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)
Firefox Build System
General
Tracking
(firefox52 fixed)
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: dmajor, Assigned: dmajor)
References
Details
Attachments
(1 file)
2.49 KB,
patch
|
glandium
:
review+
|
Details | Diff | Splinter Review |
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 2•5 years ago
|
||
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
Comment 5•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d7adf278d60b
Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Updated•3 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•