Enforce project global ownership in verification and merging
This commit is contained in:
parent
95fd65997f
commit
0e3648a174
8 changed files with 447 additions and 2 deletions
45
README.md
45
README.md
|
|
@ -111,3 +111,48 @@ agents, then approved by a human with one click per batch.
|
|||
## Tests
|
||||
|
||||
`npm test` uses synthetic PE/COFF files, so it doesn't need a compiler.
|
||||
|
||||
## Global storage checks
|
||||
|
||||
Set `"globalRules": true` in the target repository's `decomp.json` to enable the
|
||||
source-level global gate. It is opt-in; deploying this server does not enable it
|
||||
for other projects. Restart web and worker after changing project configuration.
|
||||
|
||||
The verifier applies the payload over current main before checking it, and the
|
||||
merger checks again before committing. Both ordinary and maintenance batches use
|
||||
the same rules:
|
||||
|
||||
- An external data declaration needs an actual external definition in main plus
|
||||
the submitted files. Another extern, a local/static namesake, a different
|
||||
namespace, a pending batch or a symbol-database record does not supply storage.
|
||||
- Static-storage definitions need a stable data annotation. This includes
|
||||
function pointers, multiline tables, out-of-class static members and local
|
||||
statics. Scalar constant expressions may be folded; arrays and mutable pointers
|
||||
are not exempt merely because their element or pointee type is const.
|
||||
- Data annotations belong on definitions. Source units include their owner's
|
||||
header instead of adding extern-variable declarations. DLL-import declarations
|
||||
are distinguished from missing project definitions.
|
||||
- Existing violations are grandfathered for unrelated edits. New/changed
|
||||
declarations and removed annotations are checked, as is deletion of a
|
||||
definition still required by an existing extern.
|
||||
|
||||
The scanner handles declaration scopes and balanced initializers; it is not a
|
||||
C++ type checker or a substitute for a link. Compilation still validates types
|
||||
and conditional configurations. These checks establish source ownership, not the
|
||||
correct binary address, object extent, initializer values or constructor order.
|
||||
Those require original-binary evidence and review.
|
||||
|
||||
### Structural maintenance
|
||||
|
||||
Maintainers can use `td maintenance submit --files a.cpp,b.h --notes "reason"`
|
||||
for structural changes without function tasks. The server runs the submission
|
||||
gates, compiles affected units and compares effective code against a fresh main
|
||||
build. Behavior-changing function work must use the normal task/review path.
|
||||
`td maintenance symbol <id> --size N --reason "evidence"` corrects metadata;
|
||||
`--retire` is allowed only after main no longer annotates that global ID.
|
||||
|
||||
Neither an unchanged-code fingerprint nor a successful unit compile proves that
|
||||
new global storage reproduces the original initializer/destructor behavior.
|
||||
Maintenance authors must preserve those semantics; an incomplete object layout
|
||||
must not be published as a recovered full size.
|
||||
|
||||
|
|
|
|||
244
src/server/globals.js
Normal file
244
src/server/globals.js
Normal file
|
|
@ -0,0 +1,244 @@
|
|||
// Source-level storage checks. This scanner balances C++ declaration scopes and initializers;
|
||||
// it is not a type checker. Compilation still validates types and preprocessor configurations.
|
||||
// A symbol database entry supplies provenance, never a missing C++ definition.
|
||||
|
||||
import { parseAnnotations } from '../shared/annotations.js';
|
||||
|
||||
const IDENT = /^[A-Za-z_]\w*$/;
|
||||
const NON_DECL = new Set(['using', 'typedef', 'return', 'delete', 'throw', 'break', 'continue', 'case', 'goto', 'if', 'else', 'while', 'for', 'switch', 'do', 'static_assert', 'namespace', 'enum', 'template', 'friend']);
|
||||
const SPECIFIERS = new Set(['extern', 'static', 'inline', 'thread_local', 'constexpr', 'constinit', 'const', 'volatile', 'register', 'mutable']);
|
||||
const BUILTIN = /^(?:bool|char|wchar_t|char8_t|char16_t|char32_t|short|int|long|float|double|signed|unsigned|auto|(?:std::)?(?:u?int(?:8|16|32|64)_t|size_t|ptrdiff_t))$/;
|
||||
|
||||
function tokensOf(text) {
|
||||
const out = [];
|
||||
const re = /\/\/[^\n]*|\/\*[\s\S]*?\*\/|^[ \t]*\#[^\n]*(?:\\\r?\n[^\n]*)*|(?:u8|[uUL])?"(?:\\[\s\S]|[^"\\])*"|(?:u8|[uUL])?'(?:\\[\s\S]|[^'\\])*'|[A-Za-z_]\w*|(?:\d[\w.]*|\.\d[\w.]*)|::|&&|->|\.\.\.|[^\s]/gm;
|
||||
let line = 1;
|
||||
let last = 0;
|
||||
for (const m of text.matchAll(re)) {
|
||||
for (let i = last; i < m.index; i++) if (text[i] === '\n') line++;
|
||||
const tokenLine = line;
|
||||
for (const c of m[0]) if (c === '\n') line++;
|
||||
last = m.index + m[0].length;
|
||||
if (/^(?:\/\/|\/\*|\s*#)/.test(m[0])) continue;
|
||||
out.push({ value: m[0], line: tokenLine });
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
function pairsOf(tokens) {
|
||||
const pairs = new Map();
|
||||
const stack = [];
|
||||
const closing = { ')': '(', ']': '[', '}': '{' };
|
||||
for (let i = 0; i < tokens.length; i++) {
|
||||
const v = tokens[i].value;
|
||||
if (['(', '[', '{'].includes(v)) stack.push(i);
|
||||
else if (closing[v] && tokens[stack.at(-1)]?.value === closing[v]) {
|
||||
const start = stack.pop();
|
||||
pairs.set(start, i);
|
||||
}
|
||||
}
|
||||
return pairs;
|
||||
}
|
||||
|
||||
function qualified(scope, name) {
|
||||
const prefix = scope.names.join('::');
|
||||
return prefix && !name.startsWith(prefix + '::') ? `${prefix}::${name}` : name;
|
||||
}
|
||||
|
||||
function splitDeclarators(tokens) {
|
||||
const parts = [];
|
||||
let start = 0, depth = 0, angle = 0;
|
||||
for (let i = 0; i < tokens.length; i++) {
|
||||
const v = tokens[i].value;
|
||||
if (['(', '[', '{'].includes(v)) depth++;
|
||||
else if ([')', ']', '}'].includes(v)) depth--;
|
||||
else if (!depth && v === '<') angle++;
|
||||
else if (!depth && v === '>' && angle) angle--;
|
||||
else if (!depth && !angle && v === ',') {
|
||||
parts.push(tokens.slice(start, i));
|
||||
start = i + 1;
|
||||
}
|
||||
}
|
||||
parts.push(tokens.slice(start));
|
||||
return parts;
|
||||
}
|
||||
|
||||
function variable(part, scope, inheritedType = []) {
|
||||
if (!part.length || NON_DECL.has(part[0].value)) return null;
|
||||
if (['class', 'struct', 'union'].includes(part[0].value) && part.length === 2) return null;
|
||||
if (part.some((t) => t.value === '~' || t.value === 'operator')) return null;
|
||||
let tokens = part;
|
||||
// Attributes and declspecs do not change where the variable's name is.
|
||||
const imported = tokens.some((t) => t.value === 'dllimport');
|
||||
tokens = tokens.filter((t) => !['__cdecl', '__stdcall', '__fastcall', '__vectorcall', 'WINAPI', 'WSAAPI', 'CALLBACK'].includes(t.value));
|
||||
const clean = [];
|
||||
for (let i = 0; i < tokens.length; i++) {
|
||||
if (['__declspec', 'alignas'].includes(tokens[i].value) && tokens[i + 1]?.value === '(') {
|
||||
let depth = 1;
|
||||
i += 2;
|
||||
for (; i < tokens.length && depth; i++) depth += (tokens[i].value === '(' ? 1 : tokens[i].value === ')' ? -1 : 0);
|
||||
i--;
|
||||
} else clean.push(tokens[i]);
|
||||
}
|
||||
tokens = clean;
|
||||
const pairs = pairsOf(tokens);
|
||||
let firstSpecial = tokens.findIndex((t) => ['=', '{', '[', '('].includes(t.value));
|
||||
if (firstSpecial < 0) firstSpecial = tokens.length;
|
||||
let nameIndex = firstSpecial - 1;
|
||||
let functionPointer = false;
|
||||
if (tokens[firstSpecial]?.value === '(') {
|
||||
const end = pairs.get(firstSpecial);
|
||||
const group = tokens.slice(firstSpecial + 1, end);
|
||||
const star = group.findLastIndex((t) => ['*', '&', '&&'].includes(t.value));
|
||||
if (star >= 0 && group.slice(0, star).every((t) => ['::', 'const', 'volatile'].includes(t.value)) && IDENT.test(group[star + 1]?.value ?? '')) {
|
||||
functionPointer = true;
|
||||
nameIndex = firstSpecial + star + 2;
|
||||
} else {
|
||||
const args = tokens.slice(firstSpecial + 1, end);
|
||||
// A literal-led direct initializer is unambiguous; parameter defaults in a prototype are not.
|
||||
const firstArg = args[0]?.value ?? '';
|
||||
if (!/^(?:[\d"']|(?:u8|[uUL])["']|true$|false$|nullptr$|[-+]$)/.test(firstArg)) return null;
|
||||
}
|
||||
}
|
||||
if (!IDENT.test(tokens[nameIndex]?.value ?? '')) return null;
|
||||
let nameStart = nameIndex;
|
||||
while (nameStart >= 2 && tokens[nameStart - 1].value === '::' && IDENT.test(tokens[nameStart - 2].value)) nameStart -= 2;
|
||||
const name = tokens.slice(nameStart, nameIndex + 1).map((t) => t.value).join('');
|
||||
let type = functionPointer ? tokens.slice(0, firstSpecial) : tokens.slice(0, nameStart);
|
||||
if (inheritedType.length) type = [...inheritedType, ...type];
|
||||
if (!type.some((t) => IDENT.test(t.value) && !SPECIFIERS.has(t.value)) || type.some((t) => ['=', '.', '->', 'return'].includes(t.value))) return null;
|
||||
const specs = new Set(type.map((t) => t.value));
|
||||
const initializer = tokens.findIndex((t) => t.value === '=' || t.value === '{');
|
||||
const directInit = !functionPointer && tokens[firstSpecial]?.value === '(';
|
||||
const initialized = initializer >= 0 || directInit;
|
||||
const declaredExtern = specs.has('extern');
|
||||
const definition = (!declaredExtern || initialized) && (scope.kind !== 'class' || specs.has('inline') || (specs.has('constexpr') && initialized));
|
||||
if (scope.kind === 'class' && !specs.has('static') && !specs.has('inline')) return null;
|
||||
if (scope.kind === 'function' && !specs.has('static') && !specs.has('thread_local') && !declaredExtern) return null;
|
||||
const array = tokens.slice(nameIndex + 1).some((t) => t.value === '[');
|
||||
const typeText = type.filter((t) => !SPECIFIERS.has(t.value)).map((t) => t.value).join('').replace(/\s/g, '');
|
||||
const primitive = BUILTIN.test(typeText) || /^(?:unsigned|signed)?(?:longlong|long|short|int|char)$/.test(typeText);
|
||||
const valueTokens = initializer < 0 ? [] : tokens.slice(initializer + 1);
|
||||
const constantValue = valueTokens.length > 0 && !valueTokens.some((t) => t.value === '(' || t.value === 'new' || t.value === '[');
|
||||
const folded = !functionPointer && !array && primitive && (specs.has('constexpr') || specs.has('const')) && constantValue;
|
||||
return {
|
||||
name: qualified(scope, name), line: part[0].line,
|
||||
extern: declaredExtern && !definition, declaredExtern, definition, folded, imported,
|
||||
member: scope.kind === 'class', local: scope.kind === 'function',
|
||||
internal: scope.internal || (scope.kind !== 'class' && specs.has('static')),
|
||||
signature: [...type, ...tokens.slice(nameStart)].map((t) => t.value).join(' '),
|
||||
type,
|
||||
};
|
||||
}
|
||||
|
||||
/** Records include function pointers, multiline arrays, namespace/class storage and local statics. */
|
||||
export function scanVariables(text, file = '<input>') {
|
||||
const tokens = tokensOf(String(text ?? '').replace(/\r\n/g, '\n'));
|
||||
const pairs = pairsOf(tokens);
|
||||
const out = [];
|
||||
function walk(start, end, scope) {
|
||||
let statement = start;
|
||||
for (let i = start; i < end; i++) {
|
||||
const v = tokens[i].value;
|
||||
if (['public', 'private', 'protected'].includes(v) && tokens[i + 1]?.value === ':') { statement = i + 2; i++; continue; }
|
||||
if (v === '(' || v === '[') { i = pairs.get(i) ?? i; continue; }
|
||||
if (v === '{') {
|
||||
const close = pairs.get(i);
|
||||
if (close === undefined) break;
|
||||
const prefix = tokens.slice(statement, i);
|
||||
const words = prefix.map((t) => t.value);
|
||||
const ns = words.indexOf('namespace');
|
||||
const cls = ['class', 'struct', 'union'].includes(words[0]) ? 0 : -1;
|
||||
const linkage = words[0] === 'extern' && /^"C(?:\+\+)?"$/.test(words[1] ?? '');
|
||||
if (ns >= 0 || cls >= 0 || linkage) {
|
||||
const names = ns >= 0 ? words.slice(ns + 1).filter((w) => IDENT.test(w)) : cls >= 0 ? [words[cls + 1]].filter((w) => IDENT.test(w ?? '')) : [];
|
||||
walk(i + 1, close, { kind: cls >= 0 ? 'class' : scope.kind, names: [...scope.names, ...names], internal: scope.internal || (ns >= 0 && !names.length) });
|
||||
i = close;
|
||||
statement = close + 1;
|
||||
} else if (words[0] === 'enum') {
|
||||
i = close; statement = close + 1;
|
||||
} else if ((!words.includes('=') && words.includes(')') && !variable(prefix, scope)) || (scope.kind === 'function' && !variable(prefix, scope))) {
|
||||
// A function body, or a nested control-flow block in one.
|
||||
const paren = words.indexOf('(');
|
||||
const fnName = IDENT.test(words[paren - 1] ?? '') ? words[paren - 1] : '<block>';
|
||||
walk(i + 1, close, { ...scope, kind: 'function', names: scope.kind === 'function' ? scope.names : [...scope.names, fnName] });
|
||||
i = close; statement = close + 1;
|
||||
} else {
|
||||
i = close; // brace initializer; the terminating semicolon owns the declaration
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (v !== ';') continue;
|
||||
const parts = splitDeclarators(tokens.slice(statement, i));
|
||||
let inheritedType = [];
|
||||
for (const part of parts) {
|
||||
const record = variable(part, scope, inheritedType);
|
||||
if (!record) continue;
|
||||
inheritedType = record.type;
|
||||
delete record.type;
|
||||
out.push({ ...record, file });
|
||||
}
|
||||
statement = i + 1;
|
||||
}
|
||||
}
|
||||
walk(0, tokens.length, { kind: 'file', names: [], internal: false });
|
||||
const annotations = parseAnnotations(String(text ?? ''), file).annotations;
|
||||
for (const r of out) {
|
||||
const ann = annotations.find((a) => a.targetLine === r.line && ['GLOBAL', 'STRING', 'VTABLE'].includes(a.kind));
|
||||
r.annotation = ann ? `${ann.kind}:${ann.id}` : null;
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
function recordKey(r) {
|
||||
return `${r.name}|${r.local}|${r.internal}|${r.definition}|${r.signature}|${r.annotation ?? ''}`;
|
||||
}
|
||||
|
||||
export function newVariables(after, before) {
|
||||
const old = new Set(scanVariables(before).map(recordKey));
|
||||
return scanVariables(after).filter((r) => !old.has(recordKey(r)));
|
||||
}
|
||||
|
||||
export function definesVariable(text, name) {
|
||||
return scanVariables(text).some((r) => r.definition && !r.local && !r.internal && r.name === name);
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply changed files over main. Existing violations are reported by audit mode, not re-rejected
|
||||
* on unrelated edits. Removing the last definition is checked even if the extern is unchanged.
|
||||
* `repository` is source content from main, not the symbol database or other pending batches.
|
||||
*/
|
||||
export function globalViolations(files, { before = () => null, repository = {}, audit = false } = {}) {
|
||||
const all = new Map(Object.entries(repository).map(([f, t]) => [f, scanVariables(t, f)]));
|
||||
const previous = new Map();
|
||||
for (const [file, text] of Object.entries(files)) {
|
||||
previous.set(file, scanVariables(before(file), file));
|
||||
all.set(file, scanVariables(text, file));
|
||||
}
|
||||
const definitions = new Map();
|
||||
for (const records of all.values()) for (const r of records) {
|
||||
if (!r.definition || r.local || r.internal) continue;
|
||||
if (!definitions.has(r.name)) definitions.set(r.name, []);
|
||||
definitions.get(r.name).push(r);
|
||||
}
|
||||
const removed = new Set();
|
||||
for (const records of previous.values()) for (const r of records) if (r.definition && !r.local && !r.internal && !definitions.has(r.name)) removed.add(r.name);
|
||||
const errors = [];
|
||||
for (const [file, records] of all) {
|
||||
const submitted = Object.hasOwn(files, file);
|
||||
const old = new Set((previous.get(file) ?? []).map(recordKey));
|
||||
for (const v of records) {
|
||||
const added = submitted && (audit || !old.has(recordKey(v)));
|
||||
if (!added && !removed.has(v.name)) continue;
|
||||
const report = (rule, message) => errors.push({ file, line: v.line, name: v.name, rule, message });
|
||||
if (v.extern) {
|
||||
if (added && v.annotation) report('annotated-extern', `${v.name}: put the stable ID on its definition, not on this extern declaration`);
|
||||
if (added && /\.(c|cc|cpp|cxx)$/i.test(file) && !v.imported) report('extern-in-unit', `${v.name}: include its owner's header instead of declaring an extern variable in a source unit`);
|
||||
if (!v.imported && !definitions.has(v.name)) report('undefined-extern', `${v.name}: no external definition exists in main plus this payload; a symbol ID or another extern is not a definition`);
|
||||
} else if (added && v.definition && !v.folded && !v.annotation) {
|
||||
report('unannotated-global', `${v.name}: static-storage definitions need a stable GLOBAL ID (or the appropriate STRING/VTABLE ID)`);
|
||||
}
|
||||
}
|
||||
}
|
||||
return errors;
|
||||
}
|
||||
|
|
@ -67,12 +67,12 @@ export class Maintenance {
|
|||
if (dropped.length) return back(`removes the annotation of ${dropped.join(', ')}: a maintenance batch keeps every function annotated`, { droppedAnnotations: dropped });
|
||||
const undefinedNames = this.verifier.undefinedGate(files, batch.base_commit);
|
||||
if (undefinedNames.length) return back(`declares functions that are defined nowhere: ${undefinedNames.join(', ')}`, { undefinedFunctions: undefinedNames });
|
||||
const gateErrors = this.verifier.globalsGate?.(files, batch.base_commit) ?? [];
|
||||
if (gateErrors.length) return back('global variable rules (see globalErrors)', { globalErrors: gateErrors });
|
||||
|
||||
const startHead = this.repo.head();
|
||||
const merged = this.repo.mergeOntoHead(batch.base_commit, files);
|
||||
if (merged.conflicts.length) return back('conflicts with newer work on main; sync, merge and resubmit', { conflicts: merged.conflicts });
|
||||
const globalErrors = this.verifier.globalsGate(merged.files, startHead);
|
||||
if (globalErrors.length) return back('global variable rules (see globalErrors)', { globalErrors });
|
||||
|
||||
// Every unit the change can affect: the submitted ones, and every includer of a changed header.
|
||||
const { check, rec, changedHeaders } = await this.verifier.check(merged.files, `maintenance-${batch.id}`, { dependents: true });
|
||||
|
|
|
|||
|
|
@ -303,6 +303,8 @@ export class Merger {
|
|||
const startHead = this.repo.head();
|
||||
const merged = this.repo.mergeOntoHead(batch.base_commit, files);
|
||||
if (merged.conflicts.length) return bounce('conflicts with newer work on main', { conflicts: merged.conflicts });
|
||||
const globalErrors = this.verifier.globalsGate(merged.files, startHead);
|
||||
if (globalErrors.length) return bounce('global variable rules (see globalErrors)', { globalErrors });
|
||||
|
||||
// Re-verify on top of the current main: other merges may have landed since verification.
|
||||
// The code already passed the lint; units unchanged since verification keep its compile.
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ import { listSources, isUnit } from '../shared/project.js';
|
|||
import { changedNames, unitsMentioning } from './affected.js';
|
||||
import { undefinedDeclarations } from './undefined.js';
|
||||
import { sourceIncluders } from './includers.js';
|
||||
import { globalViolations } from './globals.js';
|
||||
import { functionRegions } from './split.js';
|
||||
import { className } from './grouping.js';
|
||||
import { now } from './db.js';
|
||||
|
|
@ -282,6 +283,21 @@ export class Verifier {
|
|||
return undefinedDeclarations(files, { before: (f) => this.repo.show(baseCommit, f), existsElsewhere, known });
|
||||
}
|
||||
|
||||
/** Definitions come from the effective source tree, never from symbol metadata or pending work. */
|
||||
globalsGate(files, baseCommit) {
|
||||
if (this.project.globalRules !== true) return [];
|
||||
const head = this.repo.head();
|
||||
if (this.globalSourceHead !== head) {
|
||||
const paths = this.repo.git(['ls-tree', '-r', '--name-only', head]).trim().split('\n');
|
||||
const roots = [...this.project.sourceDirs, ...this.project.includeDirs];
|
||||
this.globalSources = Object.fromEntries(paths
|
||||
.filter((f) => /\.(c|cc|cpp|cxx|h|hpp|inl|inc)$/i.test(f) && roots.some((r) => f.startsWith(`${r.replace(/\/$/, '')}/`)))
|
||||
.map((f) => [f, fs.readFileSync(path.join(this.repo.dir, f), 'utf8')]));
|
||||
this.globalSourceHead = head;
|
||||
}
|
||||
return globalViolations(files, { before: (f) => this.repo.show(baseCommit, f), repository: this.globalSources });
|
||||
}
|
||||
|
||||
// What a merge needs from the verification's check (without listings and byte diffs).
|
||||
rememberCheck(batchId, { check, rec, changedHeaders }) {
|
||||
const compact = {
|
||||
|
|
@ -551,6 +567,8 @@ export class Verifier {
|
|||
}
|
||||
const merged = this.repo.mergeOntoHead(batch.base_commit, submitted);
|
||||
if (merged.conflicts.length) return finishAll('needs_cleanup', { summary: 'your change conflicts with newer work on main; sync, merge and resubmit', conflicts: merged.conflicts });
|
||||
const globalErrors = this.globalsGate(merged.files, this.repo.head());
|
||||
if (globalErrors.length) return finishAll('needs_cleanup', { summary: 'global variable rules (see globalErrors)', globalErrors });
|
||||
|
||||
// Stages 1-3: one compile for the whole batch, then every function on its own.
|
||||
// Verify fast: the submitted units (enough to judge and route every submitted function). The
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@ export const DEFAULT_PROJECT = {
|
|||
fileOverrides: [],
|
||||
allowedPragmas: undefined,
|
||||
lintLimits: {},
|
||||
globalRules: false, // opt-in source storage/provenance checks; no symbol-database definition exemption
|
||||
// reviewAt: 'exact' requires every relocation target to be a known symbol;
|
||||
// 'match_unresolved' also accepts byte matches that reference not-yet-known data
|
||||
// (those symbols are created automatically on merge).
|
||||
|
|
|
|||
91
test/globals.test.js
Normal file
91
test/globals.test.js
Normal file
|
|
@ -0,0 +1,91 @@
|
|||
import { test } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import { newVariables, scanVariables, definesVariable, globalViolations } from '../src/server/globals.js';
|
||||
|
||||
const rules = (files, options = {}) => globalViolations(files, options).map((e) => `${e.name}:${e.rule}`).sort();
|
||||
|
||||
test('an extern needs a definition, not a symbol-table entry, another extern, or a local namesake', () => {
|
||||
assert.deepEqual(rules({ 'a.h': 'extern int g_lost;' }, { known: () => true }), ['g_lost:undefined-extern']);
|
||||
assert.deepEqual(rules({ 'a.h': 'extern int g_lost;' }, { repository: { 'b.h': 'extern int g_lost;', 'b.cpp': 'void f() { int g_lost; }' } }), ['g_lost:undefined-extern']);
|
||||
assert.deepEqual(rules({ 'a.h': 'extern int g_owned;', 'a.cpp': '// GLOBAL: gl_00001\nint g_owned;' }), []);
|
||||
assert.deepEqual(rules({ 'a.h': 'extern int g_owned;' }, { repository: { 'a.cpp': '// GLOBAL: gl_00001\nint g_owned;' } }), []);
|
||||
});
|
||||
|
||||
test('a static or differently scoped definition cannot satisfy an external declaration', () => {
|
||||
const repository = { 'a.cpp': 'static int count; namespace A { int value; }' };
|
||||
assert.deepEqual(rules({ 'a.h': 'extern int count; extern int value;' }, { repository }), ['count:undefined-extern', 'value:undefined-extern']);
|
||||
assert.deepEqual(rules({ 'a.h': 'namespace A { extern int value; }' }, { repository }), []);
|
||||
assert.equal(definesVariable('namespace A { int value; }', 'A::value'), true);
|
||||
assert.equal(definesVariable('namespace A { int value; }', 'value'), false);
|
||||
});
|
||||
|
||||
test('local statics, namespace storage, class-static definitions and mutable pointers need IDs', () => {
|
||||
const text = [
|
||||
'int g_count;',
|
||||
'// GLOBAL: gl_00002',
|
||||
'int g_annotated = 0;',
|
||||
'constexpr int kLimit = 4;',
|
||||
'const int kMax = 16;',
|
||||
'const char* g_name = "x";',
|
||||
'namespace {',
|
||||
' float s_scale;',
|
||||
'}',
|
||||
'class C {',
|
||||
' int m_field;',
|
||||
' static int s_shared;',
|
||||
'};',
|
||||
'int C::s_shared;',
|
||||
'void f() {',
|
||||
' int local = 0;',
|
||||
' static int s_calls;',
|
||||
'}',
|
||||
].join('\n');
|
||||
assert.deepEqual(rules({ 'a.cpp': text }), ['C::s_shared:unannotated-global', 'f::s_calls:unannotated-global', 'g_count:unannotated-global', 'g_name:unannotated-global', 's_scale:unannotated-global']);
|
||||
assert.deepEqual(newVariables(text, null).filter((v) => v.member).map((v) => v.name), ['C::s_shared']);
|
||||
});
|
||||
|
||||
test('multiline tables and function-pointer declarations are checked', () => {
|
||||
const declarations = 'extern int\n (WSAAPI* g_receive)(int, char*, int, int);\nextern const int values[2];';
|
||||
assert.deepEqual(rules({ 'a.h': declarations }), ['g_receive:undefined-extern', 'values:undefined-extern']);
|
||||
const definitions = '// GLOBAL: gl_00003\nint(WSAAPI* g_receive)(int, char*, int, int);\n// GLOBAL: gl_00004\nconst int values[2] = {\n 1, 2\n};';
|
||||
assert.deepEqual(rules({ 'a.h': declarations, 'a.cpp': definitions }), []);
|
||||
assert.deepEqual(rules({ 'a.cpp': 'static int table[] = {\n 1, 2,\n};\n' }), ['table:unannotated-global']);
|
||||
assert.deepEqual(rules({ 'a.cpp': 'std::vector<int> values(16);' }), ['values:unannotated-global']);
|
||||
});
|
||||
|
||||
test('annotations live on definitions and source units include the owner header', () => {
|
||||
const repository = { 'owner.cpp': 'int g_other;' };
|
||||
assert.deepEqual(rules({ 'a.h': '// GLOBAL: gl_00005\nextern int g_other;' }, { repository }), ['g_other:annotated-extern']);
|
||||
assert.deepEqual(rules({ 'a.cpp': 'extern int g_other;' }, { repository }), ['g_other:extern-in-unit']);
|
||||
assert.deepEqual(rules({ 'a.cpp': '// GLOBAL: gl_00005\nextern int g_other = 1;' }), [], 'extern with an initializer really is a definition');
|
||||
assert.deepEqual(rules({ 'a.h': 'extern __declspec(dllimport) int importedValue;' }), [], 'DLL imports intentionally have no project definition');
|
||||
});
|
||||
|
||||
test('changed annotations and removed definitions are checked; unrelated legacy code is unchanged', () => {
|
||||
const base = { 'a.h': 'extern int g_value;', 'a.cpp': '// GLOBAL: gl_00006\nint g_value;' };
|
||||
const opts = { before: (f) => base[f] ?? null, repository: base };
|
||||
assert.deepEqual(rules({ 'a.cpp': '' }, opts), ['g_value:undefined-extern']);
|
||||
assert.deepEqual(rules({ 'a.cpp': 'int g_value;' }, opts), ['g_value:unannotated-global']);
|
||||
assert.deepEqual(rules({ 'a.h': '// GLOBAL: gl_00006\nextern int g_value;' }, opts), ['g_value:annotated-extern']);
|
||||
assert.deepEqual(rules({ 'a.cpp': '', 'owner.cpp': base['a.cpp'] }, opts), [], 'moving a definition does not orphan its extern');
|
||||
assert.deepEqual(rules({ 'a.cpp': 'int legacy;\nvoid f() {}' }, { before: () => 'int legacy;' }), []);
|
||||
});
|
||||
|
||||
test('comments, strings, prototypes and ordinary fields do not become globals', () => {
|
||||
const text = '// extern int fake;\nvoid f(int a);\nstruct C {\n int member;\n void f();\n};\nvoid g() { int x; const char* s = "extern int fake;"; }';
|
||||
assert.deepEqual(scanVariables(text), []);
|
||||
assert.deepEqual(rules({ 'a.cpp': '// STRING: st_00001\nconst char* message = "x";' }), []);
|
||||
assert.deepEqual(rules({ 'a.cpp': 'constexpr int table[2] = {1, 2};' }), ['table:unannotated-global'], 'constexpr arrays still occupy storage');
|
||||
});
|
||||
|
||||
test('forward declarations, defaulted special members and default arguments are not variables', () => {
|
||||
const text = 'class LuaScript; struct World; union Storage;\nC::C() = default; C::~C() = default;\nvoid f(int x = 4);\nint C::operator()(int x) { return x; }\n';
|
||||
assert.deepEqual(scanVariables(text), []);
|
||||
assert.deepEqual(rules({ 'a.cpp': 'void f() { if (true) { static int visits; } }' }), ['f::visits:unannotated-global']);
|
||||
});
|
||||
|
||||
test('annotation edits cannot be hidden by unchanged declaration text or whitespace', () => {
|
||||
const original = '// GLOBAL: gl_00001\nint g_count;';
|
||||
assert.deepEqual(rules({ 'a.cpp': 'int g_count;' }, { before: () => original }), ['g_count:unannotated-global']);
|
||||
assert.deepEqual(rules({ 'a.cpp': '// GLOBAL: gl_00001\n\n int\n g_count;' }, { before: () => original }), []);
|
||||
});
|
||||
|
|
@ -1803,3 +1803,47 @@ test('an admin sets a reviewer weight: it counts at once, also on verdicts alrea
|
|||
ctx.store.run(`UPDATE attempts SET state = 'superseded' WHERE id IN (?, ?, ?)`, waiting, alone, objected);
|
||||
ctx.store.run('UPDATE tasks SET state = ? WHERE id = ?', saved.state, saved.id);
|
||||
});
|
||||
|
||||
test('global rules reject undefined data in code and maintenance batches, including known symbol names', async () => {
|
||||
const author = await activeUser('globalguard');
|
||||
const maintainer = await activeUser('globalmaint', 'maintainer');
|
||||
const unit = 'src/globalguard.cpp';
|
||||
const oldRule = ctx.project.globalRules;
|
||||
ctx.project.globalRules = true;
|
||||
const v = ctx.store.currentVersion();
|
||||
const fid = ctx.store.ensureSymbol({ kind: 'function', versionId: v.id, addr: 0x402A80, size: 8, unit }).symbol.id;
|
||||
ctx.store.ensureSymbol({ kind: 'global', versionId: v.id, addr: 0x409200, name: 'g_guardMissing' });
|
||||
ctx.tasks.generate();
|
||||
const claim = await call('POST', '/api/regions/claim', { kind: 'unit', key: unit }, author);
|
||||
assert.equal(claim.status, 200, JSON.stringify(claim.body));
|
||||
const taskId = claim.body.functions.find((f) => f.symbol === fid).taskId;
|
||||
const originalHead = ctx.repo.head();
|
||||
const payload = { [unit]: `// FUNCTION: ${fid}\nint guardFunction() { return 1; }\n`, 'include/globalguard.h': 'extern int g_guardMissing;\n' };
|
||||
const settle = (id, token) => waitFor(async () => {
|
||||
const r = await call('GET', `/api/batches/${id}`, null, token);
|
||||
return !['queued', 'verifying'].includes(r.body.state) && r.body;
|
||||
});
|
||||
try {
|
||||
const submitted = await call('POST', '/api/batches', { taskIds: [taskId], baseCommit: originalHead, files: payload }, author);
|
||||
assert.equal(submitted.status, 200, JSON.stringify(submitted.body));
|
||||
assert.equal((await settle(submitted.body.batchId, author)).state, 'returned');
|
||||
const feedback = JSON.parse(ctx.store.get('SELECT result_json FROM attempts WHERE id = ?', submitted.body.attempts[0].attemptId).result_json).feedback;
|
||||
assert.deepEqual(feedback.globalErrors.map((e) => [e.name, e.rule]), [['g_guardMissing', 'undefined-extern']]);
|
||||
const maintenance = await call('POST', '/api/maintenance', { baseCommit: originalHead, files: { 'include/globalguard.h': payload['include/globalguard.h'] } }, maintainer);
|
||||
assert.equal(maintenance.status, 200);
|
||||
const rejected = await settle(maintenance.body.batchId, maintainer);
|
||||
assert.equal(rejected.state, 'returned');
|
||||
assert.deepEqual(rejected.result.globalErrors.map((e) => e.rule), ['undefined-extern']);
|
||||
assert.equal(ctx.repo.head(), originalHead);
|
||||
assert.equal(ctx.repo.show('HEAD', 'include/globalguard.h'), null);
|
||||
|
||||
ctx.repo.commitFiles({ 'src/guardOwner.cpp': '// GLOBAL: gl_00001\nint g_guardMissing;\n' }, 'seed global owner', 'x <x@y.invalid>');
|
||||
assert.deepEqual(ctx.verifier.globalsGate({ 'include/globalguard.h': payload['include/globalguard.h'] }, ctx.repo.head()), []);
|
||||
ctx.repo.commitFiles({ 'include/globalguard.h': payload['include/globalguard.h'] }, 'seed declaration', 'x <x@y.invalid>');
|
||||
assert.deepEqual(ctx.verifier.globalsGate({ 'src/guardOwner.cpp': '' }, ctx.repo.head()).map((e) => e.rule), ['undefined-extern']);
|
||||
} finally {
|
||||
ctx.project.globalRules = oldRule;
|
||||
ctx.store.run(`UPDATE tasks SET state = 'done' WHERE id = ?`, taskId);
|
||||
ctx.regions.release(ctx.accounts.byName('globalguard'), { kind: 'unit', key: unit });
|
||||
}
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue