Skip to content

Commit 7431b7a

Browse files
Qardszegedi
authored andcommitted
Fix source mapping of zero-column locations (#248)
When lineNumbers is enabled, the column is always zero. If only one occurence of a call occurs on one line then that is correctly selected. However, in cases where the same function is called multiple times in the same line it will be unable to differentiate them and would use the unmapped value. This now makes it select the first call in the line as a best-guess for the match, and in Node.js v25 will use the new column field in LineTick to select the correct column where possible.
1 parent f1cbf30 commit 7431b7a

3 files changed

Lines changed: 201 additions & 2 deletions

File tree

bindings/translate-time-profile.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616

1717
#include "translate-time-profile.hh"
18+
#include <v8-version.h>
1819
#include "profile-translator.hh"
1920

2021
namespace dd {
@@ -99,7 +100,12 @@ class TimeProfileTranslator : ProfileTranslator {
99100
node->GetScriptResourceName(),
100101
scriptId,
101102
NewInteger(entry.line),
103+
// V8 14+ (Node.js 25+) added column field to LineTick struct
104+
#if V8_MAJOR_VERSION >= 14
105+
NewInteger(entry.column),
106+
#else
102107
zero,
108+
#endif
103109
NewInteger(entry.hit_count),
104110
emptyArray,
105111
emptyArray));

ts/src/sourcemapper/sourcemapper.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,15 @@ export class SourceMapper {
268268
const consumer: sourceMap.SourceMapConsumer =
269269
entry.mapConsumer as {} as sourceMap.SourceMapConsumer;
270270

271-
const pos = consumer.originalPositionFor(generatedPos);
271+
// When column is 0, we don't have real column info (e.g., from V8's LineTick
272+
// which only provides line numbers). Use LEAST_UPPER_BOUND to find the first
273+
// mapping on this line instead of failing because there's nothing at column 0.
274+
const bias =
275+
generatedPos.column === 0
276+
? sourceMap.SourceMapConsumer.LEAST_UPPER_BOUND
277+
: sourceMap.SourceMapConsumer.GREATEST_LOWER_BOUND;
278+
279+
const pos = consumer.originalPositionFor({...generatedPos, bias});
272280
if (pos.source === null) {
273281
if (this.debug) {
274282
logger.debug(

ts/test/test-profile-serializer.ts

Lines changed: 186 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,11 @@ import {
2323
} from '../src/profile-serializer';
2424
import {SourceMapper} from '../src/sourcemapper/sourcemapper';
2525
import {Label, Profile} from 'pprof-format';
26-
import {TimeProfile} from '../src/v8-types';
26+
import {TimeProfile, TimeProfileNode} from '../src/v8-types';
2727
import {
2828
anonymousFunctionHeapProfile,
2929
getAndVerifyPresence,
30+
getAndVerifyString,
3031
heapProfile,
3132
heapSourceProfile,
3233
labelEncodingProfile,
@@ -237,4 +238,188 @@ describe('profile-serializer', () => {
237238
tmp.setGracefulCleanup();
238239
});
239240
});
241+
242+
describe('source map with column 0 (LineTick simulation)', () => {
243+
// This tests the LEAST_UPPER_BOUND fallback for when V8's LineTick
244+
// doesn't provide column information (column=0)
245+
let sourceMapper: SourceMapper;
246+
let testMapDir: string;
247+
248+
// Line in source.ts that the first call maps to (column 10)
249+
const FIRST_CALL_SOURCE_LINE = 100;
250+
// Line in source.ts that the second call maps to (column 25)
251+
const SECOND_CALL_SOURCE_LINE = 200;
252+
253+
before(async () => {
254+
// Create a source map simulating: return fib(n-1) + fib(n-2)
255+
// Same function called twice on the same line at different columns
256+
testMapDir = tmp.dirSync().name;
257+
const {SourceMapGenerator} = await import('source-map');
258+
const fs = await import('fs');
259+
const path = await import('path');
260+
261+
const mapGen = new SourceMapGenerator({file: 'generated.js'});
262+
263+
// First fib() call at column 10 -> maps to source line 100
264+
mapGen.addMapping({
265+
source: path.join(testMapDir, 'source.ts'),
266+
name: 'fib',
267+
generated: {line: 5, column: 10},
268+
original: {line: FIRST_CALL_SOURCE_LINE, column: 0},
269+
});
270+
271+
// Second fib() call at column 25 -> maps to source line 200
272+
mapGen.addMapping({
273+
source: path.join(testMapDir, 'source.ts'),
274+
name: 'fib',
275+
generated: {line: 5, column: 25},
276+
original: {line: SECOND_CALL_SOURCE_LINE, column: 0},
277+
});
278+
279+
fs.writeFileSync(
280+
path.join(testMapDir, 'generated.js.map'),
281+
mapGen.toString()
282+
);
283+
fs.writeFileSync(path.join(testMapDir, 'generated.js'), '');
284+
285+
sourceMapper = await SourceMapper.create([testMapDir]);
286+
});
287+
288+
it('should map column 0 to first mapping on line (LEAST_UPPER_BOUND fallback)', () => {
289+
const path = require('path');
290+
// Simulate LineTick entry with column=0 (no column info from V8 < 14)
291+
// This is the fallback behavior when LineTick.column is not available
292+
const childNode: TimeProfileNode = {
293+
name: 'fib',
294+
scriptName: path.join(testMapDir, 'generated.js'),
295+
scriptId: 1,
296+
lineNumber: 5,
297+
columnNumber: 0, // LineTick has no column in V8 < 14
298+
hitCount: 1,
299+
children: [],
300+
};
301+
const v8Profile: TimeProfile = {
302+
startTime: 0,
303+
endTime: 1000000,
304+
topDownRoot: {
305+
name: '(root)',
306+
scriptName: 'root',
307+
scriptId: 0,
308+
lineNumber: 0,
309+
columnNumber: 0,
310+
hitCount: 0,
311+
children: [childNode],
312+
},
313+
};
314+
315+
const profile = serializeTimeProfile(v8Profile, 1000, sourceMapper);
316+
317+
assert.strictEqual(profile.location!.length, 1);
318+
const loc = profile.location![0];
319+
const line = loc.line![0];
320+
const func = getAndVerifyPresence(
321+
profile.function!,
322+
line.functionId as number
323+
);
324+
const filename = getAndVerifyString(
325+
profile.stringTable,
326+
func,
327+
'filename'
328+
);
329+
330+
// Should be mapped to source.ts
331+
assert.ok(
332+
filename.includes('source.ts'),
333+
`Expected source.ts but got ${filename}`
334+
);
335+
// With column 0 and LEAST_UPPER_BOUND, should map to FIRST mapping (line 100)
336+
assert.strictEqual(
337+
line.line,
338+
FIRST_CALL_SOURCE_LINE,
339+
'Column 0 should use LEAST_UPPER_BOUND to find first mapping on line'
340+
);
341+
});
342+
343+
it('should map to second call when column points to it (V8 14+ with LineTick.column)', () => {
344+
const path = require('path');
345+
// Simulate V8 14+ behavior where LineTick has actual column data
346+
// Column 26 is after the second mapping at column 25
347+
const childNode: TimeProfileNode = {
348+
name: 'fib',
349+
scriptName: path.join(testMapDir, 'generated.js'),
350+
scriptId: 1,
351+
lineNumber: 5,
352+
columnNumber: 26, // V8 14+ provides actual column from LineTick
353+
hitCount: 1,
354+
children: [],
355+
};
356+
const v8Profile: TimeProfile = {
357+
startTime: 0,
358+
endTime: 1000000,
359+
topDownRoot: {
360+
name: '(root)',
361+
scriptName: 'root',
362+
scriptId: 0,
363+
lineNumber: 0,
364+
columnNumber: 0,
365+
hitCount: 0,
366+
children: [childNode],
367+
},
368+
};
369+
370+
const profile = serializeTimeProfile(v8Profile, 1000, sourceMapper);
371+
372+
assert.strictEqual(profile.location!.length, 1);
373+
const loc = profile.location![0];
374+
const line = loc.line![0];
375+
376+
// Column 26 with GREATEST_LOWER_BOUND should map to second call (line 200)
377+
assert.strictEqual(
378+
line.line,
379+
SECOND_CALL_SOURCE_LINE,
380+
'Column 26 should use GREATEST_LOWER_BOUND to find mapping at column 25'
381+
);
382+
});
383+
384+
it('should map to first call when column points to it (V8 14+ with LineTick.column)', () => {
385+
const path = require('path');
386+
// Simulate V8 14+ behavior where LineTick has actual column data
387+
// Column 11 is after the first mapping at column 10 but before second at 25
388+
const childNode: TimeProfileNode = {
389+
name: 'fib',
390+
scriptName: path.join(testMapDir, 'generated.js'),
391+
scriptId: 1,
392+
lineNumber: 5,
393+
columnNumber: 11, // V8 14+ provides actual column from LineTick
394+
hitCount: 1,
395+
children: [],
396+
};
397+
const v8Profile: TimeProfile = {
398+
startTime: 0,
399+
endTime: 1000000,
400+
topDownRoot: {
401+
name: '(root)',
402+
scriptName: 'root',
403+
scriptId: 0,
404+
lineNumber: 0,
405+
columnNumber: 0,
406+
hitCount: 0,
407+
children: [childNode],
408+
},
409+
};
410+
411+
const profile = serializeTimeProfile(v8Profile, 1000, sourceMapper);
412+
413+
assert.strictEqual(profile.location!.length, 1);
414+
const loc = profile.location![0];
415+
const line = loc.line![0];
416+
417+
// Column 11 with GREATEST_LOWER_BOUND should map to first call (line 100)
418+
assert.strictEqual(
419+
line.line,
420+
FIRST_CALL_SOURCE_LINE,
421+
'Column 11 should use GREATEST_LOWER_BOUND to find mapping at column 10'
422+
);
423+
});
424+
});
240425
});

0 commit comments

Comments
 (0)