Source/JavaScriptCore:
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 3 Aug 2018 00:14:11 +0000 (00:14 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 3 Aug 2018 00:14:11 +0000 (00:14 +0000)
Reading instructionPointer from PlatformRegisters may fail when using pointer profiling
https://bugs.webkit.org/show_bug.cgi?id=188271
<rdar://problem/42850884>

Reviewed by Michael Saboff.

This patch defends against the instructionPointer containing garbage bits.
See radar for details.

* runtime/MachineContext.h:
(JSC::MachineContext::instructionPointer):
* runtime/SamplingProfiler.cpp:
(JSC::SamplingProfiler::takeSample):
* runtime/VMTraps.cpp:
(JSC::SignalContext::SignalContext):
(JSC::SignalContext::tryCreate):
* tools/CodeProfiling.cpp:
(JSC::profilingTimer):
* tools/SigillCrashAnalyzer.cpp:
(JSC::SignalContext::SignalContext):
(JSC::SignalContext::tryCreate):
(JSC::SignalContext::dump):
(JSC::installCrashHandler):
* wasm/WasmFaultSignalHandler.cpp:
(JSC::Wasm::trapHandler):

Source/WTF:
Reading instructionPointer from PlatformRegisters may fail when using pointer tagging
https://bugs.webkit.org/show_bug.cgi?id=188271
<rdar://problem/42850884>

Reviewed by Michael Saboff.

* wtf/PtrTag.h:
(WTF::isTaggedWith):
(WTF::usesPointerTagging):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/MachineContext.h
Source/JavaScriptCore/runtime/SamplingProfiler.cpp
Source/JavaScriptCore/runtime/VMTraps.cpp
Source/JavaScriptCore/tools/CodeProfiling.cpp
Source/JavaScriptCore/tools/SigillCrashAnalyzer.cpp
Source/JavaScriptCore/wasm/WasmFaultSignalHandler.cpp
Source/WTF/ChangeLog
Source/WTF/wtf/PtrTag.h

index 0868964..4ce72a2 100644 (file)
@@ -1,3 +1,31 @@
+2018-08-02  Saam Barati  <sbarati@apple.com>
+
+        Reading instructionPointer from PlatformRegisters may fail when using pointer profiling
+        https://bugs.webkit.org/show_bug.cgi?id=188271
+        <rdar://problem/42850884>
+
+        Reviewed by Michael Saboff.
+
+        This patch defends against the instructionPointer containing garbage bits.
+        See radar for details.
+
+        * runtime/MachineContext.h:
+        (JSC::MachineContext::instructionPointer):
+        * runtime/SamplingProfiler.cpp:
+        (JSC::SamplingProfiler::takeSample):
+        * runtime/VMTraps.cpp:
+        (JSC::SignalContext::SignalContext):
+        (JSC::SignalContext::tryCreate):
+        * tools/CodeProfiling.cpp:
+        (JSC::profilingTimer):
+        * tools/SigillCrashAnalyzer.cpp:
+        (JSC::SignalContext::SignalContext):
+        (JSC::SignalContext::tryCreate):
+        (JSC::SignalContext::dump):
+        (JSC::installCrashHandler):
+        * wasm/WasmFaultSignalHandler.cpp:
+        (JSC::Wasm::trapHandler):
+
 2018-08-02  David Fenton  <david_fenton@apple.com>
 
         Unreviewed, rolling out r234489.
index 26a4fc0..0a8d257 100644 (file)
@@ -29,6 +29,7 @@
 #include "GPRInfo.h"
 #include "LLIntPCRanges.h"
 #include "MacroAssemblerCodeRef.h"
+#include <wtf/Optional.h>
 #include <wtf/PlatformRegisters.h>
 #include <wtf/PointerPreparations.h>
 #include <wtf/StdLibExtras.h>
@@ -44,7 +45,7 @@ template<typename T = void*> T framePointer(const PlatformRegisters&);
 template<typename T = void*> void setFramePointer(PlatformRegisters&, T);
 inline MacroAssemblerCodePtr<CFunctionPtrTag> linkRegister(const PlatformRegisters&);
 inline void setLinkRegister(PlatformRegisters&, MacroAssemblerCodePtr<CFunctionPtrTag>);
-inline MacroAssemblerCodePtr<CFunctionPtrTag> instructionPointer(const PlatformRegisters&);
+inline std::optional<MacroAssemblerCodePtr<CFunctionPtrTag>> instructionPointer(const PlatformRegisters&);
 inline void setInstructionPointer(PlatformRegisters&, MacroAssemblerCodePtr<CFunctionPtrTag>);
 
 template<size_t N> void*& argumentPointer(PlatformRegisters&);
@@ -431,7 +432,7 @@ static inline void*& instructionPointerImpl(PlatformRegisters& regs)
 }
 #endif // !USE(PLATFORM_REGISTERS_WITH_PROFILE)
 
-inline MacroAssemblerCodePtr<CFunctionPtrTag> instructionPointer(const PlatformRegisters& regs)
+inline std::optional<MacroAssemblerCodePtr<CFunctionPtrTag>> instructionPointer(const PlatformRegisters& regs)
 {
 #if USE(PLATFORM_REGISTERS_WITH_PROFILE)
     void* value = WTF_READ_PLATFORM_REGISTERS_PC_WITH_PROFILE(regs);
@@ -440,7 +441,11 @@ inline MacroAssemblerCodePtr<CFunctionPtrTag> instructionPointer(const PlatformR
 #endif
     if (!value)
         return MacroAssemblerCodePtr<CFunctionPtrTag>(nullptr);
-    return MacroAssemblerCodePtr<CFunctionPtrTag>(value);
+    if (!usesPointerTagging())
+        return MacroAssemblerCodePtr<CFunctionPtrTag>(value);
+    if (isTaggedWith(value, CFunctionPtrTag))
+        return MacroAssemblerCodePtr<CFunctionPtrTag>(value);
+    return std::nullopt;
 }
 
 inline void setInstructionPointer(PlatformRegisters& regs, MacroAssemblerCodePtr<CFunctionPtrTag> value)
index 67fbbbc..b6935e4 100644 (file)
@@ -356,7 +356,11 @@ void SamplingProfiler::takeSample(const AbstractLocker&, Seconds& stackTraceProc
                 m_jscExecutionThread->getRegisters(registers);
                 machineFrame = MachineContext::framePointer(registers);
                 callFrame = static_cast<ExecState*>(machineFrame);
-                machinePC = MachineContext::instructionPointer(registers).untaggedExecutableAddress();
+                auto instructionPointer = MachineContext::instructionPointer(registers);
+                if (instructionPointer)
+                    machinePC = instructionPointer->untaggedExecutableAddress();
+                else
+                    machinePC = nullptr;
                 llintPC = removeCodePtrTag(MachineContext::llintInstructionPointer(registers));
                 assertIsNotTagged(machinePC);
             }
index c263419..cf5ade1 100644 (file)
@@ -55,13 +55,23 @@ ALWAYS_INLINE VM& VMTraps::vm() const
 #if ENABLE(SIGNAL_BASED_VM_TRAPS)
 
 struct SignalContext {
-    SignalContext(PlatformRegisters& registers)
+private:
+    SignalContext(PlatformRegisters& registers, MacroAssemblerCodePtr<CFunctionPtrTag> trapPC)
         : registers(registers)
-        , trapPC(MachineContext::instructionPointer(registers))
+        , trapPC(trapPC)
         , stackPointer(MachineContext::stackPointer(registers))
         , framePointer(MachineContext::framePointer(registers))
     { }
 
+public:
+    static std::optional<SignalContext> tryCreate(PlatformRegisters& registers)
+    {
+        auto instructionPointer = MachineContext::instructionPointer(registers);
+        if (!instructionPointer)
+            return std::nullopt;
+        return SignalContext(registers, *instructionPointer);
+    }
+
     PlatformRegisters& registers;
     MacroAssemblerCodePtr<CFunctionPtrTag> trapPC;
     void* stackPointer;
@@ -186,9 +196,11 @@ public:
         static std::once_flag once;
         std::call_once(once, [] {
             installSignalHandler(Signal::BadAccess, [] (Signal, SigInfo&, PlatformRegisters& registers) -> SignalAction {
-                SignalContext context(registers);
+                auto signalContext = SignalContext::tryCreate(registers);
+                if (!signalContext)
+                    return SignalAction::NotHandled;
 
-                void* trapPC = context.trapPC.untaggedExecutableAddress();
+                void* trapPC = signalContext->trapPC.untaggedExecutableAddress();
                 if (!isJITPC(trapPC))
                     return SignalAction::NotHandled;
 
@@ -249,7 +261,9 @@ protected:
         auto optionalOwnerThread = vm.ownerThread();
         if (optionalOwnerThread) {
             sendMessage(*optionalOwnerThread.value().get(), [&] (PlatformRegisters& registers) -> void {
-                SignalContext context(registers);
+                auto signalContext = SignalContext::tryCreate(registers);
+                if (!signalContext)
+                    return;
 
                 auto ownerThread = vm.apiLock().ownerThread();
                 // We can't mess with a thread unless it's the one we suspended.
@@ -257,7 +271,7 @@ protected:
                     return;
 
                 Thread& thread = *ownerThread->get();
-                vm.traps().tryInstallTrapBreakpoints(context, thread.stack());
+                vm.traps().tryInstallTrapBreakpoints(*signalContext, thread.stack());
             });
         }
 
index 70cba8a..c3da8c2 100644 (file)
@@ -71,9 +71,11 @@ static void setProfileTimer(unsigned usec)
 static void profilingTimer(int, siginfo_t*, void* uap)
 {
     PlatformRegisters& platformRegisters = WTF::registersFromUContext(static_cast<ucontext_t*>(uap));
-    CodeProfiling::sample(
-        MachineContext::instructionPointer(platformRegisters).untaggedExecutableAddress(),
-        reinterpret_cast<void**>(MachineContext::framePointer(platformRegisters)));
+    if (auto instructionPointer = MachineContext::instructionPointer(platformRegisters)) {
+        CodeProfiling::sample(
+            instructionPointer->untaggedExecutableAddress(),
+            reinterpret_cast<void**>(MachineContext::framePointer(platformRegisters)));
+    }
 }
 #endif
 
index 7072090..9cb9d0f 100644 (file)
@@ -78,13 +78,23 @@ private:
 #endif // USE(OS_LOG)
 
 struct SignalContext {
-    SignalContext(PlatformRegisters& registers)
+private:
+    SignalContext(PlatformRegisters& registers, MacroAssemblerCodePtr<CFunctionPtrTag> machinePC)
         : registers(registers)
-        , machinePC(MachineContext::instructionPointer(registers))
+        , machinePC(machinePC)
         , stackPointer(MachineContext::stackPointer(registers))
         , framePointer(MachineContext::framePointer(registers))
     { }
 
+public:
+    static std::optional<SignalContext> tryCreate(PlatformRegisters& registers)
+    {
+        auto instructionPointer = MachineContext::instructionPointer(registers);
+        if (!instructionPointer)
+            return std::nullopt;
+        return SignalContext(registers, *instructionPointer);
+    }
+
     void dump()
     {
 #if CPU(X86_64)
@@ -132,7 +142,7 @@ struct SignalContext {
             MachineContext::linkRegister(registers).untaggedExecutableAddress<uint64_t>());
         log("sp: %016llx pc: %016llx cpsr: %08x",
             MachineContext::stackPointer<uint64_t>(registers),
-            MachineContext::instructionPointer(registers).untaggedExecutableAddress<uint64_t>(),
+            machinePC.untaggedExecutableAddress<uint64_t>(),
             registers.__cpsr);
 #endif
     }
@@ -147,14 +157,16 @@ static void installCrashHandler()
 {
 #if CPU(X86_64) || CPU(ARM64)
     installSignalHandler(Signal::Ill, [] (Signal, SigInfo&, PlatformRegisters& registers) {
-        SignalContext context(registers);
-
-        void* machinePC = context.machinePC.untaggedExecutableAddress();
+        auto signalContext = SignalContext::tryCreate(registers);
+        if (!signalContext)
+            return SignalAction::NotHandled;
+            
+        void* machinePC = signalContext->machinePC.untaggedExecutableAddress();
         if (!isJITPC(machinePC))
             return SignalAction::NotHandled;
 
         SigillCrashAnalyzer& analyzer = SigillCrashAnalyzer::instance();
-        analyzer.analyze(context);
+        analyzer.analyze(*signalContext);
         return SignalAction::NotHandled;
     });
 #endif
index 26c8fee..0d0d8cf 100644 (file)
@@ -56,7 +56,10 @@ static bool fastHandlerInstalled { false };
 
 static SignalAction trapHandler(Signal, SigInfo& sigInfo, PlatformRegisters& context)
 {
-    void* faultingInstruction = MachineContext::instructionPointer(context).untaggedExecutableAddress();
+    auto instructionPointer = MachineContext::instructionPointer(context);
+    if (!instructionPointer)
+        return SignalAction::NotHandled;
+    void* faultingInstruction = instructionPointer->untaggedExecutableAddress();
     dataLogLnIf(WasmFaultSignalHandlerInternal::verbose, "starting handler for fault at: ", RawPointer(faultingInstruction));
 
     dataLogLnIf(WasmFaultSignalHandlerInternal::verbose, "JIT memory start: ", RawPointer(startOfFixedExecutableMemoryPool()), " end: ", RawPointer(endOfFixedExecutableMemoryPool()));
index 4c1007b..bdfe9be 100644 (file)
@@ -1,3 +1,15 @@
+2018-08-02  Saam Barati  <sbarati@apple.com>
+
+        Reading instructionPointer from PlatformRegisters may fail when using pointer tagging
+        https://bugs.webkit.org/show_bug.cgi?id=188271
+        <rdar://problem/42850884>
+
+        Reviewed by Michael Saboff.
+
+        * wtf/PtrTag.h:
+        (WTF::isTaggedWith):
+        (WTF::usesPointerTagging):
+
 2018-08-02  Alex Christensen  <achristensen@webkit.org>
 
         Add some Variant types to Forward.h
index 33e7202..be2fa02 100644 (file)
@@ -154,9 +154,13 @@ template<typename PtrType> void assertIsNotTagged(PtrType) { }
 template<typename PtrType> void assertIsTagged(PtrType) { }
 template<typename PtrType> void assertIsNullOrTagged(PtrType) { }
 
+template<typename PtrType> bool isTaggedWith(PtrType, PtrTag) { return false; }
+
 template<typename PtrType> void assertIsTaggedWith(PtrType, PtrTag) { }
 template<typename PtrType> void assertIsNullOrTaggedWith(PtrType, PtrTag) { }
 
+inline bool usesPointerTagging() { return false; }
+
 #define CALL_WITH_PTRTAG(callInstructionString, targetRegisterString, tag) \
     callInstructionString " " targetRegisterString "\n"
 
@@ -186,5 +190,7 @@ using WTF::assertIsNullOrCFunctionPtr;
 using WTF::assertIsNotTagged;
 using WTF::assertIsTagged;
 using WTF::assertIsNullOrTagged;
+using WTF::isTaggedWith;
 using WTF::assertIsTaggedWith;
 using WTF::assertIsNullOrTaggedWith;
+using WTF::usesPointerTagging;