Reland "Use BigInts in processor.mjs and related code to avoid unsafe ints in calculations"

This is a reland of commit efc1a98c53

Changes since revert:
- Handle "shared-library", "code-{deopt,move,delete}", "feedback-vector", "sfi-move" events

Original change's description:
> Use BigInts in processor.mjs and related code to avoid unsafe ints in
calculations
>
> Bug: v8:13440
> Change-Id: Ie03b831b511a49fb475b9f303ef8662189bdaf3d
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4017455
> Reviewed-by: Camillo Bruni <cbruni@chromium.org>
> Commit-Queue: Camillo Bruni <cbruni@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#84698}

Change-Id: If45d38526cab887a59f60e3becfbcb084c3d41d0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4086641
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Vasili Skurydzin <vasili.skurydzin@ibm.com>
Cr-Commit-Position: refs/heads/main@{#84939}

test/mjsunit/tools/processor-bigint.mjs

@@ -0,0 +1,59 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --logfile='+' --log --log-maps --log-ic --log-code
+// Flags: --log-function-events --no-predictable
+
+import { Processor } from "../../../tools/system-analyzer/processor.mjs";
+
+// log code start
+function doWork() {
+  let array = [];
+  for (let i = 0; i < 500; i++) {
+    doWorkStep(i, array);
+  }
+  let sum = 0;
+  for (let i = 0; i < 500; i++) {
+    sum += array[i]["property" + i];
+  }
+  return sum;
+}
+
+function doWorkStep(i, array) {
+  const obj = {
+    ["property" + i]: i,
+  };
+  array.push(obj);
+  obj.custom1 = 1;
+  obj.custom2 = 2;
+}
+
+const result = doWork();
+ // log code end
+
+const logString = d8.log.getAndStop();
+assertTrue(logString.length > 0);
+const useBigInts = true;
+const processor = new Processor(useBigInts);
+await processor.processChunk(logString);
+await processor.finalize();
+
+const maps = processor.mapTimeline;
+const ics = processor.icTimeline;
+const scripts = processor.scripts;
+
+(function testResults() {
+  assertEquals(result, 124750);
+  assertTrue(maps.length > 0);
+  assertTrue(ics.length > 0);
+  assertTrue(scripts.length > 0);
+})();
+
+(function testIcKeys() {
+  const keys = new Set();
+  ics.forEach(ic => keys.add(ic.key));
+  assertTrue(keys.has("custom1"));
+  assertTrue(keys.has("custom2"));
+  assertTrue(keys.has("push"));
+})();

tools/codemap.mjs

@@ -65,6 +65,12 @@ export class CodeMap {
    */
   pages_ = new Set();
 
+  constructor(useBigInt=false) {
+    this.useBigInt = useBigInt;
+    this.kPageSize = useBigInt ? BigInt(kPageSize) : kPageSize;
+    this.kOne = useBigInt ? 1n : 1;
+    this.kZero = useBigInt ? 0n : 0;
+  }
 
   /**
    * Adds a code entry that might overlap with static code (e.g. for builtins).
@@ -73,7 +79,7 @@ export class CodeMap {
    * @param {CodeEntry} codeEntry Code entry object.
    */
   addAnyCode(start, codeEntry) {
-    const pageAddr = (start / kPageSize) | 0;
+    const pageAddr = (start / this.kPageSize) | this.kZero;
     if (!this.pages_.has(pageAddr)) return this.addCode(start, codeEntry);
     // We might have loaded static code (builtins, bytecode handlers)
     // and we get more information later in v8.log with code-creation events.
@@ -147,8 +153,8 @@ export class CodeMap {
    * @private
    */
   markPages_(start, end) {
-    for (let addr = start; addr <= end; addr += kPageSize) {
+    for (let addr = start; addr <= end; addr += this.kPageSize) {
-      this.pages_.add((addr / kPageSize) | 0);
+      this.pages_.add((addr / this.kPageSize) | this.kZero);
     }
   }
 
@@ -157,13 +163,13 @@ export class CodeMap {
    */
   deleteAllCoveredNodes_(tree, start, end) {
     const to_delete = [];
-    let addr = end - 1;
+    let addr = end - this.kOne;
     while (addr >= start) {
       const node = tree.findGreatestLessThan(addr);
       if (node === null) break;
       const start2 = node.key, end2 = start2 + node.value.size;
       if (start2 < end && start < end2) to_delete.push(start2);
-      addr = start2 - 1;
+      addr = start2 - this.kOne;
     }
     for (let i = 0, l = to_delete.length; i < l; ++i) tree.remove(to_delete[i]);
   }
@@ -191,7 +197,7 @@ export class CodeMap {
    * @param {number} addr Address.
    */
   findAddress(addr) {
-    const pageAddr = (addr / kPageSize) | 0;
+    const pageAddr = (addr / this.kPageSize) | this.kZero;
     if (this.pages_.has(pageAddr)) {
       // Static code entries can contain "holes" of unnamed code.
       // In this case, the whole library is assigned to this address.

tools/logreader.mjs

@@ -35,6 +35,16 @@
 export function parseString(field) { return field };
 export const parseVarArgs = 'parse-var-args';
 
+// Checks fields for numbers that are not safe integers. Returns true if any are
+// found.
+function containsUnsafeInts(fields) {
+  for (let i = 0; i < fields.length; i++) {
+    let field = fields[i];
+    if ('number' == typeof(field) && !Number.isSafeInteger(field)) return true;
+  }
+  return false;
+}
+
 /**
  * Base class for processing log files.
  *
@@ -44,7 +54,7 @@ export const parseVarArgs = 'parse-var-args';
  * @constructor
  */
 export class LogReader {
-  constructor(timedRange=false, pairwiseTimedRange=false) {
+  constructor(timedRange=false, pairwiseTimedRange=false, useBigInt=false) {
     this.dispatchTable_ = new Map();
     this.timedRange_ = timedRange;
     this.pairwiseTimedRange_ = pairwiseTimedRange;
@@ -54,6 +64,11 @@ export class LogReader {
     // Variables for tracking of 'current-time' log entries:
     this.hasSeenTimerMarker_ = false;
     this.logLinesSinceLastTimerMarker_ = [];
+    // Flag to parse all numeric fields as BigInt to avoid arithmetic errors
+    // caused by memory addresses being greater than MAX_SAFE_INTEGER
+    this.useBigInt = useBigInt;
+    this.parseFrame = useBigInt ? BigInt : parseInt;
+    this.hasSeenUnsafeIntegers = false;
   }
 
 /**
@@ -180,11 +195,11 @@ export class LogReader {
       const firstChar = frame[0];
       if (firstChar === '+' || firstChar === '-') {
         // An offset from the previous frame.
-        prevFrame += parseInt(frame, 16);
+        prevFrame += this.parseFrame(frame);
         fullStack.push(prevFrame);
       // Filter out possible 'overflow' string.
       } else if (firstChar !== 'o') {
-        fullStack.push(parseInt(frame, 16));
+        fullStack.push(this.parseFrame(frame));
       } else {
         console.error(`Dropping unknown tick frame: ${frame}`);
       }
@@ -216,6 +231,12 @@ export class LogReader {
         parsedFields[i] = parser(fields[1 + i]);
       }
     }
+    if (!this.useBigInt) {
+      if (!this.hasSeenUnsafeIntegers && containsUnsafeInts(parsedFields)) {
+        console.warn(`Log line containts unsafe integers: ${fields}`);
+        this.hasSeenUnsafeIntegers = true;
+      }
+    }
     // Run the processor.
     await dispatch.processor(...parsedFields);
   }

tools/profile.mjs

@@ -305,7 +305,6 @@ const kProfileOperationTick = 2;
  * @constructor
  */
 export class Profile {
-  codeMap_ = new CodeMap();
   topDownTree_ = new CallTree();
   bottomUpTree_ = new CallTree();
   c_entries_ = {__proto__:null};
@@ -313,6 +312,11 @@ export class Profile {
   urlToScript_ = new Map();
   warnings = new Set();
 
+  constructor(useBigInt=false) {
+    this.useBigInt = useBigInt;
+    this.codeMap_ = new CodeMap(useBigInt);
+  }
+
   serializeVMSymbols() {
     let result = this.codeMap_.getAllStaticEntriesWithAddresses();
     result.concat(this.codeMap_.getAllLibraryEntriesWithAddresses())
@@ -513,7 +517,7 @@ export class Profile {
     // it is safe to put them in a single code map.
     let func = this.codeMap_.findDynamicEntryByStartAddress(funcAddr);
     if (func === null) {
-      func = new FunctionEntry(name);
+      func = new FunctionEntry(name, this.useBigInt);
       this.codeMap_.addCode(funcAddr, func);
     } else if (func.name !== name) {
       // Function object has been overwritten with a new one.
@@ -961,8 +965,8 @@ class FunctionEntry extends CodeEntry {
   /** @type {Set<DynamicCodeEntry>} */
   _codeEntries = new Set();
 
-  constructor(name) {
+  constructor(name, useBigInt=false) {
-    super(0, name);
+    super(useBigInt ? 0n : 0, name);
     const index = name.lastIndexOf(' ');
     this.functionName = 1 <= index ? name.substring(0, index) : '<anonymous>';
   }

tools/system-analyzer/log/tick.mjs

@@ -42,7 +42,7 @@ export class TickLogEntry extends LogEntry {
       return 'Idle';
     }
     const topOfStack = processedStack[0];
-    if (typeof topOfStack === 'number') {
+    if (typeof topOfStack === 'number' || typeof topOfStack === 'bigint') {
       // TODO(cbruni): Handle VmStack and native ticks better.
       return 'Other';
     }

tools/system-analyzer/processor.mjs

@@ -47,7 +47,6 @@ class AsyncConsumer {
 }
 
 export class Processor extends LogReader {
-  _profile = new Profile();
   _codeTimeline = new Timeline();
   _deoptTimeline = new Timeline();
   _icTimeline = new Timeline();
@@ -70,12 +69,16 @@ export class Processor extends LogReader {
 
   MAJOR_VERSION = 7;
   MINOR_VERSION = 6;
-  constructor() {
+  constructor(useBigInt = false) {
-    super();
+    super(false, false, useBigInt);
+    this.useBigInt = useBigInt;
+    this.kZero = useBigInt ? 0n : 0;
+    this.parseAddress = useBigInt ? BigInt : parseInt;
     this._chunkConsumer =
         new AsyncConsumer((chunk) => this._processChunk(chunk));
+    this._profile = new Profile(useBigInt);
     const propertyICParser = [
-      parseInt, parseInt, parseInt, parseInt, parseString, parseString,
+      this.parseAddress, parseInt, parseInt, parseInt, parseString, parseString,
       parseString, parseString, parseString, parseString
     ];
     this.setDispatchTable({
@@ -88,46 +91,47 @@ export class Processor extends LogReader {
         processor: this.processV8Version,
       },
       'shared-library': {
-        parsers: [parseString, parseInt, parseInt, parseInt],
+        parsers: [
+          parseString, this.parseAddress, this.parseAddress, this.parseAddress
+        ],
         processor: this.processSharedLibrary.bind(this),
         isAsync: true,
       },
       'code-creation': {
         parsers: [
-          parseString, parseInt, parseInt, parseInt, parseInt, parseString,
+          parseString, parseInt, parseInt, this.parseAddress, this.parseAddress,
-          parseVarArgs
+          parseString, parseVarArgs
         ],
         processor: this.processCodeCreation
       },
       'code-deopt': {
         parsers: [
-          parseInt, parseInt, parseInt, parseInt, parseInt, parseString,
+          parseInt, parseInt, this.parseAddress, parseInt, parseInt,
-          parseString, parseString
+          parseString, parseString, parseString
         ],
         processor: this.processCodeDeopt
       },
-      'code-move':
+      'code-move': {
-          {parsers: [parseInt, parseInt], processor: this.processCodeMove},
+        parsers: [this.parseAddress, this.parseAddress],
-      'code-delete': {parsers: [parseInt], processor: this.processCodeDelete},
+        processor: this.processCodeMove
+      },
+      'code-delete':
+          {parsers: [this.parseAddress], processor: this.processCodeDelete},
       'code-source-info': {
         parsers: [
-          parseInt, parseInt, parseInt, parseInt, parseString, parseString,
+          this.parseAddress, parseInt, parseInt, parseInt, parseString,
-          parseString
+          parseString, parseString
         ],
         processor: this.processCodeSourceInfo
       },
       'code-disassemble': {
-        parsers: [
+        parsers: [this.parseAddress, parseString, parseString],
-          parseInt,
-          parseString,
-          parseString,
-        ],
         processor: this.processCodeDisassemble
       },
       'feedback-vector': {
         parsers: [
-          parseInt, parseString, parseInt, parseInt, parseString, parseString,
+          parseInt, parseString, parseInt, this.parseAddress, parseString,
-          parseInt, parseInt, parseString
+          parseString, parseInt, parseInt, parseString
         ],
         processor: this.processFeedbackVector
       },
@@ -135,11 +139,15 @@ export class Processor extends LogReader {
         parsers: [parseInt, parseString, parseString],
         processor: this.processScriptSource
       },
-      'sfi-move':
+      'sfi-move': {
-          {parsers: [parseInt, parseInt], processor: this.processFunctionMove},
+        parsers: [this.parseAddress, this.parseAddress],
+        processor: this.processFunctionMove
+      },
       'tick': {
-        parsers:
+        parsers: [
-            [parseInt, parseInt, parseInt, parseInt, parseInt, parseVarArgs],
+          this.parseAddress, parseInt, parseInt, this.parseAddress, parseInt,
+          parseVarArgs
+        ],
         processor: this.processTick
       },
       'active-runtime-timer': undefined,
@@ -157,8 +165,8 @@ export class Processor extends LogReader {
           {parsers: [parseInt, parseString], processor: this.processMapCreate},
       'map': {
         parsers: [
-          parseString, parseInt, parseString, parseString, parseInt, parseInt,
+          parseString, parseInt, parseString, parseString, this.parseAddress,
-          parseInt, parseString, parseString
+          parseInt, parseInt, parseString, parseString
         ],
         processor: this.processMap
       },
@@ -352,7 +360,7 @@ export class Processor extends LogReader {
     let profilerEntry;
     let stateName = '';
     if (maybe_func.length) {
-      const funcAddr = parseInt(maybe_func[0]);
+      const funcAddr = this.parseAddress(maybe_func[0]);
       stateName = maybe_func[1] ?? '';
       const state = Profile.parseState(maybe_func[1]);
       profilerEntry = this._profile.addFuncCode(
@@ -404,7 +412,7 @@ export class Processor extends LogReader {
       optimization_tier, invocation_count, profiler_ticks, fbv_string) {
     const profCodeEntry = this._profile.findEntry(instructionStart);
     if (!profCodeEntry) {
-      console.warn('Didn\'t find code for FBV', {fbv, instructionStart});
+      console.warn('Didn\'t find code for FBV', {fbv_string, instructionStart});
       return;
     }
     const fbv = new FeedbackVectorEntry(
@@ -439,13 +447,13 @@ export class Processor extends LogReader {
       // that a callback calls itself. Instead we use tos_or_external_callback,
       // as simply resetting PC will produce unaccounted ticks.
       pc = tos_or_external_callback;
-      tos_or_external_callback = 0;
+      tos_or_external_callback = this.kZero;
     } else if (tos_or_external_callback) {
       // Find out, if top of stack was pointing inside a JS function
       // meaning that we have encountered a frameless invocation.
       const funcEntry = this._profile.findEntry(tos_or_external_callback);
       if (!funcEntry?.isJSFunction?.()) {
-        tos_or_external_callback = 0;
+        tos_or_external_callback = this.kZero;
       }
     }
     const entryStack = this._profile.recordTick(