Skip to content

Commit 25d0fff

Browse files
manojVivekflorianl
andauthored
Sample frames to include column information in addition to line numbers (#1014)
Co-authored-by: Florian Lehner <florianl@users.noreply.github.com>
1 parent b4e74cc commit 25d0fff

19 files changed

+210
-187
lines changed

interpreter/nodev8/v8.go

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1126,8 +1126,7 @@ func (i *v8Instance) getSFI(taggedPtr libpf.Address) (*v8SFI, error) {
11261126
if sodiType == vms.Type.Script {
11271127
sfi.source = i.getSource(sodiAddr)
11281128
if sfi.funcStartPos != sfi.funcEndPos {
1129-
sfi.funcStartLine = mapPositionToLine(sfi.source.lineTable,
1130-
int32(sfi.funcStartPos))
1129+
sfi.funcStartLine, _ = mapPositionToLine(sfi.source.lineTable, int32(sfi.funcStartPos))
11311130
}
11321131
}
11331132

@@ -1451,33 +1450,43 @@ func decodePosition(table []byte, delta uint64) sourcePosition {
14511450
}
14521451
}
14531452

1454-
// mapPositionToLine maps a file position (byte offset) to a line number. This is
1455-
// done against a table containing a offsets where each line ends.
1456-
func mapPositionToLine(lineEnds []uint32, pos int32) libpf.SourceLineno {
1453+
// mapPositionToLine maps a file position (byte offset) to a line number and column
1454+
// number. This is done against a table containing offsets where each line ends.
1455+
func mapPositionToLine(lineEnds []uint32, pos int32) (libpf.SourceLineno, libpf.SourceColumn) {
14571456
if len(lineEnds) == 0 || pos < 0 {
1458-
return 0
1457+
return 0, 0
14591458
}
14601459
// Use binary search to locate the line number
14611460
index := sort.Search(len(lineEnds), func(ndx int) bool {
14621461
return lineEnds[ndx] >= uint32(pos)
14631462
})
1464-
return libpf.SourceLineno(index + 1)
1463+
1464+
// Calculate column: position - start of line
1465+
// The start of line is the end of previous line + 1 (or 0 for first line)
1466+
var lineStart uint32
1467+
if index > 0 {
1468+
lineStart = lineEnds[index-1] + 1
1469+
}
1470+
1471+
column := uint32(pos) - lineStart
1472+
1473+
return libpf.SourceLineno(index + 1), libpf.SourceColumn(column)
14651474
}
14661475

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

14791488
// appendFrame adds a new frame to frames.
1480-
func (i *v8Instance) appendFrame(frames *libpf.Frames, sfi *v8SFI, lineNo libpf.SourceLineno) {
1489+
func (i *v8Instance) appendFrame(frames *libpf.Frames, sfi *v8SFI, lineNo libpf.SourceLineno, column libpf.SourceColumn) {
14811490
funcOffset := uint32(0)
14821491
if lineNo > sfi.funcStartLine {
14831492
funcOffset = uint32(lineNo - sfi.funcStartLine)
@@ -1487,6 +1496,7 @@ func (i *v8Instance) appendFrame(frames *libpf.Frames, sfi *v8SFI, lineNo libpf.
14871496
FunctionName: sfi.funcName,
14881497
SourceFile: sfi.source.fileName,
14891498
SourceLine: lineNo,
1499+
SourceColumn: column,
14901500
FunctionOffset: funcOffset,
14911501
})
14921502
}
@@ -1505,15 +1515,15 @@ func (i *v8Instance) generateNativeFrame(sourcePos sourcePosition, sfi *v8SFI,
15051515
return
15061516
}
15071517

1508-
lineNo := sfi.scriptOffsetToLine(sourcePos)
1509-
i.appendFrame(frames, sfi, lineNo)
1518+
lineNo, column := sfi.scriptOffsetToLine(sourcePos)
1519+
i.appendFrame(frames, sfi, lineNo, column)
15101520
}
15111521

15121522
// appendBytecodeFrame symbolizes and records to a trace a Bytecode based frame.
15131523
func (i *v8Instance) appendBytecodeFrame(sfi *v8SFI, delta uint64, frames *libpf.Frames) {
15141524
sourcePos := decodePosition(sfi.bytecodePositionTable, delta)
1515-
lineNo := sfi.scriptOffsetToLine(sourcePos)
1516-
i.appendFrame(frames, sfi, lineNo)
1525+
lineNo, column := sfi.scriptOffsetToLine(sourcePos)
1526+
i.appendFrame(frames, sfi, lineNo, column)
15171527
}
15181528

15191529
// symbolizeSFI symbolizes and records to a trace a SharedFunctionInfo based frame.

libpf/libpf.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,5 +52,9 @@ type Void struct{}
5252
// interpreted code.
5353
type SourceLineno uint64
5454

55+
// SourceColumn represents a column number within a source file line. It is intended to be used
56+
// for the source column numbers in interpreted code.
57+
type SourceColumn uint64
58+
5559
// Origin determines the source of a trace.
5660
type Origin int

libpf/trace.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ type Frame struct {
7979
SourceFile String
8080
// SourceLine is the source code level line number of this frame.
8181
SourceLine SourceLineno
82+
// SourceColumn is the source code level column number of this frame.
83+
SourceColumn SourceColumn
8284
// An address in ELF VA space (native frame) or line number (interpreted frame).
8385
AddressOrLineno AddressOrLineno
8486
// Mapping is a reference to the mapping data to which this Frame corresponds to.

reporter/internal/pdata/generate.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ func (p *Pdata) setProfile(
199199
// Store interpreted frame information as a Line message
200200
locInfo.hasLine = true
201201
locInfo.lineNumber = int64(frame.SourceLine)
202+
locInfo.columnNumber = int64(frame.SourceColumn)
202203
fi := funcInfo{
203204
nameIdx: stringSet.Add(frame.FunctionName.String()),
204205
fileNameIdx: stringSet.Add(frame.SourceFile.String()),
@@ -215,6 +216,7 @@ func (p *Pdata) setProfile(
215216
if locInfo.hasLine {
216217
line := loc.Lines().AppendEmpty()
217218
line.SetLine(locInfo.lineNumber)
219+
line.SetColumn(locInfo.columnNumber)
218220
line.SetFunctionIndex(locInfo.functionIndex)
219221
}
220222
attrMgr.AppendOptionalString(loc.AttributeIndices(),

reporter/internal/pdata/helper.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ type locationInfo struct {
1313
frameType string
1414
hasLine bool
1515
lineNumber int64
16+
columnNumber int64
1617
functionIndex int32
1718
}
1819

tools/coredump/coredump.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,13 @@ func formatFrame(frame *libpf.Frame) (string, error) {
7878
}
7979

8080
if frame.FunctionName != libpf.NullString {
81-
return fmt.Sprintf("%s+%d in %s:%d",
81+
columnInfo := ""
82+
if frame.SourceColumn != 0 {
83+
columnInfo = fmt.Sprintf(":%d", frame.SourceColumn)
84+
}
85+
return fmt.Sprintf("%s+%d in %s:%d%s",
8286
frame.FunctionName, frame.FunctionOffset,
83-
frame.SourceFile, frame.SourceLine), nil
87+
frame.SourceFile, frame.SourceLine, columnInfo), nil
8488
}
8589

8690
if frame.Mapping.Valid() {

tools/coredump/testdata/amd64/node1610.26681.json

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,13 @@
77
"node+0x9e7c50",
88
"node+0x7ec150",
99
"V8::ExitFrame+0 in :0",
10-
"writeSync+15 in node:fs:873",
10+
"writeSync+15 in node:fs:873:21",
1111
"SyncWriteStream._write+0 in node:internal/fs/sync_write_stream:0",
12-
"writeOrBuffer+24 in node:internal/streams/writable:389",
13-
"_write+47 in node:internal/streams/writable:330",
14-
"Writable.write+1 in node:internal/streams/writable:334",
15-
"value+28 in node:internal/console/constructor:289",
16-
"log+1 in node:internal/console/constructor:363",
12+
"writeOrBuffer+24 in node:internal/streams/writable:389:11",
13+
"_write+47 in node:internal/streams/writable:330:9",
14+
"Writable.write+1 in node:internal/streams/writable:334:9",
15+
"value+28 in node:internal/console/constructor:289:15",
16+
"log+1 in node:internal/console/constructor:363:25",
1717
"V8::InternalFrame+0 in :0",
1818
"V8::EntryFrame+0 in :0",
1919
"node+0xb8ee01",
@@ -22,12 +22,12 @@
2222
"node+0x905af6",
2323
"V8::ExitFrame+0 in :0",
2424
"<anonymous>+11 in /home/tteras/optimyze/node/hello.js:12",
25-
"Module._compile+46 in node:internal/modules/cjs/loader:1101",
26-
"Module._extensions..js+43 in node:internal/modules/cjs/loader:1153",
27-
"Module.load+12 in node:internal/modules/cjs/loader:981",
28-
"Module._load+65 in node:internal/modules/cjs/loader:822",
29-
"executeUserEntryPoint+7 in node:internal/modules/run_main:79",
30-
"<anonymous>+16 in node:internal/main/run_main_module:17",
25+
"Module._compile+46 in node:internal/modules/cjs/loader:1101:13",
26+
"Module._extensions..js+43 in node:internal/modules/cjs/loader:1153:9",
27+
"Module.load+12 in node:internal/modules/cjs/loader:981:31",
28+
"Module._load+65 in node:internal/modules/cjs/loader:822:11",
29+
"executeUserEntryPoint+7 in node:internal/modules/run_main:79:11",
30+
"<anonymous>+16 in node:internal/main/run_main_module:17:46",
3131
"V8::InternalFrame+0 in :0",
3232
"V8::EntryFrame+0 in :0",
3333
"node+0xb8ee01",

tools/coredump/testdata/amd64/node1617-inlining.json

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,24 +50,24 @@
5050
"node+0xcb6a6d",
5151
"node+0x958702",
5252
"V8::BuiltinExitFrame+0 in :0",
53-
"trace+5 in node:internal/console/constructor:426",
53+
"trace+5 in node:internal/console/constructor:426:4",
5454
"V8::InternalFrame+0 in :0",
5555
"V8::EntryFrame+0 in :0",
5656
"node+0xa1aab4",
5757
"node+0xa1b8e1",
5858
"node+0x8ee8bb",
5959
"node+0x8055bf",
6060
"V8::ExitFrame+0 in :0",
61-
"add+1 in /home/tteras/optimyze/node/test.js:3",
62-
"add3+1 in /home/tteras/optimyze/node/test.js:8",
63-
"test+1 in /home/tteras/optimyze/node/test.js:12",
64-
"submain+2 in /home/tteras/optimyze/node/test.js:17",
65-
"main+2 in /home/tteras/optimyze/node/test.js:23",
61+
"add+1 in /home/tteras/optimyze/node/test.js:3:9",
62+
"add3+1 in /home/tteras/optimyze/node/test.js:8:15",
63+
"test+1 in /home/tteras/optimyze/node/test.js:12:8",
64+
"submain+2 in /home/tteras/optimyze/node/test.js:17:2",
65+
"main+2 in /home/tteras/optimyze/node/test.js:23:2",
6666
"<anonymous>+26 in /home/tteras/optimyze/node/test.js:27",
67-
"Module._compile+46 in node:internal/modules/cjs/loader:1126",
68-
"Module._extensions..js+45 in node:internal/modules/cjs/loader:1180",
69-
"Module.load+12 in node:internal/modules/cjs/loader:1004",
70-
"Module._load+68 in node:internal/modules/cjs/loader:839",
67+
"Module._compile+46 in node:internal/modules/cjs/loader:1126:13",
68+
"Module._extensions..js+45 in node:internal/modules/cjs/loader:1180:9",
69+
"Module.load+12 in node:internal/modules/cjs/loader:1004:31",
70+
"Module._load+68 in node:internal/modules/cjs/loader:839:11",
7171
"executeUserEntryPoint+0 in node:internal/modules/run_main:0",
7272
"<anonymous>+0 in node:internal/main/run_main_module:0",
7373
"V8::InternalFrame+0 in :0",

tools/coredump/testdata/amd64/node1815-baseline.json

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,34 +6,34 @@
66
"frames": [
77
"V8::ExitFrame+0 in :0",
88
"getColorDepth+0 in node:internal/tty:0",
9-
"value+5 in node:internal/console/constructor:320",
10-
"value+1 in node:internal/console/constructor:347",
11-
"warn+1 in node:internal/console/constructor:382",
9+
"value+5 in node:internal/console/constructor:320:19",
10+
"value+1 in node:internal/console/constructor:347:43",
11+
"warn+1 in node:internal/console/constructor:382:60",
1212
"V8::InternalFrame+0 in :0",
1313
"V8::EntryFrame+0 in :0",
1414
"node+0x9bfced",
1515
"node+0x9c0cb1",
1616
"node+0x877853",
1717
"node+0x7906bf",
1818
"V8::ExitFrame+0 in :0",
19-
"trace+6 in node:internal/console/constructor:428",
19+
"trace+6 in node:internal/console/constructor:428:9",
2020
"V8::InternalFrame+0 in :0",
2121
"V8::EntryFrame+0 in :0",
2222
"node+0x9bfced",
2323
"node+0x9c0cb1",
2424
"node+0x877853",
2525
"node+0x7906bf",
2626
"V8::ExitFrame+0 in :0",
27-
"add+1 in /home/tteras/work/elastic/node/test.js:3",
28-
"add3+1 in /home/tteras/work/elastic/node/test.js:8",
29-
"test+1 in /home/tteras/work/elastic/node/test.js:12",
30-
"submain+1 in /home/tteras/work/elastic/node/test.js:16",
31-
"main+2 in /home/tteras/work/elastic/node/test.js:23",
27+
"add+1 in /home/tteras/work/elastic/node/test.js:3:9",
28+
"add3+1 in /home/tteras/work/elastic/node/test.js:8:15",
29+
"test+1 in /home/tteras/work/elastic/node/test.js:12:8",
30+
"submain+1 in /home/tteras/work/elastic/node/test.js:16:28",
31+
"main+2 in /home/tteras/work/elastic/node/test.js:23:2",
3232
"<anonymous>+26 in /home/tteras/work/elastic/node/test.js:27",
33-
"Module._compile+46 in node:internal/modules/cjs/loader:1254",
34-
"Module._extensions..js+45 in node:internal/modules/cjs/loader:1308",
35-
"Module.load+12 in node:internal/modules/cjs/loader:1117",
36-
"Module._load+72 in node:internal/modules/cjs/loader:958",
33+
"Module._compile+46 in node:internal/modules/cjs/loader:1254:13",
34+
"Module._extensions..js+45 in node:internal/modules/cjs/loader:1308:9",
35+
"Module.load+12 in node:internal/modules/cjs/loader:1117:31",
36+
"Module._load+72 in node:internal/modules/cjs/loader:958:11",
3737
"executeUserEntryPoint+0 in node:internal/modules/run_main:0",
3838
"<anonymous>+0 in node:internal/main/run_main_module:0",
3939
"V8::InternalFrame+0 in :0",

tools/coredump/testdata/amd64/node1816-async.json

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,20 +13,20 @@
1313
"node+0x9fdd6f",
1414
"node+0x8f569c",
1515
"V8::BuiltinExitFrame+0 in :0",
16-
"trace+5 in node:internal/console/constructor:427",
16+
"trace+5 in node:internal/console/constructor:427:4",
1717
"V8::InternalFrame+0 in :0",
1818
"V8::EntryFrame+0 in :0",
1919
"node+0x9d5a90",
2020
"node+0x9d6a9c",
2121
"node+0x8884b6",
2222
"node+0x79c955",
2323
"V8::ExitFrame+0 in :0",
24-
"<anonymous>+2 in /home/tteras/work/elastic/node/async.js:29",
24+
"<anonymous>+2 in /home/tteras/work/elastic/node/async.js:29:9",
2525
"Promise+0 in <unknown>:0",
2626
"V8::ConstructFrame+0 in :0",
2727
"V8::StubFrame+0 in :0",
28-
"serveCustomer+1 in /home/tteras/work/elastic/node/async.js:27",
29-
"runRestaurant+4 in /home/tteras/work/elastic/node/async.js:43",
28+
"serveCustomer+1 in /home/tteras/work/elastic/node/async.js:27:9",
29+
"runRestaurant+4 in /home/tteras/work/elastic/node/async.js:43:8",
3030
"<anonymous>+0 in <unknown>:0",
3131
"V8::StubFrame+0 in :0",
3232
"V8::StubFrame+0 in :0",
@@ -39,10 +39,10 @@
3939
"node+0x8cd006",
4040
"node+0x8ce178",
4141
"V8::BuiltinExitFrame+0 in :0",
42-
"processTicksAndRejections+29 in node:internal/process/task_queues:96",
43-
"runNextTicks+6 in node:internal/process/task_queues:64",
44-
"listOnTimeout+21 in node:internal/timers:538",
45-
"processTimers+6 in node:internal/timers:503",
42+
"processTicksAndRejections+29 in node:internal/process/task_queues:96:28",
43+
"runNextTicks+6 in node:internal/process/task_queues:64:2",
44+
"listOnTimeout+21 in node:internal/timers:538:8",
45+
"processTimers+6 in node:internal/timers:503:4",
4646
"V8::InternalFrame+0 in :0",
4747
"V8::EntryFrame+0 in :0",
4848
"node+0x9d5a90",

0 commit comments

Comments
 (0)