From 7063c2cbdd3011ca016e4fe5e5aa741a192dc47b Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Fri, 18 Sep 2026 23:39:52 -0700 Subject: [PATCH] fix: read crash diagnostics without loading whole dumps into memory Read crash diagnostics incrementally to avoid whole-dump memory spikes. --- .gitignore | 3 + docs/audits/crashpad-read-limit/README.md | 73 +++++ docs/audits/crashpad-read-limit/reproduce.mjs | 155 +++++++++ docs/audits/crashpad-read-limit/results.json | 48 +++ .../crashpad-read-limit/source-hashes.json | 25 ++ .../stream-signature-parity.cjs | 155 +++++++++ .../stream-signature-results.json | 28 ++ .../crashpad-capture-read-limit.test.ts | 303 ++++++++++++++++++ .../crash-reporting/crashpad-capture.test.ts | 5 +- src/main/crash-reporting/crashpad-capture.ts | 46 ++- .../minidump-crash-signature.test.ts | 80 ++--- .../minidump-crash-signature.ts | 119 ++----- .../minidump-crashpad-annotations.ts | 52 +-- .../minidump-embedded-check.ts | 112 +++++++ .../minidump-file-source.test.ts | 201 ++++++++++++ .../crash-reporting/minidump-file-source.ts | 99 ++++++ .../crash-reporting/minidump-stream-reader.ts | 80 +++-- 17 files changed, 1384 insertions(+), 200 deletions(-) create mode 100644 docs/audits/crashpad-read-limit/README.md create mode 100644 docs/audits/crashpad-read-limit/reproduce.mjs create mode 100644 docs/audits/crashpad-read-limit/results.json create mode 100644 docs/audits/crashpad-read-limit/source-hashes.json create mode 100644 docs/audits/crashpad-read-limit/stream-signature-parity.cjs create mode 100644 docs/audits/crashpad-read-limit/stream-signature-results.json create mode 100644 src/main/crash-reporting/crashpad-capture-read-limit.test.ts create mode 100644 src/main/crash-reporting/minidump-embedded-check.ts create mode 100644 src/main/crash-reporting/minidump-file-source.test.ts create mode 100644 src/main/crash-reporting/minidump-file-source.ts diff --git a/.gitignore b/.gitignore index 424cf5a2a0c..d81493eca6f 100644 --- a/.gitignore +++ b/.gitignore @@ -103,6 +103,9 @@ docs/** !docs/readme/ !docs/readme/** !docs/STYLEGUIDE.md +!docs/audits/ +!docs/audits/crashpad-read-limit/ +!docs/audits/crashpad-read-limit/source-hashes.json !docs/agent-skill-sharing-implementation-checklist.md !docs/mobile-terminal-shortcut-bar.md !docs/reference/ diff --git a/docs/audits/crashpad-read-limit/README.md b/docs/audits/crashpad-read-limit/README.md new file mode 100644 index 00000000000..91a1850864f --- /dev/null +++ b/docs/audits/crashpad-read-limit/README.md @@ -0,0 +1,73 @@ +# Crashpad bounded reads + +After a crash, Orca extracts a short diagnostic signature from a potentially large dump. +The old reader loaded the complete dump. The current parser reads bounded ranges through +four retained 64 KiB pages and scans embedded diagnostic text in roughly 1 MiB windows. +It can capture a report that grows beyond the existing 64 MiB directory-discovery limit +without keeping that report in one large buffer. Reports already beyond that limit when +discovered retain the existing exclusion policy. + +## Reproduction + +```sh +ORCA_BACKGROUND_LAUNCH=1 node docs/audits/crashpad-read-limit/reproduce.mjs +``` + +Use the repository's installed dependencies and supported Node version. The runner checks +committed SHA-256 values in `source-hashes.json` before executing any tests. This includes +the capture/parser, test fixtures, runner and its local imports, plus package and lockfile +versions. It refuses changed sources instead of silently recording new hashes. Updating +the manifest requires reviewing the changed evidence and rerunning the proof. + +The runner executes the actual capture and file-source tests with synthetic files, then +repeats them with only the zero-size extent deadline disabled through an in-memory Vite +transform. The mutation changes no files. This is a regression control for the deadline, +not a simulation of every behavior in the original implementation. + +| Phase | Pass | Fail | Skip | Exit | +| ---------------------- | ---: | ---: | ---: | ---: | +| Deadline removed | 24 | 2 | 0 | 1 | +| Current implementation | 26 | 0 | 0 | 0 | + +The two expected failures verify that a file reported as size zero stops being read at +the deadline, after either one or two pages, even though more bytes remain available. +The controls fail both on the observed extent and the number of actual file reads. +The runner checks their exact names and exits unsuccessfully for unexpected results. +`results.json` records the verified hashes, counts, failed cases, exit codes and timeouts. + +Other controls cover 80 MiB sparse dumps, metadata beyond 64 MiB, marker/check-message +boundaries, growth and replacement during capture, zero-size growth, truncation, descriptor +cleanup and partial-header retry behavior. A symlink swap, deleted candidate, or directory replacement now skips only that candidate; another valid dump can still supply the signature. The symlink-swap test is skipped on platforms without `O_NOFOLLOW`. The largest requested parser read is 1 MiB + +4,096 bytes, plus four retained 64 KiB metadata pages. A 1,042-module fixture requires no +more than eight reads, preventing repeated reads between module and name pages. + +## Parser compatibility + +```sh +ORCA_BACKGROUND_LAUNCH=1 node docs/audits/crashpad-read-limit/stream-signature-parity.cjs +``` + +The separate parity script compares 47 fixtures with the published parser at +`09dbe227547fadaec8d9163f35fd127b0dc1c3ed`, using both in-memory buffers and real file handles. +It covers annotations, modules, exceptions, chunk boundaries and marker exhaustion. It +also prints five warm timings for 8/64 MiB sparse dumps; these local synthetic timings +are not a platform-wide performance guarantee. This older parity script does not enforce +the source manifest; use the manifest-checking regression runner first. + +## Limits and user impact + +This removes a potentially large transient allocation after a crash; it does not establish +the cause of normal-session OOMs or an aggregate process-memory cap. Independent captures +can overlap. No evidence ties this file-growth race to #19831 or #19768. + +The opened file supplies the parsed ranges. In-place rewrites are not an atomic snapshot. +Nonempty files retain their opened extent. A file opened at size zero receives at least one +read, then observes growth only until the capture deadline; later bytes may therefore be +omitted from that diagnostic signature. This keeps a continuously growing report from +holding capture open indefinitely. Dump files themselves are not truncated by this reader. + +Crashpad normally writes an invalid header first and promotes a completed file later: +[database implementation](https://chromium.googlesource.com/crashpad/crashpad/+/refs/heads/main/client/crash_report_database_mac.mm) +and [minidump writer](https://chromium.googlesource.com/crashpad/crashpad/+/HEAD/minidump/minidump_file_writer.cc). +The fixtures represent that ordering; they do not reproduce a field incident or a native +Crashpad process. All checks run in background Node processes without app windows. diff --git a/docs/audits/crashpad-read-limit/reproduce.mjs b/docs/audits/crashpad-read-limit/reproduce.mjs new file mode 100644 index 00000000000..8492d31fe50 --- /dev/null +++ b/docs/audits/crashpad-read-limit/reproduce.mjs @@ -0,0 +1,155 @@ +import { createHash } from 'node:crypto' +import { constants as fsConstants } from 'node:fs' +import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import { createRequire } from 'node:module' +import { tmpdir } from 'node:os' +import { join, resolve } from 'node:path' +import { fileURLToPath } from 'node:url' +import { build } from 'esbuild' + +if (process.env.ORCA_BACKGROUND_LAUNCH !== '1') { + throw new Error('Run with ORCA_BACKGROUND_LAUNCH=1.') +} + +const root = fileURLToPath(new URL('../../../', import.meta.url)) +const expectedHashes = JSON.parse( + await readFile(new URL('./source-hashes.json', import.meta.url), 'utf8') +) +const sourceHashes = {} +for (const [path, expected] of Object.entries(expectedHashes)) { + const actual = createHash('sha256') + .update(await readFile(resolve(root, path))) + .digest('hex') + if (actual !== expected) { + throw new Error(`Source hash changed; review this evidence: ${path}`) + } + sourceHashes[path] = actual +} + +const scratch = await mkdtemp(join(tmpdir(), 'orca-crashpad-read-limit-')) +const require = createRequire(import.meta.url) +let runnerModuleId +try { + const runnerPath = join(scratch, 'run-process.cjs') + await build({ + absWorkingDir: root, + entryPoints: [resolve(root, 'src/shared/child-process/run-process.ts')], + outfile: runnerPath, + bundle: true, + platform: 'node', + format: 'cjs', + logLevel: 'silent' + }) + runnerModuleId = require.resolve(runnerPath) + const { runProcess } = require(runnerModuleId) + const includes = [ + 'src/main/crash-reporting/crashpad-capture-read-limit.test.ts', + 'src/main/crash-reporting/minidump-file-source.test.ts' + ] + const deadlinePath = resolve(root, 'src/main/crash-reporting/minidump-file-source.ts') + const current = await readFile(deadlinePath, 'utf8') + const deadlineCondition = + 'size > 0 && options.deadlineMs !== undefined && now() >= options.deadlineMs' + if (current.split(deadlineCondition).length !== 2) { + throw new Error('Deadline mutation no longer identifies exactly one branch.') + } + const withoutDeadline = current.replace(deadlineCondition, 'false') + const deadlineConfig = join(scratch, 'without-deadline.config.mjs') + const fixedConfig = join(scratch, 'fixed.config.mjs') + const config = { + test: { + environment: 'node', + include: includes, + testTimeout: 30_000, + execArgv: ['--no-experimental-webstorage'] + } + } + await writeFile( + deadlineConfig, + `export default {...${JSON.stringify(config)}, plugins: [{ + name: 'disable-extent-deadline', enforce: 'pre', + transform(_code, id) { + return id.split('?')[0].replaceAll('\\\\', '/') === ${JSON.stringify(deadlinePath.replaceAll('\\', '/'))} + ? {code: ${JSON.stringify(withoutDeadline)}, map: null} : null; + } + }]};\n` + ) + await writeFile(fixedConfig, `export default ${JSON.stringify(config)};\n`) + + async function run(label, configPath) { + const report = join(scratch, `${label}.json`) + const result = await runProcess({ + program: process.execPath, + args: [ + resolve(root, 'node_modules/vitest/vitest.mjs'), + 'run', + '--config', + configPath, + '--reporter=json', + `--outputFile=${report}` + ], + cwd: root, + env: process.env, + timeoutMs: 90_000, + maxOutputBytes: 4 * 1024 * 1024 + }) + let parsed + try { + parsed = JSON.parse(await readFile(report, 'utf8')) + } catch (error) { + throw new Error(`${label} runner failed: ${result.stderr || result.stdout}`, { cause: error }) + } + return { + exitCode: result.code, + timedOut: result.timedOut, + passed: parsed.numPassedTests, + failed: parsed.numFailedTests, + skipped: parsed.numPendingTests, + failedCases: parsed.testResults.flatMap((suite) => + suite.assertionResults + .filter((test) => test.status === 'failed') + .map((test) => test.fullName) + ) + } + } + + const withoutDeadlineResult = await run('without-deadline', deadlineConfig) + const fixed = await run('fixed', fixedConfig) + const expectedFailures = [1, 2].map( + (pages) => `stops observing a growing size-zero dump after the deadline at page ${pages}` + ) + const skippedSymlink = fsConstants.O_NOFOLLOW ? 0 : 1 + const passed = + withoutDeadlineResult.exitCode === 1 && + !withoutDeadlineResult.timedOut && + withoutDeadlineResult.failed === 2 && + withoutDeadlineResult.passed === 24 - skippedSymlink && + withoutDeadlineResult.skipped === skippedSymlink && + JSON.stringify(withoutDeadlineResult.failedCases) === JSON.stringify(expectedFailures) && + fixed.exitCode === 0 && + !fixed.timedOut && + fixed.passed === 26 - skippedSymlink && + fixed.failed === 0 && + fixed.skipped === skippedSymlink + const result = { + comparison: + 'Current capture and file-source regressions; negative control removes only the extent deadline in memory.', + sourceHashes, + withoutDeadline: withoutDeadlineResult, + fixed, + passed + } + await writeFile( + new URL('./results.json', import.meta.url), + `${JSON.stringify(result, null, 2)}\n` + ) + console.log(JSON.stringify(result, null, 2)) + if (!passed) { + process.exitCode = 1 + } +} finally { + if (runnerModuleId) { + delete require.cache[runnerModuleId] + } + await rm(scratch, { recursive: true, force: true }) +} diff --git a/docs/audits/crashpad-read-limit/results.json b/docs/audits/crashpad-read-limit/results.json new file mode 100644 index 00000000000..09fcbcad884 --- /dev/null +++ b/docs/audits/crashpad-read-limit/results.json @@ -0,0 +1,48 @@ +{ + "comparison": "Current capture and file-source regressions; negative control removes only the extent deadline in memory.", + "sourceHashes": { + "docs/audits/crashpad-read-limit/reproduce.mjs": "f933664d7956dc935dc54873f43f1e9628830e4eb30f85759ba7dc2590b3c276", + "docs/audits/crashpad-read-limit/stream-signature-parity.cjs": "7ed52f7c94ff888e48b0dcdcdce0ca3b5135dd9dd8b237309321b2a87b64426d", + "package.json": "039a5464a296a1ad4cd0456154da71843e1bd3c63337e24c3767119f44a573ec", + "pnpm-lock.yaml": "759c01a2687e953a9adc5c09332bafa193471ba6ec4875468565abb4ddf0d498", + "src/main/crash-reporting/crashpad-capture-read-limit.test.ts": "259050b4b65d68cccf2597aed5f3776237028dd9b5653b0963a391ba4171a1f3", + "src/main/crash-reporting/crashpad-capture.ts": "43ebef6ef55c0639df7c69fbfdb8a9f4d899db67dd4de7d56adae7e47bef8c77", + "src/main/crash-reporting/minidump-crash-signature.test.ts": "2a393e98c5cf5c70e371e17baf7c689871180db0e84585f547d094a2ae05e6b9", + "src/main/crash-reporting/minidump-crash-signature.ts": "e82d9739727ec04cbae845495d68ae08d1dc5c5045445832882b8a0547396fca", + "src/main/crash-reporting/minidump-crashpad-annotations.ts": "34b136b27eb95b713c857df4f40eb4e82e63889883426c606c0f94210c69050e", + "src/main/crash-reporting/minidump-embedded-check.ts": "3f6bbe5532d5d2ef1d719ceacec3d83f10544424cbbc36254785e8819e2c0de4", + "src/main/crash-reporting/minidump-file-source.test.ts": "083b0b74fed8ebb103c800256479799348144b38dfa4d46e20d746652bc877ad", + "src/main/crash-reporting/minidump-file-source.ts": "4b869262332187ed822fc3df9b2c268ed0b06dc7b2ebe5f7bb635059076ce66d", + "src/main/crash-reporting/minidump-stream-reader.ts": "11361dce22b1ec60ac6b7d100df8f33999ecf124811956cdba365c8e892f2701", + "src/shared/child-process/bounded-output-sink.ts": "7dbab9fc70dac8e5d19dd03d743b55609092b45ca13423e2dc80a69cf7fc4865", + "src/shared/child-process/child-termination-reporter.ts": "d91ff09b26c9a23e26c2a2f92b129879cd6d0bddea14cd3bcac5d4489fecfca0", + "src/shared/child-process/process-spec.ts": "97e52b5e320da1eed4cfb2c0b73a88696b62f355eb4b5fe0df5389764afb8289", + "src/shared/child-process/process-tree-kill-gate.ts": "62848f063785e15cb0231deb622b043e81484ed3040c612afdc4f98fa2db06ab", + "src/shared/child-process/process-tree-termination.ts": "49f001659c872e23040fe5bab2dd387f2589178f5bc3ce9361373303d23ff120", + "src/shared/child-process/run-process.ts": "ae4dad239273351ce61269cd7f7abb8325f3f620be600fa9c0ae760c1dd7cbad", + "src/shared/child-process/spawn-resolution.ts": "8c62c3c14d6f110f25bab23e3f4b2633a68e2f36252bd3aac68cffd7b2e274dc", + "src/shared/child-process/windows-cmd-shim-resolution.ts": "5044e3a9140a19c8bb713044de655b611244659a754f32e5a5df7f519df40eb3", + "src/shared/child-process/windows-command-line.ts": "4bf94aa90ee63d965bc6e1198ddafa4a715c05d78be979025d17dc59d90557ca", + "tsconfig.json": "d96e07b4a56a47f45f62be9dd1d0d1150d3be42b4c91d918d5b182b6a78cf16c" + }, + "withoutDeadline": { + "exitCode": 1, + "timedOut": false, + "passed": 24, + "failed": 2, + "skipped": 0, + "failedCases": [ + "stops observing a growing size-zero dump after the deadline at page 1", + "stops observing a growing size-zero dump after the deadline at page 2" + ] + }, + "fixed": { + "exitCode": 0, + "timedOut": false, + "passed": 26, + "failed": 0, + "skipped": 0, + "failedCases": [] + }, + "passed": true +} diff --git a/docs/audits/crashpad-read-limit/source-hashes.json b/docs/audits/crashpad-read-limit/source-hashes.json new file mode 100644 index 00000000000..7b0adc79614 --- /dev/null +++ b/docs/audits/crashpad-read-limit/source-hashes.json @@ -0,0 +1,25 @@ +{ + "docs/audits/crashpad-read-limit/reproduce.mjs": "f933664d7956dc935dc54873f43f1e9628830e4eb30f85759ba7dc2590b3c276", + "docs/audits/crashpad-read-limit/stream-signature-parity.cjs": "7ed52f7c94ff888e48b0dcdcdce0ca3b5135dd9dd8b237309321b2a87b64426d", + "package.json": "039a5464a296a1ad4cd0456154da71843e1bd3c63337e24c3767119f44a573ec", + "pnpm-lock.yaml": "759c01a2687e953a9adc5c09332bafa193471ba6ec4875468565abb4ddf0d498", + "src/main/crash-reporting/crashpad-capture-read-limit.test.ts": "259050b4b65d68cccf2597aed5f3776237028dd9b5653b0963a391ba4171a1f3", + "src/main/crash-reporting/crashpad-capture.ts": "43ebef6ef55c0639df7c69fbfdb8a9f4d899db67dd4de7d56adae7e47bef8c77", + "src/main/crash-reporting/minidump-crash-signature.test.ts": "2a393e98c5cf5c70e371e17baf7c689871180db0e84585f547d094a2ae05e6b9", + "src/main/crash-reporting/minidump-crash-signature.ts": "e82d9739727ec04cbae845495d68ae08d1dc5c5045445832882b8a0547396fca", + "src/main/crash-reporting/minidump-crashpad-annotations.ts": "34b136b27eb95b713c857df4f40eb4e82e63889883426c606c0f94210c69050e", + "src/main/crash-reporting/minidump-embedded-check.ts": "3f6bbe5532d5d2ef1d719ceacec3d83f10544424cbbc36254785e8819e2c0de4", + "src/main/crash-reporting/minidump-file-source.test.ts": "083b0b74fed8ebb103c800256479799348144b38dfa4d46e20d746652bc877ad", + "src/main/crash-reporting/minidump-file-source.ts": "4b869262332187ed822fc3df9b2c268ed0b06dc7b2ebe5f7bb635059076ce66d", + "src/main/crash-reporting/minidump-stream-reader.ts": "11361dce22b1ec60ac6b7d100df8f33999ecf124811956cdba365c8e892f2701", + "src/shared/child-process/bounded-output-sink.ts": "7dbab9fc70dac8e5d19dd03d743b55609092b45ca13423e2dc80a69cf7fc4865", + "src/shared/child-process/child-termination-reporter.ts": "d91ff09b26c9a23e26c2a2f92b129879cd6d0bddea14cd3bcac5d4489fecfca0", + "src/shared/child-process/process-spec.ts": "97e52b5e320da1eed4cfb2c0b73a88696b62f355eb4b5fe0df5389764afb8289", + "src/shared/child-process/process-tree-kill-gate.ts": "62848f063785e15cb0231deb622b043e81484ed3040c612afdc4f98fa2db06ab", + "src/shared/child-process/process-tree-termination.ts": "49f001659c872e23040fe5bab2dd387f2589178f5bc3ce9361373303d23ff120", + "src/shared/child-process/run-process.ts": "ae4dad239273351ce61269cd7f7abb8325f3f620be600fa9c0ae760c1dd7cbad", + "src/shared/child-process/spawn-resolution.ts": "8c62c3c14d6f110f25bab23e3f4b2633a68e2f36252bd3aac68cffd7b2e274dc", + "src/shared/child-process/windows-cmd-shim-resolution.ts": "5044e3a9140a19c8bb713044de655b611244659a754f32e5a5df7f519df40eb3", + "src/shared/child-process/windows-command-line.ts": "4bf94aa90ee63d965bc6e1198ddafa4a715c05d78be979025d17dc59d90557ca", + "tsconfig.json": "d96e07b4a56a47f45f62be9dd1d0d1150d3be42b4c91d918d5b182b6a78cf16c" +} diff --git a/docs/audits/crashpad-read-limit/stream-signature-parity.cjs b/docs/audits/crashpad-read-limit/stream-signature-parity.cjs new file mode 100644 index 00000000000..be9bc1707fb --- /dev/null +++ b/docs/audits/crashpad-read-limit/stream-signature-parity.cjs @@ -0,0 +1,155 @@ +const fs = require('node:fs') +const fsp = require('node:fs/promises') +const cp = require('node:child_process') +const path = require('node:path') +const os = require('node:os') +const assert = require('node:assert/strict') +const esbuild = require(path.join(process.cwd(), 'node_modules/esbuild')) +const before = '09dbe227547fadaec8d9163f35fd127b0dc1c3ed' +const prefix = 'src/main/crash-reporting/' +const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'orca-crash-stream-parity-')) +const source = fs.readFileSync(`${prefix}minidump-crash-signature.test.ts`, 'utf8') +const fixture = `const expect = (v) => ({toBe: (e) => {if(v!==e) throw new Error('fixture invariant')}});\n${source.slice( + source.indexOf('const STREAM_TYPE_'), + source.indexOf("describe('parseMinidumpCrashSignature'") +)}` +const line = '[8104:1234:0815/143022.123456:FATAL:render_frame_impl.cc(4821)] Check failed: !x.' +async function build() { + for (const variant of ['before', 'after']) { + await esbuild.build({ + entryPoints: [`${prefix}minidump-crash-signature.ts`], + outfile: path.join(dir, `${variant}.cjs`), + bundle: true, + platform: 'node', + format: 'cjs', + logLevel: 'silent', + plugins: + variant === 'before' + ? [ + { + name: 'baseline', + setup(b) { + b.onLoad( + { + filter: /minidump-(crash-signature|stream-reader|crashpad-annotations)\.ts$/ + }, + (a) => ({ + contents: cp.execFileSync( + 'git', + ['show', `${before}:${path.relative(process.cwd(), a.path)}`], + { encoding: 'utf8' } + ), + loader: 'ts' + }) + ) + } + } + ] + : [] + }) + } + await esbuild.build({ + stdin: { contents: `${fixture}\nexport {buildDump}`, loader: 'ts', resolveDir: process.cwd() }, + outfile: path.join(dir, 'fixture.cjs'), + bundle: true, + platform: 'node', + format: 'cjs', + logLevel: 'silent' + }) + await esbuild.build({ + entryPoints: [`${prefix}minidump-file-source.ts`], + outfile: path.join(dir, 'file.cjs'), + bundle: true, + platform: 'node', + format: 'cjs', + logLevel: 'silent' + }) +} +;(async () => { + try { + await build() + const baseline = require(path.join(dir, 'before.cjs')).parseMinidumpCrashSignature + const candidate = require(path.join(dir, 'after.cjs')).parseMinidumpCrashSignature + const create = require(path.join(dir, 'file.cjs')).createMinidumpFileSource + const { buildDump } = require(path.join(dir, 'fixture.cjs')) + let checked = 0 + const inputs = [] + for (const annotations of [ + {}, + { ptype: 'renderer' }, + { ptype: 'gpu-process', LOG_FATAL: line }, + { ptype: 'renderer', 'abort-message': line } + ]) { + for (const moduleCount of [0, 1, 8, 128]) { + const { dump } = buildDump({ + annotations, + modules: Array.from({ length: moduleCount }, (_, i) => ({ + base: BigInt(0x1000 + i * 0x1000), + size: 0x1000, + name: `x${i}.dll` + })), + exception: { code: 0x80000003, address: 0x1001n } + }) + inputs.push(dump) + } + } + const header = buildDump({ annotations: { ptype: 'renderer' } }).dump + for (const shift of [-100, -40, -1, 0, 1, 40, 100]) { + for (const severity of ['FATAL', 'CHECK', 'DFATAL', 'ERROR']) { + const text = line.replace('FATAL', severity) + 'x'.repeat(4000 - line.length) + const bytes = Buffer.alloc(1024 * 1024 + shift + text.length + 2) + header.copy(bytes) + bytes.write(text, 1024 * 1024 + shift) + inputs.push(bytes) + } + } + for (const n of [255, 256, 257]) { + inputs.push(Buffer.concat([header, Buffer.from(`${':FATAL:bad\0'.repeat(n) + line}\0`)])) + } + for (const bytes of inputs) { + const expected = baseline(bytes) + assert.deepEqual(await candidate(bytes), expected) + const p = path.join(dir, 'sample.dmp') + await fsp.writeFile(p, bytes) + const h = await fsp.open(p, 'r') + try { + assert.deepEqual(await candidate(create(h, bytes.length)), expected) + } finally { + await h.close() + } + checked++ + } + const timing = [] + for (const size of [8, 64]) { + const p = path.join(dir, 'large.dmp') + await fsp.writeFile(p, header) + await fsp.truncate(p, size * 1024 * 1024) + const samples = { before: [], after: [] } + for (let i = 0; i < 6; i++) { + for (const mode of ['before', 'after']) { + const t = performance.now() + if (mode === 'before') { + baseline(await fsp.readFile(p)) + } else { + const h = await fsp.open(p, 'r') + try { + await candidate(create(h, size * 1024 * 1024)) + } finally { + await h.close() + } + } + if (i > 0) { + samples[mode].push(performance.now() - t) + } + } + } + timing.push({ sizeMiB: size, ...samples }) + } + console.log(JSON.stringify({ before, parityCases: checked, timing }, null, 2)) + } finally { + fs.rmSync(dir, { recursive: true, force: true }) + } +})().catch((e) => { + console.error(e) + process.exitCode = 1 +}) diff --git a/docs/audits/crashpad-read-limit/stream-signature-results.json b/docs/audits/crashpad-read-limit/stream-signature-results.json new file mode 100644 index 00000000000..788cb19cfe7 --- /dev/null +++ b/docs/audits/crashpad-read-limit/stream-signature-results.json @@ -0,0 +1,28 @@ +{ + "before": "09dbe227547fadaec8d9163f35fd127b0dc1c3ed", + "parityCases": 47, + "timing": [ + { + "sizeMiB": 8, + "before": [ + 1.695459000000028, 1.2253749999999854, 1.7137500000000045, 1.7986250000000155, + 1.6254159999999729 + ], + "after": [ + 2.4827920000000177, 1.3731669999999667, 1.6862090000000762, 1.6049580000000105, + 1.7520840000000817 + ] + }, + { + "sizeMiB": 64, + "before": [ + 14.819833000000017, 12.504792000000066, 10.920583000000079, 10.27516700000001, + 10.765292000000045 + ], + "after": [ + 10.903083000000038, 10.107749999999896, 10.629999999999995, 9.732708000000002, + 9.414375000000064 + ] + } + ] +} diff --git a/src/main/crash-reporting/crashpad-capture-read-limit.test.ts b/src/main/crash-reporting/crashpad-capture-read-limit.test.ts new file mode 100644 index 00000000000..6c285958f7c --- /dev/null +++ b/src/main/crash-reporting/crashpad-capture-read-limit.test.ts @@ -0,0 +1,303 @@ +import type * as FsModule from 'node:fs/promises' +import type * as ParserModule from './minidump-crash-signature' +import { constants as fsConstants } from 'node:fs' +import { + mkdtemp, + mkdir, + open, + rm, + rename, + symlink, + truncate, + utimes, + writeFile +} from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, expect, it, vi } from 'vitest' + +const state = vi.hoisted(() => { + const callbacks: { + afterStat?: (path: string) => Promise + afterOpenStat?: (path: string) => Promise + } = {} + const parsedBytes: number[] = [] + const closedPaths: string[] = [] + return { callbacks, parsedBytes, closedPaths } +}) +vi.mock('electron', () => ({ + app: { getPath: () => '/unused' }, + crashReporter: { start: vi.fn() } +})) +vi.mock('node:fs/promises', async (original) => { + const fs = await original() + return { + ...fs, + open: async (...args: Parameters) => { + const handle = await fs.open(...args) + const readStat = handle.stat.bind(handle) + const close = handle.close.bind(handle) + return Object.assign(handle, { + stat: async () => { + const stats = await readStat() + await state.callbacks.afterOpenStat?.(String(args[0])) + return stats + }, + close: async () => { + await close() + state.closedPaths.push(String(args[0])) + } + }) + }, + stat: async (...args: Parameters) => { + const stats = await fs.stat(...args) + await state.callbacks.afterStat?.(String(args[0])) + return stats + } + } +}) +vi.mock('./minidump-crash-signature', async (original) => { + const parser = await original() + return { + ...parser, + parseMinidumpCrashSignature: ( + ...args: Parameters + ) => { + state.parsedBytes.push(args[0].byteLength) + return parser.parseMinidumpCrashSignature(...args) + } + } +}) +import { _setCrashpadCaptureStateForTest, captureMinidumpSignature } from './crashpad-capture' + +const LIMIT = 64 * 1024 * 1024 +const CRASHED_AT = 1_700_000_000_000 +let directory: string +function rendererDump() { + const dump = Buffer.alloc(131) + dump.writeUInt32LE(0x504d444d, 0) + dump.writeUInt32LE(0xa793, 4) + dump.writeUInt32LE(1, 8) + dump.writeUInt32LE(32, 12) + dump.writeUInt32LE(0x43500001, 32) + dump.writeUInt32LE(52, 36) + dump.writeUInt32LE(44, 40) + dump.writeUInt32LE(1, 44) + dump.writeUInt32LE(12, 80) + dump.writeUInt32LE(96, 84) + dump.writeUInt32LE(1, 96) + dump.writeUInt32LE(108, 100) + dump.writeUInt32LE(118, 104) + dump.writeUInt32LE(5, 108) + dump.write('ptype', 112) + dump.writeUInt32LE(8, 118) + dump.write('renderer', 122) + return dump +} +async function file(name: string, newer = false, bytes = rendererDump()) { + const path = join(directory, 'new', name) + await writeFile(path, bytes) + await utimes(path, CRASHED_AT / 1000, (CRASHED_AT + (newer ? 100 : 0)) / 1000) + return path +} +function capture() { + return captureMinidumpSignature(CRASHED_AT, { + expectedProcessType: 'renderer', + timeoutMs: 0, + now: () => CRASHED_AT + }) +} +beforeEach(async () => { + directory = await mkdtemp(join(tmpdir(), 'orca-crashpad-limit-')) + await mkdir(join(directory, 'new')) + state.callbacks.afterStat = undefined + state.callbacks.afterOpenStat = undefined + state.parsedBytes.length = 0 + state.closedPaths.length = 0 + _setCrashpadCaptureStateForTest({ dumpDirectory: directory, started: true }) +}) +afterEach(async () => { + _setCrashpadCaptureStateForTest(null) + await rm(directory, { recursive: true, force: true }) +}) + +it.each(['growth', 'replacement'] as const)( + 'captures %s past the initial limit without a whole-file allocation', + async (kind) => { + const next = await file('next.dmp') + const unfinished = rendererDump() + unfinished.writeUInt32LE(0, 0) + const race = await file('racing.dmp', true, unfinished) + state.callbacks.afterStat = async (path) => { + if (path !== race) { + return + } + state.callbacks.afterStat = undefined + if (kind === 'replacement') { + await rename(race, join(directory, 'retired.bin')) + await writeFile(race, unfinished) + } + await truncate(race, LIMIT + 1024 * 1024) + const handle = await open(race, 'r+') + try { + await handle.write(rendererDump().subarray(0, 4), 0, 4, 0) + } finally { + await handle.close() + } + } + const result = await capture() + expect(state.parsedBytes).toEqual([LIMIT + 1024 * 1024]) + expect(result?.filePath).toBe(race) + expect(next).not.toBe(race) + } +) + +it('accepts exactly the existing 64 MiB limit', async () => { + const path = await file('limit.dmp') + await truncate(path, LIMIT) + const result = await capture() + expect(result?.filePath).toBe(path) + expect(result?.sizeBytes).toBe(LIMIT) + expect(state.parsedBytes).toEqual([LIMIT]) +}) + +it('ignores an already oversize candidate without parsing it', async () => { + const next = await file('next.dmp') + const path = await file('oversize.dmp', true) + await truncate(path, LIMIT + 1024 * 1024) + const result = await capture() + expect(result?.filePath).toBe(next) + expect(state.parsedBytes).toEqual([131]) +}) + +it('documents same-path partial-header rejection for this capture window', async () => { + const path = await file('partial.dmp', false, Buffer.from('MDMP')) + let clock = CRASHED_AT + const result = await captureMinidumpSignature(CRASHED_AT, { + expectedProcessType: 'renderer', + timeoutMs: 500, + now: () => clock, + sleep: async (ms) => { + clock += ms + await writeFile(path, rendererDump()) + } + }) + expect(result).toBeNull() + expect(state.parsedBytes).toEqual([4]) + expect((await capture())?.filePath).toBe(path) +}) + +it('can recover partial-header rejection after completed-path promotion', async () => { + const path = await file('partial.dmp', false, Buffer.from('MDMP')) + const promoted = join(directory, 'pending', 'partial.dmp') + let clock = CRASHED_AT + const result = await captureMinidumpSignature(CRASHED_AT, { + expectedProcessType: 'renderer', + timeoutMs: 500, + now: () => clock, + sleep: async (ms) => { + clock += ms + await writeFile(path, rendererDump()) + await mkdir(join(directory, 'pending')) + await rename(path, promoted) + } + }) + expect(result?.filePath).toBe(promoted) + expect(state.parsedBytes).toEqual([4, 131]) +}) + +it('preserves the opened dump when later growth exceeds the limit', async () => { + const next = await file('next.dmp') + const race = await file('racing.dmp', true) + let grew = false + state.callbacks.afterOpenStat = async (path) => { + if (path !== race) { + return + } + state.callbacks.afterOpenStat = undefined + grew = true + await truncate(race, LIMIT + 1024 * 1024) + } + const result = await capture() + expect(grew).toBe(true) + expect(result?.filePath).toBe(race) + expect(result?.sizeBytes).toBe(131) + expect(state.parsedBytes).toEqual([131]) + expect(state.closedPaths).toEqual([race]) + expect((await capture())?.filePath).toBe(next) +}) + +it('records bytes actually parsed when permitted growth follows directory stat', async () => { + const race = await file('racing.dmp') + state.callbacks.afterStat = async (path) => { + if (path !== race) { + return + } + state.callbacks.afterStat = undefined + await truncate(race, 132) + } + expect((await capture())?.sizeBytes).toBe(132) + expect(state.parsedBytes).toEqual([132]) +}) + +it('captures a size-zero opened dump that gains contents before its first read', async () => { + const race = await file('racing.dmp', false, Buffer.alloc(0)) + state.callbacks.afterOpenStat = async (path) => { + if (path === race) { + state.callbacks.afterOpenStat = undefined + await writeFile(path, rendererDump()) + } + } + const result = await capture() + expect(result?.filePath).toBe(race) + expect(result?.signature.processType).toBe('renderer') + expect(result?.sizeBytes).toBe(131) +}) + +it('reports the observed shorter extent when the opened dump shrinks while reading', async () => { + const race = await file('shrinking.dmp', false, Buffer.concat([rendererDump(), Buffer.alloc(10)])) + state.callbacks.afterOpenStat = async (path) => { + if (path === race) { + state.callbacks.afterOpenStat = undefined + await truncate(path, 131) + } + } + const result = await capture() + expect(result?.signature.processType).toBe('renderer') + expect(result?.sizeBytes).toBe(131) +}) + +it.skipIf(!fsConstants.O_NOFOLLOW)( + 'skips a swapped symlink and captures another valid dump', + async () => { + const next = await file('next.dmp') + const race = await file('swapped.dmp', true) + state.callbacks.afterStat = async (path) => { + if (path === race) { + state.callbacks.afterStat = undefined + await rm(race) + await symlink(next, race) + } + } + expect((await capture())?.filePath).toBe(next) + expect(state.parsedBytes).toEqual([131]) + expect(state.closedPaths).toEqual([next]) + } +) + +it.each(['missing', 'directory'] as const)('skips a candidate replaced by %s', async (kind) => { + const next = await file('next.dmp') + const race = await file('swapped.dmp', true) + state.callbacks.afterStat = async (path) => { + if (path === race) { + state.callbacks.afterStat = undefined + await rm(race) + if (kind === 'directory') { + await mkdir(race) + } + } + } + expect((await capture())?.filePath).toBe(next) + expect(state.parsedBytes).toEqual([131]) + expect(state.closedPaths).toContain(next) +}) diff --git a/src/main/crash-reporting/crashpad-capture.test.ts b/src/main/crash-reporting/crashpad-capture.test.ts index 093dc092cf6..cde6341d524 100644 --- a/src/main/crash-reporting/crashpad-capture.test.ts +++ b/src/main/crash-reporting/crashpad-capture.test.ts @@ -1,3 +1,4 @@ +import type { MinidumpSource } from './minidump-stream-reader' import { mkdtemp, mkdir, readdir, rm, utimes, writeFile } from 'node:fs/promises' import os from 'node:os' import path from 'node:path' @@ -200,8 +201,8 @@ describe('captureMinidumpSignature', () => { Buffer.from('renderer') ) await writeDump(path.join('reports', 'gpu.dmp'), CRASHED_AT + 200, Buffer.from('gpu-process')) - parseMinidumpCrashSignatureMock.mockImplementation((dump: Buffer) => ({ - processType: dump.toString('utf8'), + parseMinidumpCrashSignatureMock.mockImplementation(async (dump: MinidumpSource) => ({ + processType: (await dump.read(0, dump.byteLength)).toString('utf8'), annotations: {} })) diff --git a/src/main/crash-reporting/crashpad-capture.ts b/src/main/crash-reporting/crashpad-capture.ts index 1f628fe9c24..beff750c960 100644 --- a/src/main/crash-reporting/crashpad-capture.ts +++ b/src/main/crash-reporting/crashpad-capture.ts @@ -6,10 +6,12 @@ // bundle. We keep dumps on disk and lift the *text* signature out of them, so // a CHECK failure becomes nameable without shipping raw memory anywhere. +import { constants as fsConstants } from 'node:fs' import type { Dirent } from 'node:fs' -import { readdir, readFile, rm, stat } from 'node:fs/promises' +import { open, readdir, rm, stat } from 'node:fs/promises' import path from 'node:path' import { app, crashReporter } from 'electron' +import { createMinidumpFileSource, observeMinidumpExtent } from './minidump-file-source' import { parseMinidumpCrashSignature, type MinidumpCrashSignature @@ -230,7 +232,7 @@ function freshDumpCandidates(candidates: DumpCandidate[], crashedAtMs: number): async function pollDumpCandidates( crashedAtMs: number, options: DumpPollingOptions, - select: (candidate: DumpCandidate) => Promise + select: (candidate: DumpCandidate, deadlineMs: number) => Promise ): Promise { const directory = crashpadDumpDirectory if (!directory) { @@ -245,7 +247,7 @@ async function pollDumpCandidates( for (;;) { const fresh = freshDumpCandidates(await collectDumpCandidates(directory), crashedAtMs) for (const candidate of fresh) { - const selected = await select(candidate) + const selected = await select(candidate, deadline) if (selected !== null) { return selected } @@ -282,7 +284,7 @@ export async function captureMinidumpSignature( ): Promise { const rejectedDumpPaths = new Set() try { - return await pollDumpCandidates(crashedAtMs, options, async (dump) => { + return await pollDumpCandidates(crashedAtMs, options, async (dump, deadlineMs) => { if ( rejectedDumpPaths.has(dump.filePath) || claimedDumpPaths.has(dump.filePath) || @@ -292,9 +294,39 @@ export async function captureMinidumpSignature( } reservedDumpPaths.add(dump.filePath) try { - const signature = parseMinidumpCrashSignature(await readFile(dump.filePath), { - expectedProcessType: options.expectedProcessType + // Reject symlink swaps where the platform exposes O_NOFOLLOW; the regular-file + // check below covers descriptors opened on every platform. + const noFollow = fsConstants.O_NOFOLLOW ?? 0 + const handle = await open( + dump.filePath, + noFollow === 0 ? 'r' : fsConstants.O_RDONLY | noFollow + ).catch(() => { + rejectedDumpPaths.add(dump.filePath) + return null }) + if (handle === null) { + return null + } + let signature: MinidumpCrashSignature | null + let sizeBytes: number + try { + const stats = await handle.stat() + if (!stats.isFile()) { + rejectedDumpPaths.add(dump.filePath) + return null + } + sizeBytes = await observeMinidumpExtent(handle, stats.size, { + deadlineMs, + now: options.now + }) + const source = createMinidumpFileSource(handle, sizeBytes) + signature = await parseMinidumpCrashSignature(source, { + expectedProcessType: options.expectedProcessType + }) + sizeBytes = source.byteLength + } finally { + await handle.close() + } if ( !signature || (options.expectedProcessType !== undefined && @@ -304,7 +336,7 @@ export async function captureMinidumpSignature( return null } claimedDumpPaths.set(dump.filePath, dump.mtimeMs) - return { filePath: dump.filePath, sizeBytes: dump.size, signature } + return { filePath: dump.filePath, sizeBytes, signature } } finally { reservedDumpPaths.delete(dump.filePath) } diff --git a/src/main/crash-reporting/minidump-crash-signature.test.ts b/src/main/crash-reporting/minidump-crash-signature.test.ts index 8e878cf3d23..73274fec3a8 100644 --- a/src/main/crash-reporting/minidump-crash-signature.test.ts +++ b/src/main/crash-reporting/minidump-crash-signature.test.ts @@ -217,12 +217,12 @@ const ELECTRON_43_CHECK_LINE = '[29136:0815/232206.330:ERROR:third_party\\blink\\common\\chrome_debug_urls.cc:180] Intentionally causing CHECK because user navigated to chrome://checkcrash/' describe('parseMinidumpCrashSignature', () => { - it('names the failing CHECK from the LOG_FATAL annotation', () => { + it('names the failing CHECK from the LOG_FATAL annotation', async () => { const { dump } = buildDump({ annotations: { LOG_FATAL: FATAL_LINE, ptype: 'renderer' } }) - const signature = parseMinidumpCrashSignature(dump) + const signature = await parseMinidumpCrashSignature(dump) expect(signature?.checkMessage).toBe(FATAL_LINE) expect(signature?.checkFile).toBe('render_frame_impl.cc') @@ -230,14 +230,14 @@ describe('parseMinidumpCrashSignature', () => { expect(signature?.processType).toBe('renderer') }) - it('recovers a CHECK line from Electron 43 dump memory without LOG_FATAL', () => { + it('recovers a CHECK line from Electron 43 dump memory without LOG_FATAL', async () => { const { dump } = buildDump({ annotations: { ptype: 'renderer' } }) const dumpWithMemory = Buffer.concat([ dump, Buffer.from(`\0${ELECTRON_43_CHECK_LINE}\0`, 'utf8') ]) - const signature = parseMinidumpCrashSignature(dumpWithMemory) + const signature = await parseMinidumpCrashSignature(dumpWithMemory) expect(signature?.checkMessage).toBe(ELECTRON_43_CHECK_LINE) expect(signature?.checkFile).toBe('chrome_debug_urls.cc') @@ -245,14 +245,14 @@ describe('parseMinidumpCrashSignature', () => { expect(signature?.processType).toBe('renderer') }) - it('stops at the process type when the dump belongs to another process', () => { + it('stops at the process type when the dump belongs to another process', async () => { const { dump } = buildDump({ annotations: { ptype: 'gpu-process' } }) const dumpWithMemory = Buffer.concat([ dump, Buffer.from(`\0${ELECTRON_43_CHECK_LINE}\0`, 'utf8') ]) - const signature = parseMinidumpCrashSignature(dumpWithMemory, { + const signature = await parseMinidumpCrashSignature(dumpWithMemory, { expectedProcessType: 'renderer' }) @@ -261,49 +261,49 @@ describe('parseMinidumpCrashSignature', () => { expect(signature?.checkMessage).toBeUndefined() }) - it('still parses fully when the process type matches', () => { + it('still parses fully when the process type matches', async () => { const { dump } = buildDump({ annotations: { ptype: 'renderer' } }) const dumpWithMemory = Buffer.concat([ dump, Buffer.from(`\0${ELECTRON_43_CHECK_LINE}\0`, 'utf8') ]) - const signature = parseMinidumpCrashSignature(dumpWithMemory, { + const signature = await parseMinidumpCrashSignature(dumpWithMemory, { expectedProcessType: 'renderer' }) expect(signature?.checkMessage).toBe(ELECTRON_43_CHECK_LINE) }) - it('ignores a log prefix further back than the prefix limit', () => { + it('ignores a log prefix further back than the prefix limit', async () => { const { dump } = buildDump({ annotations: { ptype: 'renderer' } }) // `[` separated from the marker by more than MAX_LOG_PREFIX_BYTES (96). const farPrefix = `[${'x'.repeat(200)}:FATAL:render_frame_impl.cc(4821)] Check failed: far.` const dumpWithMemory = Buffer.concat([dump, Buffer.from(`\0${farPrefix}\0`, 'utf8')]) - expect(parseMinidumpCrashSignature(dumpWithMemory)?.checkMessage).toBeUndefined() + expect((await parseMinidumpCrashSignature(dumpWithMemory))?.checkMessage).toBeUndefined() }) - it('does not promote an unrelated Chromium ERROR line containing CHECK', () => { + it('does not promote an unrelated Chromium ERROR line containing CHECK', async () => { const { dump } = buildDump({}) const unrelated = '[29136:0815/232206.330:ERROR:settings.cc:44] Opened the CHECK settings panel.' const dumpWithMemory = Buffer.concat([dump, Buffer.from(`\0${unrelated}\0`, 'utf8')]) - expect(parseMinidumpCrashSignature(dumpWithMemory)?.checkMessage).toBeUndefined() + expect((await parseMinidumpCrashSignature(dumpWithMemory))?.checkMessage).toBeUndefined() }) - it('prefers the structured annotation over a dump-memory candidate', () => { + it('prefers the structured annotation over a dump-memory candidate', async () => { const { dump } = buildDump({ annotations: { LOG_FATAL: FATAL_LINE } }) const dumpWithMemory = Buffer.concat([ dump, Buffer.from(`\0${ELECTRON_43_CHECK_LINE}\0`, 'utf8') ]) - expect(parseMinidumpCrashSignature(dumpWithMemory)?.checkMessage).toBe(FATAL_LINE) + expect((await parseMinidumpCrashSignature(dumpWithMemory))?.checkMessage).toBe(FATAL_LINE) }) - it('reads annotations from the process-level simple string dictionary', () => { + it('reads annotations from the process-level simple string dictionary', async () => { const { dump } = buildDump({ simpleAnnotations: { ptype: 'gpu-process', @@ -311,13 +311,13 @@ describe('parseMinidumpCrashSignature', () => { } }) - const signature = parseMinidumpCrashSignature(dump) + const signature = await parseMinidumpCrashSignature(dump) expect(signature?.processType).toBe('gpu-process') expect(signature?.annotations['gpu-gl-vendor']).toBe('Intel Inc.') }) - it('drops annotations outside the allowlist', () => { + it('drops annotations outside the allowlist', async () => { const { dump } = buildDump({ annotations: { LOG_FATAL: FATAL_LINE, @@ -325,13 +325,13 @@ describe('parseMinidumpCrashSignature', () => { } }) - const signature = parseMinidumpCrashSignature(dump) + const signature = await parseMinidumpCrashSignature(dump) expect(signature?.annotations['switch-3']).toBeUndefined() expect(Object.keys(signature?.annotations ?? {})).toEqual(['LOG_FATAL']) }) - it('resolves the faulting module from the exception address', () => { + it('resolves the faulting module from the exception address', async () => { const { dump } = buildDump({ exception: { code: 0x80000003, address: 0x7ff8_0000_1234n }, modules: [ @@ -348,7 +348,7 @@ describe('parseMinidumpCrashSignature', () => { ] }) - const signature = parseMinidumpCrashSignature(dump) + const signature = await parseMinidumpCrashSignature(dump) expect(signature?.exceptionCode).toBe(0x80000003) expect(signature?.exceptionAddress).toBe('0x7ff800001234') @@ -356,7 +356,7 @@ describe('parseMinidumpCrashSignature', () => { expect(signature?.faultingModuleOffset).toBe('0x1234') }) - it('resolves a faulting module past index 1024 on a real macOS image count', () => { + it('resolves a faulting module past index 1024 on a real macOS image count', async () => { // A measured macOS renderer carries 1042 loaded images; a cap below that // dropped the whole module list, so no macOS report could name a module. const modules = Array.from({ length: 1042 }, (_, index) => ({ @@ -369,13 +369,13 @@ describe('parseMinidumpCrashSignature', () => { modules }) - const signature = parseMinidumpCrashSignature(dump) + const signature = await parseMinidumpCrashSignature(dump) expect(signature?.faultingModule).toBe('lib1030.dylib') expect(signature?.faultingModuleOffset).toBe('0x24') }) - it('still drops the module list when the claimed module count is absurd', () => { + it('still drops the module list when the claimed module count is absurd', async () => { const { dump } = buildDump({ exception: { code: 11, address: 0x7ff7_0000_0010n }, modules: [{ base: 0x7ff7_0000_0000n, size: 0x1000, name: '/opt/orca/orca' }] @@ -383,48 +383,48 @@ describe('parseMinidumpCrashSignature', () => { const corrupt = Buffer.from(dump) corrupt.writeUInt32LE(0xffff_ffff, moduleListRva(corrupt)) - expect(() => parseMinidumpCrashSignature(corrupt)).not.toThrow() - expect(parseMinidumpCrashSignature(corrupt)?.faultingModule).toBeUndefined() + await expect(parseMinidumpCrashSignature(corrupt)).resolves.not.toBeNull() + expect((await parseMinidumpCrashSignature(corrupt))?.faultingModule).toBeUndefined() }) - it('omits the faulting module when no image range covers the address', () => { + it('omits the faulting module when no image range covers the address', async () => { const { dump } = buildDump({ exception: { code: 11, address: 0x10n }, modules: [{ base: 0x7ff7_0000_0000n, size: 0x1000, name: '/opt/orca/orca' }] }) - const signature = parseMinidumpCrashSignature(dump) + const signature = await parseMinidumpCrashSignature(dump) expect(signature?.exceptionAddress).toBe('0x10') expect(signature?.faultingModule).toBeUndefined() }) - it('returns null for a buffer that is not a minidump', () => { - expect(parseMinidumpCrashSignature(Buffer.from('not a dump at all', 'utf8'))).toBeNull() - expect(parseMinidumpCrashSignature(Buffer.alloc(0))).toBeNull() + it('returns null for a buffer that is not a minidump', async () => { + expect(await parseMinidumpCrashSignature(Buffer.from('not a dump at all', 'utf8'))).toBeNull() + expect(await parseMinidumpCrashSignature(Buffer.alloc(0))).toBeNull() }) - it('degrades instead of throwing on a truncated dump', () => { + it('degrades instead of throwing on a truncated dump', async () => { const { dump } = buildDump({ annotations: { LOG_FATAL: FATAL_LINE } }) const truncated = dump.subarray(0, 48) - expect(() => parseMinidumpCrashSignature(truncated)).not.toThrow() - expect(parseMinidumpCrashSignature(truncated)?.checkMessage).toBeUndefined() + await expect(parseMinidumpCrashSignature(truncated)).resolves.not.toBeNull() + expect((await parseMinidumpCrashSignature(truncated))?.checkMessage).toBeUndefined() }) - it('degrades instead of throwing when stream counts are corrupt', () => { + it('degrades instead of throwing when stream counts are corrupt', async () => { const { dump } = buildDump({ annotations: { LOG_FATAL: FATAL_LINE } }) const corrupt = Buffer.from(dump) corrupt.writeUInt32LE(0xffff_ffff, 8) - expect(() => parseMinidumpCrashSignature(corrupt)).not.toThrow() - expect(parseMinidumpCrashSignature(corrupt)?.annotations).toEqual({}) + await expect(parseMinidumpCrashSignature(corrupt)).resolves.not.toBeNull() + expect((await parseMinidumpCrashSignature(corrupt))?.annotations).toEqual({}) }) }) describe('minidumpSignatureDetails', () => { - it('flattens the check location and faulting module into detail keys', () => { + it('flattens the check location and faulting module into detail keys', async () => { const { dump } = buildDump({ annotations: { LOG_FATAL: FATAL_LINE, @@ -435,7 +435,7 @@ describe('minidumpSignatureDetails', () => { modules: [{ base: 0x7ff8_0000_0000n, size: 0x10_0000, name: 'chrome_elf.dll' }] }) - const details = minidumpSignatureDetails(parseMinidumpCrashSignature(dump)!) + const details = minidumpSignatureDetails((await parseMinidumpCrashSignature(dump))!) expect(details).toMatchObject({ minidumpCheckMessage: FATAL_LINE, @@ -448,10 +448,10 @@ describe('minidumpSignatureDetails', () => { }) }) - it('does not duplicate the fatal line into an annotation key', () => { + it('does not duplicate the fatal line into an annotation key', async () => { const { dump } = buildDump({ annotations: { LOG_FATAL: FATAL_LINE } }) - const details = minidumpSignatureDetails(parseMinidumpCrashSignature(dump)!) + const details = minidumpSignatureDetails((await parseMinidumpCrashSignature(dump))!) expect(details.minidumpAnnotation_LOG_FATAL).toBeUndefined() }) diff --git a/src/main/crash-reporting/minidump-crash-signature.ts b/src/main/crash-reporting/minidump-crash-signature.ts index fe4e930504d..f122f2b1eb8 100644 --- a/src/main/crash-reporting/minidump-crash-signature.ts +++ b/src/main/crash-reporting/minidump-crash-signature.ts @@ -11,8 +11,9 @@ // rather than throwing: a truncated dump must degrade, not break crash // reporting. -import { findStream, isMinidump, MinidumpView } from './minidump-stream-reader' +import { findStream, isMinidump, MinidumpView, type MinidumpSource } from './minidump-stream-reader' import { readCrashpadAnnotations } from './minidump-crashpad-annotations' +import { findEmbeddedCheckMessage } from './minidump-embedded-check' const STREAM_TYPE_MODULE_LIST = 4 const STREAM_TYPE_EXCEPTION = 6 @@ -30,19 +31,6 @@ const EXCEPTION_RECORD_OFFSET = 8 const EXCEPTION_CODE_OFFSET = EXCEPTION_RECORD_OFFSET + 0 const EXCEPTION_ADDRESS_OFFSET = EXCEPTION_RECORD_OFFSET + 16 -const CHROMIUM_LOG_MARKERS = [ - Buffer.from(':FATAL:', 'ascii'), - Buffer.from(':CHECK:', 'ascii'), - Buffer.from(':DFATAL:', 'ascii'), - Buffer.from(':ERROR:', 'ascii') -] -const MAX_LOG_PREFIX_BYTES = 96 -const MAX_CHECK_LOG_BYTES = 4_000 -const MAX_MARKERS_PER_SEVERITY = 256 -const CHECK_LOG_PATTERN = - /^\[(?:\d+:){1,2}\d{4}\/\d{6}\.\d{3,6}:(FATAL|CHECK|DFATAL|ERROR)(?::[^:\]\r\n]{1,80})*:([^:\]\r\n]{1,512}?)(?:\((\d+)\)|:(\d+))\]\s*(.+)$/ -const ERROR_CHECK_PATTERN = /\b(?:Check failed:|D?CHECK failed:|Intentionally causing D?CHECK\b)/i - export type MinidumpCrashSignature = { /** Chromium's fatal log line, e.g. `[...:FATAL:node.cc(123)] Check failed: !x.` */ readonly checkMessage?: string @@ -67,25 +55,25 @@ type ModuleRecord = { readonly name: string } -function readModules(view: MinidumpView): ModuleRecord[] { - const stream = findStream(view, STREAM_TYPE_MODULE_LIST) +async function readModules(view: MinidumpView): Promise { + const stream = await findStream(view, STREAM_TYPE_MODULE_LIST) if (!stream) { return [] } - const count = view.u32(stream.rva) + const count = await view.u32(stream.rva) if (count === null || count > MAX_MODULE_LIST_MODULES) { return [] } const modules: ModuleRecord[] = [] for (let index = 0; index < count; index += 1) { const record = stream.rva + 4 + index * MODULE_RECORD_SIZE - const base = view.u64(record + MODULE_BASE_OFFSET) - const size = view.u32(record + MODULE_SIZE_OFFSET) - const nameRva = view.u32(record + MODULE_NAME_RVA_OFFSET) + const base = await view.u64(record + MODULE_BASE_OFFSET) + const size = await view.u32(record + MODULE_SIZE_OFFSET) + const nameRva = await view.u32(record + MODULE_NAME_RVA_OFFSET) if (base === null || size === null || nameRva === null) { break } - const name = view.utf16String(nameRva, 2_048) + const name = await view.utf16String(nameRva, 2_048) modules.push({ base, size, name: name ?? 'unknown' }) } return modules @@ -100,67 +88,6 @@ function toHex(value: bigint): string { return `0x${value.toString(16)}` } -type LocatedCheckMessage = { - readonly message: string - readonly file?: string - readonly line?: number -} - -function isPrintableLogByte(value: number): boolean { - return value === 0x09 || (value >= 0x20 && value <= 0x7e) -} - -/** - * `lastIndexOf(byte, from)` restricted to `within` bytes before `from`. An - * unbounded search scans the whole dump backward on a miss only for the result - * to be thrown away by the same prefix limit; zero-filled regions are normal in - * a minidump, so that miss is the common case, not the adversarial one. - */ -function lastIndexOfWithin(dump: Buffer, byte: number, from: number, within: number): number { - const floor = Math.max(0, from - within) - for (let at = from; at >= floor; at -= 1) { - if (dump[at] === byte) { - return at - } - } - return -1 -} - -/** Electron 43 omits LOG_FATAL but keeps Chromium's formatted log line in memory. */ -function findEmbeddedCheckMessage(dump: Buffer): LocatedCheckMessage | undefined { - for (const marker of CHROMIUM_LOG_MARKERS) { - let from = 0 - for (let inspected = 0; inspected < MAX_MARKERS_PER_SEVERITY; inspected += 1) { - const markerAt = dump.indexOf(marker, from) - if (markerAt === -1) { - break - } - from = markerAt + marker.length - const start = lastIndexOfWithin(dump, 0x5b, markerAt, MAX_LOG_PREFIX_BYTES) - if (start === -1) { - continue - } - let end = markerAt + marker.length - const limit = Math.min(dump.length, start + MAX_CHECK_LOG_BYTES) - while (end < limit && isPrintableLogByte(dump[end])) { - end += 1 - } - const candidate = dump.subarray(start, end).toString('utf8') - const match = CHECK_LOG_PATTERN.exec(candidate) - if (!match || (match[1] === 'ERROR' && !ERROR_CHECK_PATTERN.test(match[5]))) { - continue - } - const line = Number.parseInt(match[3] ?? match[4], 10) - return { - message: candidate, - file: moduleBasename(match[2]), - line: Number.isFinite(line) ? line : undefined - } - } - } - return undefined -} - /** Parses the annotation form, which uses `file.cc(123)`. */ function parseCheckLocation(checkMessage: string): { file?: string @@ -203,15 +130,21 @@ export type MinidumpParseOptions = { * Parses a Crashpad minidump into the fields that make a CHECK failure * nameable. Returns null when the buffer is not a minidump. */ -export function parseMinidumpCrashSignature( - dump: Buffer, +export async function parseMinidumpCrashSignature( + dump: Buffer | MinidumpSource, options: MinidumpParseOptions = {} -): MinidumpCrashSignature | null { - if (!isMinidump(dump)) { +): Promise { + const source: MinidumpSource = Buffer.isBuffer(dump) + ? { + byteLength: dump.length, + read: async (offset, size) => dump.subarray(offset, offset + size) + } + : dump + if (!isMinidump(await source.read(0, 32))) { return null } - const view = new MinidumpView(dump) - const annotations = readCrashpadAnnotations(view) + const view = new MinidumpView(source) + const annotations = await readCrashpadAnnotations(view) const signature: { -readonly [K in keyof MinidumpCrashSignature]: MinidumpCrashSignature[K] @@ -228,7 +161,7 @@ export function parseMinidumpCrashSignature( } const annotatedCheckMessage = annotations['LOG_FATAL'] ?? annotations['abort-message'] - const embeddedCheck = annotatedCheckMessage ? undefined : findEmbeddedCheckMessage(dump) + const embeddedCheck = annotatedCheckMessage ? undefined : await findEmbeddedCheckMessage(source) const checkMessage = annotatedCheckMessage ?? embeddedCheck?.message if (checkMessage) { signature.checkMessage = checkMessage @@ -240,16 +173,16 @@ export function parseMinidumpCrashSignature( signature.checkLine = location.line } } - const exception = findStream(view, STREAM_TYPE_EXCEPTION) + const exception = await findStream(view, STREAM_TYPE_EXCEPTION) if (exception) { - const code = view.u32(exception.rva + EXCEPTION_CODE_OFFSET) - const address = view.u64(exception.rva + EXCEPTION_ADDRESS_OFFSET) + const code = await view.u32(exception.rva + EXCEPTION_CODE_OFFSET) + const address = await view.u64(exception.rva + EXCEPTION_ADDRESS_OFFSET) if (code !== null) { signature.exceptionCode = code } if (address !== null) { signature.exceptionAddress = toHex(address) - const faulting = findFaultingModule(readModules(view), address) + const faulting = findFaultingModule(await readModules(view), address) if (faulting) { signature.faultingModule = faulting.name signature.faultingModuleOffset = faulting.offset diff --git a/src/main/crash-reporting/minidump-crashpad-annotations.ts b/src/main/crash-reporting/minidump-crashpad-annotations.ts index c79dad9b831..2b7ec540654 100644 --- a/src/main/crash-reporting/minidump-crashpad-annotations.ts +++ b/src/main/crash-reporting/minidump-crashpad-annotations.ts @@ -60,30 +60,30 @@ const ANNOTATION_ALLOWLIST = new Set([ ]) /** MinidumpSimpleStringDictionary: u32 count, then {key rva, value rva} pairs. */ -function readSimpleAnnotations( +async function readSimpleAnnotations( view: MinidumpView, location: LocationDescriptor | null, into: Record -): void { +): Promise { if (!location) { return } - const count = view.u32(location.rva) + const count = await view.u32(location.rva) if (count === null || count > MAX_ANNOTATIONS || 4 + count * 8 > location.size) { return } for (let index = 0; index < count; index += 1) { const entry = location.rva + 4 + index * 8 - const keyRva = view.u32(entry) - const valueRva = view.u32(entry + 4) + const keyRva = await view.u32(entry) + const valueRva = await view.u32(entry + 4) if (keyRva === null || valueRva === null) { return } - const key = view.utf8String(keyRva, 256) + const key = await view.utf8String(keyRva, 256) if (key === null || !ANNOTATION_ALLOWLIST.has(key)) { continue } - const value = view.utf8String(valueRva) + const value = await view.utf8String(valueRva) if (value !== null) { into[key] = value } @@ -91,15 +91,15 @@ function readSimpleAnnotations( } /** MinidumpAnnotationList: u32 count, then MinidumpAnnotation records. */ -function readAnnotationObjects( +async function readAnnotationObjects( view: MinidumpView, location: LocationDescriptor | null, into: Record -): void { +): Promise { if (!location) { return } - const count = view.u32(location.rva) + const count = await view.u32(location.rva) if ( count === null || count > MAX_ANNOTATIONS || @@ -109,20 +109,20 @@ function readAnnotationObjects( } for (let index = 0; index < count; index += 1) { const entry = location.rva + 4 + index * ANNOTATION_RECORD_SIZE - const nameRva = view.u32(entry) - const type = view.u16(entry + 4) - const valueRva = view.u32(entry + 8) + const nameRva = await view.u32(entry) + const type = await view.u16(entry + 4) + const valueRva = await view.u32(entry + 8) if (nameRva === null || type === null || valueRva === null) { return } if (type !== ANNOTATION_TYPE_STRING || valueRva === 0) { continue } - const name = view.utf8String(nameRva, 256) + const name = await view.utf8String(nameRva, 256) if (name === null || !ANNOTATION_ALLOWLIST.has(name)) { continue } - const raw = view.byteArray(valueRva) + const raw = await view.byteArray(valueRva) if (raw) { // Annotation strings are not NUL-terminated; trim a trailing one anyway. let value = raw.toString('utf8') @@ -134,43 +134,43 @@ function readAnnotationObjects( } } -export function readCrashpadAnnotations(view: MinidumpView): Record { +export async function readCrashpadAnnotations(view: MinidumpView): Promise> { const annotations: Record = {} - const info = findStream(view, STREAM_TYPE_CRASHPAD_INFO) + const info = await findStream(view, STREAM_TYPE_CRASHPAD_INFO) if (!info || info.size < CRASHPAD_INFO_MIN_SIZE) { return annotations } - readSimpleAnnotations( + await readSimpleAnnotations( view, - view.location(info.rva + CRASHPAD_INFO_SIMPLE_ANNOTATIONS_OFFSET), + await view.location(info.rva + CRASHPAD_INFO_SIMPLE_ANNOTATIONS_OFFSET), annotations ) - const moduleList = view.location(info.rva + CRASHPAD_INFO_MODULE_LIST_OFFSET) + const moduleList = await view.location(info.rva + CRASHPAD_INFO_MODULE_LIST_OFFSET) if (!moduleList) { return annotations } - const moduleCount = view.u32(moduleList.rva) + const moduleCount = await view.u32(moduleList.rva) if (moduleCount === null || moduleCount > MAX_MODULES) { return annotations } for (let index = 0; index < moduleCount; index += 1) { const link = moduleList.rva + 4 + index * MODULE_CRASHPAD_INFO_LINK_SIZE - const moduleInfo = view.location(link + 4) + const moduleInfo = await view.location(link + 4) if (!moduleInfo || moduleInfo.size < MODULE_CRASHPAD_INFO_MIN_SIZE) { continue } // Why: Chromium's crash keys land in annotation_objects on current // Crashpad, but older modules still populate the two legacy shapes. - readSimpleAnnotations( + await readSimpleAnnotations( view, - view.location(moduleInfo.rva + MODULE_CRASHPAD_INFO_SIMPLE_ANNOTATIONS_OFFSET), + await view.location(moduleInfo.rva + MODULE_CRASHPAD_INFO_SIMPLE_ANNOTATIONS_OFFSET), annotations ) - readAnnotationObjects( + await readAnnotationObjects( view, - view.location(moduleInfo.rva + MODULE_CRASHPAD_INFO_ANNOTATION_OBJECTS_OFFSET), + await view.location(moduleInfo.rva + MODULE_CRASHPAD_INFO_ANNOTATION_OBJECTS_OFFSET), annotations ) } diff --git a/src/main/crash-reporting/minidump-embedded-check.ts b/src/main/crash-reporting/minidump-embedded-check.ts new file mode 100644 index 00000000000..ea72103e19a --- /dev/null +++ b/src/main/crash-reporting/minidump-embedded-check.ts @@ -0,0 +1,112 @@ +import type { MinidumpSource } from './minidump-stream-reader' + +const CHROMIUM_LOG_MARKERS = [ + Buffer.from(':FATAL:', 'ascii'), + Buffer.from(':CHECK:', 'ascii'), + Buffer.from(':DFATAL:', 'ascii'), + Buffer.from(':ERROR:', 'ascii') +] +const MAX_LOG_PREFIX_BYTES = 96 +const MAX_CHECK_LOG_BYTES = 4_000 +const MAX_MARKERS_PER_SEVERITY = 256 +const CHECK_LOG_PATTERN = + /^\[(?:\d+:){1,2}\d{4}\/\d{6}\.\d{3,6}:(FATAL|CHECK|DFATAL|ERROR)(?::[^:\]\r\n]{1,80})*:([^:\]\r\n]{1,512}?)(?:\((\d+)\)|:(\d+))\]\s*(.+)$/ +const ERROR_CHECK_PATTERN = /\b(?:Check failed:|D?CHECK failed:|Intentionally causing D?CHECK\b)/i + +export type LocatedCheckMessage = { + readonly message: string + readonly file?: string + readonly line?: number +} + +function isPrintableLogByte(value: number): boolean { + return value === 0x09 || (value >= 0x20 && value <= 0x7e) +} + +/** + * `lastIndexOf(byte, from)` restricted to `within` bytes before `from`. An + * unbounded search scans the whole dump backward on a miss only for the result + * to be thrown away by the same prefix limit; zero-filled regions are normal in + * a minidump, so that miss is the common case, not the adversarial one. + */ +function lastIndexOfWithin(dump: Buffer, byte: number, from: number, within: number): number { + const floor = Math.max(0, from - within) + for (let at = from; at >= floor; at -= 1) { + if (dump[at] === byte) { + return at + } + } + return -1 +} + +/** Electron 43 omits LOG_FATAL but keeps Chromium's formatted log line in memory. */ +export async function findEmbeddedCheckMessage( + source: MinidumpSource +): Promise { + const blockBytes = 1024 * 1024 + const states: { marker: Buffer; inspected: number; found?: LocatedCheckMessage }[] = + CHROMIUM_LOG_MARKERS.map((marker) => ({ marker, inspected: 0 })) + let from = 0 + while (from < source.byteLength) { + const startAt = Math.max(0, from - MAX_LOG_PREFIX_BYTES) + const bytes = await source.read( + startAt, + Math.min(from - startAt + blockBytes + MAX_CHECK_LOG_BYTES, source.byteLength - startAt) + ) + if (bytes.length === 0) { + break + } + const scanEnd = Math.min(bytes.length, from - startAt + blockBytes) + for (const state of states) { + let next = from - startAt + while (!state.found && next < scanEnd && state.inspected < MAX_MARKERS_PER_SEVERITY) { + const markerAt = bytes.indexOf(state.marker, next) + if (markerAt === -1 || markerAt >= scanEnd) { + break + } + state.inspected++ + next = markerAt + state.marker.length + state.found = readCheckAt(bytes, markerAt, next) + } + } + // A lower severity cannot win until each earlier severity is exhausted. + for (const state of states) { + if (state.found) { + return state.found + } + if (state.inspected < MAX_MARKERS_PER_SEVERITY) { + break + } + } + from = startAt + scanEnd + } + return states.find((state) => state.found)?.found +} + +function readCheckAt( + bytes: Buffer, + markerAt: number, + afterMarker: number +): LocatedCheckMessage | undefined { + const start = lastIndexOfWithin(bytes, 0x5b, markerAt, MAX_LOG_PREFIX_BYTES) + if (start === -1) { + return undefined + } + let end = afterMarker + const limit = Math.min(bytes.length, start + MAX_CHECK_LOG_BYTES) + while (end < limit && isPrintableLogByte(bytes[end])) { + end++ + } + const candidate = bytes.subarray(start, end).toString('utf8') + const match = CHECK_LOG_PATTERN.exec(candidate) + if (!match || (match[1] === 'ERROR' && !ERROR_CHECK_PATTERN.test(match[5]))) { + return undefined + } + const line = Number.parseInt(match[3] ?? match[4], 10) + const separator = Math.max(match[2].lastIndexOf('/'), match[2].lastIndexOf('\\')) + return { + message: candidate, + file: match[2].slice(separator + 1), + line: Number.isFinite(line) ? line : undefined + } +} diff --git a/src/main/crash-reporting/minidump-file-source.test.ts b/src/main/crash-reporting/minidump-file-source.test.ts new file mode 100644 index 00000000000..780ba9347bc --- /dev/null +++ b/src/main/crash-reporting/minidump-file-source.test.ts @@ -0,0 +1,201 @@ +import { mkdtemp, open, rm, truncate, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, expect, it, vi } from 'vitest' +import { createMinidumpFileSource, observeMinidumpExtent } from './minidump-file-source' +import { parseMinidumpCrashSignature } from './minidump-crash-signature' + +const roots: string[] = [] +const LINE = '[8104:1234:0815/143022.123456:FATAL:render_frame_impl.cc(4821)] Check failed: !x.' +const ERROR = '[8104:1234:0815/143022.123456:ERROR:file.cc(12)] Check failed: earlier.' +const BLOCK = 1024 * 1024 + +function header(): Buffer { + const bytes = Buffer.alloc(32) + bytes.writeUInt32LE(0x504d444d, 0) + bytes.writeUInt32LE(32, 12) + return bytes +} + +async function sourceFile() { + const root = await mkdtemp(join(tmpdir(), 'orca-minidump-source-')) + roots.push(root) + const path = join(root, 'dump.dmp') + await writeFile(path, header()) + return path +} + +afterEach(async () => { + await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))) +}) + +it.each([1, 2])( + 'stops observing a growing size-zero dump after the deadline at page %i', + async (pages) => { + const path = await sourceFile() + const handle = await open(path, 'r+') + const pageBytes = 64 * 1024 + try { + await handle.truncate(3 * pageBytes) + const read = vi.spyOn(handle, 'read') + let observations = 0 + const now = () => (++observations >= pages ? 10 : 0) + + expect(await observeMinidumpExtent(handle, 0, { deadlineMs: 10, now })).toBe( + pages * pageBytes + ) + expect(read).toHaveBeenCalledTimes(pages) + expect(observations).toBe(pages) + } finally { + await handle.close() + } + } +) + +it.each([-96, -40, -1, 0, 1, 40, 96])( + 'preserves marker and full text at block boundary %+d', + async (delta) => { + const path = await sourceFile() + const handle = await open(path, 'r+') + const line = LINE + 'x'.repeat(4000 - LINE.length) + try { + await handle.write(Buffer.from(`${line}\0`), 0, line.length + 1, BLOCK + delta) + const { size } = await handle.stat() + const streamed = await parseMinidumpCrashSignature(createMinidumpFileSource(handle, size)) + const complete = await handle.readFile() + expect(streamed).toEqual(await parseMinidumpCrashSignature(complete)) + expect(streamed?.checkMessage).toBe(line) + } finally { + await handle.close() + } + } +) + +it('preserves severity priority across blocks and scans a large sparse dump with bounded reads', async () => { + const path = await sourceFile() + const handle = await open(path, 'r+') + const size = 80 * 1024 * 1024 + try { + await handle.write(Buffer.from(`${ERROR}\0`), 0, ERROR.length + 1, 4096) + await handle.write(Buffer.from(`${LINE}\0`), 0, LINE.length + 1, size - LINE.length - 1) + const source = createMinidumpFileSource(handle, size) + let maxRead = 0 + let reads = 0 + const result = await parseMinidumpCrashSignature({ + byteLength: size, + read: async (offset, length) => { + maxRead = Math.max(maxRead, length) + reads++ + return source.read(offset, length) + } + }) + expect(result?.checkMessage).toBe(LINE) + expect(maxRead).toBeLessThanOrEqual(BLOCK + 4096) + expect(reads).toBeLessThan(400) + } finally { + await handle.close() + } +}) + +it('reads annotations through large RVAs without buffering skipped process memory', async () => { + const path = await sourceFile() + const handle = await open(path, 'r+') + const rva = 80 * 1024 * 1024 + try { + const prefix = Buffer.alloc(44) + header().copy(prefix) + prefix.writeUInt32LE(1, 8) + prefix.writeUInt32LE(0x43500001, 32) + prefix.writeUInt32LE(52, 36) + prefix.writeUInt32LE(rva, 40) + await handle.write(prefix, 0, prefix.length, 0) + const info = Buffer.alloc(52 + 12 + 10 + 13) + info.writeUInt32LE(1, 0) + info.writeUInt32LE(12, 36) + info.writeUInt32LE(rva + 52, 40) + info.writeUInt32LE(1, 52) + info.writeUInt32LE(rva + 64, 56) + info.writeUInt32LE(rva + 74, 60) + info.writeUInt32LE(5, 64) + info.write('ptype', 68) + info.writeUInt32LE(8, 74) + info.write('renderer', 78) + await handle.write(info, 0, info.length, rva) + const source = createMinidumpFileSource(handle, rva + info.length) + const reads: number[] = [] + const result = await parseMinidumpCrashSignature( + { + byteLength: source.byteLength, + read: async (offset, length) => { + reads.push(length) + return source.read(offset, length) + } + }, + { expectedProcessType: 'browser' } + ) + expect(result).toEqual({ annotations: { ptype: 'renderer' }, processType: 'renderer' }) + expect(Math.max(...reads)).toBeLessThan(64) + } finally { + await handle.close() + } +}) + +it('does not read outside the opened extent, including truncation and EOF', async () => { + const path = await sourceFile() + const handle = await open(path, 'r+') + try { + const source = createMinidumpFileSource(handle, 32) + await handle.write(Buffer.from(LINE), 0, LINE.length, 32) + expect(await source.read(32, 4)).toEqual(Buffer.alloc(0)) + expect((await parseMinidumpCrashSignature(source))?.checkMessage).toBeUndefined() + await truncate(path, 4) + const truncated = createMinidumpFileSource(handle, 32) + expect(await parseMinidumpCrashSignature(truncated)).toBeNull() + } finally { + await handle.close() + } +}) + +it('avoids rereading distant name and module-table pages for a 1,042-module renderer', async () => { + const path = await sourceFile() + const handle = await open(path, 'r+') + const count = 1042 + const namesOffset = 1024 * 1024 + const moduleOffset = 56 + const exceptionOffset = moduleOffset + 4 + count * 108 + try { + const table = Buffer.alloc(exceptionOffset + 40) + header().copy(table) + table.writeUInt32LE(2, 8) + table.writeUInt32LE(4, 32) + table.writeUInt32LE(4 + count * 108, 36) + table.writeUInt32LE(moduleOffset, 40) + table.writeUInt32LE(6, 44) + table.writeUInt32LE(40, 48) + table.writeUInt32LE(exceptionOffset, 52) + table.writeUInt32LE(count, moduleOffset) + const names = Buffer.alloc(count * 32) + for (let index = 0; index < count; index++) { + const record = moduleOffset + 4 + index * 108 + table.writeBigUInt64LE(BigInt(index * 4096), record) + table.writeUInt32LE(4096, record + 8) + table.writeUInt32LE(namesOffset + index * 32, record + 20) + const name = Buffer.from(`module${index}`, 'utf16le') + names.writeUInt32LE(name.length, index * 32) + name.copy(names, index * 32 + 4) + } + table.writeUInt32LE(0x80000003, exceptionOffset + 8) + table.writeBigUInt64LE(BigInt((count - 1) * 4096 + 1), exceptionOffset + 24) + await handle.write(table, 0, table.length, 0) + await handle.write(names, 0, names.length, namesOffset) + const read = vi.spyOn(handle, 'read') + const result = await parseMinidumpCrashSignature( + createMinidumpFileSource(handle, namesOffset + names.length) + ) + expect(result?.faultingModule).toBe(`module${count - 1}`) + expect(result?.faultingModuleOffset).toBe('0x1') + expect(read.mock.calls.length).toBeLessThanOrEqual(8) + } finally { + await handle.close() + } +}) diff --git a/src/main/crash-reporting/minidump-file-source.ts b/src/main/crash-reporting/minidump-file-source.ts new file mode 100644 index 00000000000..596c0ec5cda --- /dev/null +++ b/src/main/crash-reporting/minidump-file-source.ts @@ -0,0 +1,99 @@ +import type { FileHandle } from 'node:fs/promises' +import type { MinidumpSource } from './minidump-stream-reader' + +const PAGE_BYTES = 64 * 1024 + +/** Size-zero readFile follows bytes until EOF, even when an open races the writer. */ +export type MinidumpExtentObservationOptions = { + readonly deadlineMs?: number + readonly now?: () => number +} + +export async function observeMinidumpExtent( + handle: FileHandle, + initialSize: number, + options: MinidumpExtentObservationOptions = {} +): Promise { + if (initialSize !== 0) { + return initialSize + } + const bytes = Buffer.allocUnsafe(PAGE_BYTES) + let size = 0 + const now = options.now ?? Date.now + while (true) { + // A zero-length file still gets one read so a dump that is being promoted + // can be captured; stop before the next page once the polling deadline wins. + if (size > 0 && options.deadlineMs !== undefined && now() >= options.deadlineMs) { + return size + } + const result = await handle.read(bytes, 0, bytes.length, size) + if (result.bytesRead === 0) { + return size + } + size += result.bytesRead + } +} + +/** Keep metadata seeks cheap without retaining the dump's captured process memory. */ +export function createMinidumpFileSource(handle: FileHandle, byteLength: number): MinidumpSource { + const pages = new Map() + let extent = byteLength + + async function readRange(offset: number, size: number): Promise { + const bytes = Buffer.allocUnsafe(size) + let read = 0 + while (read < size) { + const result = await handle.read(bytes, read, size - read, offset + read) + if (result.bytesRead === 0) { + extent = Math.min(extent, offset + read) + break + } + read += result.bytesRead + } + return bytes.subarray(0, read) + } + + async function pageAt(offset: number): Promise { + const cached = pages.get(offset) + if (cached) { + pages.delete(offset) + pages.set(offset, cached) + return cached + } + const page = await readRange(offset, Math.min(PAGE_BYTES, extent - offset)) + pages.set(offset, page) + if (pages.size > 4) { + const oldest = pages.keys().next().value + if (oldest !== undefined) { + pages.delete(oldest) + } + } + return page + } + + return { + get byteLength() { + return extent + }, + async read(offset, size) { + const length = Math.max(0, Math.min(size, extent - offset)) + if (length === 0) { + return Buffer.alloc(0) + } + if (length > PAGE_BYTES) { + return readRange(offset, length) + } + const start = Math.floor(offset / PAGE_BYTES) * PAGE_BYTES + const page = await pageAt(start) + const inPage = offset - start + if (inPage + length <= PAGE_BYTES || page.length < PAGE_BYTES) { + return page.subarray(inPage, inPage + length) + } + const nextPage = await pageAt(start + PAGE_BYTES) + return Buffer.concat([ + page.subarray(inPage), + nextPage.subarray(0, length - (PAGE_BYTES - inPage)) + ]) + } + } +} diff --git a/src/main/crash-reporting/minidump-stream-reader.ts b/src/main/crash-reporting/minidump-stream-reader.ts index 4b9f32a07a3..bb4eadaa737 100644 --- a/src/main/crash-reporting/minidump-stream-reader.ts +++ b/src/main/crash-reporting/minidump-stream-reader.ts @@ -21,75 +21,88 @@ export type LocationDescriptor = { readonly rva: number } +export type MinidumpSource = { + readonly byteLength: number + read(offset: number, size: number): Promise +} + export class MinidumpView { - constructor(private readonly buf: Buffer) {} + constructor(private readonly source: MinidumpSource) {} get byteLength(): number { - return this.buf.length + return this.source.byteLength } - u32(offset: number): number | null { - if (offset < 0 || offset + 4 > this.buf.length) { + async u32(offset: number): Promise { + if (offset < 0 || offset + 4 > this.source.byteLength) { return null } - return this.buf.readUInt32LE(offset) + const bytes = await this.source.read(offset, 4) + return bytes.length === 4 ? bytes.readUInt32LE(0) : null } - u16(offset: number): number | null { - if (offset < 0 || offset + 2 > this.buf.length) { + async u16(offset: number): Promise { + if (offset < 0 || offset + 2 > this.source.byteLength) { return null } - return this.buf.readUInt16LE(offset) + const bytes = await this.source.read(offset, 2) + return bytes.length === 2 ? bytes.readUInt16LE(0) : null } - u64(offset: number): bigint | null { - if (offset < 0 || offset + 8 > this.buf.length) { + async u64(offset: number): Promise { + if (offset < 0 || offset + 8 > this.source.byteLength) { return null } - return this.buf.readBigUInt64LE(offset) + const bytes = await this.source.read(offset, 8) + return bytes.length === 8 ? bytes.readBigUInt64LE(0) : null } - location(offset: number): LocationDescriptor | null { - const size = this.u32(offset) - const rva = this.u32(offset + 4) + async location(offset: number): Promise { + const size = await this.u32(offset) + const rva = await this.u32(offset + 4) if (size === null || rva === null) { return null } // A zero rva means "absent", which is normal for optional sub-structures. - if (rva === 0 || rva >= this.buf.length) { + if (rva === 0 || rva >= this.source.byteLength) { return null } return { size, rva } } /** MinidumpUTF8String: u32 byte length, then NUL-terminated UTF-8. */ - utf8String(rva: number, maxBytes = MAX_ANNOTATION_VALUE_BYTES): string | null { - return this.byteArray(rva, maxBytes)?.toString('utf8') ?? null + async utf8String(rva: number, maxBytes = MAX_ANNOTATION_VALUE_BYTES): Promise { + return (await this.byteArray(rva, maxBytes))?.toString('utf8') ?? null } /** MINIDUMP_STRING: u32 byte length, then UTF-16LE. Used for module names. */ - utf16String(rva: number, maxBytes = MAX_ANNOTATION_VALUE_BYTES): string | null { - const length = this.u32(rva) + async utf16String(rva: number, maxBytes = MAX_ANNOTATION_VALUE_BYTES): Promise { + const length = await this.u32(rva) if (length === null || length > maxBytes || length % 2 !== 0) { return null } const start = rva + 4 - if (start + length > this.buf.length) { + if (start + length > this.source.byteLength) { return null } - return this.buf.toString('utf16le', start, start + length) + const bytes = await this.source.read(start, length) + return bytes.length === length ? bytes.toString('utf16le') : null } - bytes(location: LocationDescriptor, maxBytes = MAX_ANNOTATION_VALUE_BYTES): Buffer | null { - if (location.size > maxBytes || location.rva + location.size > this.buf.length) { + async bytes( + location: LocationDescriptor, + maxBytes = MAX_ANNOTATION_VALUE_BYTES + ): Promise { + if (location.size > maxBytes || location.rva + location.size > this.source.byteLength) { return null } - return this.buf.subarray(location.rva, location.rva + location.size) + const bytes = await this.source.read(location.rva, location.size) + return bytes.length === location.size ? bytes : null } /** MinidumpByteArray: u32 byte length, then the bytes. */ - byteArray(rva: number, maxBytes = MAX_ANNOTATION_VALUE_BYTES): Buffer | null { - const length = this.u32(rva) + async byteArray(rva: number, maxBytes = MAX_ANNOTATION_VALUE_BYTES): Promise { + const length = await this.u32(rva) if (length === null || length > maxBytes) { return null } @@ -102,23 +115,26 @@ export function isMinidump(dump: Buffer): boolean { } /** Locates a stream by type in the header's directory, or null if absent. */ -export function findStream(view: MinidumpView, streamType: number): LocationDescriptor | null { - const streamCount = view.u32(8) - const directoryRva = view.u32(12) +export async function findStream( + view: MinidumpView, + streamType: number +): Promise { + const streamCount = await view.u32(8) + const directoryRva = await view.u32(12) if (streamCount === null || directoryRva === null || streamCount > MAX_STREAMS) { return null } for (let index = 0; index < streamCount; index += 1) { const entry = directoryRva + index * DIRECTORY_ENTRY_SIZE - const type = view.u32(entry) + const type = await view.u32(entry) if (type === null) { return null } if (type !== streamType) { continue } - const size = view.u32(entry + 4) - const rva = view.u32(entry + 8) + const size = await view.u32(entry + 4) + const rva = await view.u32(entry + 8) if (size === null || rva === null || rva === 0 || rva >= view.byteLength) { return null }