WTFReportBacktrace fails to print backtrace and crashes on ASAN

https://bugs.webkit.org/show_bug.cgi?id=245826
rdar://problem/100557350

Reviewed by Mark Lam and Yusuke Suzuki.

WTFReportBacktrace() would use StackTrace in the "borrowed" mode.
The borrowed mode stack pointer would be obtained by reading
uninitialized !m_capacity. This would cause read to random memory.

Fix by removing the StackTrace borrowed mode. When printing
the backtrace, use the new class StackTracePrinter which
is able to use the void* array WTFGetStacktrace returns.

Move the size/isBorrowed members out of the union that was
intended to optimize skipping of the two initial, skipped
frames. This was relying on two counts of UB:
- reading of non-active member of the union:
  Filled the m_stack by writing into one union m_skippedFrame0
  making it active, but then reading from passive union
  member m_size.

- Writing into one member, and expecting the overflowing
  of the write being defined to match the subsequent member:
  Oversized write into union member m_skippedFrame0
  was assumend to spill over to member of other, subsequent union
  member m_stack. This is not well-defined.

The above UB invocations were done to trying to save two
void* worth of memory in the StackTrace, while avoiding
copying of the void* array.

Instead, first read the stack trace into the memory storage
of StackTrace, then overwrite the initial portion with the
real class data.

Fix bugs in the unborrowed mode, where
captureStackTrace(maxFrames, framesToSkip)
would not actually skip the framesToSkip amount of frames.
If left unfixed, we could not assert that the borrowed
mode fix did not accidentally cause regressions to the
unborrowed mode.

Move the printing related StackTrace::m_prefix to
StackTracePrinter::m_prefix. This way StackTrace size shrinks.

Test the equivalent of WTFReportBacktrace in a scenario
that caused the uninitialized read.

Test the captureStackTrace().

(WTF::StackTrace::instanceSize):
(WTF::StackTrace::captureStackTrace):
* Source/WTF/wtf/StackTrace.h:
(WTF::StackTrace::StackTrace):
(WTF::StackTrace::stack const):
* Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* Tools/TestWebKitAPI/Tests/WTF/StackTraceTest.cpp: Added.
(TestWebKitAPI::TEST):
(TestWebKitAPI::expectEqualFrames):

Canonical link: https://commits.webkit.org/255164@main

Source/JavaScriptCore/b3/B3Value.cpp

@@ -62,7 +62,7 @@ String Value::generateCompilerConstructionSite()
     int frames = framesToShow + framesToSkip;
 
     WTFGetBacktrace(samples, &frames);
-    StackTrace stackTrace(samples + framesToSkip, frames - framesToSkip, "");
+    StackTraceSymbolResolver stackTrace({ samples + framesToSkip, static_cast<size_t>(frames - framesToSkip) });
 
     s.print("[");
     bool firstPrinted = false;

Source/JavaScriptCore/heap/VerifierSlotVisitor.cpp

@@ -301,9 +301,7 @@ void VerifierSlotVisitor::dumpMarkerData(HeapCell* cell)
             opaqueRoot = nullptr;
         }
 
-        markerData->stack()->dump(WTF::dataFile(), "    ");
-        dataLogLn();
-
+        dataLogLn(StackTracePrinter { *markerData->stack(), "    " });
     } while (cell || opaqueRoot);
 }
 

Source/JavaScriptCore/inspector/JSGlobalObjectInspectorController.cpp

@@ -163,7 +163,7 @@ void JSGlobalObjectInspectorController::appendAPIBacktrace(ScriptCallStack& call
     void** stack = samples + framesToSkip;
     int size = frames - framesToSkip;
     for (int i = 0; i < size; ++i) {
-        auto demangled = StackTrace::demangle(stack[i]);
+        auto demangled = StackTraceSymbolResolver::demangle(stack[i]);
         if (demangled)
             callStack.append(ScriptCallFrame(String::fromLatin1(demangled->demangledName() ? demangled->demangledName() : demangled->mangledName()), "[native code]"_s, noSourceID, 0, 0));
         else

Source/JavaScriptCore/runtime/ExceptionScope.cpp

@@ -55,13 +55,13 @@ CString ExceptionScope::unexpectedExceptionMessage()
 
     out.println("Unexpected exception observed on thread ", Thread::current(), " at:");
     auto currentStack = StackTrace::captureStackTrace(Options::unexpectedExceptionStackTraceLimit(), 1);
-    currentStack->dump(out, "    ");
+    out.print(StackTracePrinter { *currentStack, "    " });
 
     if (!m_vm.nativeStackTraceOfLastThrow())
         return CString();
 
     out.println("The exception was thrown from thread ", *m_vm.throwingThread(), " at:");
-    m_vm.nativeStackTraceOfLastThrow()->dump(out, "    ");
+    out.print(StackTracePrinter { *m_vm.nativeStackTraceOfLastThrow(), "    " });
 
     return out.toCString();
 }

Source/JavaScriptCore/runtime/SamplingProfiler.cpp

@@ -809,7 +809,7 @@ String SamplingProfiler::StackFrame::displayName(VM& vm)
     case FrameType::C:
 #if HAVE(DLADDR)
         if (frameType == FrameType::C) {
-            auto demangled = WTF::StackTrace::demangle(const_cast<void*>(cCodePC));
+            auto demangled = StackTraceSymbolResolver::demangle(const_cast<void*>(cCodePC));
             if (demangled)
                 return String::fromLatin1(demangled->demangledName() ? demangled->demangledName() : demangled->mangledName());
             WTF::dataLog("couldn't get a name");

Source/JavaScriptCore/runtime/VM.cpp

@@ -1336,12 +1336,10 @@ void VM::verifyExceptionCheckNeedIsSatisfied(unsigned recursionDepth, ExceptionE
 
         if (Options::dumpSimulatedThrows()) {
             out.println("The simulated exception was thrown at:");
-            m_nativeStackTraceOfLastSimulatedThrow->dump(out, "    ");
-            out.println();
+            out.println(StackTracePrinter { *m_nativeStackTraceOfLastSimulatedThrow, "    " });
         }
         out.println("Unchecked exception detected at:");
-        currentTrace->dump(out, "    ");
-        out.println();
+        out.println(StackTracePrinter { *currentTrace, "    " });
 
         dataLog(out.toCString());
         RELEASE_ASSERT(!m_needExceptionCheck);

Source/WTF/wtf/Assertions.cpp

@@ -308,8 +308,7 @@ void WTFReportBacktrace()
 
 void WTFPrintBacktraceWithPrefixAndPrintStream(PrintStream& out, void** stack, int size, const char* prefix)
 {
-    StackTrace stackTrace(stack, size, prefix);
-    out.print(stackTrace);
+    out.print(StackTracePrinter { { stack, static_cast<size_t>(size) }, prefix });
 }
 
 void WTFPrintBacktrace(void** stack, int size)
@@ -599,35 +598,30 @@ void WTFInitializeLogChannelStatesFromString(WTFLogChannel* channels[], size_t c
 #if !RELEASE_LOG_DISABLED
 void WTFReleaseLogStackTrace(WTFLogChannel* channel)
 {
-    auto stackTrace = WTF::StackTrace::captureStackTrace(30, 0);
-    if (stackTrace && stackTrace->stack()) {
-        auto stack = stackTrace->stack();
-        for (int frameNumber = 1; frameNumber < stackTrace->size(); ++frameNumber) {
-            auto stackFrame = stack[frameNumber];
-            auto demangled = WTF::StackTrace::demangle(stackFrame);
+    static constexpr int framesToShow = 32;
+    static constexpr int framesToSkip = 2;
+    void* stack[framesToShow + framesToSkip];
+    int frames = framesToShow + framesToSkip;
+    WTFGetBacktrace(stack, &frames);
+    StackTraceSymbolResolver { { stack, static_cast<size_t>(frames) } }.forEach([&](int frameNumber, void* stackFrame, const char* name) {
 #if USE(OS_LOG)
-            if (demangled && demangled->demangledName())
-                os_log(channel->osLogChannel, "%-3d %p %{public}s", frameNumber, stackFrame, demangled->demangledName());
-            else if (demangled && demangled->mangledName())
-                os_log(channel->osLogChannel, "%-3d %p %{public}s", frameNumber, stackFrame, demangled->mangledName());
-            else
-                os_log(channel->osLogChannel, "%-3d %p", frameNumber, stackFrame);
+        if (name)
+            os_log(channel->osLogChannel, "%-3d %p %{public}s", frameNumber, stackFrame, name);
+        else
+            os_log(channel->osLogChannel, "%-3d %p", frameNumber, stackFrame);
 #else
-            StringPrintStream out;
-            if (demangled && demangled->demangledName())
-                out.printf("%-3d %p %s", frameNumber, stackFrame, demangled->demangledName());
-            else if (demangled && demangled->mangledName())
-                out.printf("%-3d %p %s", frameNumber, stackFrame, demangled->mangledName());
-            else
-                out.printf("%-3d %p", frameNumber, stackFrame);
+        StringPrintStream out;
+        if (name)
+            out.printf("%-3d %p %s", frameNumber, stackFrame, name);
+        else
+            out.printf("%-3d %p", frameNumber, stackFrame);
 #if ENABLE(JOURNALD_LOG)
-            sd_journal_send("WEBKIT_SUBSYSTEM=%s", channel->subsystem, "WEBKIT_CHANNEL=%s", channel->name, "MESSAGE=%s", out.toCString().data(), nullptr);
+        sd_journal_send("WEBKIT_SUBSYSTEM=%s", channel->subsystem, "WEBKIT_CHANNEL=%s", channel->name, "MESSAGE=%s", out.toCString().data(), nullptr);
 #else
-            fprintf(stderr, "[%s:%s:-] %s\n", channel->subsystem, channel->name, out.toCString().data());
+        fprintf(stderr, "[%s:%s:-] %s\n", channel->subsystem, channel->name, out.toCString().data());
 #endif
 #endif
-        }
-    }
+    });
 }
 #endif
 

Source/WTF/wtf/Assertions.h

@@ -228,7 +228,7 @@ WTF_EXPORT_PRIVATE void WTFLogWithLevel(WTFLogChannel*, WTFLogLevel, const char*
 WTF_EXPORT_PRIVATE void WTFSetLogChannelLevel(WTFLogChannel*, WTFLogLevel);
 WTF_EXPORT_PRIVATE bool WTFWillLogWithLevel(WTFLogChannel*, WTFLogLevel);
 
-WTF_EXPORT_PRIVATE void WTFGetBacktrace(void** stack, int* size);
+WTF_EXPORT_PRIVATE NEVER_INLINE void WTFGetBacktrace(void** stack, int* size);
 WTF_EXPORT_PRIVATE void WTFReportBacktraceWithPrefix(const char*);
 WTF_EXPORT_PRIVATE void WTFReportBacktrace(void);
 #ifdef __cplusplus

Source/WTF/wtf/StackCheck.cpp

@@ -48,11 +48,9 @@ NO_RETURN_DUE_TO_CRASH void StackCheck::Scope::reportVerificationFailureAndCrash
     dataLogLn();
     if constexpr (verboseStackCheckVerification) {
         dataLogLn("    Stack at previous checkpoint:");
-        m_savedLastCheckpointStackTrace->dump(WTF::dataFile(), "      ");
-        dataLogLn();
+        dataLogLn(StackTracePrinter { m_savedLastCheckpointStackTrace->stack(), "      " });
         dataLogLn("    Stack at current checkpoint:");
-        m_checker.m_lastCheckpointStackTrace->dump(WTF::dataFile(), "      ");
-        dataLogLn();
+        dataLogLn(StackTracePrinter { m_checker.m_lastCheckpointStackTrace->stack(), "      " });
     } else {
         dataLogLn("    To see the stack traces at the 2 checkpoints, set verboseStackCheckVerification to true in StackCheck.h, rebuild, and re-run your test.");
         dataLogLn();

Source/WTF/wtf/StackShotProfiler.h

@@ -68,8 +68,7 @@ class StackShotProfiler {
                 for (size_t i = list.size(), count = 0; i-- && count < m_stacksToReport; count++) {
                     auto& entry = list[i];
                     dataLog("\nTop #", count + 1, " stack: ", entry.count * 100 / m_totalCount, "%\n");
-                    StackTrace trace(entry.key->array() + m_framesToSkip, entry.key->size() - m_framesToSkip);
-                    dataLog(trace);
+                    dataLog(StackTracePrinter { { entry.key->array() + m_framesToSkip, entry.key->size() - m_framesToSkip } });
                 }
                 dataLog("\n");
             }

Source/WTF/wtf/StackTrace.cpp

@@ -27,6 +27,7 @@
 #include "config.h"
 #include <wtf/StackTrace.h>
 
+#include <type_traits>
 #include <wtf/Assertions.h>
 #include <wtf/PrintStream.h>
 
@@ -44,35 +45,29 @@ void WTFGetBacktrace(void** stack, int* size)
 
 namespace WTF {
 
-ALWAYS_INLINE size_t StackTrace::instanceSize(int capacity)
+std::unique_ptr<StackTrace> StackTrace::captureStackTrace(size_t maxFrames, size_t framesToSkip)
 {
-    ASSERT(capacity >= 1);
-    return sizeof(StackTrace) + (capacity - 1) * sizeof(void*);
-}
-
-std::unique_ptr<StackTrace> StackTrace::captureStackTrace(int maxFrames, int framesToSkip)
-{
-    maxFrames = std::max(1, maxFrames);
-    size_t sizeToAllocate = instanceSize(maxFrames);
-    std::unique_ptr<StackTrace> trace(new (NotNull, fastMalloc(sizeToAllocate)) StackTrace());
+    static_assert(sizeof(StackTrace) == sizeof(void*) * 3);
+    // We overwrite the memory of the two first skipped frames, m_stack[0] will hold the third one.
+    static_assert(offsetof(StackTrace, m_stack) == sizeof(void*) * 2);
 
+    maxFrames = std::max<size_t>(1, maxFrames);
     // Skip 2 additional frames i.e. StackTrace::captureStackTrace and WTFGetBacktrace.
     framesToSkip += 2;
-    int numberOfFrames = maxFrames + framesToSkip;
-
-    WTFGetBacktrace(&trace->m_skippedFrame0, &numberOfFrames);
-    if (numberOfFrames) {
-        RELEASE_ASSERT(numberOfFrames >= framesToSkip);
-        trace->m_size = numberOfFrames - framesToSkip;
-    } else
-        trace->m_size = 0;
-
-    trace->m_capacity = maxFrames;
-
-    return trace;
+    size_t capacity = maxFrames + framesToSkip;
+    void** storage = static_cast<void**>(fastMalloc(capacity * sizeof(void*)));
+    size_t size = 0;
+    size_t initialFrame = 0;
+    int capturedFrames = static_cast<int>(capacity);
+    WTFGetBacktrace(storage, &capturedFrames);
+    if (static_cast<size_t>(capturedFrames) > framesToSkip) {
+        size = static_cast<size_t>(capturedFrames) - framesToSkip;
+        initialFrame = framesToSkip - 2; 
+    }
+    return std::unique_ptr<StackTrace> { new (NotNull, storage) StackTrace(size, initialFrame) };
 }
 
-auto StackTrace::demangle(void* pc) -> std::optional<DemangleEntry>
+auto StackTraceSymbolResolver::demangle(void* pc) -> std::optional<DemangleEntry>
 {
 #if HAVE(DLADDR)
     const char* mangledName = nullptr;
@@ -93,12 +88,10 @@ auto StackTrace::demangle(void* pc) -> std::optional<DemangleEntry>
     return std::nullopt;
 }
 
-void StackTrace::dump(PrintStream& out, const char* indentString) const
+void StackTracePrinter::dump(PrintStream& out) const
 {
-    if (!indentString)
-        indentString = "";
-    forEach([&](int frameNumber, void* stackFrame, const char* name) {
-        out.printf("%s%s%-3d %p %s\n", m_prefix ? m_prefix : "", indentString, frameNumber, stackFrame, name);
+    StackTraceSymbolResolver { m_stack }.forEach([&](int frameNumber, void* stackFrame, const char* name) {
+        out.printf("%s%-3d %p %s\n", m_prefix ? m_prefix : "", frameNumber, stackFrame, name);
     });
 }