Skip to content

Commit

Permalink
module: fix column offset on first line stack trace
Browse files Browse the repository at this point in the history
When modules are loaded by require(), they are wrapped in
a function. This added text causes errors on the first line
of a file to be offset by the length of the function
declaration. For example, an error on the first character
of a module would appear as if it were on column 63.

To fix this problem I added line and column offset support
to the ContextifyScript object and added options to the
runInContext(and friends) function. I then added a column
offset to module._compile that compensates for the function
wrapper.

Fixes nodejs#9445
  • Loading branch information
brendan0powers committed May 16, 2015
1 parent 0df5e1c commit 52a44e9
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 2 deletions.
4 changes: 4 additions & 0 deletions doc/api/vm.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,10 @@ e.g. `(0,eval)('code')`. However, it also has the following additional options:

- `filename`: allows you to control the filename that shows up in any stack
traces produced.
- `lineOffset`: allows you to add an offset to the line number that is
displayed in stack traces
- `columnOffset`: allows you to add an offset to the column number that is
displayed in stack traces
- `displayErrors`: whether or not to print any errors to stderr, with the
line of code that caused them highlighted, before throwing an exception.
Will capture both syntax errors from compiling `code` and runtime errors
Expand Down
4 changes: 3 additions & 1 deletion lib/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -441,8 +441,10 @@ Module.prototype._compile = function(content, filename) {

// create wrapper function
var wrapper = Module.wrap(content);
var wrapperOffset = Module.wrapper[0].length;

var compiledWrapper = runInThisContext(wrapper, { filename: filename });
var compiledWrapper = runInThisContext(wrapper,
{ filename: filename, columnOffset: -wrapperOffset });
if (global.v8debug) {
if (!resolvedArgv) {
// we enter the repl if we're not given a filename argument.
Expand Down
49 changes: 48 additions & 1 deletion src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -485,13 +485,15 @@ class ContextifyScript : public BaseObject {
TryCatch try_catch;
Local<String> code = args[0]->ToString();
Local<String> filename = GetFilenameArg(args, 1);
Local<Integer> lineOffset = GetLineOffsetArg(args, 1);
Local<Integer> columnOffset = GetColumnOffsetArg(args, 1);
bool display_errors = GetDisplayErrorsArg(args, 1);
if (try_catch.HasCaught()) {
try_catch.ReThrow();
return;
}

ScriptOrigin origin(filename);
ScriptOrigin origin(filename, lineOffset, columnOffset);
ScriptCompiler::Source source(code, origin);
Local<UnboundScript> v8_script =
ScriptCompiler::CompileUnbound(env->isolate(), &source);
Expand Down Expand Up @@ -658,6 +660,51 @@ class ContextifyScript : public BaseObject {
}


static Local<Integer> GetLineOffsetArg(
const FunctionCallbackInfo<Value>& args,
const int i) {
Local<Integer> defaultLineOffset = Integer::New(args.GetIsolate(), 0);

if (args[i]->IsUndefined()) {
return defaultLineOffset;
}
if (args[i]->IsInt32()) {
return args[i].As<Integer>();
}
if (!args[i]->IsObject()) {
return defaultLineOffset;
}

Local<String> key = FIXED_ONE_BYTE_STRING(args.GetIsolate(), "lineOffset");
Local<Value> value = args[i].As<Object>()->Get(key);

return value->IsUndefined() ? defaultLineOffset : value->ToInteger();
}


static Local<Integer> GetColumnOffsetArg(
const FunctionCallbackInfo<Value>& args,
const int i) {
Local<Integer> defaultColumnOffset = Integer::New(args.GetIsolate(), 0);

if (args[i]->IsUndefined()) {
return defaultColumnOffset;
}
if (args[i]->IsInt32()) {
return args[i].As<Integer>();
}
if (!args[i]->IsObject()) {
return defaultColumnOffset;
}

Local<String> key = FIXED_ONE_BYTE_STRING(args.GetIsolate(),
"columnOffset");
Local<Value> value = args[i].As<Object>()->Get(key);

return value->IsUndefined() ? defaultColumnOffset : value->ToInteger();
}


static bool EvalMachine(Environment* env,
const int64_t timeout,
const bool display_errors,
Expand Down
16 changes: 16 additions & 0 deletions test/simple/test-vm-context.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,22 @@ catch (e) {
assert.ok(gh1140Exception,
'expected exception from runInContext signature test');

// Issue GH-9445:
console.error('test runInContext offset');
var gh9445Exception;
try {
vm.runInContext('throw new Error()', context, { filename: 'expected-filename.js',
lineOffset: 32,
columnOffset: 123});
}
catch (e) {
gh9445Exception = e;
assert.ok(/expected-filename.js:33:130/.test(e.stack),
'expected appearance of proper offset in Error stack');
}
assert.ok(gh9445Exception,
'expected exception from runInContext offset test');

// GH-558, non-context argument segfaults / raises assertion
[undefined, null, 0, 0.0, '', {}, []].forEach(function(e) {
assert.throws(function() { script.runInContext(e); }, TypeError);
Expand Down

0 comments on commit 52a44e9

Please sign in to comment.