Fixes

Author: tgross35

crates/symbol-check/src/main.rs

@@ -1,16 +1,20 @@
 //! Tool used by CI to inspect compiler-builtins archives and help ensure we won't run into any
 //! linking errors.
 
+#![allow(unused)] // TODO
+
 use std::collections::{BTreeMap, BTreeSet};
 use std::fs;
 use std::io::{BufRead, BufReader};
 use std::path::{Path, PathBuf};
 use std::process::{Command, Stdio};
 
 use object::read::archive::ArchiveFile;
+use object::read::elf::SectionHeader;
 use object::{
-    BinaryFormat, File as ObjFile, Object, ObjectSection, ObjectSymbol, Result as ObjResult,
-    SectionFlags, Symbol, SymbolKind, SymbolScope, elf,
+    Architecture, BinaryFormat, Bytes, Endianness, File as ObjFile, LittleEndian, Object,
+    ObjectSection, ObjectSymbol, Result as ObjResult, SectionFlags, SectionKind, Symbol,
+    SymbolKind, SymbolScope, elf,
 };
 use serde_json::Value;
 
@@ -45,6 +49,9 @@ fn main() {
             let target = &host_target();
             run_build_and_check(target, args);
         }
+        ["check", "--ignore-exe-stack", paths @ ..] if !paths.is_empty() => {
+            check_paths(paths);
+        }
         ["check", paths @ ..] if !paths.is_empty() => {
             check_paths(paths);
         }
@@ -75,8 +82,8 @@ fn check_paths<P: AsRef<Path>>(paths: &[P]) {
         println!("Checking {}", path.display());
         let archive = BinFile::from_path(path);
 
-        verify_no_duplicates(&archive);
-        verify_core_symbols(&archive);
+        // verify_no_duplicates(&archive);
+        // verify_core_symbols(&archive);
         verify_no_exec_stack(&archive);
     }
 }
@@ -300,18 +307,7 @@ fn verify_core_symbols(archive: &BinFile) {
     println!("    success: no undefined references to core found");
 }
 
-/// Check that all object files contain a section named `.note.GNU-stack`, indicating a
-/// nonexecutable stack.
-///
-/// Paraphrased from <https://www.man7.org/linux/man-pages/man1/ld.1.html>:
-///
-/// - A `.note.GNU-stack` section with the exe flag means this needs an executable stack
-/// - A `.note.GNU-stack` section without the exe flag means there is no executable stack needed
-/// - Without the section, behavior is target-specific and on some targets means an executable
-///   stack is required.
-///
-/// Now says
-/// deprecated <https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=0d38576a34ec64a1b4500c9277a8e9d0f07e6774>.
+/// Ensure that the object/archive will not require an executable stack.
 fn verify_no_exec_stack(archive: &BinFile) {
     let mut problem_objfiles = Vec::new();
 
@@ -322,42 +318,91 @@ fn verify_no_exec_stack(archive: &BinFile) {
     });
 
     if !problem_objfiles.is_empty() {
-        panic!("the following archive members require an executable stack: {problem_objfiles:#?}");
+        panic!("the following object files require an executable stack: {problem_objfiles:#?}");
     }
 
-    println!("    success: no writeable-executable sections found");
+    println!("    success: no writeable+executable sections found");
 }
 
+/// True if the section/flag combination indicates that the object file should be linked with an
+/// executable stack.
+///
+/// Paraphrased from <https://www.man7.org/linux/man-pages/man1/ld.1.html>:
+///
+/// - A `.note.GNU-stack` section with the exe flag means this needs an executable stack
+/// - A `.note.GNU-stack` section without the exe flag means there is no executable stack needed
+/// - Without the section, behavior is target-specific and on some targets means an executable
+///   stack is required.
+///
+/// If any object files meet the requirements for an executable stack, any final binary that links
+/// it will have a program header with a `PT_GNU_STACK` section, which will be marked `RWE` rather
+/// than the desired `RW`. (We don't check final binaries).
+///
+/// Per [1], it is now deprecated behavior for a missing `.note.GNU-stack` section to imply an
+/// executable stack. However, we shouldn't assume that tooling has caught up to this.
+///
+/// [1]: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=0d38576a34ec64a1b4500c9277a8e9d0f07e6774>
 fn obj_requires_exe_stack(obj: &ObjFile) -> bool {
-    // Files other than elf likely do not use the same convention.
+    // Files other than elf do not use the same convention.
     if obj.format() != BinaryFormat::Elf {
         return false;
     }
 
+    let mut return_immediate = None;
     let mut has_exe_sections = false;
     for sec in obj.sections() {
         let SectionFlags::Elf { sh_flags } = sec.flags() else {
             unreachable!("only elf files are being checked");
         };
 
-        let exe = (sh_flags & elf::SHF_EXECINSTR as u64) != 0;
+        if sec.kind() == SectionKind::Elf(elf::SHT_ARM_ATTRIBUTES) {
+            let data = sec.data().unwrap();
+            parse_arm_thing(data);
+        }
+
+        let is_exe = (sh_flags & elf::SHF_EXECINSTR as u64) != 0;
 
         // If the magic section is present, its exe bit tells us whether or not the object
         // file requires an executable stack.
         if sec.name().unwrap_or_default() == ".note.GNU-stack" {
-            return exe;
+            return_immediate = Some(is_exe);
         }
 
         // Otherwise, just keep track of whether or not we have exeuctable sections
-        has_exe_sections |= exe;
+        has_exe_sections |= is_exe;
+    }
+
+    if let Some(imm) = return_immediate {
+        return imm;
     }
 
     // Ignore object files that have no executable sections, like rmeta
     if !has_exe_sections {
         return false;
     }
 
-    true
+    platform_default_exe_stack_required(obj.architecture())
+}
+
+/// Default if there is no `.note.GNU-stack` section.
+fn platform_default_exe_stack_required(arch: Architecture) -> bool {
+    match arch {
+        // PPC64 doesn't set `.note.GNU-stack` since GNU nested functions don't need a trampoline,
+        // <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=21098>.
+        Architecture::PowerPc64 => false,
+        _ => true,
+    }
+}
+
+fn parse_arm_thing(data: &[u8]) {
+    eprintln!("data: {data:x?}");
+    eprintln!("data string: {:?}", String::from_utf8_lossy(data));
+    eprintln!("data: {:x?}", &data[16..]);
+    // let mut rest = &data[16..];
+    let mut b = Bytes(data);
+    b.skip(16).unwrap();
+
+    // while !rest.is_empty() {}
 }
 
 /// Thin wrapper for owning data used by `object`.
@@ -448,8 +493,12 @@ fn check_no_gnu_stack_obj() {
 }
 
 #[test]
-#[cfg_attr(not(target_env = "gnu"), ignore = "requires a gnu toolchain to build")]
+#[cfg_attr(
+    any(target_os = "windows", target_vendor = "apple"),
+    ignore = "requires elf format"
+)]
 fn check_obj() {
+    #[expect(clippy::option_env_unwrap, reason = "test is ignored")]
     let p = option_env!("HAS_EXE_STACK_OBJ").expect("has_exe_stack.o not present");
     let f = fs::read(p).unwrap();
     let obj = ObjFile::parse(f.as_slice()).unwrap();

crates/symbol-check/tests/has_exe_stack.c

@@ -1,5 +1,5 @@
-/* GNU nested functions are the only way I could fine to force an executable
- * stack. Supported by GCC only, not Clang. */
+/* GNU nested functions are the only way I could find to force an explicitly
+ * executable stack. Supported by GCC only, not Clang. */
 
 void intermediate(void (*)(int, int), int);