feat: Support SFrame#1287
Conversation
|
Results from testing with the |
|
Since current CI environments can't verify this feature, adding tests using Arch Docker might be worthwhile. Given its relative simplicity, I'd like to consider implementing this. |
|
From looking at maskray's blog posts on the topic - Stack walking: space and time trade-offs and Remarks on SFrame - I'd have expected that we'd need to do a few other things to support SFrames. In particular, it sounds like the entries need to be sorted by the addresses of the functions to which they refer. It'd also be interesting to verify if |
| "segment.NOTE.*", | ||
| // TODO: RISC-V | ||
| "segment.LOAD.RW.alignment", | ||
| // GNU ld and lld sometimes don’t generate .sframe sections in cases where we do. |
There was a problem hiding this comment.
It would be good to find out when and why it happens.
Maybe it's related to GC?
There was a problem hiding this comment.
Yeah, did some testing and this is indeed related to GC.
GNU ld and LLD seem to always output .sframe, unless --gc-sections is added. Since Wild does that by default, you had to mark sframe sections as must keep.
Sure enough, all tests pass with this patch:
diff --git a/libwild/src/args.rs b/libwild/src/args.rs
index 0ff9701e7c..74b5c6a766 100644
--- a/libwild/src/args.rs
+++ b/libwild/src/args.rs
@@ -379,7 +379,7 @@
// but do end up writing more, so --no-gc-sections will almost always be as
// slow or slower than --gc-sections. For that reason, the latter is
// probably a good default.
- gc_sections: true,
+ gc_sections: false,
prepopulate_maps: false,
sym_info: None,
merge_sections: true,
diff --git a/libwild/src/layout_rules.rs b/libwild/src/layout_rules.rs
index e37d17d41f..44992fb295 100644
--- a/libwild/src/layout_rules.rs
+++ b/libwild/src/layout_rules.rs
@@ -359,7 +359,7 @@
SectionRule::exact(secnames::EH_FRAME_SECTION_NAME, SectionRuleOutcome::EhFrame),
SectionRule::exact(
secnames::SFRAME_SECTION_NAME,
- SectionRuleOutcome::Section(SectionOutputInfo::keep(output_section_id::SFRAME)),
+ SectionRuleOutcome::Section(SectionOutputInfo::regular(output_section_id::SFRAME)),
),
SectionRule::exact(
secnames::NOTE_GNU_PROPERTY_SECTION_NAME,
diff --git a/linker-diff/src/lib.rs b/linker-diff/src/lib.rs
index b08bad44de..40e652d820 100644
--- a/linker-diff/src/lib.rs
+++ b/linker-diff/src/lib.rs
@@ -257,9 +257,6 @@
"segment.GNU_PROPERTY.alignment",
"segment.GNU_PROPERTY.flags",
// GNU ld and lld sometimes don’t generate .sframe sections in cases where we do.
- // TODO: Figure out why this is happening.
- "segment.GNU_SFRAME.alignment",
- "segment.GNU_SFRAME.flags",
]
.into_iter()
.map(ToOwned::to_owned),I think, the current behaviour of always emitting sframe is fine. It doesn't slow us down much.
99a1d79 to
6a17029
Compare
db93e93 to
74580bb
Compare
|
OK, this should now allow us to add functionality for sorting outputHowever, there are several points of concern. All these issues have been documented as TODO comments in the code:
|
davidlattimore
left a comment
There was a problem hiding this comment.
If there are two (or more) input objects containing sframe sections, do we emit an sframe section with multiple headers? Is it valid to do that, or do the sections need to be merged and have a single header?
Is there a consumer of sframes that we can test this with? I'm not sure what uses SFrames, perhaps gdb or libunwind? It'd be good confirm that it works with some consumer - ideally with eh_frames disabled so that we can be sure it's using the SFrames.
|
To verify SFrame's behavior beyond what
First, here's the result of And here's the result after removal: |
|
Note that the perf support for SFrames is still not yet upstream (but has been making recent progress). The only support is in glibc's |
|
Thanks, that's good to know that glibc's backtrace supports it. I wrote a test - #1353 - the commented out part of the test removes eh_frames and relies on sframes instead. It fails at head and passes with this PR. |
|
I just realised that my test wasn't testing multiple object files. Unfortunately, once I extended the test to have a second object file, it failed, presumably because the sframe sections need to be merged. |
|
While still in debugging, I pushed because I wanted to observe CI results across various distributions. The main changes include:
These changes allows the |
|
To get CI green, I needed to take the following points into account, so I implemented the corresponding changes:
There may be a better approach, but at least this one is reliable I think. |
davidlattimore
left a comment
There was a problem hiding this comment.
Nice work! This looks like it's really close to being ready to merge :)
SFrame is a relatively new mechanism for storing frame information that occasionally appears in environments such as Arch Linux. I confirmed that all tests are green on Arch.
close #1025