Browse Source

Improved the accuracy of source locations. Fixes #347.

Dmitry Panov 3 years ago
parent
commit
acd374ca9c
3 changed files with 101 additions and 20 deletions
  1. 17 16
      compiler_expr.go
  2. 73 0
      compiler_test.go
  3. 11 4
      func.go

+ 17 - 16
compiler_expr.go

@@ -299,7 +299,7 @@ func (e *baseCompiledExpr) emitUnary(func(), func(), bool, bool) {
 }
 
 func (e *baseCompiledExpr) addSrcMap() {
-	if e.offset > 0 {
+	if e.offset >= 0 {
 		e.c.p.srcMap = append(e.c.p.srcMap, srcMapItem{pc: len(e.c.p.code), srcPos: e.offset})
 	}
 }
@@ -370,12 +370,14 @@ func (e *compiledIdentifierExpr) emitGetterAndCallee() {
 	}
 }
 
-func (c *compiler) emitVarSetter1(name unistring.String, offset int, putOnStack bool, emitRight func(isRef bool)) {
+func (e *compiledIdentifierExpr) emitVarSetter1(putOnStack bool, emitRight func(isRef bool)) {
+	e.addSrcMap()
+	c := e.c
 	if c.scope.strict {
-		c.checkIdentifierLName(name, offset)
+		c.checkIdentifierLName(e.name, e.offset)
 	}
 
-	if b, noDynamics := c.scope.lookupName(name); noDynamics {
+	if b, noDynamics := c.scope.lookupName(e.name); noDynamics {
 		emitRight(false)
 		if b != nil {
 			if putOnStack {
@@ -385,9 +387,9 @@ func (c *compiler) emitVarSetter1(name unistring.String, offset int, putOnStack
 			}
 		} else {
 			if c.scope.strict {
-				c.emit(setGlobalStrict(name))
+				c.emit(setGlobalStrict(e.name))
 			} else {
-				c.emit(setGlobal(name))
+				c.emit(setGlobal(e.name))
 			}
 			if !putOnStack {
 				c.emit(pop)
@@ -398,9 +400,9 @@ func (c *compiler) emitVarSetter1(name unistring.String, offset int, putOnStack
 			b.emitResolveVar(c.scope.strict)
 		} else {
 			if c.scope.strict {
-				c.emit(resolveVar1Strict(name))
+				c.emit(resolveVar1Strict(e.name))
 			} else {
-				c.emit(resolveVar1(name))
+				c.emit(resolveVar1(e.name))
 			}
 		}
 		emitRight(true)
@@ -412,9 +414,9 @@ func (c *compiler) emitVarSetter1(name unistring.String, offset int, putOnStack
 	}
 }
 
-func (c *compiler) emitVarSetter(name unistring.String, offset int, valueExpr compiledExpr, putOnStack bool) {
-	c.emitVarSetter1(name, offset, putOnStack, func(bool) {
-		c.emitExpr(valueExpr, true)
+func (e *compiledIdentifierExpr) emitVarSetter(valueExpr compiledExpr, putOnStack bool) {
+	e.emitVarSetter1(putOnStack, func(bool) {
+		e.c.emitExpr(valueExpr, true)
 	})
 }
 
@@ -440,12 +442,12 @@ func (e *compiledIdentifierExpr) emitRef() {
 }
 
 func (e *compiledIdentifierExpr) emitSetter(valueExpr compiledExpr, putOnStack bool) {
-	e.c.emitVarSetter(e.name, e.offset, valueExpr, putOnStack)
+	e.emitVarSetter(valueExpr, putOnStack)
 }
 
 func (e *compiledIdentifierExpr) emitUnary(prepare, body func(), postfix, putOnStack bool) {
 	if putOnStack {
-		e.c.emitVarSetter1(e.name, e.offset, true, func(isRef bool) {
+		e.emitVarSetter1(true, func(isRef bool) {
 			e.c.emit(loadUndef)
 			if isRef {
 				e.c.emit(getValue)
@@ -465,7 +467,7 @@ func (e *compiledIdentifierExpr) emitUnary(prepare, body func(), postfix, putOnS
 		})
 		e.c.emit(pop)
 	} else {
-		e.c.emitVarSetter1(e.name, e.offset, false, func(isRef bool) {
+		e.emitVarSetter1(false, func(isRef bool) {
 			if isRef {
 				e.c.emit(getValue)
 			} else {
@@ -747,7 +749,6 @@ func (e *deleteGlobalExpr) emitGetter(putOnStack bool) {
 }
 
 func (e *compiledAssignExpr) emitGetter(putOnStack bool) {
-	e.addSrcMap()
 	switch e.operator {
 	case token.ASSIGN:
 		if fn, ok := e.right.(*compiledFunctionLiteral); ok {
@@ -820,7 +821,6 @@ func (e *compiledAssignExpr) emitGetter(putOnStack bool) {
 
 func (e *compiledLiteral) emitGetter(putOnStack bool) {
 	if putOnStack {
-		e.addSrcMap()
 		e.c.emit(loadVal(e.c.p.defineLiteralValue(e.val)))
 	}
 }
@@ -1499,6 +1499,7 @@ func (e *compiledUnaryExpr) emitGetter(putOnStack bool) {
 	var prepare, body func()
 
 	toNumber := func() {
+		e.addSrcMap()
 		e.c.emit(toNumber)
 	}
 

+ 73 - 0
compiler_test.go

@@ -4442,6 +4442,79 @@ func TestDuplicateFunc(t *testing.T) {
 	testScript(SCRIPT, asciiString("b"), t)
 }
 
+func TestSrcLocations(t *testing.T) {
+	// Do not reformat, assertions depend on line and column numbers
+	const SCRIPT = `
+	let i = {
+		valueOf() {
+			throw new Error();
+		}
+	};
+	try {
+		i++;
+	} catch(e) {
+		assertStack(e, [["test.js", "valueOf", 4, 10],
+						["test.js", "", 8, 3]
+						]);
+	}
+
+	Object.defineProperty(globalThis, "x", {
+		get() {
+			throw new Error();
+		},
+		set() {
+			throw new Error();
+		}
+	});
+
+	try {
+		x;
+	} catch(e) {
+		assertStack(e, [["test.js", "get", 17, 10],
+						["test.js", "", 25, 3]
+						]);
+	}
+
+	try {
+		x++;
+	} catch(e) {
+		assertStack(e, [["test.js", "get", 17, 10],
+						["test.js", "", 33, 3]
+						]);
+	}
+
+	try {
+		x = 2;
+	} catch(e) {
+		assertStack(e, [["test.js", "set", 20, 10],
+						["test.js", "", 41, 3]
+						]);
+	}
+
+	try {
+		+i;
+	} catch(e) {
+		assertStack(e, [["test.js", "valueOf", 4, 10],
+						["test.js", "", 49, 4]
+						]);
+	}
+
+
+	function assertStack(e, expected) {
+		const lines = e.stack.split('\n');
+		let lnum = 1;
+		for (const [file, func, line, col] of expected) {
+			const expLine = func === "" ?
+				"\tat " + file + ":" + line + ":" + col + "(" :
+				"\tat " + func + " (" + file + ":" + line + ":" + col + "(";
+			assert.sameValue(lines[lnum].substring(0, expLine.length), expLine, "line " + lnum);
+			lnum++;
+		}
+	}
+	`
+	testScriptWithTestLib(SCRIPT, _undefined, t)
+}
+
 /*
 func TestBabel(t *testing.T) {
 	src, err := ioutil.ReadFile("babel7.js")

+ 11 - 4
func.go

@@ -162,7 +162,6 @@ func (f *arrowFuncObject) Call(call FunctionCall) Value {
 
 func (f *baseJsFuncObject) _call(call FunctionCall, newTarget, this Value) Value {
 	vm := f.val.runtime.vm
-	pc := vm.pc
 
 	vm.stack.expand(vm.sp + len(call.Arguments) + 1)
 	vm.stack[vm.sp] = f.val
@@ -178,18 +177,26 @@ func (f *baseJsFuncObject) _call(call FunctionCall, newTarget, this Value) Value
 		vm.sp++
 	}
 
-	vm.pc = -1
-	vm.pushCtx()
+	pc := vm.pc
+	if pc != -1 {
+		vm.pc++ // fake "return address" so that captureStack() records the correct call location
+		vm.pushCtx()
+		vm.callStack = append(vm.callStack, context{pc: -1}) // extra frame so that run() halts after ret
+	} else {
+		vm.pushCtx()
+	}
 	vm.args = len(call.Arguments)
 	vm.prg = f.prg
 	vm.stash = f.stash
 	vm.newTarget = newTarget
 	vm.pc = 0
 	vm.run()
+	if pc != -1 {
+		vm.popCtx()
+	}
 	vm.pc = pc
 	vm.halt = false
 	return vm.pop()
-
 }
 
 func (f *baseJsFuncObject) call(call FunctionCall, newTarget Value) Value {