From 64b892b2b09f00c0e2df6fa6c53a7aec0677a6ed Mon Sep 17 00:00:00 2001 From: ragnarok-clanker Date: Mon, 26 Oct 2026 18:55:10 +0100 Subject: [PATCH] Maintenance batches compare linked code, not symbol names effectiveHash fingerprints a function by its bytes and where each reference lands (symbol address + addend). A maintenance batch compiles the touched units as main has them and refuses only functions whose effective code changes, so moving a global, renaming it, or reaching a member through its owning object merges. --- src/server/maintenance.js | 22 +++++++++++++++------- src/shared/compare.js | 16 ++++++++++++++++ test/server.test.js | 29 +++++++++++++++++++++-------- 3 files changed, 52 insertions(+), 15 deletions(-) diff --git a/src/server/maintenance.js b/src/server/maintenance.js index 7ceca41..a69f692 100644 --- a/src/server/maintenance.js +++ b/src/server/maintenance.js @@ -86,7 +86,7 @@ export class Maintenance { await this.verifier.refreshBaselines(check, merged.files, `maintenance-${batch.id}`); const regressions = this.verifier.regressions(check, merged.files); if (regressions.length) return back(`breaks ${regressions.length} function(s) that matched before`, { regressions }); - const changed = this.changedFunctions(check); + const changed = await this.changedFunctions(check, merged.files, startHead); if (changed.length) { return back(`changes the compiled code of ${changed.length} function(s): that is function work; submit those functions with their tasks`, { changedFunctions: changed.slice(0, 200) }); } @@ -111,16 +111,24 @@ export class Maintenance { return { state: 'merged', commit, result: { units: check.units.length, changedHeaders } }; } - /** Functions whose compiled code differs from what main recorded for them. */ - changedFunctions(check) { + /** + * Functions whose code, once linked, differs from main's. Main's units that the change touches + * are compiled as main has them, and every function is compared on where its references land + * (effectiveHash): moving a global, renaming it, or reaching a member through its object's symbol + * changes names, not code. + */ + async changedFunctions(check, files, head) { + const touched = check.units.map((u) => u.file).filter((f) => this.repo.show(head, f) !== null); + const before = touched.length ? (await this.verifier.check({}, `maintenance-base-${head.slice(0, 10)}`, { onlyUnits: touched, lint: false })).check : { units: [] }; + const was = new Map([...bestResults(before)].map(([id, b]) => [id, b.f])); const out = []; for (const [id, { f, unit }] of bestResults(check)) { - const main = this.store.get('SELECT code_hash, status, score FROM func_status WHERE symbol_id = ?', id); - if (!main) { - if (f.codeHash) out.push({ id, file: unit, why: 'not compiled on main before' }); + const old = was.get(id); + if (!old) { + out.push({ id, file: unit, why: 'not compiled on main' }); continue; } - if (main.code_hash && f.codeHash && main.code_hash !== f.codeHash) out.push({ id, file: unit, why: 'compiled code changed' }); + if (old.effectiveHash !== f.effectiveHash) out.push({ id, file: unit, why: 'compiled code changed' }); } return out; } diff --git a/src/shared/compare.js b/src/shared/compare.js index 7138f33..7d7e83e 100644 --- a/src/shared/compare.js +++ b/src/shared/compare.js @@ -244,6 +244,9 @@ export function compareFunction({ fn, addr, origSize, pe, resolve, symbols, proc const orig = pe.read(addr, compareLen); const masked = new Uint8Array(compareLen); const relocResults = []; + // What each reference reaches: a symbol's address plus the addend, so `g_object + 0x18` and a + // name bound to that same address count as the same code (effectiveHash). + const effective = []; const inferred = []; const baseRelocs = pe.baseRelocations(); @@ -295,6 +298,7 @@ export function compareFunction({ fn, addr, origSize, pe, resolve, symbols, proc if (!isSection) inferred.push({ symbol: symName, addr: actual }); } relocResults.push({ offset: r.offset, symbol: symName, id, ok, unresolved: ok && !(target && target.addr != null), actual, reason }); + effective.push(`${r.offset}:${r.kind}:${target && target.addr != null ? wrap(target.addr + addend) : `${symName}+${addend}`}`); } const sameLength = !origSize || origSize === ours.length; @@ -333,6 +337,7 @@ export function compareFunction({ fn, addr, origSize, pe, resolve, symbols, proc score: Math.round(score * 10000) / 10000, size: { ours: ours.length, original: origSize ?? null }, codeHash: codeHash(ours, proc), + effectiveHash: effectiveHash(ours, masked, proc, effective), diffRegions: sameLength ? diffRegions(diffs, ours, orig, proc) : [], relocations: relocResults.filter((r) => !r.ok || r.unresolved), // Where the original points is only meaningful when the code lines up. @@ -450,3 +455,14 @@ export function codeHash(bytes, proc) { } return crypto.createHash('sha1').update(b).digest('hex').slice(0, 20); } + +/** + * Fingerprint of what a function does once linked: its bytes outside relocated fields (as + * codeHash) plus where each reference lands. Renaming a global, or reaching a member of an object + * through the object's symbol instead of a symbol of its own at that address, leaves it unchanged. + */ +export function effectiveHash(bytes, masked, proc, effective) { + const b = Buffer.from(bytes); + for (let i = 0; i < b.length; i++) if (masked[i]) b[i] = 0; + return crypto.createHash('sha1').update(codeHash(b, proc)).update(effective.join('|')).digest('hex').slice(0, 20); +} diff --git a/test/server.test.js b/test/server.test.js index d717e6d..b28d2dc 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -1351,12 +1351,18 @@ test('maintenance batches: maintainers only; merge when no compiled code changes ctx.store.run(`INSERT INTO func_status (symbol_id, file, status, score, commit_hash, updated_at, confirmed, code_hash) VALUES (?, ?, 'mismatch', 0.7, 'x', ?, 1, 'mf-main')`, fid, unit, now()); const realCheck = ctx.verifier.check; - ctx.verifier.check = async (files) => { - const text = files[unit] ?? ''; - const body = /int mf\(\) \{ ([^}]*) \}/.exec(text)?.[1] ?? ''; - const functions = [{ id: fid, status: STATUS.MISMATCH, score: 0.7, file: unit, codeHash: body === 'return 1;' ? 'mf-main' : `mf-${body}`, relocations: [], size: { ours: 8, original: 8 } }]; - const units = Object.keys(files).filter((f) => f.endsWith('.cpp')).map((file) => ({ file, compiled: true, lintErrors: [], unannotated: [], bindings: {}, inferred: [], includes: [], errors: [], functions: file === unit ? functions : [] })); - return { check: { annotationErrors: [], units }, rec: { conflicts: [], unknown: [], learned: [], learnedAddresses: [] }, changedHeaders: [], units: units.map((u) => u.file) }; + // Stubbed compiler: a unit's text comes from the submission, else from main (a base compile). A + // renamed reference (g_old -> g_new at the same address) keeps the effective code. + ctx.verifier.check = async (files, job, { onlyUnits = null } = {}) => { + const names = onlyUnits ?? Object.keys(files).filter((f) => f.endsWith('.cpp')); + const units = names.map((file) => { + const text = files[file] ?? ctx.repo.show('HEAD', file) ?? ''; + const body = /int mf\(\) \{ ([^}]*) \}/.exec(text)?.[1]; + const effective = body?.replace('g_renamed', 'g_original'); + const functions = file === unit && body !== undefined ? [{ id: fid, status: STATUS.MISMATCH, score: 0.7, file: unit, codeHash: `mf-${body}`, effectiveHash: `mf-${effective}`, relocations: [], size: { ours: 8, original: 8 } }] : []; + return { file, compiled: true, lintErrors: [], unannotated: [], bindings: {}, inferred: [], includes: [], errors: [], functions }; + }); + return { check: { annotationErrors: [], units }, rec: { conflicts: [], unknown: [], learned: [], learnedAddresses: [] }, changedHeaders: [], units: names }; }; const submit = (token, files, notes = 'move a global') => call('POST', '/api/maintenance', { baseCommit: ctx.repo.head(), files, notes }, token); const settle = (id, token) => waitFor(async () => { const st = (await call('GET', `/api/batches/${id}`, null, token)).body; return !['queued', 'verifying'].includes(st.state) && st; }); @@ -1373,12 +1379,19 @@ test('maintenance batches: maintainers only; merge when no compiled code changes assert.equal(done.kind, 'maintenance'); assert.equal(ctx.repo.show('HEAD', 'src/owner.cpp'), added['src/owner.cpp']); + // Reaching the same address under another name changes names, not code: it merges. + const withRef = onMain.replace('return 1;', 'return g_original;'); + ctx.repo.commitFiles({ [unit]: withRef }, 'reference a global', 'x '); + const renamed = await submit(maintainer, { [unit]: withRef.replace('g_original', 'g_renamed') }); + assert.equal((await settle(renamed.body.batchId, maintainer)).state, 'merged'); + const current = ctx.repo.show('HEAD', unit); + // Changing a function's compiled code is function work: it goes back, main stays. - const changed = await submit(maintainer, { [unit]: onMain.replace('return 1;', 'int x = 1; return x;') }); + const changed = await submit(maintainer, { [unit]: current.replace('return g_renamed;', 'int x = 1; return x;') }); const back = await settle(changed.body.batchId, maintainer); assert.equal(back.state, 'returned'); assert.match(back.result.returned, /changes the compiled code of 1 function/); - assert.equal(ctx.repo.show('HEAD', unit), onMain); + assert.equal(ctx.repo.show('HEAD', unit), current); // Removing a function's annotation is refused before compiling. const dropped = await submit(maintainer, { [unit]: 'int mf() { return 1; }\n' });