Skip breakpoints on read-only bytecode pages instead of aborting#2021
Open
tmikov wants to merge 1 commit into
Open
Skip breakpoints on read-only bytecode pages instead of aborting#2021tmikov wants to merge 1 commit into
tmikov wants to merge 1 commit into
Conversation
|
@tmikov has imported this pull request. If you are a Meta employee, you can view this in D105190367. |
The debugger installs breakpoints by overwriting an opcode in the bytecode stream with Debugger, which requires the page to be writable. The patcher uses mprotect to make the page RW; on failure it called hermes_fatal and aborted the process. If the bytecode lives in a read-only segment (e.g. statically linked into the binary as a const array, ending up in __DATA_CONST / __TEXT_CONST on macOS), mprotect is rejected by the OS (EACCES under hardened runtime) and the abort takes down the whole process. This is reachable in practice because step / restoration / on-load breakpoints install at offset 0 of every CodeBlock about to execute, so stepping into such a CodeBlock from anywhere kills the runtime. Make the patch best-effort: - CodeBlock::installBreakpointAtOffset and the static makeWritable helper now return bool. On failure no state is modified. - Debugger::installBreakpoint returns BreakpointLocation* (nullptr on failure), rolling back the partial breakpointLocations_ entry it just inserted. - doSetNonUserBreakpoint and setOnLoadBreakpoint silently skip when installBreakpoint fails. Step / restoration / on-load breakpoints are best-effort hints; the only loss is that stepping cannot pause at the entry of a CodeBlock whose page cannot be patched. - setUserBreakpoint now returns bool so callers can propagate failure to their clients (e.g. CDP could surface a proper error back to DevTools instead of aborting). - The four call sites that re-install a previously-installed breakpoint (the page is known writable since the original install succeeded) assert the result.
|
@tmikov has updated the pull request. You must reimport the pull request before landing. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The debugger installs breakpoints by overwriting an opcode in the bytecode stream with Debugger, which requires the page to be writable. The patcher uses mprotect to make the page RW; on failure it called hermes_fatal and aborted the process.
If the bytecode lives in a read-only segment (e.g. statically linked into the binary as a const array, ending up in __DATA_CONST / __TEXT_CONST on macOS), mprotect is rejected by the OS (EACCES under hardened runtime) and the abort takes down the whole process. This is reachable in practice because step / restoration / on-load breakpoints install at offset 0 of every CodeBlock about to execute, so stepping into such a CodeBlock from anywhere kills the runtime.
Make the patch best-effort: