Use a 64-bit clang-cl.exe for 32-bit static analysis builds

RESOLVED FIXED in Firefox 64

Status

enhancement
RESOLVED FIXED
2 years ago
10 months ago

People

(Reporter: dmajor, Assigned: glandium)

Tracking

unspecified
mozilla64
Dependency tree / graph

Firefox Tracking Flags

(firefox64 fixed)

Details

Attachments

(3 attachments)

In bug 1412952 I was surprised to discover that we use a 32-bit clang-cl.exe for 32-bit static analysis builds (in contrast to all other Win32 builds where we use a 64-bit toolchain). I landed an ugly hack to let 32-bit clang-cl.exe find 64-bit link.exe in our releng package.

I think it would be better to use a 64-bit compiler with -m32. Nathan says this ought to be easy, see bug 1412952 comment 14.

Then we can remove my Hostx64 hack.
And ThinLTO builds will need this too. They OOM left and right with a 32-bit toolchain.
Blocks: WinLTO
Nathan, do you have any time to share advice on how to implement this?
Flags: needinfo?(nfroyd)
We add the -m32 flag (or --target for clang) here:

https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/toolchain.configure#518

Adding a clang-cl case somewhere in there (I think in the `not info.cpu or info.cpu != target.cpu` case) should let configure figure out the right flags to pass to clang-cl?  Does that seem sufficient?
Flags: needinfo?(nfroyd)
I assume there are some automation files that need to be touched too, to tell the 32-bit builders to pull the 64-bit package?
Indeed.
I was hoping you'd tell me which files those would be. :p
(In reply to David Major [:dmajor] from comment #6)
> I was hoping you'd tell me which files those would be. :p

taskcluster/ci/
  build/windows.yml
  searchfox/kind.yml
  static-analysis/kind.yml

The win32-clang-cl in those files should be changed to win64-clang-cl.  Once everything works, you can remove the win32-clang-cl job from taskcluster/ci/toolchain/windows.yml.
Following comment 3 + comment 7 gets me:

17:35:24     INFO -  thread '<unnamed>' panicked at 'Unable to find libclang: "the `libclang` shared library could not be opened: z:/build\\build\\src\\clang\\bin\\libclang.dll"', src\libcore\result.rs:906:4

Presumably due to wrong bitness of libclang.dll.
libclang should only be loaded by bindgen from build scripts, which are host binaries. I suppose that since we don't have a `--host=...` in our win32 mozconfigs we're building 32-bit host binaries:
https://dxr.mozilla.org/mozilla-central/source/build/win32/mozconfig.vs2017

Looking at a random win32 opt build on inbound does show:
19:05:42     INFO -  checking for host system type... i686-pc-mingw32

I don't know how much of a PITA it would be to change that to x86_64, though, since:
a) we'd have to set HOST_{CC,CXX} appropriately and the LIB/INCLUDE nonsense might be hard to get right
b) we might wind up with a reoccurrence of bug 1377157 because rustc and cargo build scripts don't have a sensible way to set the host compiler
Depends on: 1429128
Ok, so to achieve the goal of this bug title ("Use a 64-bit clang-cl.exe for 32-bit static analysis builds"), these things are needed:

* Some preparatory build changes that were already done in bug 1401647.
* Pass the -m32 flag to 64-bit clang-cl. I've split this off into bug 1429128.
* Fix the clang-plugin build. Right now we can't just flip the switch on the "S" builds to use the "win64-clang-cl" toolchain because then clang-plugin fails to build because of wrong-bitness libs. This should probably be its own bug once we know what the change needs to be.
* Then, we _can_ flip the switch on "S" builds to use the "win64-clang-cl" toolchain
* Nail in the coffin: delete the win32-clang-cl toolchain definition.
Depends on: 1401647
> * Fix the clang-plugin build. Right now we can't just flip the switch on the
> "S" builds to use the "win64-clang-cl" toolchain because then clang-plugin
> fails to build because of wrong-bitness libs. This should probably be its
> own bug once we know what the change needs to be.

I broke this out into bug 1429549, though I still don't know what the fix should be.
Depends on: 1429549
Product: Core → Firefox Build System
(In reply to David Major [:dmajor] from comment #1)
> And ThinLTO builds will need this too. They OOM left and right with a 32-bit toolchain.

This is no longer the case after bug 1443827 made the st-an builders use their own toolchain.
No longer blocks: WinLTO
Assignee: nobody → mh+mozilla
This also requires the 64-bits rust compiler and some build system
tweaks.
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/73dcff37f846
Use a 64-bits clang-cl for 32-bits static analysis builds r=dmajor
https://hg.mozilla.org/integration/autoland/rev/98bb6d9eb63c
Remove now unused win32-clang-cl-st-an toolchain r=dmajor
https://hg.mozilla.org/integration/autoland/rev/5cc491e4d9cb
Remove now unused win32-rust-1.29 toolchain r=dmajor
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/76fafdaa9216
Use a 64-bits clang-cl for 32-bits static analysis builds r=dmajor
https://hg.mozilla.org/integration/mozilla-inbound/rev/273e84414434
Remove now unused win32-clang-cl-st-an toolchain r=dmajor
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8da3d4e6da0
Remove now unused win32-rust-1.29 toolchain r=dmajor
Flags: needinfo?(mh+mozilla)
See Also: → 1497120
Blocks: 1497336
Backed out 3 changesets (Bug 1414287) for causing Bug 1497029 a=backout

Backout: https://hg.mozilla.org/mozilla-central/rev/f95186d58bfaad221b599e284b57d980ade32468
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla64 → ---
Depends on: 1497029
Depends on: 1497120
Pushed by mh@glandium.org:
https://hg.mozilla.org/integration/autoland/rev/ce3b1b7b826f
Use a 64-bits clang-cl for 32-bits static analysis builds r=dmajor
https://hg.mozilla.org/integration/autoland/rev/0a7a9e17c1ea
Remove now unused win32-clang-cl-st-an toolchain r=dmajor
https://hg.mozilla.org/integration/autoland/rev/a5fe00a3b116
Remove now unused win32-rust-1.29 toolchain r=dmajor
https://hg.mozilla.org/mozilla-central/rev/ce3b1b7b826f
https://hg.mozilla.org/mozilla-central/rev/0a7a9e17c1ea
https://hg.mozilla.org/mozilla-central/rev/a5fe00a3b116
Status: REOPENED → RESOLVED
Closed: 11 months ago10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.