Modified: trunk/Source/_javascript_Core/ChangeLog (276694 => 276695)
--- trunk/Source/_javascript_Core/ChangeLog 2021-04-28 07:07:02 UTC (rev 276694)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-04-28 07:36:48 UTC (rev 276695)
@@ -1,3 +1,22 @@
+2021-04-28 Daniel Kolesa <[email protected]>
+
+ [WPE][GTK] More correct fixes for stack size issues on musl libc
+ https://bugs.webkit.org/show_bug.cgi?id=225099
+
+ Reviewed by Adrian Perez de Castro.
+
+ Partial revert https://bugs.webkit.org/show_bug.cgi?id=210068
+
+ After fixing the thread stack issues in WTF properly, we can revert
+ the JSC options changes, which are actually harmful since they result
+ in JSC being unstable. Previously, softReservedZoneSize was causing a
+ crash when set to 128K because of the main thread stack bounds, and
+ this is now fixed. We can keep the maxPerThreadStackUsage at 5M as
+ well; there is no fundamental difference from how things are done on
+ glibc anymore.
+
+ * runtime/OptionsList.h:
+
2021-04-27 Filip Pizlo <[email protected]>
Get the bytecode profiler working again
Modified: trunk/Source/_javascript_Core/runtime/OptionsList.h (276694 => 276695)
--- trunk/Source/_javascript_Core/runtime/OptionsList.h 2021-04-28 07:07:02 UTC (rev 276694)
+++ trunk/Source/_javascript_Core/runtime/OptionsList.h 2021-04-28 07:36:48 UTC (rev 276695)
@@ -71,18 +71,6 @@
// On instantiation of the first VM instance, the Options will be write protected
// and cannot be modified thereafter.
-#if OS(LINUX) && !defined(__BIONIC__) && !defined(__GLIBC__)
-// non-glibc/non-android options on linux ( musl )
-constexpr unsigned jscMaxPerThreadStack = 128 * KB;
-constexpr unsigned jscSoftReservedZoneSize = 32 * KB;
-constexpr unsigned jscReservedZoneSize = 16 * KB;
-#else
-// default
-constexpr unsigned jscMaxPerThreadStack = 5 * MB;
-constexpr unsigned jscSoftReservedZoneSize = 128 * KB;
-constexpr unsigned jscReservedZoneSize = 64 * KB;
-#endif
-
#define FOR_EACH_JSC_OPTION(v) \
v(Bool, useKernTCSM, defaultTCSMValue(), Normal, "Note: this needs to go before other options since they depend on this value.") \
v(Bool, validateOptions, false, Normal, "crashes if mis-typed JSC options were passed to the VM") \
@@ -98,9 +86,9 @@
\
v(Bool, reportMustSucceedExecutableAllocations, false, Normal, nullptr) \
\
- v(Unsigned, maxPerThreadStackUsage, jscMaxPerThreadStack, Normal, "Max allowed stack usage by the VM") \
- v(Unsigned, softReservedZoneSize, jscSoftReservedZoneSize, Normal, "A buffer greater than reservedZoneSize that reserves space for stringifying exceptions.") \
- v(Unsigned, reservedZoneSize, jscReservedZoneSize, Normal, "The amount of stack space we guarantee to our clients (and to interal VM code that does not call out to clients).") \
+ v(Unsigned, maxPerThreadStackUsage, 5 * MB, Normal, "Max allowed stack usage by the VM") \
+ v(Unsigned, softReservedZoneSize, 128 * KB, Normal, "A buffer greater than reservedZoneSize that reserves space for stringifying exceptions.") \
+ v(Unsigned, reservedZoneSize, 64 * KB, Normal, "The amount of stack space we guarantee to our clients (and to interal VM code that does not call out to clients).") \
\
v(Bool, crashOnDisallowedVMEntry, ASSERT_ENABLED, Normal, "Forces a crash if we attempt to enter the VM when disallowed") \
v(Bool, crashIfCantAllocateJITMemory, false, Normal, nullptr) \
Modified: trunk/Source/WTF/ChangeLog (276694 => 276695)
--- trunk/Source/WTF/ChangeLog 2021-04-28 07:07:02 UTC (rev 276694)
+++ trunk/Source/WTF/ChangeLog 2021-04-28 07:36:48 UTC (rev 276695)
@@ -1,3 +1,41 @@
+2021-04-28 Daniel Kolesa <[email protected]>
+
+ [WPE][GTK] More correct fixes for stack size issues on musl libc
+ https://bugs.webkit.org/show_bug.cgi?id=225099
+
+ Reviewed by Adrian Perez de Castro.
+
+ Partial revert https://bugs.webkit.org/show_bug.cgi?id=210068
+
+ While the changes in r236306 stopped JSC from crashing outright,
+ they are not correct, since they also make it rather unstable.
+
+ To counter this, increase stack size for threads on Linux with
+ non-glibc/bionic libcs to 1 megabyte, which is a robust enough
+ value that should always be sufficient.
+
+ While at it, the previous approach to musl thread stack size was
+ breaking use of DEFAULT_THREAD_STACK_SIZE_IN_KB (if defined) as
+ well as not properly taking care of the unused parameter. Move
+ the code to a more appropriate place, which solves these problems.
+
+ All this is however not enough, since there is still the main thread;
+ using pthread_attr_getstack on a main thread is not reliable since main
+ thread stacks are allowed to grow, and we expect the bounds to always
+ be constant. On glibc, this already behaved right, but e.g. on musl
+ (and possibly other C libraries) this is not necessarily the case - at
+ the point of the check, it was returning 128k (since that's the initial
+ size reserved by the kernel). Therefore, do the same thing as on Darwin
+ and use process resource limits to get the boundary on Linux as well.
+
+ This results in _javascript_Core behaving correctly on musl libc and
+ allows us to drop the options special-casing that was in place.
+
+ * wtf/StackBounds.cpp:
+ (WTF::StackBounds::currentThreadStackBoundsInternal):
+ * wtf/Threading.cpp:
+ (WTF::stackSize):
+
2021-04-27 Kimmo Kinnunen <[email protected]>
Add a Condition type that supports thread safety analysis
Modified: trunk/Source/WTF/wtf/StackBounds.cpp (276694 => 276695)
--- trunk/Source/WTF/wtf/StackBounds.cpp 2021-04-28 07:07:02 UTC (rev 276694)
+++ trunk/Source/WTF/wtf/StackBounds.cpp 2021-04-28 07:36:48 UTC (rev 276695)
@@ -36,8 +36,14 @@
#include <pthread_np.h>
#endif
+#if OS(LINUX)
+#include <sys/resource.h>
+#include <sys/syscall.h>
+#include <unistd.h>
#endif
+#endif
+
namespace WTF {
#if OS(DARWIN)
@@ -107,7 +113,25 @@
StackBounds StackBounds::currentThreadStackBoundsInternal()
{
- return newThreadStackBounds(pthread_self());
+ auto ret = newThreadStackBounds(pthread_self());
+#if OS(LINUX)
+ // on glibc, pthread_attr_getstack will generally return the limit size (minus a guard page)
+ // for the main thread; this is however not necessarily always true on every libc - for example
+ // on musl, it will return the currently reserved size - since the stack bounds are expected to
+ // be constant (and they are for every thread except main, which is allowed to grow), check
+ // resource limits and use that as the boundary instead (and prevent stack overflows in JSC)
+ if (getpid() == static_cast<pid_t>(syscall(SYS_gettid))) {
+ void* origin = ret.origin();
+ rlimit limit;
+ getrlimit(RLIMIT_STACK, &limit);
+ rlim_t size = limit.rlim_cur;
+ // account for a guard page
+ size -= static_cast<rlim_t>(sysconf(_SC_PAGESIZE));
+ void* bound = static_cast<char*>(origin) - size;
+ return StackBounds { origin, bound };
+ }
+#endif
+ return ret;
}
#elif OS(WINDOWS)
Modified: trunk/Source/WTF/wtf/Threading.cpp (276694 => 276695)
--- trunk/Source/WTF/wtf/Threading.cpp 2021-04-28 07:07:02 UTC (rev 276694)
+++ trunk/Source/WTF/wtf/Threading.cpp 2021-04-28 07:36:48 UTC (rev 276695)
@@ -52,8 +52,6 @@
#elif OS(DARWIN) && ASAN_ENABLED
if (threadType == ThreadType::Compiler)
return 1 * MB; // ASan needs more stack space (especially on Debug builds).
-#elif OS(LINUX) && !defined(__BIONIC__) && !defined(__GLIBC__) // MUSL default thread stack size.
- return 128 * KB;
#else
UNUSED_PARAM(threadType);
#endif
@@ -60,6 +58,10 @@
#if defined(DEFAULT_THREAD_STACK_SIZE_IN_KB) && DEFAULT_THREAD_STACK_SIZE_IN_KB > 0
return DEFAULT_THREAD_STACK_SIZE_IN_KB * 1024;
+#elif OS(LINUX) && !defined(__BIONIC__) && !defined(__GLIBC__)
+ // on libcs other than glibc and bionic (e.g. musl) we are either unsure how big
+ // the default thread stack is, or we know it's too small - pick a robust default
+ return 1 * MB;
#else
// Use the platform's default stack size
return WTF::nullopt;
