Don't pass sanitizer flags to the linker on Windows

RESOLVED FIXED in Firefox 52

Status

defect
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: dmajor, Assigned: dmajor)

Tracking

(Blocks 1 bug)

unspecified
mozilla52
Dependency tree / graph

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(1 attachment)

1:02.97 LINK : warning LNK4044: unrecognized option
 '/fsanitize=address'; ignored

On Windows we link using a different executable from the compiler, so we shouldn't put sanitize options in LDFLAGS.
Assignee: nobody → dmajor
Attachment #8797728 - Flags: review?(mh+mozilla)
Comment on attachment 8797728 [details] [diff] [review]
Don't pass sanitize flags to the linker on Windows.

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

This is an obvious simple fix, but I'm wondering... doesn't clang-cl support running the linker itself? Shouldn't we just switch to invoking clang-cl instead of link.exe? (and we could actually do the same for cl)
In that case, I'd expect -fsanitize=foo in LDFLAGS to actually work, and to invoke the linker with the right sanitize static library when necessary.

::: build/autoconf/sanitize.m4
@@ +64,5 @@
>      CFLAGS="-fsanitize=thread $CFLAGS"
>      CXXFLAGS="-fsanitize=thread $CXXFLAGS"
> +    if test -z "$CLANG_CL"; then
> +        LDFLAGS="-fsanitize=thread $LDFLAGS"
> +    fi    

trailing whitespaces.
Attachment #8797728 - Flags: review?(mh+mozilla) → review+
Thanks; I'll fix the whitespace. I can't speak to the matter of whether it's worthwhile to change the compiler/linker situation.
Pushed by dmajor@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7adf278d60b
Don't pass sanitize flags to the linker on Windows. r=glandium
https://hg.mozilla.org/mozilla-central/rev/d7adf278d60b
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.