Rollup merge of rust-lang#155065 - folkertdev:macho-section-specifier, r=JonathanBrouwer

error on invalid macho section specifier

The macho section specifier used by `#[link_section = "..."]` is more strict than e.g. the one for elf. LLVM will error when you get it wrong, which is easy to do if you're used to elf. So, provide some guidance for the simplest mistakes, based on the LLVM validation.

Currently compilation fails with an LLVM error, see https://godbolt.org/z/WoE8EdK1K.

The LLVM validation logic is at

https://github.com/llvm/llvm-project/blob/a0f0d6342e0cd75b7f41e0e6aae0944393b68a62/llvm/lib/MC/MCSectionMachO.cpp#L199-L203

LLVM validates the other components of the section specifier too, but it feels a bit fragile to duplicate those checks. If you get that far, hopefully the LLVM errors will be sufficient to get unstuck.

---

sidequest from rust-lang#147811

r? JonathanBrouwer

specifically, is this the right place for this sort of validation? `rustc_attr_parsing` also does some validation.

Author: JonathanBrouwer

compiler/rustc_attr_parsing/src/attributes/link_attrs.rs

@@ -14,8 +14,9 @@ use crate::attributes::cfg::parse_cfg_entry;
 use crate::session_diagnostics::{
     AsNeededCompatibility, BundleNeedsStatic, EmptyLinkName, ExportSymbolsNeedsStatic,
     ImportNameTypeRaw, ImportNameTypeX86, IncompatibleWasmLink, InvalidLinkModifier,
-    LinkFrameworkApple, LinkOrdinalOutOfRange, LinkRequiresName, MultipleModifiers,
-    NullOnLinkSection, RawDylibNoNul, RawDylibOnlyWindows, WholeArchiveNeedsStatic,
+    InvalidMachoSection, InvalidMachoSectionReason, LinkFrameworkApple, LinkOrdinalOutOfRange,
+    LinkRequiresName, MultipleModifiers, NullOnLinkSection, RawDylibNoNul, RawDylibOnlyWindows,
+    WholeArchiveNeedsStatic,
 };
 
 pub(crate) struct LinkNameParser;
@@ -465,6 +466,29 @@ impl LinkParser {
 
 pub(crate) struct LinkSectionParser;
 
+fn check_link_section_macho(name: Symbol) -> Result<(), InvalidMachoSectionReason> {
+    let mut parts = name.as_str().split(',').map(|s| s.trim());
+
+    // The segment can be empty.
+    let _segment = parts.next();
+
+    // But the section is required.
+    let section = match parts.next() {
+        None | Some("") => return Err(InvalidMachoSectionReason::MissingSection),
+        Some(section) => section,
+    };
+
+    if section.len() > 16 {
+        return Err(InvalidMachoSectionReason::SectionTooLong { section: section.to_string() });
+    }
+
+    // LLVM also checks the other components of the section specifier, but that logic is hard to
+    // keep in sync. We skip it here for now, assuming that if you got that far you'll be able
+    // to interpret the LLVM errors.
+
+    Ok(())
+}
+
 impl<S: Stage> SingleAttributeParser<S> for LinkSectionParser {
     const PATH: &[Symbol] = &[sym::link_section];
     const ON_DUPLICATE: OnDuplicate<S> = OnDuplicate::WarnButFutureError;
@@ -497,6 +521,18 @@ impl<S: Stage> SingleAttributeParser<S> for LinkSectionParser {
             return None;
         }
 
+        // We (currently) only validate macho section specifiers.
+        match cx.sess.target.binary_format {
+            BinaryFormat::MachO => match check_link_section_macho(name) {
+                Ok(()) => {}
+                Err(reason) => {
+                    cx.emit_err(InvalidMachoSection { name_span: nv.value_span, reason });
+                    return None;
+                }
+            },
+            BinaryFormat::Coff | BinaryFormat::Elf | BinaryFormat::Wasm | BinaryFormat::Xcoff => {}
+        }
+
         Some(LinkSection { name, span: cx.attr_span })
     }
 }

compiler/rustc_attr_parsing/src/session_diagnostics.rs

@@ -1128,3 +1128,22 @@ pub(crate) struct UnstableAttrForAlreadyStableFeature {
     #[label("the stability attribute annotates this item")]
     pub item_span: Span,
 }
+
+#[derive(Diagnostic)]
+#[diag("invalid macho section specifier")]
+pub(crate) struct InvalidMachoSection {
+    #[primary_span]
+    #[label("not a valid macho section specifier")]
+    pub name_span: Span,
+    #[subdiagnostic]
+    pub reason: InvalidMachoSectionReason,
+}
+
+#[derive(Subdiagnostic)]
+pub(crate) enum InvalidMachoSectionReason {
+    #[note("a macho section specifier requires a segment and a section, separated by a comma")]
+    #[help("an example of a valid macho section specifier is `__TEXT,__cstring`")]
+    MissingSection,
+    #[note("section name `{$section}` is longer than 16 bytes")]
+    SectionTooLong { section: String },
+}

compiler/rustc_hir/src/attrs/data_structures.rs

@@ -1090,8 +1090,8 @@ pub enum AttributeKind {
 
     /// Represents [`#[link_section]`](https://doc.rust-lang.org/reference/abi.html#the-link_section-attribute)
     LinkSection {
-        name: Symbol,
         span: Span,
+        name: Symbol,
     },
 
     /// Represents `#[linkage]`.

tests/codegen-llvm/naked-fn/naked-functions.rs

@@ -145,13 +145,16 @@ pub extern "C" fn naked_with_args_and_return(a: isize, b: isize) -> isize {
 }
 
 // linux:            .pushsection .text.some_different_name,\22ax\22, @progbits
-// macos:            .pushsection .text.some_different_name,regular,pure_instructions
+// macos:            .pushsection __TEXT,different,regular,pure_instructions
 // win_x86,win_i686: .pushsection .text.some_different_name,\22xr\22
 // thumb:            .pushsection .text.some_different_name,\22ax\22, %progbits
 // CHECK-LABEL: test_link_section:
 #[no_mangle]
 #[unsafe(naked)]
-#[link_section = ".text.some_different_name"]
+// FIXME: configure this with `cfg(target_binary_format = "mach-o")`,
+// see https://github.com/rust-lang/rust/issues/152586.
+#[cfg_attr(not(target_vendor = "apple"), link_section = ".text.some_different_name")]
+#[cfg_attr(target_vendor = "apple", link_section = "__TEXT,different")]
 pub extern "C" fn test_link_section() {
     cfg_select! {
         all(target_arch = "arm", target_feature = "thumb-mode") => {

tests/ui/linkage-attr/link-section-macho.rs

@@ -0,0 +1,58 @@
+//@ add-minicore
+//@ compile-flags: --target aarch64-apple-darwin
+//@ needs-llvm-components: aarch64
+//@ ignore-backends: gcc
+#![feature(no_core, rustc_attrs, lang_items)]
+#![no_core]
+#![crate_type = "lib"]
+
+extern crate minicore;
+use minicore::*;
+
+#[unsafe(link_section = "foo")]
+//~^ ERROR invalid macho section specifier
+#[unsafe(no_mangle)]
+fn missing_section() {}
+
+#[unsafe(link_section = "foo,")]
+//~^ ERROR invalid macho section specifier
+#[unsafe(no_mangle)]
+fn empty_section() {}
+
+#[unsafe(link_section = "foo, ")]
+//~^ ERROR invalid macho section specifier
+#[unsafe(no_mangle)]
+fn whitespace_section() {}
+
+#[unsafe(link_section = "foo,somelongwindedthing")]
+//~^ ERROR invalid macho section specifier
+#[unsafe(no_mangle)]
+fn section_too_long() {}
+
+#[unsafe(link_section = "foo,bar")]
+#[unsafe(no_mangle)]
+fn segment_and_section() {}
+
+#[unsafe(link_section = "foo,bar,")]
+#[unsafe(no_mangle)]
+fn segment_and_section_and_comma() {}
+
+#[unsafe(link_section = ",foo")]
+#[unsafe(no_mangle)]
+fn missing_segment_is_fine() {}
+
+#[unsafe(link_section = "__TEXT,__stubs,symbol_stubs,none,16")]
+#[unsafe(no_mangle)]
+fn stub_size_decimal() {}
+
+#[unsafe(link_section = "__TEXT,__stubs,symbol_stubs,none,0x10")]
+#[unsafe(no_mangle)]
+fn stub_size_hex() {}
+
+#[unsafe(link_section = "__TEXT,__stubs,symbol_stubs,none,020")]
+#[unsafe(no_mangle)]
+fn stub_size_oct() {}
+
+#[unsafe(link_section = "__TEXT,__stubs,symbol_stubs,none,020,rest,is,ignored")]
+#[unsafe(no_mangle)]
+fn rest_is_ignored() {}

tests/ui/linkage-attr/link-section-macho.stderr

@@ -0,0 +1,37 @@
+error: invalid macho section specifier
+  --> $DIR/link-section-macho.rs:12:25
+   |
+LL | #[unsafe(link_section = "foo")]
+   |                         ^^^^^ not a valid macho section specifier
+   |
+   = note: a macho section specifier requires a segment and a section, separated by a comma
+   = help: an example of a valid macho section specifier is `__TEXT,__cstring`
+
+error: invalid macho section specifier
+  --> $DIR/link-section-macho.rs:17:25
+   |
+LL | #[unsafe(link_section = "foo,")]
+   |                         ^^^^^^ not a valid macho section specifier
+   |
+   = note: a macho section specifier requires a segment and a section, separated by a comma
+   = help: an example of a valid macho section specifier is `__TEXT,__cstring`
+
+error: invalid macho section specifier
+  --> $DIR/link-section-macho.rs:22:25
+   |
+LL | #[unsafe(link_section = "foo, ")]
+   |                         ^^^^^^^ not a valid macho section specifier
+   |
+   = note: a macho section specifier requires a segment and a section, separated by a comma
+   = help: an example of a valid macho section specifier is `__TEXT,__cstring`
+
+error: invalid macho section specifier
+  --> $DIR/link-section-macho.rs:27:25
+   |
+LL | #[unsafe(link_section = "foo,somelongwindedthing")]
+   |                         ^^^^^^^^^^^^^^^^^^^^^^^^^ not a valid macho section specifier
+   |
+   = note: section name `somelongwindedthing` is longer than 16 bytes
+
+error: aborting due to 4 previous errors
+