From 99cec074f7d4a296b89fe47d3a0ff646cd89f679 Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Sat, 22 Feb 2025 12:02:17 +1100 Subject: [PATCH] Load SEC when __start_SEC / __stop_SEC is referenced We previously only kept such sections if they had the retain bit set. This should match the behaviour of GNU ld and also of lld when -z nostart-stop-gc is set. We now treat `-z nostart-stop-gc` as a no-op rather than warning. The custom sections test is updated to not use the retain bit, since it isn't needed anymore. This also allowed that test to enabled for cross compilation, which was disabled due to the use of the retain bit. Fixes: #459 --- libwild/src/args.rs | 1 + libwild/src/layout.rs | 92 ++++++++++++++++++++++++---- libwild/src/resolution.rs | 13 +++- wild/tests/sources/custom_section.c | 13 ++-- wild/tests/sources/custom_section0.c | 8 +-- 5 files changed, 102 insertions(+), 25 deletions(-) diff --git a/libwild/src/args.rs b/libwild/src/args.rs index abd040440..46a619e94 100644 --- a/libwild/src/args.rs +++ b/libwild/src/args.rs @@ -297,6 +297,7 @@ pub(crate) fn parse, I: Iterator>(mut input: I) -> Resul "origin" => {} "norelro" => {} "notext" => {} + "nostart-stop-gc" => {} "execstack" => args.execstack = true, "noexecstack" => args.execstack = false, "nocopyreloc" => args.allow_copy_relocations = false, diff --git a/libwild/src/layout.rs b/libwild/src/layout.rs index b5a5fa108..8c144b81f 100644 --- a/libwild/src/layout.rs +++ b/libwild/src/layout.rs @@ -68,6 +68,7 @@ use anyhow::bail; use anyhow::ensure; use bitflags::bitflags; use crossbeam_queue::ArrayQueue; +use crossbeam_queue::SegQueue; use itertools::Itertools; use linker_utils::elf::RelocationKind; use linker_utils::elf::SectionFlags; @@ -932,10 +933,22 @@ impl<'data, S: StorageModel> SymbolRequestHandler<'data, S> for EpilogueLayoutSt fn load_symbol<'scope, A: Arch>( &mut self, _common: &mut CommonGroupState, - _symbol_id: SymbolId, - _resources: &GraphResources<'data, 'scope, S>, + symbol_id: SymbolId, + resources: &GraphResources<'data, 'scope, S>, _queue: &mut LocalWorkQueue, ) -> Result { + let def_info = + &self.internal_symbols.symbol_definitions[self.symbol_id_range.id_to_offset(symbol_id)]; + + if let Some(output_section_id) = def_info.section_id() { + // We've gotten a request to load a __start_ / __stop_ symbol, sent requests to load all + // sections that would go into that section. + let sections = resources.start_stop_sections.get(output_section_id); + while let Some(request) = sections.pop() { + resources.send_work(request.file_id, WorkItem::LoadSection(request)); + } + } + Ok(()) } } @@ -1256,6 +1269,11 @@ struct GraphResources<'data, 'scope, S: StorageModel> { merged_strings: &'scope OutputSectionMap>, has_static_tls: AtomicBool, + + /// For each OutputSectionId, this tracks a list of sections that should be loaded if that + /// section gets referenced. The sections here will only be those that are eligible for having + /// __start_ / __stop_ symbols. i.e. sections that don't start their names with a ".". + start_stop_sections: OutputSectionMap>, } struct FinaliseLayoutResources<'scope, 'data, S: StorageModel> { @@ -1277,17 +1295,27 @@ enum WorkItem { /// A direct reference to a dynamic symbol has been encountered. The symbol should be defined in /// BSS with a copy relocation. ExportCopyRelocation(SymbolId), + + /// A request to load a particular section. + LoadSection(SectionLoadRequest), +} + +#[derive(Copy, Clone, Debug)] +struct SectionLoadRequest { + file_id: FileId, + + /// The offset of the section within the file's sections. i.e. the same as object::SectionIndex, + /// but stored as a u32 for compactness. + section_index: u32, } impl WorkItem { fn file_id(self, symbol_db: &SymbolDb) -> FileId { - symbol_db.file_id_for_symbol(self.symbol_id()) - } - - fn symbol_id(self) -> SymbolId { match self { - WorkItem::LoadGlobalSymbol(s) => s, - WorkItem::ExportCopyRelocation(s) => s, + WorkItem::LoadGlobalSymbol(s) | WorkItem::ExportCopyRelocation(s) => { + symbol_db.file_id_for_symbol(s) + } + WorkItem::LoadSection(s) => s.file_id, } } } @@ -1794,6 +1822,7 @@ fn find_required_sections<'data, S: StorageModel, A: Arch>( sections_with_content: output_sections.new_section_map(), merged_strings, has_static_tls: AtomicBool::new(false), + start_stop_sections: output_sections.new_section_map(), }; let resources_ref = &resources; @@ -2175,6 +2204,9 @@ impl<'data> FileLayoutState<'data> { ) } }, + WorkItem::LoadSection(request) => { + self.handle_section_request::(common, request, resources, queue) + } } } @@ -2203,6 +2235,29 @@ impl<'data> FileLayoutState<'data> { Ok(()) } + fn handle_section_request<'scope, S: StorageModel, A: Arch>( + &mut self, + common: &mut CommonGroupState<'data>, + request: SectionLoadRequest, + resources: &GraphResources<'data, 'scope, S>, + queue: &mut LocalWorkQueue, + ) -> Result { + match self { + FileLayoutState::Object(state) => { + state + .sections_required + .push(SectionRequest::new(object::SectionIndex( + request.section_index as usize, + ))); + + state.load_sections::(common, resources, queue)?; + + Ok(()) + } + _ => bail!("Request to load section sent to non-object: {self}"), + } + } + fn finalise_layout( self, memory_offsets: &mut OutputSectionPartMap, @@ -2366,6 +2421,9 @@ impl std::fmt::Display for ObjectLayout<'_> { } } +// TODO: Experiment with unifying this with SectionLoadRequest, which also has a FileId. This might +// allow us to move the sections-to-load queue from ObjectLayoutState to the group, reducing heap +// allocations. struct SectionRequest { id: object::SectionIndex, } @@ -3201,15 +3259,26 @@ impl<'data> ObjectLayoutState<'data> { let mut note_gnu_property_section = None; let no_gc = !resources.symbol_db.args.gc_sections; + for (i, section) in self.sections.iter().enumerate() { match section { SectionSlot::MustLoad(..) | SectionSlot::UnloadedDebugInfo(..) => { self.sections_required .push(SectionRequest::new(object::SectionIndex(i))); } - SectionSlot::Unloaded(..) if no_gc => { - self.sections_required - .push(SectionRequest::new(object::SectionIndex(i))); + SectionSlot::Unloaded(sec) => { + if no_gc { + self.sections_required + .push(SectionRequest::new(object::SectionIndex(i))); + } else if sec.start_stop_eligible { + resources + .start_stop_sections + .get(sec.part_id.output_section_id()) + .push(SectionLoadRequest { + file_id: self.file_id, + section_index: i as u32, + }); + } } SectionSlot::EhFrameData(index) => { eh_frame_section = Some(*index); @@ -3220,6 +3289,7 @@ impl<'data> ObjectLayoutState<'data> { _ => (), } } + if let Some(eh_frame_section_index) = eh_frame_section { process_eh_frame_data::( self, diff --git a/libwild/src/resolution.rs b/libwild/src/resolution.rs index 0e9f6a5d1..07be01c22 100644 --- a/libwild/src/resolution.rs +++ b/libwild/src/resolution.rs @@ -532,6 +532,10 @@ pub(crate) struct UnloadedSection { /// The index of the last FDE for this section. Previous FDEs will be linked from this. pub(crate) last_frame_index: Option, + + /// Whether the section has a name that makes it eligible for generation of __start_ / __stop_ + /// symbols. In particular, the name of the section doesn't start with a ".". + pub(crate) start_stop_eligible: bool, } impl UnloadedSection { @@ -539,6 +543,7 @@ impl UnloadedSection { Self { part_id, last_frame_index: None, + start_stop_eligible: false, } } } @@ -857,9 +862,11 @@ fn resolve_sections_for_object<'data>( part_id::CUSTOM_PLACEHOLDER, )) } else { - SectionSlot::Unloaded(UnloadedSection::new( - part_id::CUSTOM_PLACEHOLDER, - )) + let mut unloaded_section = + UnloadedSection::new(part_id::CUSTOM_PLACEHOLDER); + unloaded_section.start_stop_eligible = + !section_name.starts_with(b"."); + SectionSlot::Unloaded(unloaded_section) } } TemporaryPartId::EhFrameData => { diff --git a/wild/tests/sources/custom_section.c b/wild/tests/sources/custom_section.c index 671f77450..70b4f079a 100644 --- a/wild/tests/sources/custom_section.c +++ b/wild/tests/sources/custom_section.c @@ -1,6 +1,5 @@ //#AbstractConfig:default //#Object:exit.c -//#Cross:false //#Config:archive:default //#Archive:custom_section0.c @@ -10,11 +9,11 @@ #include "exit.h" -static int foo1 __attribute__ ((used, retain, section ("foo"))) = 2; -static int foo2 __attribute__ ((used, retain, section ("foo"))) = 5; +static int foo1 __attribute__ ((used, section ("foo"))) = 2; +static int foo2 __attribute__ ((used, section ("foo"))) = 5; -static int w1a __attribute__ ((used, retain, section ("w1"))) = 88; -static int w3a __attribute__ ((used, retain, section ("w3"))) = 88; +static int w1a __attribute__ ((used, section ("w1"))) = 88; +static int w3a __attribute__ ((used, section ("w3"))) = 88; extern int __start_foo[]; extern int __stop_foo[]; @@ -28,8 +27,8 @@ extern int __stop_w1[] __attribute__ ((weak)); extern int __start_w2[] __attribute__ ((weak)); extern int __stop_w2[] __attribute__ ((weak)); -static int dot1 __attribute__ ((used, retain, section (".dot"))) = 7; -static int dot2 __attribute__ ((used, retain, section (".dot.2"))) = 8; +static int dot1 __attribute__ ((used, section (".dot"))) = 7; +static int dot2 __attribute__ ((used, section (".dot.2"))) = 8; // Make sure we don't discard this custom, alloc section just because of its name. static int debug_script __attribute__ ((section (".debug_script"))) = 15; diff --git a/wild/tests/sources/custom_section0.c b/wild/tests/sources/custom_section0.c index a03fff2c1..794626388 100644 --- a/wild/tests/sources/custom_section0.c +++ b/wild/tests/sources/custom_section0.c @@ -1,8 +1,8 @@ -static int foo1 __attribute__ ((used, retain, section ("foo"))) = 1; -static int foo2 __attribute__ ((used, retain, section ("foo"))) = 20; -static int foo3 __attribute__ ((used, retain, section ("foo"))) = 5; +static int foo1 __attribute__ ((used, section ("foo"))) = 1; +static int foo2 __attribute__ ((used, section ("foo"))) = 20; +static int foo3 __attribute__ ((used, section ("foo"))) = 5; -static int bar1 __attribute__ ((used, retain, section ("bar"))) = 7; +static int bar1 __attribute__ ((used, section ("bar"))) = 7; int fn1(void) { return 2;