1485028 - Fix warnings that are specific to clang-cl ASAN builds

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Comment 1

[Masatoshi Kimura [:emk]]

Attachment: Fix warnings that are specific clang-cl ASAN builds

Comment 2

[(Away)]

I assume that the change in XPCJSContext.cpp is to silence this warning:

22:21:06     INFO -  z:/build/build/src/js/xpconnect/src/XPCJSContext.cpp(949,1):  warning: unused function 'GetWindowsStackSize' [-Wunused-function]

But it seems like a legitimate warning. The place where GetWindowsStackSize would be called is behind an ifndef MOZ_ASAN, but I think this is historical accident from when ASan was only available on Linux builds: https://searchfox.org/mozilla-central/rev/71ef4447db179639be9eff4471f32a95423962d7/js/xpconnect/src/XPCJSContext.cpp#1098

It would be nice if we could make Windows ASan builds use GetWindowsStackSize just like the non-ASan builds. Do the tests pass if you re-arrange the checks so that defined(XP_WIN) comes before defined(MOZ_ASAN)?

Comment 3

[Masatoshi Kimura [:emk]]

Attachment: Use GetWindowsStackSize() on Windows even with ASAN builds

Comment 4

[Masatoshi Kimura [:emk]]

Attachment: Fix warnings that are specific to clang-cl ASAN builds

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 9002822 [details] [diff] [review]
Use GetWindowsStackSize() on Windows even with ASAN builds

Review of attachment 9002822 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/xpconnect/src/XPCJSContext.cpp
@@ +1091,5 @@
> +    // 1MB is the default stack size on Windows. We use the -STACK linker flag
> +    // (see WIN32_EXE_LDFLAGS in config/config.mk) 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

I wonder if it makes sense to have kTrustedScriptBuffer be 450 * 1024 in the ASan case, like we do in the Linux code right above this. If this is green it's probably fine to leave it as is, though.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e42366ac19e9
Use GetWindowsStackSize() on Windows even with ASAN builds. r=jandem
https://hg.mozilla.org/integration/mozilla-inbound/rev/eec51e192b5d
Fix warnings that are specific to clang-cl ASAN builds. r=dmajor

Comment 7

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/e42366ac19e9
https://hg.mozilla.org/mozilla-central/rev/eec51e192b5d