Bind annotations to functions by their debug-info definition line

The listing assigned each compiled function to the file of its first
source-line marker. When the optimizer schedules code inlined from a
header first (std::map::find, std::string::c_str, ...), that marker is
the header's, so a correctly compiled function was reported as "no
compiled function found after this annotation".

Checks now compile with /Z7, which puts CodeView debug info in the
object without changing codegen, and CoffObject.functionLines() reads
each function's definition file and line from its line table. The
binder uses that location and keeps the listing as the fallback for
objects without debug info; the listing still provides instruction
text and codeHash.

src/shared/build.js

@@ -38,8 +38,10 @@ export function fillPlaceholders(s, roots) {
 function clArgs({ cflags, includeDirs, src, out, toPath, roots }) {
   const flags = cflags.map((f) => fillPlaceholders(f, roots));
   const inc = includeDirs.map((d) => `/I${toPath(d)}`);
-  // /showIncludes lists every included header (dependency tracking); it does not change codegen.
+  // /showIncludes lists every included header (dependency tracking); /Z7 puts debug info in the
-  const compile = [...flags, ...inc, '/showIncludes', '/FAcs', `/Fa${toPath(out + '.cod')}`, `/Fo${toPath(out + '.obj')}`, toPath(src)];
+  // object, which tells where each function is defined (CoffObject.functionLines). Neither
+  // changes codegen.
+  const compile = [...flags, ...inc, '/showIncludes', '/Z7', '/FAcs', `/Fa${toPath(out + '.cod')}`, `/Fo${toPath(out + '.obj')}`, toPath(src)];
   const preprocess = [...flags.filter((f) => f.toLowerCase() !== '/c'), ...inc, '/P', `/Fi${toPath(out + '.i')}`, toPath(src)];
   return { compile, preprocess };
 }

src/shared/coff.js

@@ -53,6 +53,12 @@ const IMAGE_SCN_LNK_COMDAT = 0x00001000;
 const IMAGE_SCN_LNK_NRELOC_OVFL = 0x01000000;
 const IMAGE_SYM_DTYPE_FUNCTION = 0x20;
 
+// CodeView (C13) debug info, as /Z7 writes it into .debug$S.
+const CV_SIGNATURE_C13 = 4;
+const DEBUG_S_LINES = 0xf2;
+const DEBUG_S_STRINGTABLE = 0xf3;
+const DEBUG_S_FILECHKSMS = 0xf4;
+
 export class CoffObject {
   /** @param {Buffer} buf */
   constructor(buf) {
@@ -181,6 +187,53 @@ export class CoffObject {
     return out;
   }
 
+  /**
+   * Where each function is defined, from the CodeView line tables in .debug$S (compiled with /Z7):
+   * decorated name -> {file, line} of its first instruction. Code inlined into a function has
+   * records of its own, so this is the function's own definition even when the optimizer put
+   * inlined code first. Empty without debug info.
+   */
+  functionLines() {
+    const out = new Map();
+    const debug = this.sections.filter((s) => s.name === '.debug$S' && s.size >= 4 && s.data.readUInt32LE(0) === CV_SIGNATURE_C13);
+    const subsections = (s) => {
+      const list = [];
+      for (let p = 4; p + 8 <= s.size; ) {
+        const type = s.data.readUInt32LE(p);
+        const len = s.data.readUInt32LE(p + 4);
+        list.push({ type, off: p + 8, len });
+        p = (p + 8 + len + 3) & ~3;
+      }
+      return list;
+    };
+    let strings = null;
+    let checksums = null;
+    for (const s of debug) {
+      for (const sub of subsections(s)) {
+        if (sub.type === DEBUG_S_STRINGTABLE) strings ??= { data: s.data, off: sub.off };
+        if (sub.type === DEBUG_S_FILECHKSMS) checksums ??= { data: s.data, off: sub.off };
+      }
+    }
+    if (!strings || !checksums) return out;
+    const fileName = (fileId) => {
+      const start = strings.off + checksums.data.readUInt32LE(checksums.off + fileId);
+      return strings.data.toString('latin1', start, strings.data.indexOf(0, start));
+    };
+    for (const s of debug) {
+      const relocAt = new Map(s.relocs.map((r) => [r.offset, r]));
+      for (const sub of subsections(s)) {
+        // Header (offset, segment, flags, code size), then file blocks (file id, line count,
+        // block size) whose line entries (code offset, line) run in code order.
+        if (sub.type !== DEBUG_S_LINES || sub.len < 32) continue;
+        const sym = this.symbols[relocAt.get(sub.off)?.symbolIndex];
+        const block = sub.off + 12;
+        if (!sym || out.has(sym.name) || s.data.readUInt32LE(block + 4) === 0) continue;
+        out.set(sym.name, { file: fileName(s.data.readUInt32LE(block)), line: s.data.readUInt32LE(block + 16) & 0xffffff });
+      }
+    }
+    return out;
+  }
+
   /**
    * Bytes of a constant defined in this object (string literal ??_C@..., __real@/__xmm@/__mask@
    * floating-point and SIMD constants), or null. Each such constant is its own COMDAT section.

src/shared/compare.js

@@ -33,16 +33,21 @@ const MAX_DIFF_REGIONS = 24;
 // below the annotated signature; anything further is another function.
 const MAX_SIGNATURE_LINES = 12;
 
-export function bindAnnotations({ listing, file, annotations, nameIndex = new Map(), symbols = new Map(), sizes = new Map() }) {
+// `locations`: decorated name -> {file, line} of each function's definition, from the object's
+// debug info (CoffObject.functionLines). The listing's first source line belongs to whatever code
+// the optimizer scheduled first, which may be inlined from a header; it is only the fallback.
+export function bindAnnotations({ listing, file, annotations, nameIndex = new Map(), symbols = new Map(), sizes = new Map(), locations = new Map() }) {
   const byAnn = new Map();
   const skipped = new Set();
   const errors = [];
   // A compiled function whose decorated name is already known to be another ID is never taken.
   const ownerOf = (p) => nameIndex.get(p.symbol) ?? null;
 
-  // Procs per source file of the listing, and the annotation files they belong to.
+  // Procs per source file of their definition, and the annotation files they belong to.
   const procsByFile = new Map();
-  for (const p of listing.procs) {
+  for (const proc of listing.procs) {
+    const loc = locations.get(proc.symbol);
+    const p = loc ? { ...proc, sourceFile: normalizePath(loc.file), firstLine: loc.line } : proc;
     if (!p.sourceFile || p.firstLine === null) continue;
     if (!procsByFile.has(p.sourceFile)) procsByFile.set(p.sourceFile, []);
     procsByFile.get(p.sourceFile).push(p);
@@ -140,7 +145,7 @@ export function bindAnnotations({ listing, file, annotations, nameIndex = new Ma
 export function compareObject({ obj, listing, file, annotations, symbols, nameIndex = new Map(), pe }) {
   const objFns = new Map(obj.functions().map((f) => [f.name, f]));
   const sizes = new Map([...objFns].map(([n, f]) => [n, f.bytes.length]));
-  const found = bindAnnotations({ listing, file, annotations, nameIndex, symbols, sizes });
+  const found = bindAnnotations({ listing, file, annotations, nameIndex, symbols, sizes, locations: obj.functionLines() });
   const { byAnn, errors } = found;
   // A file-local static (s_ScriptFunctions, a function's static local) has the same name in every
   // unit that defines one: it is known by its object and name (Foo_Scripts.obj|s_ScriptFunctions),

test/compare.test.js

@@ -1,11 +1,11 @@
 import { test } from 'node:test';
 import assert from 'node:assert/strict';
-import { fixture, FN_ADDR, CALLEE_ADDR, GLOBAL_ADDR } from './helpers.js';
+import { fixture, buildCoff, buildDebugS, FN_ADDR, CALLEE_ADDR, GLOBAL_ADDR } from './helpers.js';
 import { PEImage } from '../src/shared/pe.js';
 import { CoffObject } from '../src/shared/coff.js';
 import { parseListing } from '../src/shared/listing.js';
 import { parseAnnotations } from '../src/shared/annotations.js';
-import { compareObject, STATUS, lcsRatio, sameOriginal } from '../src/shared/compare.js';
+import { compareObject, bindAnnotations, STATUS, lcsRatio, sameOriginal } from '../src/shared/compare.js';
 import { reconcile } from '../src/shared/reconcile.js';
 
 const FILE = 'src/player.cpp';
@@ -122,6 +122,68 @@ test('annotation with an ID that has no address in this version', () => {
   assert.equal(fn.status, STATUS.NO_ADDRESS);
 });
 
+// The optimizer may schedule code inlined from a header first: the listing then shows the
+// header's line before the function's own. Where the function is defined comes from its debug info.
+function inlinedHeaderFirst() {
+  const listing = parseListing(
+    [
+      '; File z:\\job\\src\\player.cpp',
+      '?Update@Player@@QAEXXZ PROC\t\t\t\t; Player::Update, COMDAT',
+      '; File z:\\toolchain\\include\\xtree',
+      '; 1597 :         _Tree_find_result<_Nodeptr> _Result{};',
+      '  00000\t8b 49 04\t mov\t ecx, DWORD PTR [ecx+4]',
+      '; File z:\\job\\src\\player.cpp',
+      '; 5    : {',
+      '  00003\tc3\t\t ret\t 0',
+      '?Update@Player@@QAEXXZ ENDP\t\t\t\t; Player::Update',
+      'END',
+    ].join('\r\n'),
+  );
+  const { annotations } = parseAnnotations(fixture().source, FILE);
+  return { listing, annotations };
+}
+
+test('an annotation binds to its function when inlined header code comes first', () => {
+  const { listing, annotations } = inlinedHeaderFirst();
+  const bind = (locations) => bindAnnotations({ listing, file: FILE, annotations, locations });
+  assert.equal(bind(new Map()).byAnn.size, 0, 'the listing alone puts the function in xtree');
+  const { byAnn, errors } = bind(new Map([['?Update@Player@@QAEXXZ', { file: 'z:\\job\\src\\player.cpp', line: 4 }]]));
+  assert.deepEqual(errors, []);
+  assert.deepEqual([...byAnn.values()], ['?Update@Player@@QAEXXZ']);
+});
+
+test('a function inlining code of the annotated file does not take its annotation', () => {
+  const { listing, annotations } = inlinedHeaderFirst();
+  // Defined in a header, it contains player.cpp's line 4 only through inlining.
+  const { byAnn } = bindAnnotations({ listing, file: FILE, annotations, locations: new Map([['?Update@Player@@QAEXXZ', { file: 'z:\\job\\src\\player.h', line: 30 }]]) });
+  assert.equal(byAnn.size, 0);
+});
+
+test('COFF reader takes each function\'s definition line from its CodeView line table', () => {
+  const debug = buildDebugS([
+    { symbolIndex: 1, file: 'z:\\job\\src\\player.cpp', lines: [[0, 4], [3, 6], [8, 7]] },
+    { symbolIndex: 2, file: 'z:\\job\\src\\other.cpp', lines: [[0, 12]] },
+  ]);
+  const obj = new CoffObject(
+    buildCoff({
+      sections: [
+        { name: '.text$mn', data: Buffer.alloc(16, 0xc3), characteristics: 0x60501020 },
+        { name: '.debug$S', data: debug.data, relocs: debug.relocs, characteristics: 0x42100040 },
+      ],
+      symbols: [
+        { name: '.text$mn', section: 1, storageClass: 3 },
+        { name: '?Update@Player@@QAEXXZ', section: 1, type: 0x20, storageClass: 2 },
+        { name: '?Other@@YAXXZ', section: 1, value: 8, type: 0x20, storageClass: 2 },
+      ],
+    }),
+  );
+  assert.deepEqual(Object.fromEntries(obj.functionLines()), {
+    '?Update@Player@@QAEXXZ': { file: 'z:\\job\\src\\player.cpp', line: 4 },
+    '?Other@@YAXXZ': { file: 'z:\\job\\src\\other.cpp', line: 12 },
+  });
+  assert.equal(new CoffObject(fixture().obj).functionLines().size, 0, 'no debug info, no locations');
+});
+
 test('lcsRatio', () => {
   assert.equal(lcsRatio(Buffer.from('abcd'), Buffer.from('abcd')), 1);
   assert.equal(lcsRatio(Buffer.from('abcd'), Buffer.from('abxd')), 0.75);

test/helpers.js

@@ -71,6 +71,50 @@ export function buildCoff({ sections, symbols }) {
   return Buffer.concat([header, ...secHeaders, ...body, ...symTable, strTable]);
 }
 
+/**
+ * A CodeView (C13) .debug$S section with one line table per function, as /Z7 writes it.
+ * @param {Array<{symbolIndex: number, file: string, lines: Array<[offset: number, line: number]>}>} functions
+ * @returns {{data: Buffer, relocs: Array<{offset, symbolIndex, type}>}}
+ */
+export function buildDebugS(functions) {
+  const files = [...new Set(functions.map((f) => f.file))];
+  const strings = Buffer.concat([Buffer.alloc(1), ...files.map((f) => Buffer.from(f + '\0', 'latin1'))]);
+  const nameOffset = (f) => 1 + files.slice(0, files.indexOf(f)).reduce((n, s) => n + s.length + 1, 0);
+  // File checksum entries: name offset, checksum size 0, kind 0, padded to 4 bytes.
+  const checksums = Buffer.concat(files.map((f) => { const b = Buffer.alloc(8); b.writeUInt32LE(nameOffset(f), 0); return b; }));
+  const fileId = (f) => files.indexOf(f) * 8;
+
+  const parts = [Buffer.from([4, 0, 0, 0])];
+  const relocs = [];
+  let at = 4;
+  const subsection = (type, body) => {
+    const head = Buffer.alloc(8);
+    head.writeUInt32LE(type, 0);
+    head.writeUInt32LE(body.length, 4);
+    const pad = Buffer.alloc((4 - (body.length % 4)) % 4);
+    parts.push(head, body, pad);
+    const bodyAt = at + 8;
+    at += 8 + body.length + pad.length;
+    return bodyAt;
+  };
+  for (const fn of functions) {
+    const body = Buffer.alloc(12 + 12 + fn.lines.length * 8);
+    body.writeUInt32LE(Math.max(...fn.lines.map(([o]) => o)) + 1, 8);
+    body.writeUInt32LE(fileId(fn.file), 12);
+    body.writeUInt32LE(fn.lines.length, 16);
+    body.writeUInt32LE(12 + fn.lines.length * 8, 20);
+    fn.lines.forEach(([offset, line], i) => {
+      body.writeUInt32LE(offset, 24 + i * 8);
+      body.writeUInt32LE((line | 0x80000000) >>> 0, 28 + i * 8);
+    });
+    const bodyAt = subsection(0xf2, body);
+    relocs.push({ offset: bodyAt, symbolIndex: fn.symbolIndex, type: 0x0b }, { offset: bodyAt + 4, symbolIndex: fn.symbolIndex, type: 0x0a });
+  }
+  subsection(0xf3, strings);
+  subsection(0xf4, checksums);
+  return { data: Buffer.concat(parts), relocs };
+}
+
 /**
  * @param {{imageBase?: number, sections: Array<{name, rva, data, characteristics?}>, baseRelocs?: number[]}} p
  *        baseRelocs are virtual addresses of HIGHLOW fixups.