coff split: mark linker-written regions in Sections:, not as units

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<OutputRange> {
     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(&section.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 {

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) {

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<i32>,
     /// 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

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<ObjSectionKind>,
+    /// 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<ObjSubRegion>,
+    /// The linker writes this whole section (see [`ObjSubRegion::linker`]).
+    pub linker: bool,
 }
 
 #[derive(Debug, Clone)]
@@ -191,6 +196,12 @@ impl IndexMut<SectionIndex> 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!(

src/util/coff.rs

@@ -125,6 +125,7 @@ pub fn process_coff(data: &[u8], name: &str) -> Result<(ObjInfo, Option<u32>)> {
             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();

src/util/config.rs

@@ -511,18 +511,25 @@ 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 &section.sub_regions {
-            writeln!(
+            write!(
                 w,
                 "\t{:<11} type:{} vaddr:{:#010X} end:{:#010X}",
                 region.name,
                 section_kind_to_str(region.kind.unwrap_or(section.kind)),
                 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<u8>,
     /// Influences the order of this unit relative to other ordered units.
     order: Option<i32>,
-    /// The linker writes this unit's bytes (`linker` attribute).
-    linker: bool,
 }
 
 pub struct SectionDef {
@@ -629,6 +631,10 @@ pub struct SectionDef {
     /// `unit:<name>` — owning unit for a `type:comment` range; its `.drectve` is
     /// emitted into that unit's object (else a shared object).
     pub unit: Option<String>,
+    /// `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<SplitLine> {
     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<SplitLine> {
                 _ => 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<SplitLin
             end: None,
             comment: false,
             unit: None,
+            linker: false,
         };
 
         for attr in captures["attrs"].split(' ').filter(|&s| !s.is_empty()) {
@@ -720,7 +723,10 @@ fn parse_section_line(captures: Captures, state: &SplitState) -> Result<SplitLin
                     _ => 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());
     }
 }

src/util/dol.rs

@@ -384,6 +384,7 @@ pub fn process_dol(buf: &[u8], name: &str) -> Result<ObjInfo> {
                 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<ObjInfo> {
                 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<ObjInfo> {
                 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<ObjInfo> {
                 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<ObjInfo> {
                         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<ObjInfo> {
                         section_known: false,
                         splits: Default::default(),
                         sub_regions: Vec::new(),
+                        linker: false,
                     });
                 }
                 n => bail!("Invalid number of BSS sections: {}", n),

src/util/elf.rs

@@ -104,6 +104,7 @@ pub fn process_elf(path: &Utf8NativePath) -> Result<ObjInfo> {
             section_known: true,
             splits: Default::default(),
             sub_regions: Vec::new(),
+            linker: false,
         });
     }