From a02dae4ae9d3d2bc3cf1ab6365ec016853d494bc Mon Sep 17 00:00:00 2001 From: Matt Stevens Date: Mon, 12 Oct 2026 08:06:23 +0100 Subject: [PATCH] coff split: mark linker-written regions in Sections:, not as units Instead of a `linker` attribute on a splits.txt unit, a section or sub-region in the `Sections:` group takes a bare `linker` flag: .idata$5 type:rodata vaddr:0x008A9000 end:0x008A99C0 linker dtk carves each `linker` region into its own autogenerated unit, named after the region (`linker/idata_5`, `linker/edata`), and marks that unit `linker` as before: objs.rsp leaves it out and the build config lists its ranges. The gap fill no longer splits a linker region on its symbols, since the linker writes it whole. The split fails if a splits.txt unit claims part of a `linker` region, or if generate_imports/generate_exports is on and an `.idata`/`.edata` split isn't in one. The unit-level `linker` attribute from 0.0.36 is gone. --- src/cmd/coff.rs | 32 ++++++++++-------- src/cmd/rel.rs | 1 + src/obj/mod.rs | 11 ++++++- src/obj/sections.rs | 11 +++++++ src/util/coff.rs | 3 ++ src/util/config.rs | 80 +++++++++++++++++++++++++++++---------------- src/util/dol.rs | 6 ++++ src/util/elf.rs | 1 + src/util/map.rs | 1 + src/util/rel.rs | 1 + src/util/rso.rs | 1 + src/util/split.rs | 51 +++++++++++++++-------------- 12 files changed, 132 insertions(+), 67 deletions(-) diff --git a/src/cmd/coff.rs b/src/cmd/coff.rs index e82a180c..9954a67c 100644 --- a/src/cmd/coff.rs +++ b/src/cmd/coff.rs @@ -8,7 +8,7 @@ use std::{ time::Instant, }; -use anyhow::{Context, Result, bail}; +use anyhow::{Context, Result, bail, ensure}; use argp::FromArgs; use itertools::Itertools; use tracing::{debug, info}; @@ -1820,8 +1820,10 @@ fn unit_ranges(obj: &ObjInfo, unit: &str) -> Vec { ranges } -/// With the linker generating a table, the original's bytes for it must sit in -/// `linker` units, or the split objects would link them in a second time. +/// The linker writes a `linker` region whole, so no split from splits.txt may +/// claim part of one. With the linker generating a table, the original's bytes +/// for it must sit in a `linker` region, or the split objects would link them in +/// a second time. fn check_linker_tables( module_name: &str, obj: &ObjInfo, @@ -1830,19 +1832,22 @@ fn check_linker_tables( ) -> Result<()> { for (_, section, addr, split) in obj.sections.all_splits() { let name = split.rename.as_deref().unwrap_or(§ion.name); - let (generated, what) = if name.starts_with(".idata") { - (generate_imports, "generate_imports") - } else if name == ".edata" { - (generate_exports, "generate_exports") - } else { - continue; - }; - if generated && !obj.link_order.iter().any(|u| u.name == split.unit && u.linker) { - bail!( - "{module_name}: {what} needs {name} {addr:#010X} in a `linker` unit, but it is in {}", + if section.is_linker_address(addr) { + ensure!( + split.autogenerated, + "{module_name}: {name} {addr:#010X} is marked `linker` in Sections:, but {} splits it", split.unit ); + continue; } + let what = if name.starts_with(".idata") && generate_imports { + "generate_imports" + } else if name == ".edata" && generate_exports { + "generate_exports" + } else { + continue; + }; + bail!("{module_name}: {what} needs {name} {addr:#010X} marked `linker` in Sections:"); } Ok(()) } @@ -2464,6 +2469,7 @@ mod lib_pointer_tests { section_known: true, splits: ObjSplits::default(), sub_regions: vec![], + linker: false, relocations: ObjRelocations::new(vec![( 0, ObjReloc { diff --git a/src/cmd/rel.rs b/src/cmd/rel.rs index 12ddd0f2..adcfda9b 100644 --- a/src/cmd/rel.rs +++ b/src/cmd/rel.rs @@ -534,6 +534,7 @@ fn merge(args: MergeArgs) -> Result<()> { section_known: mod_section.section_known, splits: mod_section.splits.clone(), sub_regions: mod_section.sub_regions.clone(), + linker: mod_section.linker, }); section_map.nested_insert(module.module_id, mod_section.elf_index, offset)?; for (_, mod_symbol) in module.symbols.for_section(mod_section_index) { diff --git a/src/obj/mod.rs b/src/obj/mod.rs index 599b29a1..0b0e3397 100644 --- a/src/obj/mod.rs +++ b/src/obj/mod.rs @@ -73,7 +73,8 @@ pub struct ObjUnit { /// Influences the order of this unit relative to other ordered units. pub order: Option, /// The linker writes this unit's bytes itself (an import or export table, a - /// debug directory), so its object is left out of the link. + /// debug directory), so its object is left out of the link. Set for the + /// units carved from `linker` sections and sub-regions. pub linker: bool, } @@ -372,6 +373,14 @@ impl ObjInfo { .all(|(_, _, _, split)| split.autogenerated) } + /// Whether every split of `unit` lies in a `linker` section or sub-region. + pub fn is_unit_linker(&self, unit: &str) -> bool { + let mut splits = + self.sections.all_splits().filter(|(_, _, _, split)| split.unit == unit).peekable(); + splits.peek().is_some() + && splits.all(|(_, section, addr, _)| section.is_linker_address(addr)) + } + /// Calculate the total size of all code sections. pub fn code_size(&self) -> u32 { self.sections diff --git a/src/obj/sections.rs b/src/obj/sections.rs index 93c96145..94d2e3ab 100644 --- a/src/obj/sections.rs +++ b/src/obj/sections.rs @@ -35,6 +35,9 @@ pub struct ObjSubRegion { /// page). Units carved here are emitted as uninitialized so the linker /// regenerates the zero padding via file alignment, matching the original. pub kind: Option, + /// The linker writes this region itself (an import or export table, a debug + /// directory), so its bytes go in a `linker` unit that stays out of the link. + pub linker: bool, } #[derive(Debug, Clone)] @@ -53,6 +56,8 @@ pub struct ObjSection { pub section_known: bool, pub splits: ObjSplits, pub sub_regions: Vec, + /// The linker writes this whole section (see [`ObjSubRegion::linker`]). + pub linker: bool, } #[derive(Debug, Clone)] @@ -191,6 +196,12 @@ impl IndexMut for ObjSections { } impl ObjSection { + /// Whether the linker writes the byte at `addr` (a `linker` section or + /// sub-region). + pub fn is_linker_address(&self, addr: u32) -> bool { + self.linker || self.sub_regions.iter().any(|r| r.linker && r.start <= addr && addr < r.end) + } + pub fn data_range(&self, start: u32, end: u32) -> Result<&[u8]> { if end == 0 { ensure!( diff --git a/src/util/coff.rs b/src/util/coff.rs index ccdaed36..3908ae62 100644 --- a/src/util/coff.rs +++ b/src/util/coff.rs @@ -125,6 +125,7 @@ pub fn process_coff(data: &[u8], name: &str) -> Result<(ObjInfo, Option)> { section_known: true, splits: Default::default(), sub_regions: Vec::new(), + linker: false, }); } @@ -1416,6 +1417,7 @@ mod tests { section_known: true, splits: ObjSplits::default(), sub_regions: vec![], + linker: false, } } @@ -1677,6 +1679,7 @@ mod tests { end: 0x00600004, name: ".CRT$XCU".into(), kind: None, + linker: false, }); } apply_base_relocations(&mut obj, 0x00400000).unwrap(); diff --git a/src/util/config.rs b/src/util/config.rs index c56e9f04..69d2cb60 100644 --- a/src/util/config.rs +++ b/src/util/config.rs @@ -511,11 +511,14 @@ where { write!(w, " vaddr:{:#010X}", vaddr)?; } + if section.linker { + write!(w, " linker")?; + } writeln!(w)?; // Round-trip logical sub-section declarations (e.g. .rdata$r) so they // survive `dtk coff split` regenerating splits.txt. for region in §ion.sub_regions { - writeln!( + write!( w, "\t{:<11} type:{} vaddr:{:#010X} end:{:#010X}", region.name, @@ -523,6 +526,10 @@ where region.start, region.end, )?; + if region.linker { + write!(w, " linker")?; + } + writeln!(w)?; } } for (name, start, end, unit) in &obj.pe_comment_sections { @@ -538,9 +545,6 @@ where if let Some(order) = unit.order { write!(w, " order:{order}")?; } - if unit.linker { - write!(w, " linker")?; - } writeln!(w)?; let mut split_iter = obj.sections.all_splits().peekable(); while let Some((_section_index, section, addr, split)) = split_iter.next() { @@ -608,8 +612,6 @@ struct SplitUnit { comment_version: Option, /// Influences the order of this unit relative to other ordered units. order: Option, - /// The linker writes this unit's bytes (`linker` attribute). - linker: bool, } pub struct SectionDef { @@ -629,6 +631,10 @@ pub struct SectionDef { /// `unit:` — owning unit for a `type:comment` range; its `.drectve` is /// emitted into that unit's object (else a shared object). pub unit: Option, + /// `linker` — the linker writes this section or sub-region itself (an import + /// or export table, a debug directory). Its bytes become `linker` units, + /// which stay out of the link. + pub linker: bool, } enum SplitLine { @@ -663,8 +669,7 @@ fn parse_unit_line(captures: Captures) -> Result { if name == "Sections" { return Ok(SplitLine::SectionsStart); } - let mut unit = - SplitUnit { name: name.to_string(), comment_version: None, order: None, linker: false }; + let mut unit = SplitUnit { name: name.to_string(), comment_version: None, order: None }; for attr in captures["attrs"].split(' ').filter(|&s| !s.is_empty()) { if let Some((attr, value)) = attr.split_once(':') { @@ -674,10 +679,7 @@ fn parse_unit_line(captures: Captures) -> Result { _ => bail!("Unknown unit attribute '{}'", attr), } } else { - match attr { - "linker" => unit.linker = true, - _ => bail!("Unknown unit attribute '{attr}'"), - } + bail!("Unknown unit attribute '{attr}'"); } } @@ -695,6 +697,7 @@ fn parse_section_line(captures: Captures, state: &SplitState) -> Result Result bail!("Unknown section attribute '{attr}'"), } } else { - bail!("Unknown section attribute '{attr}'"); + match attr { + "linker" => section.linker = true, + _ => bail!("Unknown section attribute '{attr}'"), + } } } @@ -800,14 +806,14 @@ where match (&mut state, split_line) { ( SplitState::None | SplitState::Unit(_) | SplitState::Sections(_), - SplitLine::Unit(SplitUnit { name, comment_version, order, linker }), + SplitLine::Unit(SplitUnit { name, comment_version, order }), ) => { obj.link_order.push(ObjUnit { name: name.clone(), autogenerated: false, comment_version, order, - linker, + linker: false, }); state = SplitState::Unit(name); } @@ -819,7 +825,16 @@ where } ( SplitState::Sections(index), - SplitLine::Section(SectionDef { name, kind, align, vaddr, end, comment, unit }), + SplitLine::Section(SectionDef { + name, + kind, + align, + vaddr, + end, + comment, + unit, + linker, + }), ) => { // Comment ranges live in the header padding, outside any physical // section; the COFF loader reads them. Retain the declaration so @@ -844,6 +859,7 @@ where end: stop, name: name.clone(), kind, + linker, }); continue; } @@ -862,6 +878,7 @@ where obj_section.name = name; obj_section.section_known = true; } + obj_section.linker = linker; if let Some(align) = align { obj_section.align = align as u64; } @@ -1177,23 +1194,28 @@ mod tests { use super::*; #[test] - fn unit_linker_attribute() { - let SplitLine::Unit(unit) = - parse_split_line("linker/imports: linker", &SplitState::None).unwrap() - else { - panic!("expected a unit line"); + fn section_linker_attribute() { + let SplitLine::Section(section) = parse_split_line( + " .idata$5 type:rodata vaddr:0x008A9000 end:0x008A99C0 linker", + &SplitState::Sections(0), + ) + .unwrap() else { + panic!("expected a section line"); }; - assert_eq!(unit.name, "linker/imports"); - assert!(unit.linker); + assert_eq!(section.name, ".idata$5"); + assert!(section.linker); - let SplitLine::Unit(unit) = - parse_split_line("Black/Abode.cpp: order:1", &SplitState::None).unwrap() + let SplitLine::Section(section) = + parse_split_line(" .rdata type:rodata align:4096", &SplitState::Sections(0)) + .unwrap() else { - panic!("expected a unit line"); + panic!("expected a section line"); }; - assert!(!unit.linker); - assert_eq!(unit.order, Some(1)); + assert!(!section.linker); - assert!(parse_split_line("Black/Abode.cpp: linkr", &SplitState::None).is_err()); + assert!( + parse_split_line(" .edata type:rodata linkr", &SplitState::Sections(0)).is_err() + ); + assert!(parse_split_line("linker/imports: linker", &SplitState::None).is_err()); } } diff --git a/src/util/dol.rs b/src/util/dol.rs index 8be1bf00..10fc4730 100644 --- a/src/util/dol.rs +++ b/src/util/dol.rs @@ -384,6 +384,7 @@ pub fn process_dol(buf: &[u8], name: &str) -> Result { section_known: known, splits: Default::default(), sub_regions: Vec::new(), + linker: false, }); } } else { @@ -438,6 +439,7 @@ pub fn process_dol(buf: &[u8], name: &str) -> Result { section_known: known, splits: Default::default(), sub_regions: Vec::new(), + linker: false, }); } } @@ -471,6 +473,7 @@ pub fn process_dol(buf: &[u8], name: &str) -> Result { section_known: false, splits: Default::default(), sub_regions: Vec::new(), + linker: false, }); } @@ -492,6 +495,7 @@ pub fn process_dol(buf: &[u8], name: &str) -> Result { section_known: false, splits: Default::default(), sub_regions: Vec::new(), + linker: false, }); let mut obj = ObjInfo::new( ObjKind::Executable, @@ -520,6 +524,7 @@ pub fn process_dol(buf: &[u8], name: &str) -> Result { section_known: false, splits: Default::default(), sub_regions: Vec::new(), + linker: false, }); sections.push(ObjSection { name: ".sbss".to_string(), @@ -535,6 +540,7 @@ pub fn process_dol(buf: &[u8], name: &str) -> Result { section_known: false, splits: Default::default(), sub_regions: Vec::new(), + linker: false, }); } n => bail!("Invalid number of BSS sections: {}", n), diff --git a/src/util/elf.rs b/src/util/elf.rs index 1f7039df..7f400af1 100644 --- a/src/util/elf.rs +++ b/src/util/elf.rs @@ -104,6 +104,7 @@ pub fn process_elf(path: &Utf8NativePath) -> Result { section_known: true, splits: Default::default(), sub_regions: Vec::new(), + linker: false, }); } diff --git a/src/util/map.rs b/src/util/map.rs index cd3b9b52..97677027 100644 --- a/src/util/map.rs +++ b/src/util/map.rs @@ -1161,6 +1161,7 @@ pub fn create_obj(result: &MapInfo) -> Result { section_known: true, splits: Default::default(), sub_regions: Vec::new(), + linker: false, } }) .collect(); diff --git a/src/util/rel.rs b/src/util/rel.rs index 9693c071..92fb8efd 100644 --- a/src/util/rel.rs +++ b/src/util/rel.rs @@ -453,6 +453,7 @@ where section_known, splits: Default::default(), sub_regions: Vec::new(), + linker: false, }); } ensure!( diff --git a/src/util/rso.rs b/src/util/rso.rs index 63761b6e..379600a4 100644 --- a/src/util/rso.rs +++ b/src/util/rso.rs @@ -501,6 +501,7 @@ where section_known: false, splits: Default::default(), sub_regions: Vec::new(), + linker: false, }); if offset == 0 { total_bss_size += size; diff --git a/src/util/split.rs b/src/util/split.rs index 229357bf..a2080a75 100644 --- a/src/util/split.rs +++ b/src/util/split.rs @@ -661,10 +661,7 @@ fn split_extabindex(obj: &mut ObjInfo, start: SectionAddress) -> Result<()> { /// Create splits for gaps between existing splits. fn create_gap_splits(obj: &mut ObjInfo) -> Result<()> { let mut new_splits = BTreeMap::::new(); - // The linker writes a `linker` unit's bytes, so the gap after one belongs to - // the parent section, not to the table the unit covers. - let linker_units: HashSet<&str> = - obj.link_order.iter().filter(|u| u.linker).map(|u| u.name.as_str()).collect(); + let mut linker_units = HashSet::::new(); for (section_index, section) in obj.sections.iter() { let mut current_address = SectionAddress::new(section_index, section.address as u32); @@ -735,15 +732,21 @@ fn create_gap_splits(obj: &mut ObjInfo) -> Result<()> { new_split_end.address = r.end; } } + // The linker writes a `linker` region whole, so its gap is one unit + // regardless of the symbols inside. + let in_linker = section.is_linker_address(current_address.address); // Find any duplicate symbols in this range - let symbols = obj - .symbols - .for_section_range( - section_index, - current_address.address..new_split_end.address, - ) - .filter(|&(_, s)| !s.flags.is_stripped()) - .collect_vec(); + let symbols = if in_linker { + vec![] + } else { + obj.symbols + .for_section_range( + section_index, + current_address.address..new_split_end.address, + ) + .filter(|&(_, s)| !s.flags.is_stripped()) + .collect_vec() + }; let mut existing_symbols = HashSet::new(); for &(_, symbol) in &symbols { // Isolate comdat-flagged symbols into their own auto unit @@ -819,12 +822,15 @@ fn create_gap_splits(obj: &mut ObjInfo) -> Result<()> { }); let effective_rename = region_rename.or_else(|| prev_rename.clone()); let effective_section = effective_rename.as_deref().unwrap_or(§ion.name); - let unit = format!( - "auto_{:02}_{:08X}_{}", - current_address.section, - current_address.address, - effective_section.trim_start_matches('.').replace('$', "_") - ); + let section_tag = effective_section.trim_start_matches('.').replace('$', "_"); + let unit = if in_linker && linker_units.insert(format!("linker/{section_tag}")) { + format!("linker/{section_tag}") + } else { + format!( + "auto_{:02}_{:08X}_{}", + current_address.section, current_address.address, section_tag + ) + }; new_splits.insert( current_address, ObjSplit { @@ -842,11 +848,7 @@ fn create_gap_splits(obj: &mut ObjInfo) -> Result<()> { } if let Some((_, consumed_split)) = file_iter.next() { - prev_rename = if linker_units.contains(consumed_split.unit.as_str()) { - None - } else { - consumed_split.rename.clone() - }; + prev_rename = consumed_split.rename.clone(); } if split_end.address > 0 { current_address = split_end; @@ -1618,7 +1620,7 @@ fn resolve_link_order(obj: &ObjInfo) -> Result> { autogenerated: obj.is_unit_autogenerated(name), comment_version: None, order: None, - linker: false, + linker: obj.is_unit_linker(name), } } }) @@ -1883,6 +1885,7 @@ pub fn split_obj( section_known: true, splits: Default::default(), sub_regions: Vec::new(), + linker: false, }); }