fix: Do not output SFrame section unless asked

[mati865]

With this change Wild will discard .sframe sections unless --wild-experimental-sframe flag is explicitly added.

[mati865]

@davidlattimore this is probably the easiest way to disable SFrame entirely.

Without experimental there is no SFrame at all:

❯ gcc -o /tmp/sframe_test2.wild sframe_test2.c -Wa,--gsframe -lsframe -rdynamic -B ~/Projects/wild/fakes-debug/ && readelf -W --sframe /tmp/sframe_test2.wild
readelf: Warning: Section '.sframe' was not dumped because it does not exist

With the experimental flag we get the broken SFrame section on systems with Binutils 2.46+ and working SFrame on systems with older Binutils (but not too old):

❯ gcc -o /tmp/sframe_test2.wild sframe_test2.c -Wa,--gsframe -lsframe -rdynamic -B ~/Projects/wild/fakes-debug/ -Wl,--wild-experimental-sframe && readelf -W --sframe /tmp/sframe_test2.wild
wild: warning: Unsupported SFrame version 3, disabling SFrame sorting
Contents of the SFrame section .sframe:
  Header :

    Version: SFRAME_VERSION_3
    Flags: SFRAME_F_FDE_FUNC_START_PCREL
    CFA fixed RA offset: -8
    Num FDEs: 1
    Num FREs: 2

  Function Index :

    func idx [0]: pc = 0x2830, size = 38 bytes
    STARTPC         CFA       FP        RA
    0000000000002830  sp+8      u         f
    0000000000002834  RA undefined

I haven't tried to add abstractions and hide it in Elf impl yet.

[davidlattimore]

What about doing something like this?

diff --git a/libwild/src/elf.rs b/libwild/src/elf.rs
index bfbadb889e..e2bd57da1f 100644
--- a/libwild/src/elf.rs
+++ b/libwild/src/elf.rs
@@ -1694,8 +1694,8 @@
         }
     }

-    fn default_layout_rules() -> &'static [SectionRule<'static>] {
-        DEFAULT_SECTION_RULES
+    fn default_layout_rules(_args: &Self::Args) -> Vec<SectionRule<'static>> {
+        DEFAULT_SECTION_RULES.to_vec()
     }

     fn linker_script_rules_pre_build(rule_builder: &mut crate::layout_rules::LayoutRulesBuilder) {
diff --git a/libwild/src/layout_rules.rs b/libwild/src/layout_rules.rs
index b1dfa5d9b4..fc5e7348a7 100644
--- a/libwild/src/layout_rules.rs
+++ b/libwild/src/layout_rules.rs
@@ -351,9 +351,9 @@
         })
     }

-    pub(crate) fn build<P: Platform>(mut self) -> LayoutRules<'data> {
+    pub(crate) fn build<P: Platform>(mut self, args: &P::Args) -> LayoutRules<'data> {
         let section_rules = if self.rules.is_empty() {
-            SectionRules::from_rules(P::default_layout_rules())
+            SectionRules::from_rules(&P::default_layout_rules(args))
         } else {
             P::linker_script_rules_pre_build(&mut self);
             SectionRules::from_rules(&self.rules)
@@ -581,7 +581,9 @@

 #[test]
 fn test_section_mapping() {
-    let rules = SectionRules::from_rules(crate::elf::Elf::default_layout_rules());
+    let rules = SectionRules::from_rules(&crate::elf::Elf::default_layout_rules(
+        &crate::args::elf::ElfArgs::new().unwrap(),
+    ));
     let header = crate::elf::SectionHeader {
         sh_name: Default::default(),
         sh_type: Default::default(),
diff --git a/libwild/src/lib.rs b/libwild/src/lib.rs
index d5c80527a3..b7adfadd2f 100644
--- a/libwild/src/lib.rs
+++ b/libwild/src/lib.rs
@@ -325,7 +325,7 @@
             symbol_db.handle_rust_version_script(rust_vscript, &mut per_symbol_flags);
         }

-        let layout_rules = layout_rules_builder.build::<P>();
+        let layout_rules = layout_rules_builder.build::<P>(args);

         let resolved = resolver.resolve_sections_and_canonicalise_undefined(
             &mut symbol_db,
diff --git a/libwild/src/macho.rs b/libwild/src/macho.rs
index 18da9ccac9..00b9a26eaa 100644
--- a/libwild/src/macho.rs
+++ b/libwild/src/macho.rs
@@ -1328,8 +1328,8 @@
         RawSymbolName { name: name_bytes }
     }

-    fn default_layout_rules() -> &'static [crate::layout_rules::SectionRule<'static>] {
-        DEFAULT_SECTION_RULES
+    fn default_layout_rules(args: &Self::Args) -> Vec<crate::layout_rules::SectionRule<'static>> {
+        DEFAULT_SECTION_RULES.to_vec()
     }

     fn build_output_order_and_program_segments<'data>(
diff --git a/libwild/src/platform.rs b/libwild/src/platform.rs
index c5d43d1f9b..0052e11579 100644
--- a/libwild/src/platform.rs
+++ b/libwild/src/platform.rs
@@ -652,7 +652,7 @@
         <Self::RawSymbolName<'data> as RawSymbolName>::parse(name_bytes)
     }

-    fn default_layout_rules() -> &'static [SectionRule<'static>];
+    fn default_layout_rules(args: &Self::Args) -> Vec<SectionRule<'static>>;

     /// Only called if a linker script that provides custom sections and layout rules is present.
     /// Gives the platform a chance to add extra built-in rules that need to be present even when a
diff --git a/libwild/src/wasm.rs b/libwild/src/wasm.rs
index 4935fd9390..c0eec1de88 100644
--- a/libwild/src/wasm.rs
+++ b/libwild/src/wasm.rs
@@ -1034,8 +1034,8 @@
         RawSymbolName { name: name_bytes }
     }

-    fn default_layout_rules() -> &'static [crate::layout_rules::SectionRule<'static>] {
-        &[]
+    fn default_layout_rules(_args: &Self::Args) -> Vec<crate::layout_rules::SectionRule<'static>> {
+        Vec::new()
     }

     fn align_load_segment_start(

Then if sframes are disabled, mapping the section to SectionRuleOutcome::Discard.

[mati865]

Thanks, it checks out. Do you want to commit that as a separate PR?

I guess we could move more of the rules into ELF impl as well, but we don't know the overlap with other platforms yet.

The performance difference is within noise margins:

❯ OUT=/tmp/bin powerprofilesctl launch -p performance hyperfine -w 100 -L variant base,refactor,discard-sframe './run-with ~/Projects/wild/target/release/wild-{variant} --no-fork'
Benchmark 1: ./run-with ~/Projects/wild/target/release/wild-base --no-fork
  Time (mean ± σ):      68.4 ms ±   1.0 ms    [User: 1119.5 ms, System: 266.5 ms]
  Range (min … max):    66.4 ms …  71.0 ms    43 runs

Benchmark 2: ./run-with ~/Projects/wild/target/release/wild-refactor --no-fork
  Time (mean ± σ):      68.2 ms ±   1.1 ms    [User: 1116.8 ms, System: 267.7 ms]
  Range (min … max):    65.9 ms …  70.8 ms    44 runs

Benchmark 3: ./run-with ~/Projects/wild/target/release/wild-discard-sframe --no-fork
  Time (mean ± σ):      68.6 ms ±   1.9 ms    [User: 1111.9 ms, System: 271.4 ms]
  Range (min … max):    65.7 ms …  76.3 ms    43 runs

Summary
  ./run-with ~/Projects/wild/target/release/wild-refactor --no-fork ran
    1.00 ± 0.02 times faster than ./run-with ~/Projects/wild/target/release/wild-base --no-fork
    1.01 ± 0.03 times faster than ./run-with ~/Projects/wild/target/release/wild-discard-sframe --no-fork

[mati865]

Perhaps this should land in an own test file?

[davidlattimore]

I think it's fine here. Having it here means that we're testing that backtraces work in this scenario and without duplicating the testing logic to do that.

[mati865]

@marxin FYI

[marxin]

Love it!

[davidlattimore]

I think it's fine here. Having it here means that we're testing that backtraces work in this scenario and without duplicating the testing logic to do that.