Switch VMTraps to use halt instructions rather than breakpoint instructions
authorkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 24 Jun 2017 02:54:02 +0000 (02:54 +0000)
committerkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 24 Jun 2017 02:54:02 +0000 (02:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173677
Source/JavaScriptCore:

<rdar://problem/32178892>

Reviewed by JF Bastien.

Using the breakpoint instruction for VMTraps caused issues with lldb.
Since we only need some way to stop execution we can, in theory, use
any exceptioning instruction we want. I went with the halt instruction
on X86 since that is the only one byte instruction that does not
breakpoint (in my tests both 0xf1 and 0xd6 produced EXC_BREAKPOINT).
On ARM we use the data cache clearing instruction with the zero register,
which triggers a segmentation fault.

Also, update the platform code to only use signaling VMTraps
on where we have an appropriate instruction (x86 and ARM64).

* API/tests/ExecutionTimeLimitTest.cpp:
(testExecutionTimeLimit):
* assembler/ARM64Assembler.h:
(JSC::ARM64Assembler::replaceWithVMHalt):
(JSC::ARM64Assembler::dataCacheZeroVirtualAddress):
(JSC::ARM64Assembler::replaceWithBkpt): Deleted.
* assembler/ARMAssembler.h:
(JSC::ARMAssembler::replaceWithBkpt): Deleted.
* assembler/ARMv7Assembler.h:
(JSC::ARMv7Assembler::replaceWithBkpt): Deleted.
* assembler/MIPSAssembler.h:
(JSC::MIPSAssembler::replaceWithBkpt): Deleted.
* assembler/MacroAssemblerARM.h:
(JSC::MacroAssemblerARM::replaceWithBreakpoint): Deleted.
* assembler/MacroAssemblerARM64.h:
(JSC::MacroAssemblerARM64::replaceWithVMHalt):
(JSC::MacroAssemblerARM64::replaceWithBreakpoint): Deleted.
* assembler/MacroAssemblerARMv7.h:
(JSC::MacroAssemblerARMv7::storeFence):
(JSC::MacroAssemblerARMv7::replaceWithBreakpoint): Deleted.
* assembler/MacroAssemblerMIPS.h:
(JSC::MacroAssemblerMIPS::replaceWithBreakpoint): Deleted.
* assembler/MacroAssemblerX86Common.h:
(JSC::MacroAssemblerX86Common::replaceWithVMHalt):
(JSC::MacroAssemblerX86Common::replaceWithBreakpoint): Deleted.
* assembler/X86Assembler.h:
(JSC::X86Assembler::replaceWithHlt):
(JSC::X86Assembler::replaceWithInt3): Deleted.
* dfg/DFGJumpReplacement.cpp:
(JSC::DFG::JumpReplacement::installVMTrapBreakpoint):
* runtime/VMTraps.cpp:
(JSC::SignalContext::SignalContext):
(JSC::installSignalHandler):
(JSC::SignalContext::adjustPCToPointToTrappingInstruction): Deleted.
* wasm/WasmFaultSignalHandler.cpp:
(JSC::Wasm::enableFastMemory):

Source/WTF:

<rdar://problem/32178892>

Reviewed by JF Bastien.

Remove the Trap signal handler code since it plays badly with lldb and combine
SIGBUS with SIGSEGV since distiguishing them is generally non-portable.

Also, update the platform code to only use signaling VMTraps
on where we have an appropriate instruction (x86 and ARM64).

* wtf/Platform.h:
* wtf/threads/Signals.cpp:
(WTF::fromMachException):
(WTF::toMachMask):
(WTF::installSignalHandler):
(WTF::jscSignalHandler):
* wtf/threads/Signals.h:
(WTF::toSystemSignal):
(WTF::fromSystemSignal):

Tools:

Reviewed by JF Bastien.

* TestWebKitAPI/Tests/WTF/ThreadMessages.cpp:
(TEST):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@218782 268f45cc-cd09-0410-ab3c-d52691b4dbfc

22 files changed:
Source/JavaScriptCore/API/tests/ExecutionTimeLimitTest.cpp
Source/JavaScriptCore/API/tests/testapi.c
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/assembler/ARM64Assembler.h
Source/JavaScriptCore/assembler/ARMAssembler.h
Source/JavaScriptCore/assembler/ARMv7Assembler.h
Source/JavaScriptCore/assembler/MIPSAssembler.h
Source/JavaScriptCore/assembler/MacroAssemblerARM.h
Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
Source/JavaScriptCore/assembler/MacroAssemblerARMv7.h
Source/JavaScriptCore/assembler/MacroAssemblerMIPS.h
Source/JavaScriptCore/assembler/MacroAssemblerX86Common.h
Source/JavaScriptCore/assembler/X86Assembler.h
Source/JavaScriptCore/dfg/DFGJumpReplacement.cpp
Source/JavaScriptCore/runtime/VMTraps.cpp
Source/JavaScriptCore/wasm/WasmFaultSignalHandler.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/Platform.h
Source/WTF/wtf/threads/Signals.cpp
Source/WTF/wtf/threads/Signals.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WTF/ThreadMessages.cpp

index a3c39a7..73f324d 100644 (file)
@@ -121,8 +121,8 @@ int testExecutionTimeLimit()
     static const TierOptions tierOptionsList[] = {
         { "LLINT",    0,   "--useConcurrentJIT=false --useLLInt=true --useJIT=false" },
         { "Baseline", 0,   "--useConcurrentJIT=false --useLLInt=true --useJIT=true --useDFGJIT=false" },
-        { "DFG",      0,   "--useConcurrentJIT=false --useLLInt=true --useJIT=true --useDFGJIT=true --useFTLJIT=false" },
-        { "FTL",      200, "--useConcurrentJIT=false --useLLInt=true --useJIT=true --useDFGJIT=true --useFTLJIT=true" },
+        { "DFG",      200,   "--useConcurrentJIT=false --useLLInt=true --useJIT=true --useDFGJIT=true --useFTLJIT=false" },
+        { "FTL",      500, "--useConcurrentJIT=false --useLLInt=true --useJIT=true --useDFGJIT=true --useFTLJIT=true" },
     };
     
     bool failed = false;
@@ -151,7 +151,39 @@ int testExecutionTimeLimit()
         JSObjectRef currentCPUTimeFunction = JSObjectMakeFunctionWithCallback(context, currentCPUTimeStr, currentCPUTimeAsJSFunctionCallback);
         JSObjectSetProperty(context, globalObject, currentCPUTimeStr, currentCPUTimeFunction, kJSPropertyAttributeNone, nullptr);
         JSStringRelease(currentCPUTimeStr);
-        
+
+        /* Test script on another thread: */
+        timeLimit = (100 + tierAdjustmentMillis) / 1000.0;
+        JSContextGroupSetExecutionTimeLimit(contextGroup, timeLimit, shouldTerminateCallback, 0);
+        {
+            unsigned timeAfterWatchdogShouldHaveFired = 300 + tierAdjustmentMillis;
+
+            JSStringRef script = JSStringCreateWithUTF8CString("function foo() { while (true) { } } foo();");
+            exception = nullptr;
+            JSValueRef* exn = &exception;
+            shouldTerminateCallbackWasCalled = false;
+            auto thread = Thread::create("Rogue thread", [=] {
+                JSEvaluateScript(context, script, nullptr, nullptr, 1, exn);
+            });
+
+            sleep(Seconds(timeAfterWatchdogShouldHaveFired / 1000.0));
+
+            if (shouldTerminateCallbackWasCalled)
+                printf("PASS: %s script timed out as expected.\n", tierOptions.tier);
+            else {
+                printf("FAIL: %s script timeout callback was not called.\n", tierOptions.tier);
+                exit(1);
+            }
+
+            if (!exception) {
+                printf("FAIL: %s TerminatedExecutionException was not thrown.\n", tierOptions.tier);
+                exit(1);
+            }
+
+            thread->waitForCompletion();
+            testResetAfterTimeout(failed);
+        }
+
         /* Test script timeout: */
         timeLimit = (100 + tierAdjustmentMillis) / 1000.0;
         JSContextGroupSetExecutionTimeLimit(contextGroup, timeLimit, shouldTerminateCallback, 0);
index f12cc2e..9d3b456 100644 (file)
@@ -1348,6 +1348,7 @@ int main(int argc, char* argv[])
     ::SetErrorMode(0);
 #endif
 
+    testExecutionTimeLimit();
     testCompareAndSwap();
     startMultithreadedMultiVMExecutionTest();
 
index 63066de..d9ea7ca 100644 (file)
@@ -1,3 +1,59 @@
+2017-06-23  Keith Miller  <keith_miller@apple.com>
+
+        Switch VMTraps to use halt instructions rather than breakpoint instructions
+        https://bugs.webkit.org/show_bug.cgi?id=173677
+        <rdar://problem/32178892>
+
+        Reviewed by JF Bastien.
+
+        Using the breakpoint instruction for VMTraps caused issues with lldb.
+        Since we only need some way to stop execution we can, in theory, use
+        any exceptioning instruction we want. I went with the halt instruction
+        on X86 since that is the only one byte instruction that does not
+        breakpoint (in my tests both 0xf1 and 0xd6 produced EXC_BREAKPOINT).
+        On ARM we use the data cache clearing instruction with the zero register,
+        which triggers a segmentation fault.
+
+        Also, update the platform code to only use signaling VMTraps
+        on where we have an appropriate instruction (x86 and ARM64).
+
+        * API/tests/ExecutionTimeLimitTest.cpp:
+        (testExecutionTimeLimit):
+        * assembler/ARM64Assembler.h:
+        (JSC::ARM64Assembler::replaceWithVMHalt):
+        (JSC::ARM64Assembler::dataCacheZeroVirtualAddress):
+        (JSC::ARM64Assembler::replaceWithBkpt): Deleted.
+        * assembler/ARMAssembler.h:
+        (JSC::ARMAssembler::replaceWithBkpt): Deleted.
+        * assembler/ARMv7Assembler.h:
+        (JSC::ARMv7Assembler::replaceWithBkpt): Deleted.
+        * assembler/MIPSAssembler.h:
+        (JSC::MIPSAssembler::replaceWithBkpt): Deleted.
+        * assembler/MacroAssemblerARM.h:
+        (JSC::MacroAssemblerARM::replaceWithBreakpoint): Deleted.
+        * assembler/MacroAssemblerARM64.h:
+        (JSC::MacroAssemblerARM64::replaceWithVMHalt):
+        (JSC::MacroAssemblerARM64::replaceWithBreakpoint): Deleted.
+        * assembler/MacroAssemblerARMv7.h:
+        (JSC::MacroAssemblerARMv7::storeFence):
+        (JSC::MacroAssemblerARMv7::replaceWithBreakpoint): Deleted.
+        * assembler/MacroAssemblerMIPS.h:
+        (JSC::MacroAssemblerMIPS::replaceWithBreakpoint): Deleted.
+        * assembler/MacroAssemblerX86Common.h:
+        (JSC::MacroAssemblerX86Common::replaceWithVMHalt):
+        (JSC::MacroAssemblerX86Common::replaceWithBreakpoint): Deleted.
+        * assembler/X86Assembler.h:
+        (JSC::X86Assembler::replaceWithHlt):
+        (JSC::X86Assembler::replaceWithInt3): Deleted.
+        * dfg/DFGJumpReplacement.cpp:
+        (JSC::DFG::JumpReplacement::installVMTrapBreakpoint):
+        * runtime/VMTraps.cpp:
+        (JSC::SignalContext::SignalContext):
+        (JSC::installSignalHandler):
+        (JSC::SignalContext::adjustPCToPointToTrappingInstruction): Deleted.
+        * wasm/WasmFaultSignalHandler.cpp:
+        (JSC::Wasm::enableFastMemory):
+
 2017-06-22  Saam Barati  <sbarati@apple.com>
 
         The lowering of Identity in the DFG backend needs to use ManualOperandSpeculation
index 6ad46de..b850673 100644 (file)
@@ -2602,9 +2602,10 @@ public:
         linkPointer(addressOf(code, where), valuePtr);
     }
 
-    static void replaceWithBkpt(void* where)
+    static void replaceWithVMHalt(void* where)
     {
-        int insn = excepnGeneration(ExcepnOp_BREAKPOINT, 0, 0);
+        // This should try to write to null which should always Segfault.
+        int insn = dataCacheZeroVirtualAddress(ARM64Registers::zr);
         performJITMemcpy(where, &insn, sizeof(int));
         cacheFlush(where, sizeof(int));
     }
@@ -3655,6 +3656,11 @@ private:
     {
         return hintPseudo(0);
     }
+
+    ALWAYS_INLINE static int dataCacheZeroVirtualAddress(RegisterID rt)
+    {
+        return system(0, 1, 0x3, 0x7, 0x4, 0x1, rt);
+    }
     
     // 'op' means negate
     ALWAYS_INLINE static int testAndBranchImmediate(bool op, int b50, int imm14, RegisterID rt)
index 91ce0b2..48e1101 100644 (file)
@@ -995,13 +995,6 @@ namespace JSC {
             return reinterpret_cast<void*>(readPointer(reinterpret_cast<void*>(getAbsoluteJumpAddress(from))));
         }
 
-        static void replaceWithBkpt(void* instructionStart)
-        {
-            ARMWord* instruction = reinterpret_cast<ARMWord*>(instructionStart);
-            instruction[0] = BKPT;
-            cacheFlush(instruction, sizeof(ARMWord));
-        }
-
         static void replaceWithJump(void* instructionStart, void* to)
         {
             ARMWord* instruction = reinterpret_cast<ARMWord*>(instructionStart);
index 6cd4025..eb61509 100644 (file)
@@ -2328,16 +2328,6 @@ public:
         return reinterpret_cast<void*>(readInt32(where));
     }
 
-    static void replaceWithBkpt(void* instructionStart)
-    {
-        ASSERT(!(bitwise_cast<uintptr_t>(instructionStart) & 1));
-
-        uint16_t* ptr = reinterpret_cast<uint16_t*>(instructionStart);
-        uint16_t instructions = OP_BKPT;
-        performJITMemcpy(ptr, &instructions, sizeof(uint16_t));
-        cacheFlush(ptr, sizeof(uint16_t));
-    }
-
     static void replaceWithJump(void* instructionStart, void* to)
     {
         ASSERT(!(bitwise_cast<uintptr_t>(instructionStart) & 1));
index 5fa0fa0..270cce3 100644 (file)
@@ -941,15 +941,6 @@ public:
         cacheFlush(insn, codeSize);
     }
 
-    static void replaceWithBkpt(void* instructionStart)
-    {
-        ASSERT(!(bitwise_cast<uintptr_t>(instructionStart) & 3));
-        MIPSWord* insn = reinterpret_cast<MIPSWord*>(reinterpret_cast<intptr_t>(instructionStart));
-        int value = 512; /* BRK_BUG */
-        insn[0] = (0x0000000d | ((value & 0x3ff) << OP_SH_CODE));
-        cacheFlush(instructionStart, sizeof(MIPSWord));
-    }
-
     static void replaceWithJump(void* instructionStart, void* to)
     {
         ASSERT(!(bitwise_cast<uintptr_t>(instructionStart) & 3));
index 6f5f37e..2b80d24 100644 (file)
@@ -1499,11 +1499,6 @@ public:
         return FunctionPtr(reinterpret_cast<void(*)()>(ARMAssembler::readCallTarget(call.dataLocation())));
     }
 
-    static void replaceWithBreakpoint(CodeLocationLabel instructionStart)
-    {
-        ARMAssembler::replaceWithBkpt(instructionStart.executableAddress());
-    }
-
     static void replaceWithJump(CodeLocationLabel instructionStart, CodeLocationLabel destination)
     {
         ARMAssembler::replaceWithJump(instructionStart.dataLocation(), destination.dataLocation());
index fd494a0..c37e00b 100644 (file)
@@ -3713,9 +3713,9 @@ public:
         return FunctionPtr(reinterpret_cast<void(*)()>(ARM64Assembler::readCallTarget(call.dataLocation())));
     }
 
-    static void replaceWithBreakpoint(CodeLocationLabel instructionStart)
+    static void replaceWithVMHalt(CodeLocationLabel instructionStart)
     {
-        ARM64Assembler::replaceWithBkpt(instructionStart.executableAddress());
+        ARM64Assembler::replaceWithVMHalt(instructionStart.executableAddress());
     }
 
     static void replaceWithJump(CodeLocationLabel instructionStart, CodeLocationLabel destination)
index 54335a2..f3fe61d 100644 (file)
@@ -1354,11 +1354,6 @@ public:
     {
         m_assembler.dmbISHST();
     }
-    
-    static void replaceWithBreakpoint(CodeLocationLabel instructionStart)
-    {
-        ARMv7Assembler::replaceWithBkpt(instructionStart.dataLocation());
-    }
 
     static void replaceWithJump(CodeLocationLabel instructionStart, CodeLocationLabel destination)
     {
index ff40090..fa05be1 100644 (file)
@@ -3072,11 +3072,6 @@ public:
         return FunctionPtr(reinterpret_cast<void(*)()>(MIPSAssembler::readCallTarget(call.dataLocation())));
     }
 
-    static void replaceWithBreakpoint(CodeLocationLabel instructionStart)
-    {
-        MIPSAssembler::replaceWithBkpt(instructionStart.executableAddress());
-    }
-
     static void replaceWithJump(CodeLocationLabel instructionStart, CodeLocationLabel destination)
     {
         MIPSAssembler::replaceWithJump(instructionStart.dataLocation(), destination.dataLocation());
index 32da450..6acc2a9 100644 (file)
@@ -3803,9 +3803,9 @@ public:
     }
 #endif
 
-    static void replaceWithBreakpoint(CodeLocationLabel instructionStart)
+    static void replaceWithVMHalt(CodeLocationLabel instructionStart)
     {
-        X86Assembler::replaceWithInt3(instructionStart.executableAddress());
+        X86Assembler::replaceWithHlt(instructionStart.executableAddress());
     }
 
     static void replaceWithJump(CodeLocationLabel instructionStart, CodeLocationLabel destination)
index addc7d8..5b64f02 100644 (file)
@@ -3637,10 +3637,10 @@ public:
         return reinterpret_cast<void**>(where)[-1];
     }
 
-    static void replaceWithInt3(void* instructionStart)
+    static void replaceWithHlt(void* instructionStart)
     {
         uint8_t* ptr = reinterpret_cast<uint8_t*>(instructionStart);
-        ptr[0] = static_cast<uint8_t>(OP_INT3);
+        ptr[0] = static_cast<uint8_t>(OP_HLT);
     }
 
     static void replaceWithJump(void* instructionStart, void* to)
index 247bda9..1ebf694 100644 (file)
@@ -43,7 +43,11 @@ void JumpReplacement::fire()
 
 void JumpReplacement::installVMTrapBreakpoint()
 {
-    MacroAssembler::replaceWithBreakpoint(m_source);
+#if ENABLE(SIGNAL_BASED_VM_TRAPS)
+    MacroAssembler::replaceWithVMHalt(m_source);
+#else
+    UNREACHABLE_FOR_PLATFORM();
+#endif
 }
 
 } } // namespace JSC::DFG
index 51f28d3..44d53ea 100644 (file)
@@ -59,17 +59,6 @@ struct SignalContext {
         , stackPointer(MachineContext::stackPointer(registers))
         , framePointer(MachineContext::framePointer(registers))
     {
-#if CPU(X86_64) || CPU(X86)
-        // On X86_64, SIGTRAP reports the address after the trapping PC. So, dec by 1.
-        trapPC = reinterpret_cast<uint8_t*>(trapPC) - 1;
-#endif
-    }
-
-    void adjustPCToPointToTrappingInstruction()
-    {
-#if CPU(X86_64) || CPU(X86)
-        MachineContext::instructionPointer(registers) = trapPC;
-#endif
     }
 
     PlatformRegisters& registers;
@@ -130,7 +119,7 @@ static Expected<VMAndStackBounds, VMTraps::Error> findActiveVMAndStackBounds(Sig
 
 static void installSignalHandler()
 {
-    installSignalHandler(Signal::Trap, [] (Signal, SigInfo&, PlatformRegisters& registers) -> SignalAction {
+    installSignalHandler(Signal::BadAccess, [] (Signal, SigInfo&, PlatformRegisters& registers) -> SignalAction {
         SignalContext context(registers);
 
         if (!isJITPC(context.trapPC))
@@ -297,10 +286,6 @@ auto VMTraps::tryJettisonCodeBlocksOnStack(SignalContext& context) -> Expected<b
         return false;
 
     invalidateCodeBlocksOnStack(codeBlockSetLocker, topCallFrame);
-
-    // Re-run the trapping instruction now that we've patched it with the invalidation
-    // OSR exit off-ramp.
-    context.adjustPCToPointToTrappingInstruction();
     return true;
 }
 
index 264c34d..e86fd47 100644 (file)
@@ -123,11 +123,7 @@ void enableFastMemory()
             return;
 
 #if ENABLE(WEBASSEMBLY_FAST_MEMORY)
-        installSignalHandler(Signal::Bus, [] (Signal signal, SigInfo& sigInfo, PlatformRegisters& ucontext) {
-            return trapHandler(signal, sigInfo, ucontext);
-        });
-
-        installSignalHandler(Signal::SegV, [] (Signal signal, SigInfo& sigInfo, PlatformRegisters& ucontext) {
+        installSignalHandler(Signal::BadAccess, [] (Signal signal, SigInfo& sigInfo, PlatformRegisters& ucontext) {
             return trapHandler(signal, sigInfo, ucontext);
         });
 
index 68075a9..23012e4 100644 (file)
@@ -1,3 +1,27 @@
+2017-06-23  Keith Miller  <keith_miller@apple.com>
+
+        Switch VMTraps to use halt instructions rather than breakpoint instructions
+        https://bugs.webkit.org/show_bug.cgi?id=173677
+        <rdar://problem/32178892>
+
+        Reviewed by JF Bastien.
+
+        Remove the Trap signal handler code since it plays badly with lldb and combine
+        SIGBUS with SIGSEGV since distiguishing them is generally non-portable.
+
+        Also, update the platform code to only use signaling VMTraps
+        on where we have an appropriate instruction (x86 and ARM64).
+
+        * wtf/Platform.h:
+        * wtf/threads/Signals.cpp:
+        (WTF::fromMachException):
+        (WTF::toMachMask):
+        (WTF::installSignalHandler):
+        (WTF::jscSignalHandler):
+        * wtf/threads/Signals.h:
+        (WTF::toSystemSignal):
+        (WTF::fromSystemSignal):
+
 2017-06-23  Antti Koivisto  <antti@apple.com>
 
         Add notifyutil registrations for going in and out of simulated low memory state
index 5004077..87bb3bb 100644 (file)
 #endif
 #endif
 
-#if ENABLE(JIT) && HAVE(MACHINE_CONTEXT)
+#if ENABLE(JIT) && HAVE(MACHINE_CONTEXT) && (CPU(X86) || CPU(X86_64) || CPU(ARM64))
 #define ENABLE_SIGNAL_BASED_VM_TRAPS 1
 #endif
 
index f01e391..56973a9 100644 (file)
@@ -117,9 +117,8 @@ static void startMachExceptionHandlerThread()
 static Signal fromMachException(exception_type_t type)
 {
     switch (type) {
-    case EXC_BAD_ACCESS: return Signal::SegV;
+    case EXC_BAD_ACCESS: return Signal::BadAccess;
     case EXC_BAD_INSTRUCTION: return Signal::Ill;
-    case EXC_BREAKPOINT: return Signal::Trap;
     default: break;
     }
     return Signal::Unknown;
@@ -128,9 +127,8 @@ static Signal fromMachException(exception_type_t type)
 static exception_mask_t toMachMask(Signal signal)
 {
     switch (signal) {
-    case Signal::SegV: return EXC_MASK_BAD_ACCESS;
+    case Signal::BadAccess: return EXC_MASK_BAD_ACCESS;
     case Signal::Ill: return EXC_MASK_BAD_INSTRUCTION;
-    case Signal::Trap: return EXC_MASK_BREAKPOINT;
     default: break;
     }
     RELEASE_ASSERT_NOT_REACHED();
@@ -166,6 +164,11 @@ kern_return_t catch_mach_exception_raise_state(
     mach_msg_type_number_t* outStateCount)
 {
     RELEASE_ASSERT(port == exceptionPort);
+    // If we wanted to distinguish between SIGBUS and SIGSEGV for EXC_BAD_ACCESS on Darwin we could do:
+    // if (exceptionData[0] == KERN_INVALID_ADDRESS)
+    //    signal = SIGSEGV;
+    // else
+    //    signal = SIGBUS;
     Signal signal = fromMachException(exceptionType);
     RELEASE_ASSERT(signal != Signal::Unknown);
 
@@ -187,7 +190,7 @@ kern_return_t catch_mach_exception_raise_state(
 #endif
 
     SigInfo info;
-    if (signal == Signal::SegV) {
+    if (signal == Signal::BadAccess) {
         ASSERT_UNUSED(dataCount, dataCount == 2);
         info.faultingAddress = reinterpret_cast<void*>(exceptionData[1]);
     }
@@ -249,18 +252,19 @@ void unregisterThreadForMachExceptionHandling(Thread* thread)
 static constexpr bool useMach = false;
 #endif // HAVE(MACH_EXCEPTIONS)
 
+
+inline size_t offsetForSystemSignal(int sig)
+{
+    Signal signal = fromSystemSignal(sig);
+    return static_cast<size_t>(signal) + (sig == SIGBUS);
+}
+
 static void jscSignalHandler(int, siginfo_t*, void*);
 
 void installSignalHandler(Signal signal, SignalHandler&& handler)
 {
     ASSERT(signal < Signal::Unknown);
 #if HAVE(MACH_EXCEPTIONS)
-    // Since mach only has EXC_BAD_ACCESS, which covers both SegV and Bus, we arbitarily choose to make
-    // mach EXC_BAD_ACCESSes map to SegV.
-    // FIXME: We should just use a single Signal::BadAccess value instead of SegV/Bus.
-    // See: https://bugs.webkit.org/show_bug.cgi?id=172063
-    if (signal == Signal::Bus && useMach)
-        return;
     ASSERT(!useMach || signal != Signal::Usr);
 
     if (useMach)
@@ -276,7 +280,10 @@ void installSignalHandler(Signal signal, SignalHandler&& handler)
             auto result = sigfillset(&action.sa_mask);
             RELEASE_ASSERT(!result);
             action.sa_flags = SA_SIGINFO;
-            result = sigaction(toSystemSignal(signal), &action, &oldActions[static_cast<size_t>(signal)]);
+            auto systemSignals = toSystemSignal(signal);
+            result = sigaction(std::get<0>(systemSignals), &action, &oldActions[offsetForSystemSignal(std::get<0>(systemSignals))]);
+            if (std::get<1>(systemSignals))
+                result |= sigaction(*std::get<1>(systemSignals), &action, &oldActions[offsetForSystemSignal(*std::get<1>(systemSignals))]);
             RELEASE_ASSERT(!result);
         }
 
@@ -316,7 +323,7 @@ void jscSignalHandler(int sig, siginfo_t* info, void* ucontext)
     }
 
     SigInfo sigInfo;
-    if (signal == Signal::SegV || signal == Signal::Bus)
+    if (signal == Signal::BadAccess)
         sigInfo.faultingAddress = info->si_addr;
 
     PlatformRegisters& registers = registersFromUContext(reinterpret_cast<ucontext_t*>(ucontext));
@@ -341,7 +348,8 @@ void jscSignalHandler(int sig, siginfo_t* info, void* ucontext)
         return;
     }
 
-    struct sigaction& oldAction = oldActions[static_cast<size_t>(signal)];
+    unsigned oldActionIndex = static_cast<size_t>(signal) + (sig == SIGBUS);
+    struct sigaction& oldAction = oldActions[static_cast<size_t>(oldActionIndex)];
     if (signal == Signal::Usr) {
         if (oldAction.sa_sigaction)
             oldAction.sa_sigaction(sig, info, ucontext);
index 2576fad..faae0a7 100644 (file)
@@ -29,6 +29,7 @@
 
 #include <signal.h>
 #include <wtf/Function.h>
+#include <wtf/Optional.h>
 #include <wtf/PlatformRegisters.h>
 
 namespace WTF {
@@ -41,22 +42,18 @@ enum class Signal {
 
     // These signals will only chain if we don't have a handler that can process them. If there is nothing
     // to chain to we restore the default handler and crash.
-    Trap,
     Ill,
-    SegV,
-    Bus,
-    NumberOfSignals,
+    BadAccess, // For posix this is both SIGSEGV and SIGBUS
+    NumberOfSignals = BadAccess + 2, // BadAccess is really two signals.
     Unknown = NumberOfSignals
 };
 
-inline int toSystemSignal(Signal signal)
+inline std::tuple<int, std::optional<int>> toSystemSignal(Signal signal)
 {
     switch (signal) {
-    case Signal::SegV: return SIGSEGV;
-    case Signal::Bus: return SIGBUS;
-    case Signal::Ill: return SIGILL;
-    case Signal::Usr: return SIGUSR2;
-    case Signal::Trap: return SIGTRAP;
+    case Signal::BadAccess: return std::make_tuple(SIGSEGV, SIGBUS);
+    case Signal::Ill: return std::make_tuple(SIGILL, std::nullopt);
+    case Signal::Usr: return std::make_tuple(SIGILL, std::nullopt);
     default: break;
     }
     RELEASE_ASSERT_NOT_REACHED();
@@ -65,11 +62,10 @@ inline int toSystemSignal(Signal signal)
 inline Signal fromSystemSignal(int signal)
 {
     switch (signal) {
-    case SIGSEGV: return Signal::SegV;
-    case SIGBUS: return Signal::Bus;
+    case SIGSEGV: return Signal::BadAccess;
+    case SIGBUS: return Signal::BadAccess;
     case SIGILL: return Signal::Ill;
     case SIGUSR2: return Signal::Usr;
-    case SIGTRAP: return Signal::Trap;
     default: return Signal::Unknown;
     }
 }
index 582b180..d7cdba5 100644 (file)
@@ -1,3 +1,13 @@
+2017-06-23  Keith Miller  <keith_miller@apple.com>
+
+        Switch VMTraps to use halt instructions rather than breakpoint instructions
+        https://bugs.webkit.org/show_bug.cgi?id=173677
+
+        Reviewed by JF Bastien.
+
+        * TestWebKitAPI/Tests/WTF/ThreadMessages.cpp:
+        (TEST):
+
 2017-06-23  Youenn Fablet  <youenn@apple.com>
 
         Set getUserMedia permission to true by default on WTR
index e6a5789..266f20e 100644 (file)
@@ -114,7 +114,7 @@ TEST(ThreadMessage, SignalsWorkOnExit)
         receiverShouldKeepRunning.store(false);
         EXPECT_FALSE(static_cast<ReflectedThread*>(receiverThread.get())->hasExited());
         sleep(1);
-        signalFired = !pthread_kill(static_cast<ReflectedThread*>(receiverThread.get())->m_handle, toSystemSignal(Signal::Usr));
+        signalFired = !pthread_kill(static_cast<ReflectedThread*>(receiverThread.get())->m_handle, std::get<0>(toSystemSignal(Signal::Usr)));
     }
 
     receiverThread->waitForCompletion();