cranelift-object: emit .eh_frame for System V unwind info#13383
Conversation
HueCodes
requested review from
cfallin and
fitzgen
and removed request for
a team
|
Maybe put this behind a feature flag? This wouldn't be usable for rustc_codegen_cranelift as it doesn't allow writing an LSDA. On arm64 Mach-O it should be possible to emit |
github-actions
Bot
added
cranelift
Add `ObjectBuilder::unwind_info(bool)`. When enabled, the emitted object gains a `.eh_frame` section with one shared CIE and one FDE per defined function, relocated against the function symbols. The implementation routes cranelift-codegen's existing `UnwindInfo::SystemV` through gimli's `RelocateWriter` into an `object`-crate section. Off by default. Supported on ELF and COFF; Mach-O and Windows unwind formats are deferred and currently no-op or error rather than emit wrong data. Gated behind a new default-on `unwind` Cargo feature mirroring cranelift-codegen's `unwind`. `gimli` is now optional and pulled in only by that feature, so consumers writing their own `.eh_frame` (e.g. rustc_codegen_cranelift, which needs LSDA support) drop it via `default-features = false`. Closes bytecodealliance#1282.
HueCodes
force-pushed
the
feat/eh-frame-emission
branch
from
0b7cc7f to
99dadc9
Compare
|
@bjorn3 thanks. Done: default-on "unwind" Cargo feature mirroring cranelift-codegen's "unwind", with gimli and ObjectBuilder::unwind_info both gated behind it, so cg_clif drops the dep and API via default-features = false. Kept Mach-O scoped out for now (wrong-reloc risk); TODO left pointing at |
fitzgen
left a comment
fitzgen
left a comment
There was a problem hiding this comment.
Thanks! I think we could clean up the boundary between cfg(feature = "unwind") vs cfg(not(feature = "unwind")) a bit.
| #![deny(missing_docs)] | ||
|
|
||
| mod backend; | ||
| mod unwind; |
There was a problem hiding this comment.
And we also don't want to build any of this module unless #[cfg(feature = "unwind")].
| /// No-op stand-in used when the `unwind` feature is disabled. It carries no | ||
| /// state and emits nothing, so callers need no `#[cfg]` and pull in no | ||
| /// `gimli` dependency. | ||
| #[cfg(not(feature = "unwind"))] | ||
| pub(crate) struct UnwindBuilder; | ||
|
|
||
| #[cfg(not(feature = "unwind"))] | ||
| impl UnwindBuilder { | ||
| pub(crate) fn new(_endian: object::Endianness) -> Self { | ||
| UnwindBuilder | ||
| } | ||
|
|
||
| pub(crate) fn add_function( | ||
| &mut self, | ||
| _isa: &dyn TargetIsa, | ||
| _func_symbol: SymbolId, | ||
| _info: UnwindInfo, | ||
| ) { | ||
| } | ||
|
|
||
| pub(crate) fn finish(self, _object: &mut Object<'static>, _isa: &dyn TargetIsa) -> Result<()> { | ||
| Ok(()) | ||
| } | ||
| } |
There was a problem hiding this comment.
Yeah better to avoid this by moving the #[cfg(feature = "unwind")] "up" and avoiding this whole module or any calls into it when the feature is disabled.
Move `#[cfg(feature = "unwind")]` from inside the `unwind` module up to the field declarations, struct initializers, and call sites in `backend.rs`. The no-op `UnwindBuilder` stub is gone; the module itself no longer compiles when the feature is off.
|
@fitzgen thanks! Changes made: stub gone, #[cfg(feature = "unwind")] lifted to the field decls, struct init, |
3 participants
Add
ObjectBuilder::unwind_info(bool). When enabled, the emitted object gains a.eh_framesection with one shared CIE and one FDE per defined function, relocated against the function symbols.The implementation routes cranelift-codegen's existing
UnwindInfo::SystemVthrough gimli'sRelocateWriterinto anobject-crate section. Off by default. Supported on ELF and COFF; Mach-O and Windows unwind formats are deferred and currently no-op or error rather than emit wrong data.Closes #1282.