Closed Bug 1485028 Opened Last year Closed Last year

Fix warnings that are specific to clang-cl ASAN builds


(Firefox Build System :: General, enhancement)

Not set


(firefox63 fixed)

Tracking Status
firefox63 --- fixed


(Reporter: emk, Assigned: emk)




(2 files, 1 obsolete file)

No description provided.
Attachment #9002775 - Flags: review?(core-build-config-reviews)
Assignee: nobody → VYV03354
Summary: Fix warnings that are specific clang-cl ASAN builds → Fix warnings that are specific to clang-cl ASAN builds
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:

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)?
Attachment #9002823 - Flags: review?(core-build-config-reviews)
Attachment #9002775 - Attachment is obsolete: true
Attachment #9002775 - Flags: review?(core-build-config-reviews)
Attachment #9002823 - Flags: review?(core-build-config-reviews) → review+
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/ 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.
Attachment #9002822 - Flags: review?(jdemooij) → review+
Pushed by
Use GetWindowsStackSize() on Windows even with ASAN builds. r=jandem
Fix warnings that are specific to clang-cl ASAN builds. r=dmajor
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.