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; }