Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (288884 => 288885)
--- trunk/Source/_javascript_Core/ChangeLog 2022-02-01 17:18:59 UTC (rev 288884)
+++ trunk/Source/_javascript_Core/ChangeLog 2022-02-01 17:33:58 UTC (rev 288885)
@@ -1,3 +1,47 @@
+2022-02-01 Mark Lam <[email protected]>
+
+ Enhance sanitizeStackForVM() to assist with crash analysis.
+ https://bugs.webkit.org/show_bug.cgi?id=235752
+ rdar://81014601
+
+ Reviewed by Michael Saboff.
+
+ 1. Remove the AssemblyHelpers version of sanitizeStack. Instead, make the 3
+ JIT operation functions call sanitizeStackForVM() instead. This ensures
+ that sanitizeStack crashes are not obscured as generic JIT crashes.
+
+ 2. Add sanity check RELEASE_ASSERTs to VM::setLastStackTop() with a capture of
+ the relevant variables for crash analysis.
+
+ 3. Fix logSanitizeStack() so that it no longer relies on vm.topCallFrame.
+ vm.topCallFrame may not be properly initialized at all the places that
+ sanitizeStackForVM() is called.
+
+ 4. Add a JSLock check to sanitizeStackForVM(), and return early if not owned by
+ the current thread. If the JSLock is not owned by the current thread, we can't
+ rely on vm.lastStackTop() being a sane value. Hence, it's not possible to
+ do stack sanitization correctly.
+
+ Add sanity check RELEASE_ASSERTs to sanitizeStackForVM() with a capture of
+ the relevant variables for crash analysis.
+
+ * jit/AssemblyHelpers.cpp:
+ (JSC::AssemblyHelpers::sanitizeStackInline): Deleted.
+ * jit/AssemblyHelpers.h:
+ * jit/JITOperations.cpp:
+ (JSC::JSC_DEFINE_JIT_OPERATION):
+ * jit/ThunkGenerators.cpp:
+ (JSC::slowPathFor):
+ * runtime/JSLock.cpp:
+ (JSC::JSLock::didAcquireLock):
+ (JSC::JSLock::grabAllLocks):
+ * runtime/VM.cpp:
+ (JSC::VM::VM):
+ (JSC::VM::setLastStackTop):
+ (JSC::logSanitizeStack):
+ (JSC::sanitizeStackForVM):
+ * runtime/VM.h:
+
2022-01-31 Yusuke Suzuki <[email protected]>
Revert OSAllocator behavior to pre-Structure-Allocator change one
Modified: trunk/Source/_javascript_Core/jit/AssemblyHelpers.cpp (288884 => 288885)
--- trunk/Source/_javascript_Core/jit/AssemblyHelpers.cpp 2022-02-01 17:18:59 UTC (rev 288884)
+++ trunk/Source/_javascript_Core/jit/AssemblyHelpers.cpp 2022-02-01 17:33:58 UTC (rev 288885)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011-2021 Apple Inc. All rights reserved.
+ * Copyright (C) 2011-2022 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -1113,19 +1113,6 @@
#endif
}
-void AssemblyHelpers::sanitizeStackInline(VM& vm, GPRReg scratch)
-{
- loadPtr(vm.addressOfLastStackTop(), scratch);
- Jump done = branchPtr(BelowOrEqual, stackPointerRegister, scratch);
- Label loop = label();
- storePtr(TrustedImmPtr(nullptr), Address(scratch));
- addPtr(TrustedImmPtr(sizeof(void*)), scratch);
- branchPtr(Above, stackPointerRegister, scratch).linkTo(loop, this);
- done.link(this);
- move(stackPointerRegister, scratch);
- storePtr(scratch, vm.addressOfLastStackTop());
-}
-
void AssemblyHelpers::cageWithoutUntagging(Gigacage::Kind kind, GPRReg storage)
{
#if GIGACAGE_ENABLED
Modified: trunk/Source/_javascript_Core/jit/AssemblyHelpers.h (288884 => 288885)
--- trunk/Source/_javascript_Core/jit/AssemblyHelpers.h 2022-02-01 17:18:59 UTC (rev 288884)
+++ trunk/Source/_javascript_Core/jit/AssemblyHelpers.h 2022-02-01 17:33:58 UTC (rev 288885)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011-2021 Apple Inc. All rights reserved.
+ * Copyright (C) 2011-2022 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -1710,8 +1710,6 @@
return branchTest8(Zero, AbsoluteAddress(vm.heap.addressOfMutatorShouldBeFenced()));
}
- void sanitizeStackInline(VM&, GPRReg scratch);
-
// Emits the branch structure for typeof. The code emitted by this doesn't fall through. The
// functor is called at those points where we have pinpointed a type. One way to use this is to
// have the functor emit the code to put the type string into an appropriate register and then
Modified: trunk/Source/_javascript_Core/jit/JITOperations.cpp (288884 => 288885)
--- trunk/Source/_javascript_Core/jit/JITOperations.cpp 2022-02-01 17:18:59 UTC (rev 288884)
+++ trunk/Source/_javascript_Core/jit/JITOperations.cpp 2022-02-01 17:33:58 UTC (rev 288885)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2013-2021 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2022 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -1448,11 +1448,13 @@
JSC_DEFINE_JIT_OPERATION(operationLinkCall, SlowPathReturnType, (CallFrame* calleeFrame, JSGlobalObject* globalObject, CallLinkInfo* callLinkInfo))
{
+ sanitizeStackForVM(globalObject->vm());
return linkFor(calleeFrame, globalObject, callLinkInfo);
}
JSC_DEFINE_JIT_OPERATION(operationLinkPolymorphicCall, SlowPathReturnType, (CallFrame* calleeFrame, JSGlobalObject* globalObject, CallLinkInfo* callLinkInfo))
{
+ sanitizeStackForVM(globalObject->vm());
ASSERT(callLinkInfo->specializationKind() == CodeForCall);
JSCell* calleeAsFunctionCell;
SlowPathReturnType result = virtualForWithFunction(globalObject, calleeFrame, callLinkInfo, calleeAsFunctionCell);
@@ -1464,6 +1466,7 @@
JSC_DEFINE_JIT_OPERATION(operationVirtualCall, SlowPathReturnType, (CallFrame* calleeFrame, JSGlobalObject* globalObject, CallLinkInfo* callLinkInfo))
{
+ sanitizeStackForVM(globalObject->vm());
JSCell* calleeAsFunctionCellIgnored;
return virtualForWithFunction(globalObject, calleeFrame, callLinkInfo, calleeAsFunctionCellIgnored);
}
Modified: trunk/Source/_javascript_Core/jit/ThunkGenerators.cpp (288884 => 288885)
--- trunk/Source/_javascript_Core/jit/ThunkGenerators.cpp 2022-02-01 17:18:59 UTC (rev 288884)
+++ trunk/Source/_javascript_Core/jit/ThunkGenerators.cpp 2022-02-01 17:33:58 UTC (rev 288885)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2010-2021 Apple Inc. All rights reserved.
+ * Copyright (C) 2010-2022 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -192,7 +192,6 @@
static void slowPathFor(CCallHelpers& jit, VM& vm, Sprt_JITOperation_EGCli slowPathFunction)
{
- jit.sanitizeStackInline(vm, GPRInfo::nonArgGPR0);
jit.emitFunctionPrologue();
jit.storePtr(GPRInfo::callFrameRegister, &vm.topCallFrame);
#if OS(WINDOWS) && CPU(X86_64)
Modified: trunk/Source/_javascript_Core/runtime/JSLock.cpp (288884 => 288885)
--- trunk/Source/_javascript_Core/runtime/JSLock.cpp 2022-02-01 17:18:59 UTC (rev 288884)
+++ trunk/Source/_javascript_Core/runtime/JSLock.cpp 2022-02-01 17:33:58 UTC (rev 288885)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2005-2021 Apple Inc. All rights reserved.
+ * Copyright (C) 2005-2022 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
@@ -138,8 +138,7 @@
m_entryAtomStringTable = thread.setCurrentAtomStringTable(m_vm->atomStringTable());
ASSERT(m_entryAtomStringTable);
- m_vm->setLastStackTop(thread.savedLastStackTop());
- ASSERT(thread.stack().contains(m_vm->lastStackTop()));
+ m_vm->setLastStackTop(thread);
if (m_vm->heap.hasAccess())
m_shouldReleaseHeapAccess = false;
@@ -277,7 +276,7 @@
Thread& thread = Thread::current();
m_vm->setStackPointerAtVMEntry(thread.savedStackPointerAtVMEntry());
- m_vm->setLastStackTop(thread.savedLastStackTop());
+ m_vm->setLastStackTop(thread);
}
JSLock::DropAllLocks::DropAllLocks(VM* vm)
Modified: trunk/Source/_javascript_Core/runtime/VM.cpp (288884 => 288885)
--- trunk/Source/_javascript_Core/runtime/VM.cpp 2022-02-01 17:18:59 UTC (rev 288884)
+++ trunk/Source/_javascript_Core/runtime/VM.cpp 2022-02-01 17:33:58 UTC (rev 288885)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2008-2021 Apple Inc. All rights reserved.
+ * Copyright (C) 2008-2022 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -242,9 +242,8 @@
CRASH_WITH_INFO(0x4242424220202020, 0xbadbeef0badbeef, 0x1234123412341234, 0x1337133713371337);
interpreter = new Interpreter(*this);
- StackBounds stack = Thread::current().stack();
updateSoftReservedZoneSize(Options::softReservedZoneSize());
- setLastStackTop(stack.origin());
+ setLastStackTop(Thread::current());
JSRunLoopTimer::Manager::shared().registerVM(*this);
@@ -503,9 +502,11 @@
m_needToFirePrimitiveGigacageEnabled = true;
}
-void VM::setLastStackTop(void* lastStackTop)
-{
- m_lastStackTop = lastStackTop;
+void VM::setLastStackTop(const Thread& thread)
+{
+ m_lastStackTop = thread.savedLastStackTop();
+ auto& stack = thread.stack();
+ RELEASE_ASSERT(stack.contains(m_lastStackTop), 0x5510, m_lastStackTop, stack.origin(), stack.end());
}
Ref<VM> VM::createContextGroup(HeapType heapType)
@@ -1050,16 +1051,11 @@
m_checkpointSideState.shrinkToFit();
}
-void logSanitizeStack(VM& vm)
+static void logSanitizeStack(VM& vm)
{
- if (Options::verboseSanitizeStack() && vm.topCallFrame) {
- int dummy;
+ if (UNLIKELY(Options::verboseSanitizeStack())) {
auto& stackBounds = Thread::current().stack();
- dataLog(
- "Sanitizing stack for VM = ", RawPointer(&vm), " with top call frame at ", RawPointer(vm.topCallFrame),
- ", current stack pointer at ", RawPointer(&dummy), ", in ",
- pointerDump(vm.topCallFrame->codeBlock()), ", last code origin = ",
- vm.topCallFrame->codeOrigin(), ", last stack top = ", RawPointer(vm.lastStackTop()), ", in stack range [", RawPointer(stackBounds.origin()), ", ", RawPointer(stackBounds.end()), "]\n");
+ dataLogLn("Sanitizing stack for VM = ", RawPointer(&vm), ", current stack pointer at ", RawPointer(currentStackPointer()), ", last stack top = ", RawPointer(vm.lastStackTop()), ", in stack range (", RawPointer(stackBounds.end()), ", ", RawPointer(stackBounds.origin()), "]");
}
}
@@ -1270,17 +1266,20 @@
void sanitizeStackForVM(VM& vm)
{
+ auto& thread = Thread::current();
+ auto& stack = thread.stack();
+ if (!vm.currentThreadIsHoldingAPILock())
+ return; // vm.lastStackTop() may not be set up correctly if JSLock is not held.
+
logSanitizeStack(vm);
- if (vm.topCallFrame) {
- auto& stackBounds = Thread::current().stack();
- ASSERT(vm.currentThreadIsHoldingAPILock());
- ASSERT_UNUSED(stackBounds, stackBounds.contains(vm.lastStackTop()));
- }
+
+ RELEASE_ASSERT(stack.contains(vm.lastStackTop()), 0xaa10, vm.lastStackTop(), stack.origin(), stack.end());
#if ENABLE(C_LOOP)
vm.interpreter->cloopStack().sanitizeStack();
#else
sanitizeStackForVMImpl(&vm);
#endif
+ RELEASE_ASSERT(stack.contains(vm.lastStackTop()), 0xaa20, vm.lastStackTop(), stack.origin(), stack.end());
}
size_t VM::committedStackByteCount()
Modified: trunk/Source/_javascript_Core/runtime/VM.h (288884 => 288885)
--- trunk/Source/_javascript_Core/runtime/VM.h 2022-02-01 17:18:59 UTC (rev 288884)
+++ trunk/Source/_javascript_Core/runtime/VM.h 2022-02-01 17:33:58 UTC (rev 288885)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2008-2021 Apple Inc. All rights reserved.
+ * Copyright (C) 2008-2022 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -774,7 +774,7 @@
void** addressOfLastStackTop() { return &m_lastStackTop; }
void* lastStackTop() { return m_lastStackTop; }
- void setLastStackTop(void*);
+ void setLastStackTop(const Thread&);
void firePrimitiveGigacageEnabledIfNecessary()
{
@@ -1162,6 +1162,5 @@
#endif
JS_EXPORT_PRIVATE void sanitizeStackForVM(VM&);
-void logSanitizeStack(VM&);
} // namespace JSC
Modified: trunk/Source/WTF/ChangeLog (288884 => 288885)
--- trunk/Source/WTF/ChangeLog 2022-02-01 17:18:59 UTC (rev 288884)
+++ trunk/Source/WTF/ChangeLog 2022-02-01 17:33:58 UTC (rev 288885)
@@ -1,3 +1,17 @@
+2022-02-01 Mark Lam <[email protected]>
+
+ Enhance sanitizeStackForVM() to assist with crash analysis.
+ https://bugs.webkit.org/show_bug.cgi?id=235752
+ rdar://81014601
+
+ Reviewed by Michael Saboff.
+
+ * wtf/Threading.h:
+ (WTF::Thread::savedStackPointerAtVMEntry const):
+ (WTF::Thread::savedLastStackTop const):
+ (WTF::Thread::savedStackPointerAtVMEntry): Deleted.
+ (WTF::Thread::savedLastStackTop): Deleted.
+
2022-01-31 Yusuke Suzuki <[email protected]>
Revert OSAllocator behavior to pre-Structure-Allocator change one
Modified: trunk/Source/WTF/wtf/Threading.h (288884 => 288885)
--- trunk/Source/WTF/wtf/Threading.h 2022-02-01 17:18:59 UTC (rev 288884)
+++ trunk/Source/WTF/wtf/Threading.h 2022-02-01 17:33:58 UTC (rev 288885)
@@ -231,7 +231,7 @@
}
#endif
- void* savedStackPointerAtVMEntry()
+ void* savedStackPointerAtVMEntry() const
{
return m_savedStackPointerAtVMEntry;
}
@@ -241,7 +241,7 @@
m_savedStackPointerAtVMEntry = stackPointerAtVMEntry;
}
- void* savedLastStackTop()
+ void* savedLastStackTop() const
{
return m_savedLastStackTop;
}
