Closed Bug 1428349 Opened 6 years ago Closed 6 years ago

clang on win32/64 is either a tier 1 platform or it is not

Categories

(Firefox Build System :: General, defect)

x86
Windows
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: gcp, Unassigned)

References

Details

I am trying to land a security fix, bug 1297740. It fundamentally changes process launch on all desktop platforms.

It was backed out because of build failures. One of those I can fix trivially. The other one though, has been more problematic. I'm essentially running into a C++ standard compat issue between MSVC2015/2017 and clang-cl, which is specific to the Windows environment. Note that because it's a security bug, I can't just spam try with attempted build fixes (which I'd be desperate enough to do at this point).

My conundrum is this: We don't support building with clang on win32. It's not a tier 1 platform (https://developer.mozilla.org/en-US/docs/Mozilla/Supported_build_configurations), it is known to be broken (bug 752004). I did not manage to get it working even after manually fixing up various suggestions in various bugs (e.g. bug 1402915, bug 1397263).

Yet, we back out patches that fail (e.g. Windows 2012 opt static-analysis-win32-st-an/opt) with clang on Windows! So the platform *is* tier 1: you can't land patches unless it works (or at least compiles). But it's expected not to work due to the above.

The instructions for static analysis contain helpful hints such as "These instructions will only work where Mozilla already compiles with Clang" (not win32 clearly!). Attempting to run ./mach boostrap; ./mach static-analysis install; ./mach static-analysis check <anything> fails due to issues pointed out in the above clang-on-win32-isn't-supported bugs.

If running clang static analysis on Windows is something that is supposed to be trivial and I'm being stupid, or there's a simple mach bug, fine - mea culpa. But if not, then can we please decide whether clang on win32 is tier 1 or not?
The lame answer is "yes"; obviously it's a problem if people can't really compile things locally, though. :(  dmajor, do you have hints for gcp here?
Flags: needinfo?(dmajor)
With dmajor's help, I managed to get the build until my problem.

Key issues that were blocking me:

1) It is required to have MSVC2015 installed + an SDK update. MSVC2017 with the MSVC2015 toolset does not work (it works fine for clang, but not with our build-system).
2) MozillaBuild 2.2.0 is required.

I think this is pretty terrible for a tier-1 platform. (dmajor also had no idea how to get the static analysis build on Windows working IIRC)
Flags: needinfo?(dmajor)
I think bug 1402915 is a large (if not entire) portion of the pain here.
Depends on: 1402915
Flags: needinfo?(gpascutto)
>I think bug 1402915 is a large (if not entire) portion of the pain here.

I tried to work around that manually by setting the vc_path, but got various other problems later in the build.
(In reply to Gian-Carlo Pascutto [:gcp] from comment #5)
> >I think bug 1402915 is a large (if not entire) portion of the pain here.
> 
> I tried to work around that manually by setting the vc_path, but got various
> other problems later in the build.

As I understand that bug, there are more factors in play than just vc_path. If you got a successful build by using VS2015 + MozillaBuild 2, then I expect that fixing bug 1402915 would address the issues you encountered.
Build instructions on MDN have been updated.
Flags: needinfo?(gpascutto)
With current m-c you ought to be able to build with VS2017 and MozillaBuild 3. The only remaining gotcha is you have to use SDK 15063 or older; 16299 doesn't work. (I'll add a configure message for this shortly)

Do you have time to give it a try? It would be good to know if I've missed anything before I update MDN.
dmajor and I discussed briefly on IRC, but as an additional nicety we could fix configure to look in ~/.mozbuild/clang/bin when looking for compilers, so that developers could simply add `CC=clang-cl CXX=clang-cl` to their mozconfigs and have it Just Work with the toolchain that `mach bootstrap` installs.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #9)
> dmajor and I discussed briefly on IRC, but as an additional nicety we could
> fix configure to look in ~/.mozbuild/clang/bin when looking for compilers,
> so that developers could simply add `CC=clang-cl CXX=clang-cl` to their
> mozconfigs and have it Just Work with the toolchain that `mach bootstrap`
> installs.

I opened bug 1439767 for this.
Product: Core → Firefox Build System
:dmajor since bug 1439767 and bug 1402915 are resolved - can this bug be closed or are there further issues to address?
Flags: needinfo?(dmajor)
(In reply to Kim Moir [:kmoir] ET from comment #11)
> :dmajor since bug 1439767 and bug 1402915 are resolved - can this bug be
> closed or are there further issues to address?

gcp has this bug been resolved to your satisfaction?
Flags: needinfo?(dmajor) → needinfo?(gpascutto)
> The only remaining gotcha is you have to use SDK 15063 or older; 16299
> doesn't work. (I'll add a configure message for this shortly)

I fixed this part in bug 1439762.
This should be OK now.
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(gpascutto)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.