From 294ec5ca7e2d17eba44f9bb2b9c21125362c6b3e Mon Sep 17 00:00:00 2001 From: Tzvetan Mikov Date: Tue, 12 May 2026 21:48:39 -0700 Subject: [PATCH] Skip breakpoints on read-only bytecode pages instead of aborting 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. --- include/hermes/VM/CodeBlock.h | 6 +- include/hermes/VM/Debugger/Debugger.h | 15 ++++- lib/VM/CodeBlock.cpp | 18 +++--- lib/VM/Debugger/Debugger.cpp | 89 +++++++++++++++++++-------- 4 files changed, 91 insertions(+), 37 deletions(-) diff --git a/include/hermes/VM/CodeBlock.h b/include/hermes/VM/CodeBlock.h index 449b9c1241d..8aeb77bbb8e 100644 --- a/include/hermes/VM/CodeBlock.h +++ b/include/hermes/VM/CodeBlock.h @@ -371,7 +371,11 @@ class CodeBlock final : private llvh::TrailingObjects< /// at location \p offset. /// Requires that there's a breakpoint registered at \p offset. /// Increments the user count of the associated runtime module. - void installBreakpointAtOffset(uint32_t offset); + /// \return true on success; false if the bytecode page cannot be made + /// writable (e.g. statically embedded bytecode in a read-only segment + /// that the OS refuses to remap, such as macOS __DATA_CONST under + /// hardened runtime). On failure, no state is modified. + LLVM_NODISCARD bool installBreakpointAtOffset(uint32_t offset); /// Uninstalls the debugger instruction from the opcode stream /// at location \p offset, replacing it with \p opCode. diff --git a/include/hermes/VM/Debugger/Debugger.h b/include/hermes/VM/Debugger/Debugger.h index b9158ddc9dc..43c40f84a0a 100644 --- a/include/hermes/VM/Debugger/Debugger.h +++ b/include/hermes/VM/Debugger/Debugger.h @@ -473,8 +473,13 @@ class Debugger { /// for the given codeBlock and offset, else creates one. /// Used by other functions which should be called to set breakpoints. /// Installs a breakpoint at that location, doesn't modify it. - /// \return the location at which the breakpoint was installed. - BreakpointLocation &installBreakpoint(CodeBlock *codeBlock, uint32_t offset); + /// \return a pointer to the location at which the breakpoint was installed, + /// or nullptr if the bytecode page cannot be made writable (e.g. + /// statically embedded bytecode in a read-only segment that the OS + /// refuses to remap). On failure, no state is modified. + LLVM_NODISCARD BreakpointLocation *installBreakpoint( + CodeBlock *codeBlock, + uint32_t offset); /// Helper function to uninstall a breakpoint. Always use this function to /// uninstall breakpoints. This takes care of the case when we purposely keep @@ -489,7 +494,11 @@ class Debugger { /// If the physical breakpoint isn't enabled yet, patches the debugger /// instruction in. /// Sets the breakpoint ID to \p id. - void + /// \return true on success; false if the bytecode page cannot be made + /// writable. On failure, no state is modified and the breakpoint is not + /// physically installed (the caller may still keep it in + /// userBreakpoints_ as a record). + bool setUserBreakpoint(CodeBlock *codeBlock, uint32_t offset, BreakpointID id); /// Should not be called directly except from \p setStepBreakpoint() or \p diff --git a/lib/VM/CodeBlock.cpp b/lib/VM/CodeBlock.cpp index 0517b161b60..a4cb904cc18 100644 --- a/lib/VM/CodeBlock.cpp +++ b/lib/VM/CodeBlock.cpp @@ -332,8 +332,10 @@ uint32_t CodeBlock::getVirtualOffset() const { #ifdef HERMES_ENABLE_DEBUGGER /// Makes the page that \p address is in writable. -/// If it fails, aborts execution. -static void makeWritable(void *address, size_t length) { +/// \return true on success, false if the page cannot be made writable (e.g. +/// the bytecode lives in a read-only segment of the binary that the OS +/// refuses to remap, such as macOS __DATA_CONST under hardened runtime). +static bool makeWritable(void *address, size_t length) { void *endAddress = static_cast(static_cast(address) + length); // Align the address to page size before setting the pagesize. @@ -343,14 +345,11 @@ static void makeWritable(void *address, size_t length) { size_t totalLength = static_cast(endAddress) - static_cast(alignedAddress); - bool success = oscompat::vm_protect( + return oscompat::vm_protect( alignedAddress, totalLength, oscompat::ProtectMode::ReadWrite); - if (!success) { - hermes_fatal("mprotect failed before modifying breakpoint"); - } } -void CodeBlock::installBreakpointAtOffset(uint32_t offset) { +bool CodeBlock::installBreakpointAtOffset(uint32_t offset) { auto opcodes = getOpcodeArray(); assert(offset < opcodes.size() && "patch offset out of bounds"); hbc::opcode_atom_t *address = @@ -362,9 +361,12 @@ void CodeBlock::installBreakpointAtOffset(uint32_t offset) { sizeof(inst::DebuggerInst) == 1, "debugger instruction can only be a single opcode atom"); - makeWritable(address, sizeof(inst::DebuggerInst)); + if (!makeWritable(address, sizeof(inst::DebuggerInst))) { + return false; + } *address = debuggerOpcode; ++numInstalledBreakpoints_; + return true; } void CodeBlock::uninstallBreakpointAtOffset( diff --git a/lib/VM/Debugger/Debugger.cpp b/lib/VM/Debugger/Debugger.cpp index 58cc688ae86..38d379606b6 100644 --- a/lib/VM/Debugger/Debugger.cpp +++ b/lib/VM/Debugger/Debugger.cpp @@ -421,7 +421,12 @@ ExecutionStatus Debugger::debuggerLoop( } breakAtPossibleNextInstructions(state); if (breakpointOpt) { - state.codeBlock->installBreakpointAtOffset(state.offset); + // Re-installing a previously-installed breakpoint: the page is + // already known writable. + bool ok = + state.codeBlock->installBreakpointAtOffset(state.offset); + (void)ok; + assert(ok && "re-installing existing breakpoint cannot fail"); } if (stepMode == StepMode::Into) { // Stepping in could enter another code block, @@ -767,18 +772,26 @@ Debugger::getBreakpointLocation(CodeBlock *codeBlock, uint32_t offset) const { } auto Debugger::installBreakpoint(CodeBlock *codeBlock, uint32_t offset) - -> BreakpointLocation & { + -> BreakpointLocation * { auto opcodes = codeBlock->getOpcodeArray(); assert(offset < opcodes.size() && "invalid offset to set breakpoint"); - auto &location = - breakpointLocations_ - .try_emplace(codeBlock->getOffsetPtr(offset), opcodes[offset]) - .first->second; + auto *opcodePtr = codeBlock->getOffsetPtr(offset); + auto emplaceResult = + breakpointLocations_.try_emplace(opcodePtr, opcodes[offset]); + auto &location = emplaceResult.first->second; if (location.count() == 0) { // count used to be 0, so patch this in now that the count > 0. - codeBlock->installBreakpointAtOffset(offset); + if (!codeBlock->installBreakpointAtOffset(offset)) { + // Patching failed (e.g. statically embedded bytecode in a read-only + // segment that the OS refuses to remap). Roll back the entry we just + // inserted so state stays consistent, and signal failure to caller. + if (emplaceResult.second) { + breakpointLocations_.erase(emplaceResult.first); + } + return nullptr; + } } - return location; + return &location; } void Debugger::uninstallBreakpoint( @@ -796,12 +809,16 @@ void Debugger::uninstallBreakpoint( } } -void Debugger::setUserBreakpoint( +bool Debugger::setUserBreakpoint( CodeBlock *codeBlock, uint32_t offset, BreakpointID id) { - BreakpointLocation &location = installBreakpoint(codeBlock, offset); - location.userBreakpointIDs.insert(id); + BreakpointLocation *location = installBreakpoint(codeBlock, offset); + if (!location) { + return false; + } + location->userBreakpointIDs.insert(id); + return true; } void Debugger::doSetNonUserBreakpoint( @@ -809,15 +826,22 @@ void Debugger::doSetNonUserBreakpoint( uint32_t offset, uint32_t callStackDepth, bool isStepBreakpoint) { - BreakpointLocation &location = installBreakpoint(codeBlock, offset); + BreakpointLocation *location = installBreakpoint(codeBlock, offset); + if (!location) { + // Best-effort: step/restoration breakpoints are an optimization for + // pause-on-entry while stepping. If the target lives in a read-only + // bytecode segment we cannot patch, just skip it -- stepping will simply + // not pause at the entry of that code block. + return; + } std::vector &breakpoints = isStepBreakpoint ? tempBreakpoints_ : restorationBreakpoints_; - if (location.callStackDepths.count(callStackDepth) == 0) { - location.callStackDepths.insert(callStackDepth); + if (location->callStackDepths.count(callStackDepth) == 0) { + location->callStackDepths.insert(callStackDepth); } - if ((isStepBreakpoint && !location.hasStepBreakpoint) || - (!isStepBreakpoint && !location.hasRestorationBreakpoint)) { + if ((isStepBreakpoint && !location->hasStepBreakpoint) || + (!isStepBreakpoint && !location->hasRestorationBreakpoint)) { // Leave the resolved location empty for now, // let the caller fill it in lazily. Breakpoint breakpoint{}; @@ -828,9 +852,9 @@ void Debugger::doSetNonUserBreakpoint( } if (isStepBreakpoint) { - location.hasStepBreakpoint = true; + location->hasStepBreakpoint = true; } else { - location.hasRestorationBreakpoint = true; + location->hasRestorationBreakpoint = true; } } @@ -843,17 +867,22 @@ void Debugger::setStepBreakpoint( } void Debugger::setOnLoadBreakpoint(CodeBlock *codeBlock, uint32_t offset) { - BreakpointLocation &location = installBreakpoint(codeBlock, offset); + BreakpointLocation *location = installBreakpoint(codeBlock, offset); + if (!location) { + // Best-effort: pauseOnScriptLoad cannot pause inside a code block whose + // bytecode page is not writable. Skip. + return; + } // Leave the resolved location empty for now, // let the caller fill it in lazily. Breakpoint breakpoint{}; breakpoint.codeBlock = codeBlock; breakpoint.offset = offset; breakpoint.enabled = true; - assert(!location.onLoad && "can't set duplicate on-load breakpoint"); - location.onLoad = true; + assert(!location->onLoad && "can't set duplicate on-load breakpoint"); + location->onLoad = true; tempBreakpoints_.push_back(breakpoint); - assert(location.count() && "invalid count following set breakpoint"); + assert(location->count() && "invalid count following set breakpoint"); } void Debugger::unsetUserBreakpoint( @@ -1017,8 +1046,12 @@ void Debugger::setRestorationBreakpoint( bool Debugger::restoreBreakpointIfAny() { if (breakpointToRestore_.first != nullptr) { - breakpointToRestore_.first->installBreakpointAtOffset( + // Re-installing a previously-installed breakpoint: the page is already + // known writable. + bool ok = breakpointToRestore_.first->installBreakpointAtOffset( breakpointToRestore_.second); + (void)ok; + assert(ok && "re-installing existing breakpoint cannot fail"); breakpointToRestore_ = {nullptr, 0}; return true; } @@ -1046,7 +1079,10 @@ ExecutionStatus Debugger::stepInstruction(InterpreterState &state) { // Temporarily uninstall the breakpoint so we can run the real instruction. uninstallBreakpoint(codeBlock, offset, locationOpt->opCode); status = runtime_.stepFunction(newState); - codeBlock->installBreakpointAtOffset(offset); + // Re-install: page already known writable. + bool ok = codeBlock->installBreakpointAtOffset(offset); + (void)ok; + assert(ok && "re-installing existing breakpoint cannot fail"); } else { status = runtime_.stepFunction(newState); } @@ -1092,7 +1128,10 @@ ExecutionStatus Debugger::processInstUnderDebuggerOpCode( runtime_.setCurrentIP(ip); ExecutionStatus status = runtime_.stepFunction(newState); runtime_.invalidateCurrentIP(); - codeBlock->installBreakpointAtOffset(offset); + // Re-install: page already known writable. + bool ok = codeBlock->installBreakpointAtOffset(offset); + (void)ok; + assert(ok && "re-installing existing breakpoint cannot fail"); if (status == ExecutionStatus::EXCEPTION) { return status; }