Skip to content

Commit

Permalink
Error checking for accepting only first-pass computed expression valu…
Browse files Browse the repository at this point in the history 
…es (#60)

Machinery to accept only expression values in segment start/end that
are possible to compute in the first pass of compilation.

This should handle most cases, although I guess conditional compilation
could still leak forward label values into segment arguments.  Not sure
if there's a reasonable way to fix this.
  • Loading branch information
@nurpax
nurpax committed Feb 6, 2021
1 parent 7c75378 commit 4bdc585
Show file tree
Hide file tree
Showing 11 changed files with 144 additions and 32 deletions.
104 changes: 72 additions & 32 deletions src/asm.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,20 +45,28 @@ interface LabelAddr {
interface EvalValue<T> {
value: T;
errors: boolean;
completeFirstPass: boolean; // fully evaluated in first pass?
}

function mkErrorValue(v: number) {
return { value: v, errors: true };
function mkErrorValue(v: number): EvalValue<number> {
return { value: v, errors: true, completeFirstPass: false };
}

function mkEvalValue<T>(v: T) {
return { value: v, errors: false };
function mkEvalValue<T>(v: T, complete: boolean): EvalValue<T> {
return { value: v, errors: false, completeFirstPass: complete };
}

function anyErrors(...args: (EvalValue<any> | undefined)[]) {
return args.some(e => e !== undefined && e.errors);
}

// Compute "computeFirstPass" info for a multiple expression values
// Any non-first pass value means the result expression is also
// not evaluatable to a value in the first pass.
function combineEvalPassInfo(...args: (EvalValue<any> | undefined)[]) {
return args.every(e => e !== undefined && e.completeFirstPass);
}

class NamedScope<T> {
syms: Map<string, T & {seen: number}> = new Map();
readonly parent: NamedScope<T> | null = null;
Expand Down Expand Up @@ -207,10 +215,11 @@ class Scopes {
// scopes for label declarations, we end up
// mutating some unrelated, but same-named label names.
const prevLabel = this.curSymtab.syms.get(name);
if (prevLabel == undefined) {
if (prevLabel === undefined) {
// TODO completeFirstPass maybe too strict here?
const lblsym: SymLabel = {
type: 'label',
data: mkEvalValue({ addr: codePC, loc })
data: mkEvalValue({ addr: codePC, loc }, false)
};
this.curSymtab.addSymbol(name, lblsym, this.passCount);
return false;
Expand Down Expand Up @@ -358,10 +367,11 @@ interface BranchOffset {

const runBinop = (a: EvalValue<number>, b: EvalValue<number>, f: (a: number, b: number) => number | boolean): EvalValue<number> => {
const res = f(a.value as number, b.value as number);
const firstPassComplete = combineEvalPassInfo(a, b);
if (typeof res == 'boolean') {
return mkEvalValue(res ? 1 : 0);
return mkEvalValue(res ? 1 : 0, firstPassComplete);
}
return mkEvalValue(res);
return mkEvalValue(res, firstPassComplete);
}

const runUnaryOp = (a: EvalValue<number>, f: (a: number) => number | boolean): EvalValue<number> => {
Expand All @@ -370,9 +380,9 @@ const runUnaryOp = (a: EvalValue<number>, f: (a: number) => number | boolean): E
}
const res = f(a.value as number);
if (typeof res == 'boolean') {
return mkEvalValue(res ? 1 : 0);
return mkEvalValue(res ? 1 : 0, a.completeFirstPass);
}
return mkEvalValue(res);
return mkEvalValue(res, a.completeFirstPass);
}

// true if running in Node.js
Expand Down Expand Up @@ -456,7 +466,7 @@ class Assembler {
if (newPlugin === undefined) {
return mkErrorValue(0);
}
const m = mkEvalValue(newPlugin);
const m = mkEvalValue(newPlugin, true);
this.pluginCache.set(fname, m);
return m;
} catch(err) {
Expand Down Expand Up @@ -560,7 +570,7 @@ class Assembler {
const { filename } = ast;
const evalFname = this.evalExprToString(filename, "!binary filename");

let offset = mkEvalValue(0);
let offset = mkEvalValue(0, true);
let size = undefined;
if (ast.size !== null) {
if (ast.offset !== null) {
Expand All @@ -576,6 +586,15 @@ class Assembler {
return;
}

// Require that !binary offset and size arguments
// evaluate to a value in the first pass.
if (ast.offset !== null && !offset.completeFirstPass) {
this.addError("!binary 'offset' must evaluate to a value in the first pass", ast.offset.loc);
}
if (ast.size !== null && !size?.completeFirstPass) {
this.addError("!binary 'size' must evaluate to a value in the first pass", ast.size.loc);
}

const fname = this.makeSourceRelativePath(evalFname.value);
const buf = this.guardedReadFileSync(fname, ast.loc);
if (buf === undefined) { // can happen if fname is not found
Expand All @@ -596,11 +615,13 @@ class Assembler {
// Type-error checking variant of evalExpr
evalExprType<T>(node: ast.Expr, ty: 'number'|'string'|'object', msg: string): EvalValue<T> {
const res = this.evalExpr(node);
const { errors, value } = res;
const { errors, value, completeFirstPass } = res;
if (!errors && typeof value !== ty) {
this.addError(`Expecting ${msg} to be '${ty}' type, got '${formatTypename(value)}'`, node.loc);
return {
errors: true, value
errors: true,
completeFirstPass,
value
}
}
return res;
Expand Down Expand Up @@ -676,13 +697,14 @@ class Assembler {
}
}
case 'literal': {
return mkEvalValue(node.lit);
return mkEvalValue(node.lit, true);
}
case 'array': {
const evals = node.list.map(v => this.evalExpr(v));
return {
value: evals.map(e => e.value),
errors: anyErrors(...evals)
errors: anyErrors(...evals),
completeFirstPass: combineEvalPassInfo(...evals)
}
}
case 'object': {
Expand All @@ -692,7 +714,8 @@ class Assembler {
});
return {
value: kvs.reduce((o, [key, value]) => ({...o, [key]: value.value}), {}),
errors: anyErrors(...kvs.map(([_, e]) => e))
errors: anyErrors(...kvs.map(([_, e]) => e)),
completeFirstPass: combineEvalPassInfo(...kvs.map(([_, e]) => e))
}
}
case 'ident': {
Expand All @@ -708,14 +731,16 @@ class Assembler {
}
// Return a placeholder that should be resolved in the next pass
this.needPass = true;
return mkEvalValue(0);
// Evaluated value is marked as "incomplete in first pass"
return mkEvalValue(0, false);
}

switch (sym.type) {
case 'label':
return {
errors: sym.data.errors,
value: sym.data.value.addr
value: sym.data.value.addr,
completeFirstPass: sym.seen == this.pass
}
case 'var':
if (sym.seen < this.pass) {
Expand Down Expand Up @@ -755,7 +780,7 @@ class Assembler {
this.addError(`${typeName} property must be a string, got ${formatTypename(node.property.type)}`, node.loc);
} else {
if (checkProp(node.property.name, node.property.loc)) {
return mkEvalValue((object as any)[node.property.name])
return mkEvalValue((object as any)[node.property.name], evaledObject.completeFirstPass);
}
}
return mkErrorValue(0);
Expand All @@ -765,20 +790,20 @@ class Assembler {
if (!node.computed) {
return evalProperty(node, 'Array');
}
const { errors, value: idx } = this.evalExprToInt(node.property, 'array index');
const { errors, value: idx, completeFirstPass } = this.evalExprToInt(node.property, 'array index');
if (errors) {
return mkErrorValue(0);
}
if (!(idx in object)) {
this.addError(`Out of bounds array index ${idx}`, node.property.loc)
return mkErrorValue(0);
}
return mkEvalValue(object[idx]);
return mkEvalValue(object[idx], evaledObject.completeFirstPass && completeFirstPass);
} else if (typeof object == 'object') {
if (!node.computed) {
return evalProperty(node, 'Object');
} else {
let { errors, value: prop } = this.evalExpr(node.property);
let { errors, value: prop, completeFirstPass } = this.evalExpr(node.property);
if (errors) {
return mkErrorValue(0);
}
Expand All @@ -787,7 +812,9 @@ class Assembler {
return mkErrorValue(0);
}
if (checkProp(prop, node.property.loc)) {
return mkEvalValue(object[prop]);
// TODO losing completeInFirstPass bit here because
// it was not stored with expression value for objects?
return mkEvalValue(object[prop], completeFirstPass && evaledObject.completeFirstPass);
}
return mkErrorValue(0);
}
Expand All @@ -806,7 +833,7 @@ class Assembler {
}
return mkErrorValue(0);
}
return mkEvalValue(0); // dummy value as we couldn't resolve in pass 0
return mkEvalValue(0, false); // dummy value as we couldn't resolve in pass 0
}
case 'callfunc': {
const callee = this.evalExpr(node.callee);
Expand All @@ -822,7 +849,8 @@ class Assembler {
return mkErrorValue(0);
}
try {
return mkEvalValue(callee.value(...argValues.map(v => v.value)));
const complete = callee.completeFirstPass && combineEvalPassInfo(...argValues);
return mkEvalValue(callee.value(...argValues.map(v => v.value)), complete);
} catch(err) {
if (node.callee.type == 'qualified-ident') {
this.addError(`Call to '${formatSymbolPath(node.callee)}' failed with an error: ${err.message}`, node.loc);
Expand All @@ -835,7 +863,7 @@ class Assembler {
}
}
case 'getcurpc': {
return mkEvalValue(this.getPC());
return mkEvalValue(this.getPC(), true);
}
default:
break;
Expand Down Expand Up @@ -1053,7 +1081,7 @@ class Assembler {
}

bindFunction (name: ast.Ident, pluginModule: any, loc: SourceLoc) {
this.scopes.declareVar(name.name, mkEvalValue(this.makeFunction(pluginModule, loc)));
this.scopes.declareVar(name.name, mkEvalValue(this.makeFunction(pluginModule, loc), true));
}

bindPlugin (node: ast.StmtLoadPlugin, plugin: EvalValue<any>) {
Expand All @@ -1079,7 +1107,7 @@ class Assembler {
moduleObj[key] = val;
}
}
this.scopes.declareVar(moduleName.name, mkEvalValue(moduleObj));
this.scopes.declareVar(moduleName.name, mkEvalValue(moduleObj, true));
}
}

Expand Down Expand Up @@ -1151,7 +1179,7 @@ class Assembler {
scopeName = `${localScopeName}__${i}`
}
this.withAnonScope(scopeName, () => {
this.scopes.declareVar(index.name, mkEvalValue(lst[i]));
this.scopes.declareVar(index.name, mkEvalValue(lst[i], lstVal.completeFirstPass));
return this.assembleLines(body);
});
}
Expand Down Expand Up @@ -1265,7 +1293,18 @@ class Assembler {
if (anyErrors(start) || anyErrors(end)) {
return;
}
// TODO assert expression types are numbers for both start/end
let passErrors = false;
if (!start.completeFirstPass) {
this.addError("!segment 'start' must evaluate to a value in the first pass", startAddr.loc);
passErrors = true;
}
if (!end.completeFirstPass) {
this.addError("!segment 'end' must evaluate to a value in the first pass", endAddr.loc);
passErrors = true;
}
if (passErrors) {
return;
}
const segment = this.newSegment(name.name, start.value, end.value);
this.scopes.declareSegment(name.name, segment);
return;
Expand Down Expand Up @@ -1507,7 +1546,8 @@ class Assembler {
return Array(end-start).fill(null).map((_,idx) => idx + start);
};
const addPlugin = (name: string, handler: any) => {
this.scopes.declareVar(name, mkEvalValue(handler));
// TODO what about values from plugin calls?? passinfo missing
this.scopes.declareVar(name, mkEvalValue(handler, false));
}
addPlugin('loadJson', json);
addPlugin('range', range);
Expand Down
4 changes: 4 additions & 0 deletions test/cases/segment2.expected.asm
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
1000: A9 00 LDA #$00
1002: A9 01 LDA #$01
1004: A9 02 LDA #$02
1006: A9 03 LDA #$03
10 changes: 10 additions & 0 deletions test/cases/segment2.input.asm
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@

!let a = $1000

!segment code(start=a, end=a+16)

!segment code
lda #0 ; default segment
lda #1
lda #2
lda #3
4 changes: 4 additions & 0 deletions test/cases/segment3.expected.asm
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
1000: A9 00 LDA #$00
1002: A9 01 LDA #$01
1004: A9 02 LDA #$02
1006: A9 03 LDA #$03
13 changes: 13 additions & 0 deletions test/cases/segment3.input.asm
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@

* = $1000
lbl:

!let a = lbl

!segment code(start=lbl, end=lbl+100)

!segment code
lda #0
lda #1
lda #2
lda #3
2 changes: 2 additions & 0 deletions test/errors/require_1stpass1.errors.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
test/errors/require_1stpass1.input.asm:4:21: error: !segment 'start' must evaluate to a value in the first pass
test/errors/require_1stpass1.input.asm:5:32: error: !segment 'end' must evaluate to a value in the first pass
8 changes: 8 additions & 0 deletions test/errors/require_1stpass1.input.asm
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@

* = $801

!segment code1(start=lbl1, end=1000) ; forward refs not accepted for segments
!segment code2(start=$1000, end=lbl1) ; forward refs not accepted for segments

lbl1:
lda #0
2 changes: 2 additions & 0 deletions test/errors/require_1stpass2.errors.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
test/errors/require_1stpass2.input.asm:5:21: error: !segment 'start' must evaluate to a value in the first pass
test/errors/require_1stpass2.input.asm:6:31: error: !segment 'end' must evaluate to a value in the first pass
9 changes: 9 additions & 0 deletions test/errors/require_1stpass2.input.asm
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@

* = $801

!let a = lbl1
!segment code1(start=a, end=$1000) ; forward refs for start, should propagate through 'a'
!segment code2(start=$801, end=a) ; ditto but for end

lbl1:
lda #0
1 change: 1 addition & 0 deletions test/errors/require_1stpass3.errors.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test/errors/require_1stpass3.input.asm:16:33: error: !segment 'end' must evaluate to a value in the first pass
19 changes: 19 additions & 0 deletions test/errors/require_1stpass3.input.asm
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@

* = $801

!let a = $900
!let b = a + 4

!segment code(start=a, end=b) ; should be ok

!! b = b + 1

!segment code2(start=a+100, end=100+b) ; should still be ok

!let foo = lbl
!! b = b + foo

!segment code3(start=a+200, end=200+b) ; fail, first pass error propagates through foo

* = $2000
lbl:

0 comments on commit 4bdc585

Please sign in to comment.