feat: define multiple program headers per output section in linker scripts #2256
+526
−239
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,7 @@ use crate::output_kind::OutputKind; | |
| use crate::output_section_id; | ||
| use crate::output_section_id::CustomSectionIds; | ||
| use crate::output_section_id::NUM_BUILT_IN_SECTIONS; | ||
| use crate::output_section_id::OrderEvent; | ||
| use crate::output_section_id::OutputOrder; | ||
| use crate::output_section_id::OutputOrderBuilder; | ||
| use crate::output_section_id::OutputSectionId; | ||
|
|
@@ -68,7 +69,9 @@ use crate::platform::SectionType as _; | |
| use crate::platform::Symbol as _; | ||
| use crate::platform::ThunkConfig; | ||
| use crate::platform::VerneedTable as _; | ||
| use crate::program_segments::ProgramSegmentId; | ||
| use crate::program_segments::ProgramSegments; | ||
| use crate::program_segments::SegmentEntry; | ||
| use crate::resolution::LoadedMetrics; | ||
| use crate::string_merging::MergedStringStartAddresses; | ||
| use crate::string_merging::MergedStringsSection; | ||
|
|
@@ -829,6 +832,11 @@ impl platform::Platform for Elf { | |
| if segment_def.segment_type == pt::PHDR { | ||
| *keep = !args.nmagic; | ||
| } | ||
| if segment_def.segment_type == pt::RISCV_ATTRIBUTES | ||
| && args.arch != Architecture::RiscV64 | ||
| { | ||
| *keep = false; | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -840,6 +848,20 @@ impl platform::Platform for Elf { | |
| &[STACK_SEGMENT_DEF] | ||
| } | ||
|
|
||
| fn get_segment_flags_for_section(section_flags: &Self::SectionFlags) -> u32 { | ||
| let mut flags = 0; | ||
| if section_flags.contains(shf::ALLOC) { | ||
| flags |= pf::READABLE.0; | ||
| } | ||
| if section_flags.contains(shf::WRITE) { | ||
| flags |= pf::WRITABLE.0; | ||
| } | ||
| if section_flags.contains(shf::EXECINSTR) { | ||
| flags |= pf::EXECUTABLE.0; | ||
| } | ||
| flags | ||
| } | ||
|
|
||
| fn create_linker_defined_symbols( | ||
| symbols: &mut crate::parsing::InternalSymbolsBuilder<Elf>, | ||
| output_kind: OutputKind, | ||
|
|
@@ -976,9 +998,9 @@ impl platform::Platform for Elf { | |
| min_alignment: d.min_alignment, | ||
| location_info: None, | ||
| secondary_order: None, | ||
| region_name: None, | ||
| fill: None, | ||
| phdrs: Vec::new(), | ||
| }) | ||
| .collect() | ||
| } | ||
|
|
@@ -2006,7 +2028,6 @@ impl platform::Platform for Elf { | |
| output_sections: &OutputSections<'data, Self>, | ||
| secondary: &OutputSectionMap<Vec<OutputSectionId>>, | ||
| linker_scripts: &[&SequencedLinkerScript<'data, Self>], | ||
| location_counters: &[crate::layout_rules::LocationCounter<'data>], | ||
| ) -> Result<(OutputOrder<'data>, ProgramSegments<Self::ProgramSegmentDef>)> { | ||
| let mut builder = OutputOrderBuilder::<Self>::new( | ||
|
|
@@ -2017,90 +2038,189 @@ impl platform::Platform for Elf { | |
| location_counters, | ||
| ); | ||
|
|
||
| let mut segments_map = HashMap::new(); | ||
| let mut ordered_sections = Vec::new(); | ||
|
|
||
| let mut num_phdrs = 0; | ||
|
|
||
| let mut segment_has_explicit_flags = Vec::with_capacity(num_phdrs); | ||
|
|
||
| for script in linker_scripts { | ||
| num_phdrs += script.parsed.program_headers.len(); | ||
| for phdr in &script.parsed.program_headers { | ||
| let ptype = expression_eval::evaluate_const(&phdr.ptype)? as u32; | ||
| let flags = phdr | ||
| .flags | ||
| .as_ref() | ||
| .map(|f| expression_eval::evaluate_const(f).map(|c| c as u32)) | ||
| .transpose()? | ||
| .unwrap_or(0); | ||
|
|
||
| let id = builder.add_custom_segment( | ||
| <ProgramSegmentDef as platform::ProgramSegmentDef>::from_linker_script( | ||
| ptype, flags, | ||
| ), | ||
| ); | ||
| segment_has_explicit_flags.push(phdr.flags.is_some()); | ||
| segments_map.insert( | ||
| phdr.name, | ||
| SegmentEntry { | ||
| id, | ||
| ptype, | ||
| flags, | ||
| is_emitted: false, | ||
| }, | ||
| ); | ||
| } | ||
|
|
||
| for id in &script.parsed.ordered_sections { | ||
| let info = output_sections.section_infos.get(*id); | ||
| for phdr in &info.phdrs { | ||
| if phdr == b"NONE" { | ||
| continue; | ||
| } | ||
| let segment = segments_map.get_mut(phdr).with_context(|| { | ||
| format!( | ||
|
Member
There was a problem hiding this comment. If I apply the following change without this PR, the modified test passes, but fails with this PR. Do we want handle diff --git a/wild/tests/sources/elf/linker-script-phdrs/linker-script-phdrs.ld b/wild/tests/sources/elf/linker-script-phdrs/linker-script-phdrs.ld
index 49e04e67b8..4dcead98e2 100644
--- a/wild/tests/sources/elf/linker-script-phdrs/linker-script-phdrs.ld
+++ b/wild/tests/sources/elf/linker-script-phdrs/linker-script-phdrs.ld
@@ -22,6 +22,10 @@
.rodata : {
*(.rodata*)
} :rodata
+
+ /* :NONE suppresses the inherited :rodata program-header assignment. */
+ .empty : {
+ } :NONE
}
ASSERT(start_of_sections == 0x40 + ((3 + is_riscv) * 0x38), "Wrong start of sections");
Member
Author
There was a problem hiding this comment. I initially wanted to handle |
||
| "Section {} assigned to non-existent phdr `{}`", | ||
| output_sections.display_name(*id), | ||
| String::from_utf8_lossy(phdr) | ||
| ) | ||
| })?; | ||
| segment.is_emitted = true; | ||
| if segment_has_explicit_flags[segment.id.as_usize()] { | ||
| continue; | ||
| } | ||
| segment.flags |= | ||
| Self::get_segment_flags_for_section(&info.section_attributes.flags); | ||
| builder.get_segment_mut(segment.id).segment_flags |= | ||
| SegmentFlags(segment.flags); | ||
| } | ||
| ordered_sections.push(*id); | ||
| } | ||
| } | ||
|
|
||
| let mut first_load = None; | ||
| let mut starts = vec![None; num_phdrs]; | ||
| let mut ends = vec![None; num_phdrs]; | ||
| let mut first_exec: Option<&SegmentEntry> = None; | ||
| let mut first_rw: Option<&SegmentEntry> = None; | ||
| let mut first_ro: Option<&SegmentEntry> = None; | ||
|
|
||
| for (pos, id) in ordered_sections.iter().enumerate() { | ||
| let phdrs = &output_sections.section_infos.get(*id).phdrs; | ||
| for &phdr_name in phdrs { | ||
| let Some(entry) = segments_map.get(phdr_name) else { | ||
| continue; | ||
| }; | ||
| let seg_idx = entry.id.as_usize(); | ||
| if entry.ptype == pt::LOAD.0 && first_load.is_none() { | ||
| first_load = Some(entry.id); | ||
| } | ||
| if ends[seg_idx].is_none() { | ||
| starts[seg_idx] = Some(pos); | ||
| } | ||
| ends[seg_idx] = Some(pos); | ||
| if entry.ptype == pt::LOAD.0 { | ||
| if (entry.flags & pf::EXECUTABLE.0) != 0 | ||
| && first_exec.is_none_or(|e| e.flags & pf::WRITABLE.0 != 0) | ||
| { | ||
| first_exec = Some(entry); | ||
| } else if (entry.flags & pf::WRITABLE.0) != 0 | ||
| && first_rw.is_none_or(|e| e.flags & pf::EXECUTABLE.0 != 0) | ||
| { | ||
| first_rw = Some(entry); | ||
| } else if first_ro.is_none_or(|e| e.flags != pf::READABLE.0) { | ||
| first_ro = Some(entry); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if first_load.is_none() { | ||
| bail!("Final Link failed: bad value"); | ||
|
Member
There was a problem hiding this comment. I think I'd be somewhat confused seeing this error message. I assume this is the message that GNU ld emits, but I'd be inclined to see if we can give something more helpful - e.g. "Missing LOAD PHDR in linker script"? |
||
| } | ||
|
|
||
| for &hdr_id in &[ | ||
| output_section_id::FILE_HEADER, | ||
| output_section_id::PROGRAM_HEADERS, | ||
| output_section_id::SECTION_HEADERS, | ||
| ] { | ||
| builder.push_event(OrderEvent::Section(hdr_id)); | ||
| } | ||
|
|
||
| let update_flags = |builder: &mut OutputOrderBuilder<Self>, | ||
| sections: &[OutputSectionId], | ||
| segment: ProgramSegmentId| { | ||
| if segment_has_explicit_flags[segment.as_usize()] { | ||
| return; | ||
| } | ||
| for §ion_id in sections { | ||
| let info = output_sections.section_infos.get(section_id); | ||
| let flags = Self::get_segment_flags_for_section(&info.section_attributes.flags); | ||
| builder.get_segment_mut(segment).segment_flags |= SegmentFlags(flags); | ||
| } | ||
| }; | ||
|
|
||
| for (pos, section_id) in ordered_sections.iter().enumerate() { | ||
| let section_id = *section_id; | ||
|
|
||
| for (seg_idx, segment) in starts.iter().enumerate().take(num_phdrs) { | ||
| if *segment == Some(pos) { | ||
| builder.push_event(OrderEvent::SegmentStart(ProgramSegmentId::new(seg_idx))); | ||
| } | ||
| } | ||
|
|
||
| builder.add_section(section_id); | ||
|
|
||
| for (seg_idx, segment) in ends.iter().enumerate().take(num_phdrs) { | ||
| if *segment == Some(pos) { | ||
| let seg_id = ProgramSegmentId::new(seg_idx); | ||
| if Some(seg_id) == first_exec.map_or(first_load, |e| Some(e.id)) { | ||
| update_flags(&mut builder, &custom.exec, seg_id); | ||
| builder.add_sections(&custom.exec); | ||
| } | ||
| if Some(seg_id) == first_rw.map_or(first_load, |e| Some(e.id)) { | ||
| update_flags(&mut builder, &custom.tdata, seg_id); | ||
| update_flags(&mut builder, &custom.tbss, seg_id); | ||
| update_flags(&mut builder, &custom.data, seg_id); | ||
| update_flags(&mut builder, &custom.bss, seg_id); | ||
| builder.add_sections(&custom.tdata); | ||
| builder.add_sections(&custom.tbss); | ||
| builder.add_sections(&custom.data); | ||
| builder.add_sections(&custom.bss); | ||
| } | ||
| if Some(seg_id) == first_ro.map_or(first_load, |e| Some(e.id)) { | ||
| update_flags(&mut builder, &custom.ro, seg_id); | ||
| builder.add_sections(&custom.ro); | ||
| } | ||
|
Member
There was a problem hiding this comment. If there's no segment with the right permissions, then it seems that we drop the section. Can you add a test for that?
Member
Author
There was a problem hiding this comment. If there is a single |
||
| builder.push_event(OrderEvent::SegmentEnd(seg_id)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| for &id in &custom.nonalloc { | ||
| if id != output_section_id::RISCV_ATTRIBUTES { | ||
| builder.add_section(id); | ||
| } | ||
| } | ||
|
|
||
| for segment in segments_map.values() { | ||
| if !segment.is_emitted { | ||
| builder.push_event(OrderEvent::SegmentStart(segment.id)); | ||
| builder.push_event(OrderEvent::SegmentEnd(segment.id)); | ||
| } | ||
| } | ||
|
|
||
| let riscv_segment = builder.add_custom_segment( | ||
| <ProgramSegmentDef as platform::ProgramSegmentDef>::from_linker_script( | ||
| pt::RISCV_ATTRIBUTES.0, | ||
| pf::READABLE.0, | ||
| ), | ||
| ); | ||
| builder.push_event(OrderEvent::SegmentStart(riscv_segment)); | ||
| builder.add_section(output_section_id::RISCV_ATTRIBUTES); | ||
| builder.push_event(OrderEvent::SegmentEnd(riscv_segment)); | ||
|
|
||
| Ok(builder.build()) | ||
| } | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might as well do
Vec::new(), since I think it will be equivalent toVec::with_capacity(0)