Status
People
(Reporter: ting, Assigned: ting)
Tracking
Firefox Tracking Flags
(Not tracked)
Details
Attachments
(3 attachments)
In bug 1306650 :froydnj added the task for building 32-bit clang-cl. This one is for 64-bit, because ASan needs 64-bit runtime binary for 64-bit Firefox.
|
||
Comment 2•2 years ago
|
||
So our current clang-cl build task builds a 32-bit clang-cl, along with 32-bit runtime libraries. We really want a 64-bit clang-cl, with 32-bit and 64-bit runtime libraries, because our build machines are (AIUI) 64-bit machines anyway. I *think* what needs to be done for this bug is to adjust the paths and whatnot in build-clang-windows.sh to use the 64-bit cl.exe etc. rather than the 32-bit cl.exe for building, and possibly adjust how clang-cl itself gets built. Is that right, dmajor? How have you been building 64-bit clang-cl? I can do this, or Ting-Yu can do this, I think.
Flags: needinfo?(dmajor)
|
|
||
Comment 3•2 years ago
|
||
Yeah, I think it's just a matter of having the right compiler in your path. The only difference in my local workflow is whether I run cmake/ninja from the "VS2015 x64 Native Tools Command Prompt" or the x86 one.
Flags: needinfo?(dmajor)
|
|
||
Comment 4•2 years ago
|
||
One tricky thing is that even though 64-bit clang-cl can build for either target (since you can do -m32), I still end up building a separate 32-bit clang-cl just for its asan libaries. I don't know if there's an easier way around that.
|
(Assignee) | |
Comment 6•2 years ago
|
||
The other platforms runs static analysis and ASan with only 64 bit version, I am not sure why but do we need both 32-bit and 64-bit for Windows?
|
|
||
Comment 7•2 years ago
|
||
(In reply to Ting-Yu Chou [:ting] from comment #6) > The other platforms runs static analysis and ASan with only 64 bit version, > I am not sure why but do we need both 32-bit and 64-bit for Windows? I would say because our other major platforms (Linux/OSX) are both 64-bit by default, so we focus on 64-bit ports there. Whereas 32-bit Windows is our primary Windows platform, so we'd want 32-bit static analysis. But we're also going to be shipping an official 64-bit Windows port soonish, so we'd want to analyze that as well. That being said, if there a lot of overlap between our 32-bit and 64-bit Windows code, we could probably get away with doing one or the other. I don't know how much of our Windows code is specific to one architecture, though.
|
|
||
Comment 8•2 years ago
|
||
If we want to have Windows ASan be a full-fledged entry on treeherder running through the complete set of mochitests etc., then I suspect it's going to have to be 64-bit. We would OOM left and right on 32-bit builds.
|
|
||
Comment 9•2 years ago
|
||
(In reply to David Major [:dmajor] from comment #8) > If we want to have Windows ASan be a full-fledged entry on treeherder > running through the complete set of mochitests etc., then I suspect it's > going to have to be 64-bit. We would OOM left and right on 32-bit builds. Indeed. But mochitests are a ways off at this point, AIUI. At least, talking with folks on the releng side indicated that running a couple of one-off Windows builds on each push is no big deal. Running the full test suite, under ASan or otherwise, is a different kettle of fish.
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
| Comment hidden (mozreview-request) |
|
||
Comment 13•2 years ago
|
||
| mozreview-review | ||
Comment on attachment 8810710 [details] Bug 1316537 part 1 - Add 64-bit clang-cl toolchain build. https://reviewboard.mozilla.org/r/93002/#review93128
Attachment #8810710 -
Flags: review?(dustin) → review+
|
|
||
Comment 14•2 years ago
|
||
(In reply to David Major [:dmajor] from comment #4) > One tricky thing is that even though 64-bit clang-cl can build for either > target (since you can do -m32), I still end up building a separate 32-bit > clang-cl just for its asan libaries. I don't know if there's an easier way > around that. Does the 64-bit clang-cl not build any 32-bit runtime libraries, or does it just not build the ASan ones? Given the problems with 32-bit ASan test running, I'm not sure if we care about 32-bit ASan builds...fuzzers might, though?
Flags: needinfo?(dmajor)
|
||
Comment 15•2 years ago
|
||
| mozreview-review | ||
Comment on attachment 8810711 [details] Bug 1316537 part 2 - Update 64-bit clang build configuration for the changes in bug 1306650. https://reviewboard.mozilla.org/r/93004/#review93200
Attachment #8810711 -
Flags: review?(ehsan) → review+
|
|
||
Comment 16•2 years ago
|
||
| mozreview-review | ||
Comment on attachment 8810712 [details] Bug 1316537 part 3 - Fix tar package path conversion for local build. https://reviewboard.mozilla.org/r/93006/#review93202
Attachment #8810712 -
Flags: review?(ehsan) → review+
|
|
||
Comment 17•2 years ago
|
||
> Does the 64-bit clang-cl not build any 32-bit runtime libraries, or does it
> just not build the ASan ones?
afaik it doesn't build any 32-bit libraries.Flags: needinfo?(dmajor)
|
||
Comment 18•2 years ago
|
||
Pushed by tchou@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f12149f08204 part 1 - Add 64-bit clang-cl toolchain build. r=dustin https://hg.mozilla.org/integration/autoland/rev/bb4a098b3001 part 2 - Update 64-bit clang build configuration for the changes in bug 1306650. r=Ehsan https://hg.mozilla.org/integration/autoland/rev/0e6e3d0eec49 part 3 - Fix tar package path conversion for local build. r=Ehsan
|
|
(Assignee) | |
Comment 19•2 years ago
|
||
I'll check to make sure the task does work after landing.
Keywords: leave-open
|
||
Comment 20•2 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/f12149f08204 https://hg.mozilla.org/mozilla-central/rev/bb4a098b3001 https://hg.mozilla.org/mozilla-central/rev/0e6e3d0eec49
|
|
(Assignee) | |
Comment 21•2 years ago
|
||
Confirmed it works. https://treeherder.mozilla.org/#/jobs?repo=try&revision=c76375b44f68a249df35902f9684db00b77d5be7
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Removing leave-open keyword from resolved bugs, per :sylvestre.
Keywords: leave-open
|
||
Updated•11 months ago
|
Product: TaskCluster → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•