Refuse annotating a destructor with its deleting destructor

When a // FUNCTION: annotation binds a ??1 destructor to an original function that is a scalar deleting destructor (tests flags&1 of its argument, calls operator delete, returns this with retn 4), the submission goes back with guidance to claim the function no-decompile (compiler_generated). Only the submission target is checked, so existing code on main is unaffected. td check prints the same hint.

src/client/cli.js

@@ -70,6 +70,7 @@ function printCheck(r) {
     for (const e of [...u.lintErrors, ...u.errors]) console.log(`  ! ${e.message}`);
     for (const f of u.functions) {
       console.log(`  ${f.id} ${f.addr ?? '?'} ${(f.name ?? '').padEnd(30)} ${f.status.padEnd(17)} ${(f.score * 100).toFixed(1)}%  size ${f.size?.ours ?? '-'}/${f.size?.original ?? '?'}`);
+      if (f.deletingDestructor) console.log(`      ${f.id} is a scalar deleting destructor in the original: remove this annotation and claim it no-decompile (compiler_generated)`);
       for (const d of f.diffRegions ?? []) console.log(`      +0x${d.offset.toString(16)}: ours ${d.ours} orig ${d.original}  ${d.ourInstruction ?? ''}${d.sourceLine ? ` (line ${d.sourceLine})` : ''}`);
       for (const x of f.relocations ?? []) console.log(`      reloc +0x${x.offset.toString(16)} ${x.symbol}: ${x.ok ? `not yet named, original points at 0x${hex(x.actual)}` : x.reason}`);
     }

src/server/verify.js

@@ -792,6 +792,9 @@ export class Verifier {
     } else if (!target) {
       feedback.summary = `no compiled function is annotated with // FUNCTION: ${targetId}`;
       state = 'needs_cleanup';
+    } else if (target.deletingDestructor) {
+      feedback.summary = `${targetId} is a scalar deleting destructor in the original (it frees the object when flags&1 and returns this, retn 4), compiler generated from the virtual destructor: do not annotate the destructor with it; claim it no-decompile (td_mark_no_decompile, category compiler_generated)`;
+      state = 'needs_cleanup';
     } else if (regressions.length) {
       feedback.summary = `your change breaks ${regressions.length} function(s) that matched before`;
       state = 'needs_cleanup';

src/shared/check.js

@@ -128,6 +128,7 @@ export function summarizeCheck(result, rec, files = null) {
         size: f.size,
         diffRegions: f.diffRegions?.slice(0, 12),
         relocations: f.relocations,
+        ...(f.deletingDestructor && { deletingDestructor: true }),
       })),
     })),
     referenceConflicts: rec.conflicts,

src/shared/compare.js

@@ -180,7 +180,8 @@ export function compareObject({ obj, listing, file, annotations, symbols, nameIn
     const r = compareFunction({ fn: withLocalNames(fn), addr: sym.addr, origSize: sym.size, pe, resolve, symbols, proc, literalOf: (s) => obj.literal(s) });
     inferred.push(...r.inferred);
     delete r.inferred;
-    functions.push({ ...base, addr: sym.addr, ...r, asm: procText(proc) });
+    const deletingDestructor = /^\?\?1/.test(symbol) && isDeletingDestructor(pe, sym.addr, sym.size);
+    functions.push({ ...base, addr: sym.addr, ...r, ...(deletingDestructor && { deletingDestructor: true }), asm: procText(proc) });
   }
 
   const boundSymbols = new Set([...bindings.keys(), ...byAnn.values()]);
@@ -199,6 +200,21 @@ function isCompilerGenerated(name) {
   return /^\?\?_[GEH]|^__|^\$|^_\$E/.test(name);
 }
 
+// MSVC's scalar deleting destructor: tests bit 0 of its flags argument ([ebp+8] or [esp+4/8]),
+// calls operator delete on it, and returns this with `retn 4`. A plain destructor takes no argument.
+export function isDeletingDestructor(pe, addr, size) {
+  if (!size || size < 8) return false;
+  let code;
+  try {
+    code = pe.read(addr, size);
+  } catch {
+    return false;
+  }
+  const tail = code.subarray(code.length - 3);
+  if (tail[0] !== 0xc2 || tail[1] !== 0x04 || tail[2] !== 0x00) return false;
+  return [[0xf6, 0x45, 0x08, 0x01], [0xf6, 0x44, 0x24, 0x04, 0x01], [0xf6, 0x44, 0x24, 0x08, 0x01]].some((sig) => code.indexOf(Buffer.from(sig)) >= 0);
+}
+
 function dedupeInferred(list) {
   const m = new Map();
   for (const x of list) m.set(x.symbol, x);

test/compare.test.js

@@ -5,7 +5,7 @@ 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, bindAnnotations, STATUS, lcsRatio, sameOriginal } from '../src/shared/compare.js';
+import { compareObject, bindAnnotations, STATUS, lcsRatio, sameOriginal, isDeletingDestructor } from '../src/shared/compare.js';
 import { reconcile } from '../src/shared/reconcile.js';
 
 const FILE = 'src/player.cpp';
@@ -226,3 +226,17 @@ test('sameOriginal: two copies of a function match when their calls reach the sa
   assert.equal(sameOriginal(pe, 0x1000, 0x1100, 12), false, 'an immediate differs');
   assert.equal(sameOriginal(pe, 0x1000, 0x1100, 0), false, 'no size, no claim');
 });
+
+test('isDeletingDestructor tells scalar deleting destructors from plain ones', () => {
+  const at = (hexBytes) => {
+    const bytes = Buffer.from(hexBytes, 'hex');
+    return { pe: { read: (va, len) => bytes.subarray(va - 0x1000, va - 0x1000 + len) }, size: bytes.length };
+  };
+  // CSurfaceDX9 at 0x5708F0: test [ebp+8], 1 / operator delete / retn 4.
+  const deleting = at('558bec6aff689de2eb0064a100000000505156a1981b360133c5508d45f464a3000000008bf18975f0c70634d60d01c745fc000000008b4e1085c9740d8b0151ff5008c7461000000000c745fcffffffff8bcee8281a0000f6450801740b6a1456e829198f0083c4088bc68b4df464890d00000000595e8be55dc20400');
+  assert.equal(isDeletingDestructor(deleting.pe, 0x1000, deleting.size), true);
+  // ??1CFile@@UAE@XZ: a plain destructor (no flags argument).
+  const plain = at('558bec6aff68c096eb0064a100000000505156a1981b360133c5508d45f464a3000000008bf18975f0c70604960d01c745fc00000000e8d5960500c745fcffffffff8d4e14e82676fdff8b4df464890d00000000595e8be55dc3');
+  assert.equal(isDeletingDestructor(plain.pe, 0x1000, plain.size), false);
+  assert.equal(isDeletingDestructor(deleting.pe, 0x1000, 0), false, 'unknown size');
+});

test/server.test.js

@@ -209,6 +209,9 @@ test('evaluate routes results', () => {
   assert.equal(ev(check(withRef), { ...rec, conflicts: [{ symbol: '?Foo@@YAXXZ', hard: true, message: 'Foo is elsewhere' }] }).state, 'needs_cleanup');
   assert.equal(ev(check(withRef), { ...rec, conflicts: [{ symbol: '?Bar@@YAXXZ', hard: true, message: 'Bar is elsewhere' }] }).state, 'approved');
   assert.equal(ev(check(withRef), { ...rec, conflicts: [{ symbol: '?Foo@@YAXXZ', message: 'Foo seen twice' }] }).state, 'approved');
+  const deleting = ev(check({ ...fn(STATUS.MISMATCH, 0.4), deletingDestructor: true }));
+  assert.equal(deleting.state, 'needs_cleanup', 'a deleting destructor annotated on a destructor goes back');
+  assert.match(deleting.feedback.summary, /no-decompile/);
 });
 
 test('regressions are detected against func_status', () => {