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.
This commit is contained in:
parent
689a8bc85b
commit
64b892b2b0
3 changed files with 52 additions and 15 deletions
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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 <x@y.invalid>');
|
||||
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' });
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue