Merge pull request #1571 from justinmichaud/eng/stack-scanning-backport

Exclude non-user portions of the main thread stack from stack scanning on 32 bits.

Author: magomez

Source/JavaScriptCore/jsc.cpp

@@ -3869,6 +3869,7 @@ int jscmain(int argc, char** argv)
     // Need to override and enable restricted options before we start parsing options below.
     Config::enableRestrictedOptions();
 
+    WTF::StackBounds::setBottomOfMainThreadMain(__builtin_frame_address(0));
     WTF::initializeMainThread();
 
     // Note that the options parsing can affect VM creation, and thus

Source/JavaScriptCore/runtime/JSLock.cpp

@@ -169,6 +169,10 @@ void JSLock::didAcquireLock()
             samplingProfiler->noticeJSLockAcquisition();
     }
 #endif
+#if ASSERT_ENABLED
+    StackBounds::currentThreadStackBounds(); // Check stack bounds consistency.
+#endif
+
 }
 
 void JSLock::unlock()

Source/WTF/wtf/StackBounds.cpp

@@ -21,6 +21,8 @@
 #include "config.h"
 #include <wtf/StackBounds.h>
 
+#include "MainThread.h"
+
 #if OS(DARWIN)
 
 #include <pthread.h>
@@ -46,6 +48,18 @@
 
 namespace WTF {
 
+#if !CPU(ADDRESS64)
+static std::atomic<void*> bottomOfMainThreadMain = nullptr;
+#endif
+
+void StackBounds::setBottomOfMainThreadMain([[maybe_unused]] void* stack)
+{
+#if !CPU(ADDRESS64)
+    RELEASE_ASSERT(bottomOfMainThreadMain == nullptr);
+    bottomOfMainThreadMain = stack;
+#endif
+}
+
 #if OS(DARWIN)
 
 StackBounds StackBounds::newThreadStackBounds(PlatformThreadHandle thread)
@@ -135,15 +149,23 @@ StackBounds StackBounds::currentThreadStackBoundsInternal()
 
         static char** oldestEnviron = environ;
 
-        // In 32bit architecture, it is possible that environment variables are having a characters which looks like a pointer,
-        // and conservative GC will find it as a live pointer. We would like to avoid that to precisely exclude non user stack
-        // data region from this stack bounds. As the article (https://lwn.net/Articles/631631/) and the elf loader implementation
-        // explain how Linux main thread stack is organized, environment variables vector is placed on the stack, so we can exclude
-        // environment variables if we use `environ` global variable as a origin of the stack.
+        // On 32bit, it is possible that environment variables and other random data in libc have bytes which look like a pointer,
+        // and conservative GC will find it as a live pointer. We would like to avoid that, so we exclude the non-user stack
+        // data region from these stack bounds. This article explains how the main thread looks (https://lwn.net/Articles/631631/).
+        //
+        // First, we exclude environment variables by using `environ` global variable as a origin of the stack.
         // But `setenv` / `putenv` may alter `environ` variable's content. So we record the oldest `environ` variable content, and use it.
+
         StackBounds stackBounds { origin, bound };
         if (stackBounds.contains(oldestEnviron))
             stackBounds = { oldestEnviron, bound };
+
+        // Then, we exclude any greater region based on manual calls to setBottomOfMainThreadMain.
+#if !CPU(ADDRESS64)
+        if (stackBounds.contains(bottomOfMainThreadMain))
+            stackBounds = { bottomOfMainThreadMain, bound };
+#endif
+
         return stackBounds;
     }
 #endif

Source/WTF/wtf/StackBounds.h

@@ -63,6 +63,7 @@ class StackBounds {
         result.checkConsistency();
         return result;
     }
+    WTF_EXPORT_PRIVATE static void setBottomOfMainThreadMain(void*);
 
     void* origin() const
     {
@@ -142,10 +143,10 @@ class StackBounds {
 
     void checkConsistency() const
     {
-#if ASSERT_ENABLED
+#if ASSERT_ENABLED || OS(LINUX)
         void* currentPosition = currentStackPointer();
-        ASSERT(m_origin != m_bound);
-        ASSERT(currentPosition < m_origin && currentPosition > m_bound);
+        RELEASE_ASSERT(m_origin != m_bound);
+        RELEASE_ASSERT(currentPosition < m_origin && currentPosition > m_bound);
 #endif
     }
 

Source/WebKit/WebProcess/gtk/WebProcessMainGtk.cpp

@@ -31,6 +31,7 @@
 #include "WebProcess.h"
 #include <WebCore/GtkVersioning.h>
 #include <libintl.h>
+#include <wtf/StackBounds.h>
 
 #if PLATFORM(X11)
 #include <X11/Xlib.h>
@@ -95,6 +96,7 @@ int WebProcessMain(int argc, char** argv)
     // This call needs to happen before any threads begin execution
     unsetenv("GTK_THEME");
 
+    WTF::StackBounds::setBottomOfMainThreadMain(__builtin_frame_address(0));
     return AuxiliaryProcessMain<WebProcessMainGtk>(argc, argv);
 }
 

Source/WebKit/WebProcess/wpe/WebProcessMainWPE.cpp

@@ -30,6 +30,7 @@
 #include "AuxiliaryProcessMain.h"
 #include "WebProcess.h"
 #include <glib.h>
+#include <wtf/StackBounds.h>
 
 #if USE(GCRYPT)
 #include <pal/crypto/gcrypt/Initialization.h>
@@ -72,6 +73,7 @@ class WebProcessMainWPE final : public AuxiliaryProcessMainBase<WebProcess> {
 
 int WebProcessMain(int argc, char** argv)
 {
+    WTF::StackBounds::setBottomOfMainThreadMain(__builtin_frame_address(0));
     return AuxiliaryProcessMain<WebProcessMainWPE>(argc, argv);
 }