Debuggers & profilers broken on Node.js 5.2

@cjihrig

Debuggers and profilers are broken in Node.js 5.2 by the recent changes to how we wrap modules. Specifically the PRs: #4254 and #2867 are causing issues.

The following test-case works with Node.js 5.1.1, but doesn't work with Node.js 5.2:

var vm = require('vm');
var Debug = vm.runInDebugContext('Debug');
var foo = require('./foo.js');

Debug.setListener(function (e,s,d) {
  console.log('debug event', e);
});
Debug.setBreakPoint(foo);
foo('world');
console.log(Debug.showBreakPoints(foo));

Where foo.js is

module.exports = function bar(who) { console.log('hello', who); }

The expected output is:

debug event 1
hello world
(who) { [B0]console.log('hello', who); }

This has to do with how V8 interprets line_offset provided with the scripts. V8 ends up setting the breakpoint at the wrong line, or in this case, not setting the breakpoint at all because it looks like a breakpoint beyond the end of the file.

Profilers are broken for a similar reason as the line-ticks are going to be reported incorrectly.

I am still investigating the issue to figure out if it is a bug in V8. In the meanwhile, I would argue that #4254 and #2867 should be reverted.

@cjihrig @bnoordhuis Thoughts?
/cc @nodejs/v8

In the profiler case, this issue manifests as failed v8 assertions (debug builds) and garbled state (release builds) when the v8 profiler is active.

For example, profiling the script:

require('child_process').spawn('ls');

yields:

#
# Fatal error in ../deps/v8/src/profiler/profile-generator.cc, line 29
# Check failed: line > 0.
#

==== C stack trace ===============================

 1: V8_Fatal
 2: v8::internal::JITLineInfoTable::SetPosition(int, int)
 3: v8::internal::CpuProfiler::CodeCreateEvent(v8::internal::Logger::LogEventsAndTags, v8::internal::Code*, v8::internal::SharedFunctionInfo*, v8::internal::CompilationInfo*, v8::internal::Name*, int, int)
 4: v8::internal::Logger::CodeCreateEvent(v8::internal::Logger::LogEventsAndTags, v8::internal::Code*, v8::internal::SharedFunctionInfo*, v8::internal::CompilationInfo*, v8::internal::Name*, int, int)
 5: 0x100100f
 6: v8::internal::Compiler::GetSharedFunctionInfo(v8::internal::FunctionLiteral*, v8::internal::Handle<v8::internal::Script>, v8::internal::CompilationInfo*)
 7: v8::internal::FullCodeGenerator::VisitFunctionLiteral(v8::internal::FunctionLiteral*)
 8: v8::internal::FunctionLiteral::Accept(v8::internal::AstVisitor*)
 9: v8::internal::FullCodeGenerator::Visit(v8::internal::AstNode*)
10: v8::internal::FullCodeGenerator::VisitForAccumulatorValue(v8::internal::Expression*)

@ofrobots

I'm fine with delaying the release to get this sorted out. For what it's worth, the release is ready and tested, and was just getting ready to go out the door.

I don't think those PRs need to be completely reverted, but I'll submit a PR to revert the relevant pieces so we can get debugging and profiling working again.

@ofrobots can you keep us updated as you find out if this is a V8 bug or not?

@cjihrig

@cjihrig I'll update this issue as I make progress.