Revert "Cherry-pick f442fbe222f3. rdar://126892345"

This reverts commit ffd05ad.
WebKit · Apr 24, 2024 · 6fc6e82 · 6fc6e82
1 parent ab614cf
commit 6fc6e82
Show file tree

Hide file tree

Showing 9 changed files with 25 additions and 118 deletions.
diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerARM64E.h b/Source/JavaScriptCore/assembler/MacroAssemblerARM64E.h
@@ -65,12 +65,6 @@ class MacroAssemblerARM64E : public MacroAssemblerARM64 {
 
     ALWAYS_INLINE void tagPtr(PtrTag tag, RegisterID target)
     {
-        if (!tag) {
-            m_assembler.pacizb(target);
-            return;
-        }
-
-        RELEASE_ASSERT(Options::allowNonSPTagging());
         auto tagGPR = getCachedDataTempRegisterIDAndInvalidate();
         move(TrustedImm64(tag), tagGPR);
         m_assembler.pacib(target, tagGPR);
@@ -82,17 +76,11 @@ class MacroAssemblerARM64E : public MacroAssemblerARM64 {
             m_assembler.pacibsp();
             return;
         }
-        RELEASE_ASSERT(Options::allowNonSPTagging());
         m_assembler.pacib(target, tag);
     }
 
     ALWAYS_INLINE void untagPtr(PtrTag tag, RegisterID target)
     {
-        if (!tag) {
-            m_assembler.autizb(target);
-            return;
-        }
-
         auto tagGPR = getCachedDataTempRegisterIDAndInvalidate();
         move(TrustedImm64(tag), tagGPR);
         m_assembler.autib(target, tagGPR);

diff --git a/Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp b/Source/JavaScriptCore/dfg/DFGOSRExitCompilerCommon.cpp
@@ -252,16 +252,6 @@ void reifyInlinedCallFrames(CCallHelpers& jit, const OSRExitBase& exit)
     ASSERT(JITCode::isBaselineCode(jit.baselineCodeBlock()->jitType()));
     jit.storePtr(AssemblyHelpers::TrustedImmPtr(jit.baselineCodeBlock()), AssemblyHelpers::addressFor(CallFrameSlot::codeBlock));
 
-    GPRReg returnPCReg = GPRInfo::regT5;
-#if CPU(ARM64E)
-    GPRReg signingTagReg = GPRInfo::regT2;
-    if (!Options::allowNonSPTagging()) {
-        returnPCReg = ARM64Registers::lr;
-        signingTagReg = MacroAssembler::stackPointerRegister;
-        // We could save/restore lr here but we don't need to because the LLInt/Baseline will load it from the stack before returning anyway.
-    }
-#endif
-
     const CodeOrigin* codeOrigin;
     for (codeOrigin = &exit.m_codeOrigin; codeOrigin && codeOrigin->inlineCallFrame(); codeOrigin = codeOrigin->inlineCallFrame()->getCallerSkippingTailCalls()) {
         InlineCallFrame* inlineCallFrame = codeOrigin->inlineCallFrame();
@@ -274,25 +264,15 @@ void reifyInlinedCallFrames(CCallHelpers& jit, const OSRExitBase& exit)
 
         if (!trueCaller) {
             ASSERT(inlineCallFrame->isTail());
-            jit.loadPtr(AssemblyHelpers::Address(GPRInfo::callFrameRegister, CallFrame::returnPCOffset()), returnPCReg);
+            jit.loadPtr(AssemblyHelpers::Address(GPRInfo::callFrameRegister, CallFrame::returnPCOffset()), GPRInfo::regT3);
 #if CPU(ARM64E)
-            if (!Options::allowNonSPTagging()) {
-                JIT_COMMENT(jit, "lldb dynamic execution / posix signals could trash your stack"); // We don't have to worry about signals because they shouldn't fire in WebContent process in this window.
-                jit.move(MacroAssembler::stackPointerRegister, GPRInfo::regT4);
-            }
-
             jit.addPtr(AssemblyHelpers::TrustedImm32(sizeof(CallerFrameAndPC)), GPRInfo::callFrameRegister, GPRInfo::regT2);
-            jit.untagPtr(GPRInfo::regT2, returnPCReg);
-            jit.validateUntaggedPtr(returnPCReg, GPRInfo::regT2);
-            jit.addPtr(AssemblyHelpers::TrustedImm32(inlineCallFrame->returnPCOffset() + sizeof(CPURegister)), GPRInfo::callFrameRegister, signingTagReg);
-            jit.tagPtr(signingTagReg, returnPCReg);
-
-            if (!Options::allowNonSPTagging()) {
-                JIT_COMMENT(jit, "lldb dynamic execution / posix signals are ok again");
-                jit.move(GPRInfo::regT4, MacroAssembler::stackPointerRegister);
-            }
+            jit.untagPtr(GPRInfo::regT2, GPRInfo::regT3);
+            jit.addPtr(AssemblyHelpers::TrustedImm32(inlineCallFrame->returnPCOffset() + sizeof(void*)), GPRInfo::callFrameRegister, GPRInfo::regT2);
+            jit.validateUntaggedPtr(GPRInfo::regT3, GPRInfo::regT4);
+            jit.tagPtr(GPRInfo::regT2, GPRInfo::regT3);
 #endif
-            jit.storePtr(returnPCReg, AssemblyHelpers::addressForByteOffset(inlineCallFrame->returnPCOffset()));
+            jit.storePtr(GPRInfo::regT3, AssemblyHelpers::addressForByteOffset(inlineCallFrame->returnPCOffset()));
             jit.loadPtr(AssemblyHelpers::Address(GPRInfo::callFrameRegister, CallFrame::callerFrameOffset()), GPRInfo::regT3);
             callerFrameGPR = GPRInfo::regT3;
         } else {
@@ -309,20 +289,10 @@ void reifyInlinedCallFrames(CCallHelpers& jit, const OSRExitBase& exit)
             }
 
 #if CPU(ARM64E)
-            if (!Options::allowNonSPTagging()) {
-                JIT_COMMENT(jit, "lldb dynamic execution / posix signals could trash your stack"); // We don't have to worry about signals because they shouldn't fire in WebContent process in this window.
-                jit.move(MacroAssembler::stackPointerRegister, GPRInfo::regT4);
-            }
-
-            jit.addPtr(AssemblyHelpers::TrustedImm32(inlineCallFrame->returnPCOffset() + sizeof(CPURegister)), GPRInfo::callFrameRegister, signingTagReg);
-            jit.move(AssemblyHelpers::TrustedImmPtr(jumpTarget.untaggedPtr()), returnPCReg);
-            jit.tagPtr(signingTagReg, returnPCReg);
-            jit.storePtr(returnPCReg, AssemblyHelpers::addressForByteOffset(inlineCallFrame->returnPCOffset()));
-
-            if (!Options::allowNonSPTagging()) {
-                JIT_COMMENT(jit, "lldb dynamic execution / posix signals are ok again");
-                jit.move(GPRInfo::regT4, MacroAssembler::stackPointerRegister);
-            }
+            jit.addPtr(AssemblyHelpers::TrustedImm32(inlineCallFrame->returnPCOffset() + sizeof(void*)), GPRInfo::callFrameRegister, GPRInfo::regT2);
+            jit.move(AssemblyHelpers::TrustedImmPtr(jumpTarget.untaggedPtr()), GPRInfo::regT4);
+            jit.tagPtr(GPRInfo::regT2, GPRInfo::regT4);
+            jit.storePtr(GPRInfo::regT4, AssemblyHelpers::addressForByteOffset(inlineCallFrame->returnPCOffset()));
 #else
             jit.storePtr(AssemblyHelpers::TrustedImmPtr(jumpTarget.untaggedPtr()), AssemblyHelpers::addressForByteOffset(inlineCallFrame->returnPCOffset()));
 #endif

diff --git a/Source/JavaScriptCore/jit/ThunkGenerators.cpp b/Source/JavaScriptCore/jit/ThunkGenerators.cpp
@@ -626,9 +626,9 @@ MacroAssemblerCodeRef<JITThunkPtrTag> arityFixupGenerator(VM& vm)
     jit.addPtr(JSInterfaceJIT::TrustedImm32(sizeof(CallerFrameAndPC)), GPRInfo::callFrameRegister, extraTemp);
     jit.untagPtr(extraTemp, GPRInfo::regT3);
     jit.validateUntaggedPtr(GPRInfo::regT3, extraTemp);
-    // We don't want to pass a tag register to pacib because it could get hijacked to make a PAC bypass gadget so we use pacizb instead.
-    static_assert(NoPtrTag == static_cast<PtrTag>(0));
-    jit.tagPtr(NoPtrTag, GPRInfo::regT3);
+    PtrTag tempReturnPCTag = static_cast<PtrTag>(random());
+    jit.move(JSInterfaceJIT::TrustedImmPtr(tempReturnPCTag), extraTemp);
+    jit.tagPtr(extraTemp, GPRInfo::regT3);
     jit.storePtr(GPRInfo::regT3, JSInterfaceJIT::Address(GPRInfo::callFrameRegister, CallFrame::returnPCOffset()));
 #endif
     jit.move(JSInterfaceJIT::callFrameRegister, JSInterfaceJIT::regT3);
@@ -697,35 +697,18 @@ MacroAssemblerCodeRef<JITThunkPtrTag> arityFixupGenerator(VM& vm)
 
 #if CPU(ARM64E)
     jit.loadPtr(JSInterfaceJIT::Address(GPRInfo::callFrameRegister, CallFrame::returnPCOffset()), GPRInfo::regT3);
-    if (Options::allowNonSPTagging()) {
-        static_assert(NoPtrTag == static_cast<PtrTag>(0));
-        jit.untagPtr(NoPtrTag, GPRInfo::regT3);
-        jit.validateUntaggedPtr(GPRInfo::regT3, extraTemp);
-        jit.addPtr(JSInterfaceJIT::TrustedImm32(sizeof(CallerFrameAndPC)), GPRInfo::callFrameRegister, extraTemp);
-        jit.tagPtr(extraTemp, GPRInfo::regT3);
-        jit.storePtr(GPRInfo::regT3, JSInterfaceJIT::Address(GPRInfo::callFrameRegister, CallFrame::returnPCOffset()));
-    } else {
-        jit.move(ARM64Registers::lr, JSInterfaceJIT::argumentGPR0);
-        jit.move(ARM64Registers::sp, JSInterfaceJIT::argumentGPR1);
-        JIT_COMMENT(jit, "lldb dynamic execution / posix signals could trash your stack"); // We don't have to worry about signals because they shouldn't fire in WebContent process in this window.
-        jit.addPtr(JSInterfaceJIT::TrustedImm32(sizeof(CallerFrameAndPC)), GPRInfo::callFrameRegister, ARM64Registers::sp);
-        jit.move(GPRInfo::regT3, ARM64Registers::lr);
-        static_assert(NoPtrTag == static_cast<PtrTag>(0));
-        jit.untagPtr(NoPtrTag, ARM64Registers::lr);
-        jit.validateUntaggedPtr(ARM64Registers::lr, extraTemp);
-        jit.tagReturnAddress();
-        jit.storePtr(ARM64Registers::lr, JSInterfaceJIT::Address(GPRInfo::callFrameRegister, CallFrame::returnPCOffset()));
-        JIT_COMMENT(jit, "lldb dynamic execution / posix signals are ok again");
-        jit.move(JSInterfaceJIT::argumentGPR1, ARM64Registers::sp);
-        jit.move(JSInterfaceJIT::argumentGPR0, ARM64Registers::lr);
-    }
+    jit.move(JSInterfaceJIT::TrustedImmPtr(tempReturnPCTag), extraTemp);
+    jit.untagPtr(extraTemp, GPRInfo::regT3);
+    jit.validateUntaggedPtr(GPRInfo::regT3, extraTemp);
+    jit.addPtr(JSInterfaceJIT::TrustedImm32(sizeof(CallerFrameAndPC)), GPRInfo::callFrameRegister, extraTemp);
+    jit.tagPtr(extraTemp, GPRInfo::regT3);
+    jit.storePtr(GPRInfo::regT3, JSInterfaceJIT::Address(GPRInfo::callFrameRegister, CallFrame::returnPCOffset()));
 #endif
 
 #if CPU(X86_64)
     jit.push(JSInterfaceJIT::regT4);
 #endif
     jit.ret();
-
 #else // USE(JSVALUE64) section above, USE(JSVALUE32_64) section below.
     jit.move(JSInterfaceJIT::callFrameRegister, JSInterfaceJIT::regT3);
     jit.load32(JSInterfaceJIT::addressFor(CallFrameSlot::argumentCountIncludingThis), JSInterfaceJIT::argumentGPR2);

diff --git a/Source/JavaScriptCore/llint/LLIntThunks.cpp b/Source/JavaScriptCore/llint/LLIntThunks.cpp
@@ -668,22 +668,9 @@ MacroAssemblerCodeRef<NativeToJITGatePtrTag> tagGateThunk(void* pointer)
 {
     CCallHelpers jit;
 
-    GPRReg signingTagReg = GPRInfo::regT3;
-    if (!Options::allowNonSPTagging()) {
-        JIT_COMMENT(jit, "lldb dynamic execution / posix signals could trash your stack"); // We don't have to worry about signals because they shouldn't fire in WebContent process in this window.
-        jit.move(MacroAssembler::stackPointerRegister, GPRInfo::regT3);
-        signingTagReg = MacroAssembler::stackPointerRegister;
-    }
-
-    jit.addPtr(CCallHelpers::TrustedImm32(sizeof(CallerFrameAndPC)), GPRInfo::callFrameRegister, signingTagReg);
-    jit.tagPtr(signingTagReg, ARM64Registers::lr);
-    jit.storePtr(ARM64Registers::lr, CCallHelpers::Address(GPRInfo::callFrameRegister, sizeof(CPURegister)));
-
-    if (!Options::allowNonSPTagging()) {
-        JIT_COMMENT(jit, "lldb dynamic execution / posix signals are ok again");
-        jit.move(GPRInfo::regT3, MacroAssembler::stackPointerRegister);
-    }
-
+    jit.addPtr(CCallHelpers::TrustedImm32(16), GPRInfo::callFrameRegister, GPRInfo::regT3);
+    jit.tagPtr(GPRInfo::regT3, ARM64Registers::lr);
+    jit.storePtr(ARM64Registers::lr, CCallHelpers::Address(GPRInfo::callFrameRegister, 8));
     jit.move(CCallHelpers::TrustedImmPtr(pointer), GPRInfo::regT3);
     jit.farJump(GPRInfo::regT3, OperationPtrTag);
 
@@ -695,8 +682,8 @@ MacroAssemblerCodeRef<NativeToJITGatePtrTag> untagGateThunk(void* pointer)
 {
     CCallHelpers jit;
 
-    jit.loadPtr(CCallHelpers::Address(GPRInfo::callFrameRegister, sizeof(CPURegister)), ARM64Registers::lr);
-    jit.addPtr(CCallHelpers::TrustedImm32(sizeof(CallerFrameAndPC)), GPRInfo::callFrameRegister, GPRInfo::regT3);
+    jit.loadPtr(CCallHelpers::Address(GPRInfo::callFrameRegister, 8), ARM64Registers::lr);
+    jit.addPtr(CCallHelpers::TrustedImm32(16), GPRInfo::callFrameRegister, GPRInfo::regT3);
     jit.untagPtr(GPRInfo::regT3, ARM64Registers::lr);
     jit.validateUntaggedPtr(ARM64Registers::lr, GPRInfo::regT3);
     jit.move(CCallHelpers::TrustedImmPtr(pointer), GPRInfo::regT3);

diff --git a/Source/JavaScriptCore/runtime/Options.cpp b/Source/JavaScriptCore/runtime/Options.cpp
@@ -898,11 +898,6 @@ void Options::notifyOptionsChanged()
     }
 #endif
 
-    // We can't use our pacibsp system while using posix signals because the signal handler could trash our stack during reifyInlinedCallFrames.
-    // If we have JITCage we don't need to restrict ourselves to pacibsp.
-    if (!Options::useMachForExceptions() || Options::useJITCage())
-        Options::allowNonSPTagging() = true;
-
     if (!Options::useWasmFaultSignalHandler())
         Options::useWebAssemblyFastMemory() = false;
 
@@ -1273,11 +1268,6 @@ void Options::assertOptionsAreCoherent()
         coherent = false;
         dataLogLn("Bytecode profiler is not concurrent JIT safe.");
     }
-    if (!allowNonSPTagging() && !useMachForExceptions()) {
-        coherent = false;
-        dataLog("INCOHERENT OPTIONS: can't restrict pointer tagging to pacibsp and use posix signals");
-    }
-
     if (!coherent)
         CRASH();
 }

diff --git a/Source/JavaScriptCore/runtime/OptionsList.h b/Source/JavaScriptCore/runtime/OptionsList.h
@@ -476,7 +476,6 @@ bool hasCapacityToUseLargeGigacage();
     v(Bool, usePollingTraps, false, Normal, "use polling (instead of signalling) VM traps") \
     \
     v(Bool, useMachForExceptions, true, Normal, "Use mach exceptions rather than signals to handle faults and pass thread messages. (This does nothing on platforms without mach)") \
-    v(Bool, allowNonSPTagging, true, Normal, "allow use of the pacib instruction instead of just pacibsp (This can break lldb/posix signals as it puts live data below SP)") \
     \
     v(Bool, useICStats, false, Normal, nullptr) \
     \

diff --git a/Source/WTF/wtf/PtrTag.h b/Source/WTF/wtf/PtrTag.h
@@ -50,7 +50,7 @@ enum class PACKeyType {
     FOR_EACH_ADDITIONAL_WTF_PTRTAG(v) \
 
 enum PtrTag : uintptr_t {
-    NoPtrTag = 0, // Note: We use the 0 tag for temporarily holding the return PC during JSC's arity fixup.
+    NoPtrTag,
     CFunctionPtrTag,
 };
 

diff --git a/Source/WebKit/WebProcess/WebProcess.cpp b/Source/WebKit/WebProcess/WebProcess.cpp
@@ -362,12 +362,6 @@ void WebProcess::initializeProcess(const AuxiliaryProcessInitializationParameter
 {
     WTF::setProcessPrivileges({ });
 
-    {
-        JSC::Options::AllowUnfinalizedAccessScope scope;
-        JSC::Options::allowNonSPTagging() = false;
-        JSC::Options::notifyOptionsChanged();
-    }
-
     MessagePortChannelProvider::setSharedProvider(WebMessagePortChannelProvider::singleton());
 
     platformInitializeProcess(parameters);

diff --git a/Tools/Scripts/run-jsc-stress-tests b/Tools/Scripts/run-jsc-stress-tests
@@ -1130,7 +1130,6 @@ BASE_MODES = [
         "misc-ftl-no-cjit",
         [
             "--useDataICInFTL=true",
-            "--allowNonSPTagging=false",
         ] +
         FTL_OPTIONS +
         NO_CJIT_OPTIONS
@@ -1158,7 +1157,6 @@ BASE_MODES = [
             "--airForceIRCAllocator=true",
             "--useDataICInFTL=true",
             "--forceUnlinkedDFG=true",
-            "--allowNonSPTagging=false",
         ] +
         FTL_OPTIONS +
         NO_CJIT_OPTIONS
@@ -1220,7 +1218,6 @@ BASE_MODES = [
             "--useRandomizingExecutableIslandAllocation=true",
             "--forcePolyProto=true",
             "--useDataICInFTL=true",
-            "--allowNonSPTagging=false",
         ] +
         FTL_OPTIONS +
         EAGER_OPTIONS +
@@ -1284,7 +1281,6 @@ BASE_MODES = [
         "ftl-no-cjit-no-access-inlining",
         [
             "--useAccessInlining=false",
-            "--allowNonSPTagging=false",
         ] +
         FTL_OPTIONS +
         NO_CJIT_OPTIONS