Closed Bug 1307547 Opened 5 years ago Closed 5 years ago

Don't pass sanitizer flags to the linker on Windows

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox52 fixed)

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: dmajor, Assigned: dmajor)

References

Details

Attachments

(1 file)

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: 5 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.