Skip to content
40 changes: 25 additions & 15 deletions interpreter/nodev8/v8.go
Original file line number Diff line number Diff line change
Expand Up @@ -1127,8 +1127,7 @@ func (i *v8Instance) getSFI(taggedPtr libpf.Address) (*v8SFI, error) {
if sodiType == vms.Type.Script {
sfi.source = i.getSource(sodiAddr)
if sfi.funcStartPos != sfi.funcEndPos {
sfi.funcStartLine = mapPositionToLine(sfi.source.lineTable,
int32(sfi.funcStartPos))
sfi.funcStartLine, _ = mapPositionToLine(sfi.source.lineTable, int32(sfi.funcStartPos))
}
}

Expand Down Expand Up @@ -1452,33 +1451,43 @@ func decodePosition(table []byte, delta uint64) sourcePosition {
}
}

// mapPositionToLine maps a file position (byte offset) to a line number. This is
// done against a table containing a offsets where each line ends.
func mapPositionToLine(lineEnds []uint32, pos int32) libpf.SourceLineno {
// mapPositionToLine maps a file position (byte offset) to a line number and column
// number. This is done against a table containing offsets where each line ends.
func mapPositionToLine(lineEnds []uint32, pos int32) (libpf.SourceLineno, libpf.SourceColumn) {
if len(lineEnds) == 0 || pos < 0 {
return 0
return 0, 0
}
// Use binary search to locate the line number
index := sort.Search(len(lineEnds), func(ndx int) bool {
return lineEnds[ndx] >= uint32(pos)
})
return libpf.SourceLineno(index + 1)

// Calculate column: position - start of line
// The start of line is the end of previous line + 1 (or 0 for first line)
var lineStart uint32
if index > 0 {
lineStart = lineEnds[index-1] + 1
}

column := uint32(pos) - lineStart

return libpf.SourceLineno(index + 1), libpf.SourceColumn(column)
}

// scriptOffsetToLine maps a sourcePosition to a line number in the corresponding source
func (sfi *v8SFI) scriptOffsetToLine(position sourcePosition) libpf.SourceLineno {
// scriptOffsetToLine maps a sourcePosition to a line and column number in the corresponding source
func (sfi *v8SFI) scriptOffsetToLine(position sourcePosition) (libpf.SourceLineno, libpf.SourceColumn) {
scriptOffset := position.scriptOffset()
// The scriptOffset is offset by one, to make kNoSourcePosition zero.
//nolint:lll
// https://chromium.googlesource.com/v8/v8.git/+/refs/tags/9.2.230.1/src/codegen/source-position.h#93
if scriptOffset == 0 {
return sfi.funcStartLine
return sfi.funcStartLine, 0
}
return mapPositionToLine(sfi.source.lineTable, scriptOffset-1)
}

// appendFrame adds a new frame to frames.
func (i *v8Instance) appendFrame(frames *libpf.Frames, sfi *v8SFI, lineNo libpf.SourceLineno) {
func (i *v8Instance) appendFrame(frames *libpf.Frames, sfi *v8SFI, lineNo libpf.SourceLineno, column libpf.SourceColumn) {
funcOffset := uint32(0)
if lineNo > sfi.funcStartLine {
funcOffset = uint32(lineNo - sfi.funcStartLine)
Expand All @@ -1488,6 +1497,7 @@ func (i *v8Instance) appendFrame(frames *libpf.Frames, sfi *v8SFI, lineNo libpf.
FunctionName: sfi.funcName,
SourceFile: sfi.source.fileName,
SourceLine: lineNo,
SourceColumn: column,
FunctionOffset: funcOffset,
})
}
Expand All @@ -1506,15 +1516,15 @@ func (i *v8Instance) generateNativeFrame(sourcePos sourcePosition, sfi *v8SFI,
return
}

lineNo := sfi.scriptOffsetToLine(sourcePos)
i.appendFrame(frames, sfi, lineNo)
lineNo, column := sfi.scriptOffsetToLine(sourcePos)
i.appendFrame(frames, sfi, lineNo, column)
}

// appendBytecodeFrame symbolizes and records to a trace a Bytecode based frame.
func (i *v8Instance) appendBytecodeFrame(sfi *v8SFI, delta uint64, frames *libpf.Frames) {
sourcePos := decodePosition(sfi.bytecodePositionTable, delta)
lineNo := sfi.scriptOffsetToLine(sourcePos)
i.appendFrame(frames, sfi, lineNo)
lineNo, column := sfi.scriptOffsetToLine(sourcePos)
i.appendFrame(frames, sfi, lineNo, column)
}

// symbolizeSFI symbolizes and records to a trace a SharedFunctionInfo based frame.
Expand Down
4 changes: 4 additions & 0 deletions libpf/libpf.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,5 +52,9 @@ type Void struct{}
// interpreted code.
type SourceLineno uint64

// SourceColumn represents a column number within a source file line. It is intended to be used
// for the source column numbers in interpreted code.
type SourceColumn uint64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this should be made uint32 and the above SourceLineno also to reduce memory use?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


// Origin determines the source of a trace.
type Origin int
2 changes: 2 additions & 0 deletions libpf/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ type Frame struct {
SourceFile String
// SourceLine is the source code level line number of this frame.
SourceLine SourceLineno
// SourceColumn is the source code level column number of this frame.
SourceColumn SourceColumn
// An address in ELF VA space (native frame) or line number (interpreted frame).
AddressOrLineno AddressOrLineno
// Mapping is a reference to the mapping data to which this Frame corresponds to.
Expand Down
2 changes: 2 additions & 0 deletions reporter/internal/pdata/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ func (p *Pdata) setProfile(
// Store interpreted frame information as a Line message
locInfo.hasLine = true
locInfo.lineNumber = int64(frame.SourceLine)
locInfo.columnNumber = int64(frame.SourceColumn)
fi := funcInfo{
nameIdx: stringSet.Add(frame.FunctionName.String()),
fileNameIdx: stringSet.Add(frame.SourceFile.String()),
Expand All @@ -215,6 +216,7 @@ func (p *Pdata) setProfile(
if locInfo.hasLine {
line := loc.Lines().AppendEmpty()
line.SetLine(locInfo.lineNumber)
line.SetColumn(locInfo.columnNumber)
line.SetFunctionIndex(locInfo.functionIndex)
}
attrMgr.AppendOptionalString(loc.AttributeIndices(),
Expand Down
1 change: 1 addition & 0 deletions reporter/internal/pdata/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ type locationInfo struct {
frameType string
hasLine bool
lineNumber int64
columnNumber int64
Comment on lines 15 to +16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here also, line/column probably could be int32?

functionIndex int32
}

Expand Down
8 changes: 6 additions & 2 deletions tools/coredump/coredump.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,13 @@ func formatFrame(frame *libpf.Frame) (string, error) {
}

if frame.FunctionName != libpf.NullString {
return fmt.Sprintf("%s+%d in %s:%d",
columnInfo := ""
if frame.SourceColumn != 0 {
columnInfo = fmt.Sprintf(":%d", frame.SourceColumn)
}
return fmt.Sprintf("%s+%d in %s:%d%s",
frame.FunctionName, frame.FunctionOffset,
frame.SourceFile, frame.SourceLine), nil
frame.SourceFile, frame.SourceLine, columnInfo), nil
}

if frame.Mapping.Valid() {
Expand Down
24 changes: 12 additions & 12 deletions tools/coredump/testdata/amd64/node1610.26681.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@
"node+0x9e7c50",
"node+0x7ec150",
"V8::ExitFrame+0 in :0",
"writeSync+15 in node:fs:873",
"writeSync+15 in node:fs:873:21",
"SyncWriteStream._write+0 in node:internal/fs/sync_write_stream:0",
"writeOrBuffer+24 in node:internal/streams/writable:389",
"_write+47 in node:internal/streams/writable:330",
"Writable.write+1 in node:internal/streams/writable:334",
"value+28 in node:internal/console/constructor:289",
"log+1 in node:internal/console/constructor:363",
"writeOrBuffer+24 in node:internal/streams/writable:389:11",
"_write+47 in node:internal/streams/writable:330:9",
"Writable.write+1 in node:internal/streams/writable:334:9",
"value+28 in node:internal/console/constructor:289:15",
"log+1 in node:internal/console/constructor:363:25",
"V8::InternalFrame+0 in :0",
"V8::EntryFrame+0 in :0",
"node+0xb8ee01",
Expand All @@ -22,12 +22,12 @@
"node+0x905af6",
"V8::ExitFrame+0 in :0",
"<anonymous>+11 in /home/tteras/optimyze/node/hello.js:12",
"Module._compile+46 in node:internal/modules/cjs/loader:1101",
"Module._extensions..js+43 in node:internal/modules/cjs/loader:1153",
"Module.load+12 in node:internal/modules/cjs/loader:981",
"Module._load+65 in node:internal/modules/cjs/loader:822",
"executeUserEntryPoint+7 in node:internal/modules/run_main:79",
"<anonymous>+16 in node:internal/main/run_main_module:17",
"Module._compile+46 in node:internal/modules/cjs/loader:1101:13",
"Module._extensions..js+43 in node:internal/modules/cjs/loader:1153:9",
"Module.load+12 in node:internal/modules/cjs/loader:981:31",
"Module._load+65 in node:internal/modules/cjs/loader:822:11",
"executeUserEntryPoint+7 in node:internal/modules/run_main:79:11",
"<anonymous>+16 in node:internal/main/run_main_module:17:46",
"V8::InternalFrame+0 in :0",
"V8::EntryFrame+0 in :0",
"node+0xb8ee01",
Expand Down
20 changes: 10 additions & 10 deletions tools/coredump/testdata/amd64/node1617-inlining.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,24 +50,24 @@
"node+0xcb6a6d",
"node+0x958702",
"V8::BuiltinExitFrame+0 in :0",
"trace+5 in node:internal/console/constructor:426",
"trace+5 in node:internal/console/constructor:426:4",
"V8::InternalFrame+0 in :0",
"V8::EntryFrame+0 in :0",
"node+0xa1aab4",
"node+0xa1b8e1",
"node+0x8ee8bb",
"node+0x8055bf",
"V8::ExitFrame+0 in :0",
"add+1 in /home/tteras/optimyze/node/test.js:3",
"add3+1 in /home/tteras/optimyze/node/test.js:8",
"test+1 in /home/tteras/optimyze/node/test.js:12",
"submain+2 in /home/tteras/optimyze/node/test.js:17",
"main+2 in /home/tteras/optimyze/node/test.js:23",
"add+1 in /home/tteras/optimyze/node/test.js:3:9",
"add3+1 in /home/tteras/optimyze/node/test.js:8:15",
"test+1 in /home/tteras/optimyze/node/test.js:12:8",
"submain+2 in /home/tteras/optimyze/node/test.js:17:2",
"main+2 in /home/tteras/optimyze/node/test.js:23:2",
"<anonymous>+26 in /home/tteras/optimyze/node/test.js:27",
"Module._compile+46 in node:internal/modules/cjs/loader:1126",
"Module._extensions..js+45 in node:internal/modules/cjs/loader:1180",
"Module.load+12 in node:internal/modules/cjs/loader:1004",
"Module._load+68 in node:internal/modules/cjs/loader:839",
"Module._compile+46 in node:internal/modules/cjs/loader:1126:13",
"Module._extensions..js+45 in node:internal/modules/cjs/loader:1180:9",
"Module.load+12 in node:internal/modules/cjs/loader:1004:31",
"Module._load+68 in node:internal/modules/cjs/loader:839:11",
"executeUserEntryPoint+0 in node:internal/modules/run_main:0",
"<anonymous>+0 in node:internal/main/run_main_module:0",
"V8::InternalFrame+0 in :0",
Expand Down
26 changes: 13 additions & 13 deletions tools/coredump/testdata/amd64/node1815-baseline.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,34 @@
"frames": [
"V8::ExitFrame+0 in :0",
"getColorDepth+0 in node:internal/tty:0",
"value+5 in node:internal/console/constructor:320",
"value+1 in node:internal/console/constructor:347",
"warn+1 in node:internal/console/constructor:382",
"value+5 in node:internal/console/constructor:320:19",
"value+1 in node:internal/console/constructor:347:43",
"warn+1 in node:internal/console/constructor:382:60",
"V8::InternalFrame+0 in :0",
"V8::EntryFrame+0 in :0",
"node+0x9bfced",
"node+0x9c0cb1",
"node+0x877853",
"node+0x7906bf",
"V8::ExitFrame+0 in :0",
"trace+6 in node:internal/console/constructor:428",
"trace+6 in node:internal/console/constructor:428:9",
"V8::InternalFrame+0 in :0",
"V8::EntryFrame+0 in :0",
"node+0x9bfced",
"node+0x9c0cb1",
"node+0x877853",
"node+0x7906bf",
"V8::ExitFrame+0 in :0",
"add+1 in /home/tteras/work/elastic/node/test.js:3",
"add3+1 in /home/tteras/work/elastic/node/test.js:8",
"test+1 in /home/tteras/work/elastic/node/test.js:12",
"submain+1 in /home/tteras/work/elastic/node/test.js:16",
"main+2 in /home/tteras/work/elastic/node/test.js:23",
"add+1 in /home/tteras/work/elastic/node/test.js:3:9",
"add3+1 in /home/tteras/work/elastic/node/test.js:8:15",
"test+1 in /home/tteras/work/elastic/node/test.js:12:8",
"submain+1 in /home/tteras/work/elastic/node/test.js:16:28",
"main+2 in /home/tteras/work/elastic/node/test.js:23:2",
"<anonymous>+26 in /home/tteras/work/elastic/node/test.js:27",
"Module._compile+46 in node:internal/modules/cjs/loader:1254",
"Module._extensions..js+45 in node:internal/modules/cjs/loader:1308",
"Module.load+12 in node:internal/modules/cjs/loader:1117",
"Module._load+72 in node:internal/modules/cjs/loader:958",
"Module._compile+46 in node:internal/modules/cjs/loader:1254:13",
"Module._extensions..js+45 in node:internal/modules/cjs/loader:1308:9",
"Module.load+12 in node:internal/modules/cjs/loader:1117:31",
"Module._load+72 in node:internal/modules/cjs/loader:958:11",
"executeUserEntryPoint+0 in node:internal/modules/run_main:0",
"<anonymous>+0 in node:internal/main/run_main_module:0",
"V8::InternalFrame+0 in :0",
Expand Down
16 changes: 8 additions & 8 deletions tools/coredump/testdata/amd64/node1816-async.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,20 @@
"node+0x9fdd6f",
"node+0x8f569c",
"V8::BuiltinExitFrame+0 in :0",
"trace+5 in node:internal/console/constructor:427",
"trace+5 in node:internal/console/constructor:427:4",
"V8::InternalFrame+0 in :0",
"V8::EntryFrame+0 in :0",
"node+0x9d5a90",
"node+0x9d6a9c",
"node+0x8884b6",
"node+0x79c955",
"V8::ExitFrame+0 in :0",
"<anonymous>+2 in /home/tteras/work/elastic/node/async.js:29",
"<anonymous>+2 in /home/tteras/work/elastic/node/async.js:29:9",
"Promise+0 in <unknown>:0",
"V8::ConstructFrame+0 in :0",
"V8::StubFrame+0 in :0",
"serveCustomer+1 in /home/tteras/work/elastic/node/async.js:27",
"runRestaurant+4 in /home/tteras/work/elastic/node/async.js:43",
"serveCustomer+1 in /home/tteras/work/elastic/node/async.js:27:9",
"runRestaurant+4 in /home/tteras/work/elastic/node/async.js:43:8",
"<anonymous>+0 in <unknown>:0",
"V8::StubFrame+0 in :0",
"V8::StubFrame+0 in :0",
Expand All @@ -39,10 +39,10 @@
"node+0x8cd006",
"node+0x8ce178",
"V8::BuiltinExitFrame+0 in :0",
"processTicksAndRejections+29 in node:internal/process/task_queues:96",
"runNextTicks+6 in node:internal/process/task_queues:64",
"listOnTimeout+21 in node:internal/timers:538",
"processTimers+6 in node:internal/timers:503",
"processTicksAndRejections+29 in node:internal/process/task_queues:96:28",
"runNextTicks+6 in node:internal/process/task_queues:64:2",
"listOnTimeout+21 in node:internal/timers:538:8",
"processTimers+6 in node:internal/timers:503:4",
"V8::InternalFrame+0 in :0",
"V8::EntryFrame+0 in :0",
"node+0x9d5a90",
Expand Down
20 changes: 10 additions & 10 deletions tools/coredump/testdata/amd64/node189-inlining.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,24 @@
"node+0xefd2a7",
"node+0x10a7bc2",
"V8::ExitFrame+0 in :0",
"trace+6 in node:internal/console/constructor:427",
"trace+6 in node:internal/console/constructor:427:19",
"V8::InternalFrame+0 in :0",
"V8::EntryFrame+0 in :0",
"node+0xb1dd4d",
"node+0xb1ef67",
"node+0x99f5ad",
"node+0x897b4a",
"V8::ExitFrame+0 in :0",
"add+1 in /home/tteras/optimyze/node/test.js:3",
"add3+1 in /home/tteras/optimyze/node/test.js:8",
"test+1 in /home/tteras/optimyze/node/test.js:12",
"submain+2 in /home/tteras/optimyze/node/test.js:17",
"main+2 in /home/tteras/optimyze/node/test.js:23",
"add+1 in /home/tteras/optimyze/node/test.js:3:9",
"add3+1 in /home/tteras/optimyze/node/test.js:8:8",
"test+1 in /home/tteras/optimyze/node/test.js:12:8",
"submain+2 in /home/tteras/optimyze/node/test.js:17:2",
"main+2 in /home/tteras/optimyze/node/test.js:23:2",
"<anonymous>+26 in /home/tteras/optimyze/node/test.js:27",
"Module._compile+46 in node:internal/modules/cjs/loader:1119",
"Module._extensions..js+45 in node:internal/modules/cjs/loader:1173",
"Module.load+12 in node:internal/modules/cjs/loader:997",
"Module._load+68 in node:internal/modules/cjs/loader:838",
"Module._compile+46 in node:internal/modules/cjs/loader:1119:13",
"Module._extensions..js+45 in node:internal/modules/cjs/loader:1173:9",
"Module.load+12 in node:internal/modules/cjs/loader:997:31",
"Module._load+68 in node:internal/modules/cjs/loader:838:11",
"executeUserEntryPoint+0 in node:internal/modules/run_main:0",
"<anonymous>+0 in node:internal/main/run_main_module:0",
"V8::InternalFrame+0 in :0",
Expand Down
20 changes: 10 additions & 10 deletions tools/coredump/testdata/amd64/node211.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,16 @@
"node+0xb2af62",
"node+0xa2f3a4",
"V8::ExitFrame+0 in :0",
"bar+1 in /home/tteras/work/elastic/node/hello2.js:2",
"foo+1 in /home/tteras/work/elastic/node/hello2.js:7",
"doit+2 in /home/tteras/work/elastic/node/hello2.js:12",
"<anonymous>+18 in /home/tteras/work/elastic/node/hello2.js:19",
"Module._compile+46 in node:internal/modules/cjs/loader:1376",
"Module._extensions..js+46 in node:internal/modules/cjs/loader:1435",
"Module.load+13 in node:internal/modules/cjs/loader:1207",
"Module._load+73 in node:internal/modules/cjs/loader:1023",
"executeUserEntryPoint+8 in node:internal/modules/run_main:135",
"<anonymous>+27 in node:internal/main/run_main_module:28",
"bar+1 in /home/tteras/work/elastic/node/hello2.js:2:1",
"foo+1 in /home/tteras/work/elastic/node/hello2.js:7:1",
"doit+2 in /home/tteras/work/elastic/node/hello2.js:12:2",
"<anonymous>+18 in /home/tteras/work/elastic/node/hello2.js:19:1",
"Module._compile+46 in node:internal/modules/cjs/loader:1376:13",
"Module._extensions..js+46 in node:internal/modules/cjs/loader:1435:9",
"Module.load+13 in node:internal/modules/cjs/loader:1207:31",
"Module._load+73 in node:internal/modules/cjs/loader:1023:11",
"executeUserEntryPoint+8 in node:internal/modules/run_main:135:11",
"<anonymous>+27 in node:internal/main/run_main_module:28:48",
"V8::InternalFrame+0 in :0",
"V8::EntryFrame+0 in :0",
"node+0xc93180",
Expand Down
Loading