feat(coff): per-function sections, honor scope:local

docs/coff.md

@@ -0,0 +1,94 @@
+# COFF / PE support (prototype)
+
+Notes on behavior specific to the `dtk coff` subcommand family (x86 PE
+executables split into MSVC-style COFF objects).
+
+## Object file layout
+
+By default (`function_sections: true` in `config.yml`), each function in a
+unit is emitted as its own `.text` section, mirroring MSVC `/Gy`
+(function-level linking). This gives tooling (objdiff, `dumpbin`,
+`llvm-readobj`) exact per-function section sizes instead of sizes inferred
+from symbol gaps, which would include inter-function padding.
+
+Details:
+
+- COFF permits multiple sections with the same name in one object; lld-link
+  concatenates them in section-table order, so relinked output is unchanged.
+- Inter-function padding bytes (`int3`/`nop`) and trailing jump tables stay
+  attached to the preceding function's section. Non-first sections have
+  alignment 1, so the linker inserts no padding between them and the byte
+  stream is preserved.
+- Sections are not COMDAT: dtk deduplicates symbol names globally at split
+  time, so COMDAT selection semantics are unnecessary.
+
+Set `function_sections: false` to emit one section per split range instead.
+
+## Symbol visibility
+
+- `scope:local` in `symbols.txt` is honored even with `export_all: true`
+  (the default): the symbol is emitted as `IMAGE_SYM_CLASS_STATIC`.
+- If a `scope:local` symbol is referenced from another unit and
+  `globalize_symbols` is enabled (the default), it is renamed to
+  `<name>_<addr>` and promoted to external automatically.
+- With `globalize_symbols: false`, a `scope:local` symbol referenced
+  cross-unit will fail to link (undefined symbol) — same contract as the
+  ELF pipeline.
+- `noexport` still forces a symbol static regardless of scope.
+
+## symbols.txt / splits.txt changes
+
+Symbol names, scopes, and splits are baked into the emitted objects at
+`dtk coff split` time. After editing `symbols.txt` or `splits.txt`, re-run
+`dtk coff split` to regenerate the objects.
+
+## Non-contiguous units
+
+A unit may be listed in `splits.txt` with multiple, non-adjacent ranges of
+the same section (e.g. two `.text` ranges). This is supported for diffing:
+the unit's object receives one section per range.
+
+Limitations:
+
+- Link order is resolved from address adjacency; interleaved units create
+  cycles that are broken heuristically. A warning names any unit with
+  non-contiguous ranges.
+- A relinked image cannot be byte-identical to the original in this case,
+  since the linker emits one object's sections adjacently. Multi-section
+  ranges (`.text` + `.data` + `.bss` for one unit) are unaffected — that is
+  the normal case and fully supported.
+
+## Header-slack comment strings (`comment_regions`)
+
+MSVC's `#pragma comment(exestr, …)` / Intel `-?comment:"…"` directives cause the
+original linker to embed comment strings into the PE header slack (between the
+section table and the first raw section). These strings survive in the linked
+image but the `.drectve` directives that produced them are gone.
+
+Declare the byte ranges holding them per image (base exe or a DLL module):
+
+```yaml
+comment_regions:
+- offset: 912      # file offset in the source PE
+  size: 882        # byte length
+```
+
+At split time dtk lifts those bytes, splits them on NUL, and re-emits each run
+as a `-?comment:"…"` directive in a `.drectve` section of a generated
+`auto_comments` object (appended to the link order / `objs.rsp`). Bytes are
+emitted verbatim — a run truncated in the source image stays truncated.
+
+Notes:
+
+- Default is empty (feature off): no comment object is emitted and output is
+  byte-identical to a build without the field.
+- Requires a linker that accepts `-?comment` in `.drectve`. Stock `lld-link`
+  rejects it (`is not allowed in .drectve`); reproducing the strings needs a
+  linker with comment-embedding support.
+
+## Known limitations
+
+- objdiff may display function signatures as `void(void)`: COFF symbols
+  carry no parameter or return type information (only the DTYPE_FUNCTION
+  bit). Real signatures live in CodeView debug data, which dtk does not
+  emit. This is cosmetic and does not affect diffing.

src/analysis/x86.rs

@@ -345,8 +345,7 @@ pub fn analyze_x86_functions(obj: &mut ObjInfo) -> Result<X86FunctionSizeData> {
                                                 ObjSymbolKind::Function,
                                             )?
                                             .is_some();
-                                    if is_tail_call && inside_known_function(obj, tgt_sec, target)
-                                    {
+                                    if is_tail_call && inside_known_function(obj, tgt_sec, target) {
                                         // Tail JMP into the body of a known function:
                                         // emit the reference (resolves to the containing
                                         // function + addend) but don't mint a sub-entry
@@ -589,14 +588,7 @@ pub fn analyze_x86_functions(obj: &mut ObjInfo) -> Result<X86FunctionSizeData> {
                             .kind_at_section_address(sec_idx, operand_va, ObjSymbolKind::Function)?
                             .is_some();
                         if !already && emit && !on_symbol_start {
-                            add_rel32(
-                                obj,
-                                sec_idx,
-                                operand_va,
-                                tgt_sec,
-                                target,
-                                &mut sweep_count,
-                            )?;
+                            add_rel32(obj, sec_idx, operand_va, tgt_sec, target, &mut sweep_count)?;
                         }
                     }
                 }
@@ -807,14 +799,12 @@ fn is_within_decoded_span(
 /// its declared size. The reference itself is still emitted as a relocation
 /// against the containing function (with an addend).
 fn inside_known_function(obj: &ObjInfo, sec: SectionIndex, va: u32) -> bool {
-    obj.symbols
-        .for_section_range(sec, ..=va)
-        .any(|(_, s)| {
-            s.kind == ObjSymbolKind::Function
-                && s.size_known
-                && (s.address as u32) < va
-                && va < (s.address as u32).wrapping_add(s.size as u32)
-        })
+    obj.symbols.for_section_range(sec, ..=va).any(|(_, s)| {
+        s.kind == ObjSymbolKind::Function
+            && s.size_known
+            && (s.address as u32) < va
+            && va < (s.address as u32).wrapping_add(s.size as u32)
+    })
 }
 
 /// Returns `true` if `va` decodes as a valid (non-invalid) instruction.

src/cmd/coff.rs

@@ -30,14 +30,17 @@ use crate::{
         shasum::file_sha1_string,
     },
     obj::{
-        ObjInfo, ObjKind, ObjRelocKind, ObjSymbol, ObjSymbolFlags, ObjSymbolFlagSet,
-        ObjSymbolKind, ObjSymbolScope, SectionIndex, SymbolIndex, best_match_for_reloc,
+        ObjInfo, ObjKind, ObjRelocKind, ObjSymbol, ObjSymbolFlagSet, ObjSymbolFlags, ObjSymbolKind,
+        ObjSymbolScope, SectionIndex, SymbolIndex, best_match_for_reloc,
     },
     util::{
-        coff::{apply_base_relocations, create_function_splits, process_coff, write_coff},
+        coff::{
+            apply_base_relocations, create_function_splits, extract_comment_directives,
+            process_coff, write_coff,
+        },
         config::{
             apply_splits_file, apply_symbols_file, create_auto_symbol_name, is_auto_symbol,
-            write_splits_file, write_symbols_file,
+            read_comment_regions, write_splits_file, write_symbols_file,
         },
         dep::DepFile,
         file::{FileReadInfo, buf_writer, process_rsp, touch, verify_hash},
@@ -555,11 +558,31 @@ fn load_coff_module(
         let pe_header = PeHeaderInfo::parse(data);
         let (mut obj, image_base) = process_coff(data, config.name())?;
         detect_pe_symbols(&mut obj, data)?;
+        // Extract exestr comments declared as `type:comment` ranges in the
+        // splits file's Sections block, re-emitted later as `.drectve` directives.
+        if let Some(splits) = &config.splits {
+            let regions = comment_regions_from_splits(&splits.with_encoding(), image_base)?;
+            extract_comment_directives(data, &regions, &mut obj, config.name())?;
+        }
         (obj, image_base, pe_header)
     };
     Ok((obj, image_base, pe_header, object_path))
 }
 
+/// Resolve `type:comment` entries in the splits file's Sections block to
+/// `(file_offset, size)` ranges. Header padding maps 1:1, so the file offset is
+/// `vaddr - image_base`.
+fn comment_regions_from_splits(
+    splits_path: &Utf8NativePath,
+    image_base: Option<u32>,
+) -> Result<Vec<(u32, u32, Option<String>)>> {
+    let base = image_base.unwrap_or(0);
+    Ok(read_comment_regions(splits_path)?
+        .into_iter()
+        .map(|(start, end, unit)| (start - base, end - start, unit))
+        .collect())
+}
+
 /// Import authoritative symbol sizes from one prebuilt verbatim library object.
 ///
 /// The object's symbol layout (sizes inferred as the gap to the next symbol in
@@ -868,7 +891,8 @@ fn load_analyze_coff(
                     Some(s) => s,
                     None => continue,
                 };
-                if unit_at(&obj, sec_idx, reloc_addr) != unit_at(&obj, tgt_sec, sym.address as u32) {
+                if unit_at(&obj, sec_idx, reloc_addr) != unit_at(&obj, tgt_sec, sym.address as u32)
+                {
                     to_remove.push((sec_idx, reloc_addr));
                 }
             }
@@ -878,7 +902,10 @@ fn load_analyze_coff(
             obj.sections[sec_idx].relocations.remove(addr);
         }
         if dropped > 0 {
-            info!("{}: dropped {dropped} cross-unit relocation(s) to non-exportable targets", obj.name);
+            info!(
+                "{}: dropped {dropped} cross-unit relocation(s) to non-exportable targets",
+                obj.name
+            );
         }
     }
 
@@ -1102,8 +1129,10 @@ fn split_write_coff(
     };
 
     // Serialize all split objects in parallel (CPU-bound), then write serially.
-    let serialized: Vec<Result<Vec<u8>>> =
-        split_objs.par_iter().map(|split_obj| write_coff(split_obj, config.export_all)).collect();
+    let serialized: Vec<Result<Vec<u8>>> = split_objs
+        .par_iter()
+        .map(|split_obj| write_coff(split_obj, config.export_all, config.function_sections))
+        .collect();
 
     // Serial bookkeeping (path dedup, unit order, directory creation), then write
     // the objects in parallel — writing 18k+ files dominates the split otherwise.
@@ -1139,6 +1168,48 @@ fn split_write_coff(
         }
         pending_writes.push((out_path, out_obj));
     }
+
+    // Comments attributed to a unit are emitted into that unit's object by
+    // write_coff. Unattributed comments go in a shared `.drectve`-only object,
+    // appended to the link order so it lands in objs.rsp.
+    let unattributed: Vec<&[u8]> = module
+        .obj
+        .pe_comment_directives
+        .iter()
+        .filter(|(unit, _)| unit.is_none())
+        .map(|(_, bytes)| bytes.as_slice())
+        .collect();
+    if let Some(comment_obj) = crate::util::coff::write_coff_comments(&unattributed)? {
+        let unit_name = "auto_comments".to_string();
+        let obj_path = obj_path_for_unit(&unit_name);
+        let out_path = obj_dir.join(&obj_path);
+        if object_paths.contains_key(&obj_path) {
+            bail!("Comment object path {} collides with an existing unit", out_path);
+        }
+        if let Some(parent) = out_path.parent() {
+            if created_dirs.insert(parent.to_owned()) {
+                DirBuilder::new().recursive(true).create(parent)?;
+            }
+        }
+        // Link the shared comment object first: its comments sit at the lowest
+        // header addresses (before any code), so a comment-aware linker embeds
+        // them ahead of any per-unit `.drectve`, preserving header address order.
+        out_config.units.insert(0, OutputUnit {
+            object: out_path.with_unix_encoding(),
+            name: unit_name.clone(),
+            autogenerated: true,
+            code_size: 0,
+            data_size: 0,
+        });
+        module.obj.link_order.insert(0, crate::obj::ObjUnit {
+            name: unit_name,
+            autogenerated: true,
+            comment_version: None,
+            order: None,
+        });
+        pending_writes.push((out_path, comment_obj));
+    }
+
     pending_writes.par_iter().try_for_each(|(path, data)| write_if_changed(path, data))?;
 
     // Generate args.rsp (flags) and objs.rsp (object file list)

src/cmd/dol.rs

@@ -248,6 +248,10 @@ pub struct ProjectConfig {
     /// Promotes local symbols referenced by other units to global.
     #[serde(default = "bool_true", skip_serializing_if = "is_true")]
     pub globalize_symbols: bool,
+    /// Emit each function as its own section in COFF objects (MSVC /Gy style),
+    /// so tooling (objdiff, dumpbin) sees exact per-function sizes.
+    #[serde(default = "bool_true", skip_serializing_if = "is_true")]
+    pub function_sections: bool,
     /// Optional base path for all object files.
     #[serde(with = "unix_path_serde_option", default, skip_serializing_if = "is_default")]
     pub object_base: Option<Utf8UnixPathBuf>,
@@ -286,6 +290,7 @@ impl Default for ProjectConfig {
             dead_strip: false,
             export_all: true,
             globalize_symbols: true,
+            function_sections: true,
             object_base: None,
             extract_objects: true,
             x86_signatures: None,

src/obj/mod.rs

@@ -12,11 +12,11 @@ use std::{
 
 use anyhow::{Result, anyhow, bail, ensure};
 use objdiff_core::obj::split_meta::SplitMeta;
-use serde::{Deserialize, Serialize};
 pub use relocations::{ObjReloc, ObjRelocKind, ObjRelocations};
 pub use sections::{
     ObjSection, ObjSectionKind, ObjSections, ObjSubRegion, SectionIndex, section_kind_for_section,
 };
+use serde::{Deserialize, Serialize};
 pub use splits::{ObjSplit, ObjSplits};
 pub use symbols::{
     ObjDataKind, ObjSymbol, ObjSymbolFlagSet, ObjSymbolFlags, ObjSymbolKind, ObjSymbolScope,
@@ -111,6 +111,18 @@ pub struct ObjInfo {
     /// absolute relocations. Not emitted as a section; the linker regenerates it.
     pub pe_reloc_data: Vec<u8>,
 
+    /// Compiler comment strings extracted from the PE header slack via
+    /// `type:comment` splits. Each entry is `(owning unit, literal bytes)`;
+    /// emitted as an `-?comment:"..."` directive in a `.drectve` section of the
+    /// owning unit's object (or a shared object when the unit is `None`). Not a
+    /// real section.
+    pub pe_comment_directives: Vec<(Option<String>, Vec<u8>)>,
+
+    /// `type:comment` split declarations `(name, start_va, end_va, owning unit)`,
+    /// retained so `dtk coff split` round-trips them into the regenerated splits
+    /// file. A `None` unit is a shared `Sections:` entry; `Some` is under a unit.
+    pub pe_comment_sections: Vec<(String, u32, u32, Option<String>)>,
+
     /// Original PE header metadata, retained so the post-link patch can reproduce
     /// fields the relinker doesn't. Only set for COFF/PE images.
     pub pe_metadata: Option<PeMetadata>,
@@ -147,6 +159,8 @@ impl ObjInfo {
             module_id: 0,
             unresolved_relocations: vec![],
             pe_reloc_data: Vec::new(),
+            pe_comment_directives: Vec::new(),
+            pe_comment_sections: Vec::new(),
             pe_metadata: None,
         }
     }