diff --git a/src/client/cli.js b/src/client/cli.js index c5c279b..58e6d84 100644 --- a/src/client/cli.js +++ b/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}`); } diff --git a/src/server/verify.js b/src/server/verify.js index 16c9e71..d378628 100644 --- a/src/server/verify.js +++ b/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'; diff --git a/src/shared/check.js b/src/shared/check.js index 57af627..1d503e1 100644 --- a/src/shared/check.js +++ b/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, diff --git a/src/shared/compare.js b/src/shared/compare.js index 73f0cb3..3c2cb38 100644 --- a/src/shared/compare.js +++ b/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); diff --git a/test/compare.test.js b/test/compare.test.js index 0a4582c..39f5ebc 100644 --- a/test/compare.test.js +++ b/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'); +}); diff --git a/test/server.test.js b/test/server.test.js index adabb21..9a64c7a 100644 --- a/test/server.test.js +++ b/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', () => {