AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

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

Browse Source 
This reverts commit efc1a98c53.

Reason for revert: breaks gc stress tests: https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20-%20gc%20stress/1928/overview

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}

Bug: v8:13440
Change-Id: Ida91f184e24a09e873388cd7c6d35849a5734178
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Owners-Override: +1
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4083964
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Francis McCabe <fgm@chromium.org>
Owners-Override: Francis McCabe <fgm@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#84699}
This commit is contained in:
Francis McCabe 2022-12-07 00:04:45 +00:00 committed by V8 LUCI CQ
parent efc1a98c53
commit 72f7c31e23
6 changed files with 37 additions and 126 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
test/mjsunit/tools/processor-bigint.mjs
View File

@ -1,59 +0,0 @@
// 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"));
})();

18
tools/codemap.mjs
View File

@ -65,12 +65,6 @@ 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).
@ -79,7 +73,7 @@ export class CodeMap {
* @param {CodeEntry} codeEntry Code entry object.
*/
addAnyCode(start, codeEntry) {
const pageAddr = (start / this.kPageSize) | this.kZero;
const pageAddr = (start / kPageSize) | 0;
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.
@ -153,8 +147,8 @@ export class CodeMap {
* @private
*/
markPages_(start, end) {
for (let addr = start; addr <= end; addr += this.kPageSize) {
this.pages_.add((addr / this.kPageSize) | this.kZero);
for (let addr = start; addr <= end; addr += kPageSize) {
this.pages_.add((addr / kPageSize) | 0);
}
}
@ -163,13 +157,13 @@ export class CodeMap {
*/
deleteAllCoveredNodes_(tree, start, end) {
const to_delete = [];
let addr = end - this.kOne;
let addr = end - 1;
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 - this.kOne;
addr = start2 - 1;
}
for (let i = 0, l = to_delete.length; i < l; ++i) tree.remove(to_delete[i]);
}
@ -197,7 +191,7 @@ export class CodeMap {
* @param {number} addr Address.
*/
findAddress(addr) {
const pageAddr = (addr / this.kPageSize) | this.kZero;
const pageAddr = (addr / kPageSize) | 0;
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.

27
tools/logreader.mjs
View File

@ -35,16 +35,6 @@
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.
*
@ -54,7 +44,7 @@ function containsUnsafeInts(fields) {
* @constructor
*/
export class LogReader {
constructor(timedRange=false, pairwiseTimedRange=false, useBigInt=false) {
constructor(timedRange=false, pairwiseTimedRange=false) {
this.dispatchTable_ = new Map();
this.timedRange_ = timedRange;
this.pairwiseTimedRange_ = pairwiseTimedRange;
@ -64,11 +54,6 @@ 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;
}
/**
@ -195,11 +180,11 @@ export class LogReader {
const firstChar = frame[0];
if (firstChar === '+' || firstChar === '-') {
// An offset from the previous frame.
prevFrame += this.parseFrame(frame);
prevFrame += parseInt(frame, 16);
fullStack.push(prevFrame);
// Filter out possible 'overflow' string.
} else if (firstChar !== 'o') {
fullStack.push(this.parseFrame(frame));
fullStack.push(parseInt(frame, 16));
} else {
console.error(`Dropping unknown tick frame: ${frame}`);
}
@ -231,12 +216,6 @@ 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);
}

12
tools/profile.mjs
View File

@ -305,6 +305,7 @@ const kProfileOperationTick = 2;
* @constructor
*/
export class Profile {
codeMap_ = new CodeMap();
topDownTree_ = new CallTree();
bottomUpTree_ = new CallTree();
c_entries_ = {__proto__:null};
@ -312,11 +313,6 @@ 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())
@ -517,7 +513,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, this.useBigInt);
func = new FunctionEntry(name);
this.codeMap_.addCode(funcAddr, func);
} else if (func.name !== name) {
// Function object has been overwritten with a new one.
@ -965,8 +961,8 @@ class FunctionEntry extends CodeEntry {
/** @type {Set<DynamicCodeEntry>} */
_codeEntries = new Set();
constructor(name, useBigInt=false) {
super(useBigInt ? 0n : 0, name);
constructor(name) {
super(0, name);
const index = name.lastIndexOf(' ');
this.functionName = 1 <= index ? name.substring(0, index) : '<anonymous>';
}

2
tools/system-analyzer/log/tick.mjs
View File

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

45
tools/system-analyzer/processor.mjs
View File

@ -47,6 +47,7 @@ class AsyncConsumer {
}
export class Processor extends LogReader {
_profile = new Profile();
_codeTimeline = new Timeline();
_deoptTimeline = new Timeline();
_icTimeline = new Timeline();
@ -69,16 +70,12 @@ export class Processor extends LogReader {
MAJOR_VERSION = 7;
MINOR_VERSION = 6;
constructor(useBigInt = false) {
super(false, false, useBigInt);
this.useBigInt = useBigInt;
this.kZero = useBigInt ? 0n : 0;
this.parseAddress = useBigInt ? BigInt : parseInt;
constructor() {
super();
this._chunkConsumer =
new AsyncConsumer((chunk) => this._processChunk(chunk));
this._profile = new Profile(useBigInt);
const propertyICParser = [
this.parseAddress, parseInt, parseInt, parseInt, parseString, parseString,
parseInt, parseInt, parseInt, parseInt, parseString, parseString,
parseString, parseString, parseString, parseString
];
this.setDispatchTable({
@ -91,14 +88,14 @@ export class Processor extends LogReader {
processor: this.processV8Version,
},
'shared-library': {
parsers: [parseString, this.parseAddress, this.parseAddress, parseInt],
parsers: [parseString, parseInt, parseInt, parseInt],
processor: this.processSharedLibrary.bind(this),
isAsync: true,
},
'code-creation': {
parsers: [
parseString, parseInt, parseInt, this.parseAddress, this.parseAddress,
parseString, parseVarArgs
parseString, parseInt, parseInt, parseInt, parseInt, parseString,
parseVarArgs
],
processor: this.processCodeCreation
},
@ -114,13 +111,17 @@ export class Processor extends LogReader {
'code-delete': {parsers: [parseInt], processor: this.processCodeDelete},
'code-source-info': {
parsers: [
this.parseAddress, parseInt, parseInt, parseInt, parseString,
parseString, parseString
parseInt, parseInt, parseInt, parseInt, parseString, parseString,
parseString
],
processor: this.processCodeSourceInfo
},
'code-disassemble': {
parsers: [this.parseAddress, parseString, parseString],
parsers: [
parseInt,
parseString,
parseString,
],
processor: this.processCodeDisassemble
},
'feedback-vector': {
@ -137,10 +138,8 @@ export class Processor extends LogReader {
'sfi-move':
{parsers: [parseInt, parseInt], processor: this.processFunctionMove},
'tick': {
parsers: [
this.parseAddress, parseInt, parseInt, this.parseAddress, parseInt,
parseVarArgs
],
parsers:
[parseInt, parseInt, parseInt, parseInt, parseInt, parseVarArgs],
processor: this.processTick
},
'active-runtime-timer': undefined,
@ -158,8 +157,8 @@ export class Processor extends LogReader {
{parsers: [parseInt, parseString], processor: this.processMapCreate},
'map': {
parsers: [
parseString, parseInt, parseString, parseString, this.parseAddress,
parseInt, parseInt, parseString, parseString
parseString, parseInt, parseString, parseString, parseInt, parseInt,
parseInt, parseString, parseString
],
processor: this.processMap
},
@ -311,9 +310,11 @@ export class Processor extends LogReader {
// Many events rely on having a script around, creating fake entries for
// shared libraries.
this._profile.addScriptSource(-1, name, '');
if (this._cppEntriesProvider == undefined) {
await this._setupCppEntriesProvider();
}
await this._cppEntriesProvider.parseVmSymbols(
name, startAddr, endAddr, aslrSlide, (fName, fStart, fEnd) => {
const entry = this._profile.addStaticCode(fName, fStart, fEnd);
@ -351,7 +352,7 @@ export class Processor extends LogReader {
let profilerEntry;
let stateName = '';
if (maybe_func.length) {
const funcAddr = this.parseAddress(maybe_func[0]);
const funcAddr = parseInt(maybe_func[0]);
stateName = maybe_func[1] ?? '';
const state = Profile.parseState(maybe_func[1]);
profilerEntry = this._profile.addFuncCode(
@ -438,13 +439,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 = this.kZero;
tos_or_external_callback = 0;
} 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 = this.kZero;
tos_or_external_callback = 0;
}
}
const entryStack = this._profile.recordTick(