[turbolizer] Solve off-by-one source position error for non-Wasm

Wasm has the attribute sourceLineToBytecodePosition and adds the source
lines via setSourceLineToBytecodePosition in which they are 0-based.
Non-Wasm doesn't have that attribute and uses insertSourcePositions
which is 1-based. In non-wasm we are being off by one.
As a note, the sourcePositionsInRange call in insertSourcePositions
doesn't return a list for Wasm since they rely on
setSourceLineToBytecodePosition and therefore do not have that
off-by-one error.

Drive-by: Several elements have the same source position so update
addHtmlElementToSourcePosition to handle more than one element.

Drive-by: Renames due to having the same name but different
capitalization, which was confusing.

Bug: v8:7327
Change-Id: Ie8a066ca629054a5f5a754deec0ed1917bed2b33
Notry: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3008634Reviewed-by: Nico Hartmann <nicohartmann@chromium.org>
Commit-Queue: Santiago Aboy Solanes <solanes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#75655}

tools/turbolizer/src/code-view.ts

@@ -27,7 +27,7 @@ export class CodeView extends View {
   source: Source;
   sourceResolver: SourceResolver;
   codeMode: CodeMode;
-  sourcePositionToHtmlElement: Map<string, HTMLElement>;
+  sourcePositionToHtmlElements: Map<string, Array<HTMLElement>>;
   showAdditionalInliningPosition: boolean;
   selectionHandler: SelectionHandler;
   selection: MySelection;
@@ -45,7 +45,7 @@ export class CodeView extends View {
     view.sourceResolver = sourceResolver;
     view.source = sourceFunction;
     view.codeMode = codeMode;
-    this.sourcePositionToHtmlElement = new Map();
+    this.sourcePositionToHtmlElements = new Map();
     this.showAdditionalInliningPosition = false;
 
     const selectionHandler = {
@@ -87,23 +87,25 @@ export class CodeView extends View {
 
   addHtmlElementToSourcePosition(sourcePosition, element) {
     const key = sourcePositionToStringKey(sourcePosition);
-    if (this.sourcePositionToHtmlElement.has(key)) {
-      console.log("Warning: duplicate source position", sourcePosition);
+    if (!this.sourcePositionToHtmlElements.has(key)) {
+      this.sourcePositionToHtmlElements.set(key, []);
     }
-    this.sourcePositionToHtmlElement.set(key, element);
+    this.sourcePositionToHtmlElements.get(key).push(element);
   }
 
   getHtmlElementForSourcePosition(sourcePosition) {
     const key = sourcePositionToStringKey(sourcePosition);
-    return this.sourcePositionToHtmlElement.get(key);
+    return this.sourcePositionToHtmlElements.get(key);
   }
 
   updateSelection(scrollIntoView: boolean = false): void {
     const mkVisible = new ViewElements(this.divNode.parentNode as HTMLElement);
-    for (const [sp, el] of this.sourcePositionToHtmlElement.entries()) {
+    for (const [sp, els] of this.sourcePositionToHtmlElements.entries()) {
       const isSelected = this.selection.isKeySelected(sp);
-      mkVisible.consider(el, isSelected);
-      el.classList.toggle("selected", isSelected);
+      for (const el of els) {
+        mkVisible.consider(el, isSelected);
+        el.classList.toggle("selected", isSelected);
+      }
     }
     mkVisible.apply(scrollIntoView);
   }
@@ -125,7 +127,7 @@ export class CodeView extends View {
     if (doClear) {
       this.selectionHandler.clear();
     }
-    const positions = this.sourceResolver.linetoSourcePositions(lineNumber - 1);
+    const positions = this.sourceResolver.lineToSourcePositions(lineNumber - 1);
     if (positions !== undefined) {
       this.selectionHandler.select(positions, undefined);
     }
@@ -235,7 +237,9 @@ export class CodeView extends View {
     const sps = this.sourceResolver.sourcePositionsInRange(this.source.sourceId, pos - adjust, end);
     let offset = 0;
     for (const sourcePosition of sps) {
-      this.sourceResolver.addAnyPositionToLine(lineNumber, sourcePosition);
+      // Internally, line numbers are 0-based so we have to substract 1 from the line number. This
+      // path in only taken by non-Wasm code. Wasm code relies on setSourceLineToBytecodePosition.
+      this.sourceResolver.addAnyPositionToLine(lineNumber - 1, sourcePosition);
       const textnode = currentSpan.tagName == 'SPAN' ? currentSpan.lastChild : currentSpan;
       if (!(textnode instanceof Text)) continue;
       const splitLength = Math.max(0, sourcePosition.scriptOffset - pos - offset);
@@ -271,11 +275,8 @@ export class CodeView extends View {
     lineNumberElement.dataset.lineNumber = `${lineNumber}`;
     lineNumberElement.innerText = `${lineNumber}`;
     lineElement.insertBefore(lineNumberElement, lineElement.firstChild);
-    // Don't add lines to source positions of not in backwardsCompatibility mode.
-    if (this.source.backwardsCompatibility === true) {
-      for (const sourcePosition of this.sourceResolver.linetoSourcePositions(lineNumber - 1)) {
-        view.addHtmlElementToSourcePosition(sourcePosition, lineElement);
-      }
+    for (const sourcePosition of this.sourceResolver.lineToSourcePositions(lineNumber - 1)) {
+      view.addHtmlElementToSourcePosition(sourcePosition, lineElement);
     }
   }
 

tools/turbolizer/src/source-resolver.ts

@@ -165,7 +165,7 @@ export class SourceResolver {
   phases: Array<Phase>;
   phaseNames: Map<string, number>;
   disassemblyPhase: Phase;
-  lineToSourcePositions: Map<string, Array<AnyPosition>>;
+  linePositionMap: Map<string, Array<AnyPosition>>;
   nodeIdToInstructionRange: Array<[number, number]>;
   blockIdToInstructionRange: Array<[number, number]>;
   instructionToPCOffset: Array<TurbolizerInstructionStartInfo>;
@@ -193,7 +193,7 @@ export class SourceResolver {
     // The disassembly phase is stored separately.
     this.disassemblyPhase = undefined;
     // Maps line numbers to source positions
-    this.lineToSourcePositions = new Map();
+    this.linePositionMap = new Map();
     // Maps node ids to instruction ranges.
     this.nodeIdToInstructionRange = [];
     // Maps block ids to instruction ranges.
@@ -653,10 +653,10 @@ export class SourceResolver {
 
   addAnyPositionToLine(lineNumber: number | string, sourcePosition: AnyPosition) {
     const lineNumberString = anyToString(lineNumber);
-    if (!this.lineToSourcePositions.has(lineNumberString)) {
-      this.lineToSourcePositions.set(lineNumberString, []);
+    if (!this.linePositionMap.has(lineNumberString)) {
+      this.linePositionMap.set(lineNumberString, []);
     }
-    const A = this.lineToSourcePositions.get(lineNumberString);
+    const A = this.linePositionMap.get(lineNumberString);
     if (!A.includes(sourcePosition)) A.push(sourcePosition);
   }
 
@@ -667,8 +667,8 @@ export class SourceResolver {
     });
   }
 
-  linetoSourcePositions(lineNumber: number | string) {
-    const positions = this.lineToSourcePositions.get(anyToString(lineNumber));
+  lineToSourcePositions(lineNumber: number | string) {
+    const positions = this.linePositionMap.get(anyToString(lineNumber));
     if (positions === undefined) return [];
     return positions;
   }