From 00f22ddbe62f9ab4cf314b1beb18b7f9a91edae2 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Fri, 15 Jul 2022 11:56:12 +0000 Subject: Issue #1805 - Improve stack size limits for all targets. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows us to use a greater rendering depth for exceedingly-deep DOM trees in layout, better matching what mainstream browsers are capable of. Note that for 32-bit Windows the stack size MUST be set to larger than the default or Bad Things Will Happen™ - we use 1.5 MB for this as a carefully-tuned value. This needs to be capped specifically for JS use because some JavaScript obfuscators deliberately trigger stack overflows and would lock up the browser otherwise as long as there's still stack space to abuse. For web compatibility we therefore limit this to 2MB in JS only (3x for ASAN) while still allowing a greater depth for the layout engine. --- config/config.mk | 16 ++++++- js/xpconnect/src/XPCJSContext.cpp | 96 ++++++++++++++++++++++++++++----------- layout/generic/nsIFrame.h | 18 +++++++- modules/libpref/init/all.js | 14 +++++- 4 files changed, 113 insertions(+), 31 deletions(-) diff --git a/config/config.mk b/config/config.mk index b71e65fc35..2d1ff365b1 100644 --- a/config/config.mk +++ b/config/config.mk @@ -377,8 +377,20 @@ endif # WINNT ifdef _MSC_VER ifeq ($(CPU_ARCH),x86_64) -# set stack to 2MB on x64 build. See bug 582910 -WIN32_EXE_LDFLAGS += -STACK:2097152 +# Normal operation on 64-bit Windows needs 2 MB of stack. (Bug 582910) +# ASAN requires 6 MB of stack. +# Setting the stack to 8 MB to match the capability of other systems +# to deal with frame construction for unreasonably deep DOM trees +# with worst-case styling. This uses address space unnecessarily for +# non-main threads, but that should be tolerable on 64-bit systems. +WIN32_EXE_LDFLAGS += -STACK:8388608 +else +# Since this setting affects the default stack size for non-main +# threads, too, to avoid burning the address space, increase only +# 512 KB over the default. Just enough to be able to deal with +# reasonable styling applied to DOM trees whose depth is near what +# Blink's HTML parser can output. +WIN32_EXE_LDFLAGS += -STACK:1572864 endif endif diff --git a/js/xpconnect/src/XPCJSContext.cpp b/js/xpconnect/src/XPCJSContext.cpp index a02c5e103b..2a32af83bd 100644 --- a/js/xpconnect/src/XPCJSContext.cpp +++ b/js/xpconnect/src/XPCJSContext.cpp @@ -45,6 +45,7 @@ #include "mozilla/jsipc/CrossProcessObjectWrappers.h" #include "mozilla/Atomics.h" #include "mozilla/Attributes.h" +#include "mozilla/Preferences.h" #include "mozilla/ProcessHangMonitor.h" #include "mozilla/Sprintf.h" #include "mozilla/UniquePtrExtensions.h" @@ -58,6 +59,7 @@ #include "nsJSPrincipals.h" #ifdef XP_WIN +#include #include #endif @@ -3205,47 +3207,87 @@ XPCJSContext::Initialize() // on 32-bit platforms and 1MB on 64-bit platforms. const size_t kDefaultStackQuota = 128 * sizeof(size_t) * 1024; - // Set stack sizes for different configurations. It's probably not great for - // the web to base this decision primarily on the default stack size that the - // underlying platform makes available, but that seems to be what we do. :-( + // Set maximum stack size for different configurations. This value is then + // capped below because huge JS stacks are not web-compatible. + + // ASan requires more script buffer space due to red-zones, so give it more. + // We hazard a guess that ASAN builds have roughly thrice the stack + // overhead normal builds have, so we reserve 450k (50 frames @ 9k frame size) #if defined(XP_MACOSX) || defined(DARWIN) // MacOS has a gargantuan default stack size of 8MB. Go wild with 7MB, - // and give trusted script 180k extra. The stack is huge on mac anyway. - const size_t kStackQuota = 7 * 1024 * 1024; + // and give trusted script 180k extra. + const size_t kUncappedStackQuota = 7 * 1024 * 1024; const size_t kTrustedScriptBuffer = 180 * 1024; -#elif defined(MOZ_ASAN) - // ASan requires more stack space due to red-zones, so give it double the - // default (1MB on 32-bit, 2MB on 64-bit). ASAN stack frame measurements - // were not taken at the time of this writing, so we hazard a guess that - // ASAN builds have roughly thrice the stack overhead as normal builds. - // On normal builds, the largest stack frame size we might encounter is - // 9.0k (see above), so let's use a buffer of 9.0 * 5 * 10 = 450k. - const size_t kStackQuota = 2 * kDefaultStackQuota; + +#elif defined(XP_LINUX) || defined(XP_SOLARIS) + // Most Linux distributions set default stack size to 8MB. Use it as the + // maximum value. + // Solaris uses 8 or 10 MB, depending, so this is a safe max there too. + const size_t kStackQuotaMax = 8 * 1024 * 1024; +#if defined(MOZ_ASAN) || defined(DEBUG) + // Bug 803182: account for the 4x difference in the size of js::Interpret + // between optimized and debug builds. We use 2x since the JIT part + // doesn't increase much. + const size_t kStackQuotaMin = 2 * kDefaultStackQuota; +#else + const size_t kStackQuotaMin = kDefaultStackQuota; +#endif // MOZ_ASAN || DEBUG + // Allocate 128kB margin for the safe space. + const size_t kStackSafeMargin = 128 * 1024; + + struct rlimit rlim; + const size_t kUncappedStackQuota = + getrlimit(RLIMIT_STACK, &rlim) == 0 ? + std::max(std::min(size_t(rlim.rlim_cur - kStackSafeMargin), + kStackQuotaMax - kStackSafeMargin), + kStackQuotaMin) : + kStackQuotaMin; +#if defined(MOZ_ASAN) const size_t kTrustedScriptBuffer = 450 * 1024; +#else + const size_t kTrustedScriptBuffer = 180 * 1024; +#endif // MOZ_ASAN + #elif defined(XP_WIN) // 1MB is the default stack size on Windows. We use the /STACK linker flag - // to request a larger stack, so we determine the stack size at runtime. - const size_t kStackQuota = GetWindowsStackSize(); - const size_t kTrustedScriptBuffer = (sizeof(size_t) == 8) ? 180 * 1024 //win64 - : 120 * 1024; //win32 - // The following two configurations are linux-only. Given the numbers above, - // we use 50k and 100k trusted buffers on 32-bit and 64-bit respectively. -#elif defined(DEBUG) - // Bug 803182: account for the 4x difference in the size of js::Interpret - // between optimized and debug builds. - // XXXbholley - Then why do we only account for 2x of difference? - const size_t kStackQuota = 2 * kDefaultStackQuota; - const size_t kTrustedScriptBuffer = sizeof(size_t) * 12800; + // (see WIN32_EXE_LDFLAGS in config/config.mk) to request a larger stack, so + // we determine the stack size at runtime. + const size_t kUncappedStackQuota = GetWindowsStackSize(); +#if defined(MOZ_ASAN) + const size_t kTrustedScriptBuffer = 450 * 1024; #else - const size_t kStackQuota = kDefaultStackQuota; - const size_t kTrustedScriptBuffer = sizeof(size_t) * 12800; + const size_t kTrustedScriptBuffer = (sizeof(size_t) == 8) ? + 180 * 1024 : // win64 + 120 * 1024; // win32 +#endif //MOZ_ASAN + +#else + // We're not on Windows, Linux, Solaris or Mac/Darwin + // Catch-all configuration for other environments. +#if defined(MOZ_ASAN) + const size_t kUncappedStackQuota = 2 * kDefaultStackQuota; + const size_t kTrustedScriptBuffer = 450 * 1024; +#else +#if defined(DEBUG) + const size_t kUncappedStackQuota = 2 * kDefaultStackQuota; +#else + const size_t kUncappedStackQuota = kDefaultStackQuota; #endif + // Given the numbers above, we use 50k and 100k trusted buffers on 32-bit + // and 64-bit respectively. + const size_t kTrustedScriptBuffer = sizeof(size_t) * 12800; +#endif // MOZ_ASAN +#endif // OS selection // Avoid an unused variable warning on platforms where we don't use the // default. (void) kDefaultStackQuota; + // Large JS stacks are not web-compatible so cap to a smaller value. + const size_t kStackQuotaCap = Preferences::GetUint("javascript.options.main_thread_stack_quota_cap", 2 * 1024 * 1024); + const size_t kStackQuota = std::min(kUncappedStackQuota, kStackQuotaCap); + JS_SetNativeStackQuota(cx, kStackQuota, kStackQuota - kSystemCodeBuffer, diff --git a/layout/generic/nsIFrame.h b/layout/generic/nsIFrame.h index be439dbdc2..1c5dbbe9ac 100644 --- a/layout/generic/nsIFrame.h +++ b/layout/generic/nsIFrame.h @@ -12,7 +12,23 @@ #error This header/class should only be used within Mozilla code. It should not be used by extensions. #endif -#define MAX_REFLOW_DEPTH 200 +#if (defined(XP_WIN) && !defined(HAVE_64BIT_BUILD)) +// Using the same number as Blink's depth limit for 32-bit Windows for consistency. +// Note: This depth of 513 doesn't fit in the default stack of 1 MB, but it +// depth fits when the default is grown by a mere 192 KB. +// +// 32-bit Windows has a different limit compared to 64-bit desktop, because the +// default stack size affects all threads and consumes address space. +// +// Ideally, we'd get rid of this smaller limit and make 32-bit Windows +// capable of working with the Linux/Mac/Win64 number below. +#define MAX_REFLOW_DEPTH 513 +#else +// Blink's depth limit from its HTML parser times two. This just about fits +// the system default runtime stack limit of 8 MB on 64-bit Mac and Linux with +// display: table-cell. +#define MAX_REFLOW_DEPTH 1026 +#endif /* nsIFrame is in the process of being deCOMtaminated, i.e., this file is eventually going to be eliminated, and all callers will use nsFrame instead. At the moment diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index c84e3a6597..f391dd4739 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -1231,7 +1231,7 @@ pref("javascript.options.wasm_baselinejit", false); #endif pref("javascript.options.native_regexp", true); pref("javascript.options.parallel_parsing", true); -// ayncstack is used for debugging promises in devtools. +// asyncstack is used for debugging promises in devtools. pref("javascript.options.asyncstack", false); pref("javascript.options.throw_on_asmjs_validation_failure", false); pref("javascript.options.ion.offthread_compilation", true); @@ -1277,6 +1277,18 @@ pref("javascript.options.shared_memory", true); pref("javascript.options.throw_on_debuggee_would_run", false); pref("javascript.options.dump_stack_on_debuggee_would_run", false); +// Set a thread stack quota limit for the main thread. +// Default 2MB for normal builds on all OSes. Tweak this if your custom +// build explicitly requires a larger or smaller stack limit +// Do NOT touch these values unless you know exactly what you are doing! +// Neither exceedingly large nor exceedingly small values are beneficial. +#ifdef MOZ_ASAN +pref("javascript.options.main_thread_stack_quota_cap", 6291456); +#else +pref("javascript.options.main_thread_stack_quota_cap", 2097152); +#endif + + // advanced prefs pref("advanced.mailftp", false); pref("image.animation_mode", "normal"); -- cgit v1.2.3 From 325a49588aec069265e2342e9d81918ad3a5a681 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Fri, 15 Jul 2022 16:03:43 +0000 Subject: Issue #1805 - Follow-up: provide for Linux builds using rlimit and min/max. --- js/xpconnect/src/XPCJSContext.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/js/xpconnect/src/XPCJSContext.cpp b/js/xpconnect/src/XPCJSContext.cpp index 2a32af83bd..61a1d3b274 100644 --- a/js/xpconnect/src/XPCJSContext.cpp +++ b/js/xpconnect/src/XPCJSContext.cpp @@ -58,7 +58,14 @@ #include "nsIXULRuntime.h" #include "nsJSPrincipals.h" +#if defined(XP_LINUX) +// For getrlimit and min/max. +#include +#include +#endif + #ifdef XP_WIN +// For min/max. #include #include #endif -- cgit v1.2.3 From f63a7ee3fd540b700e0d23bdef5e3ea580905ca4 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Fri, 15 Jul 2022 19:48:18 +0000 Subject: Issue #1805 - Follow-up: Include the same headers for Solaris as well to future-proof gcc versions. --- js/xpconnect/src/XPCJSContext.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/xpconnect/src/XPCJSContext.cpp b/js/xpconnect/src/XPCJSContext.cpp index 61a1d3b274..e9d07b17c9 100644 --- a/js/xpconnect/src/XPCJSContext.cpp +++ b/js/xpconnect/src/XPCJSContext.cpp @@ -58,7 +58,7 @@ #include "nsIXULRuntime.h" #include "nsJSPrincipals.h" -#if defined(XP_LINUX) +#if defined(XP_LINUX) || defined(XP_SOLARIS) // For getrlimit and min/max. #include #include -- cgit v1.2.3