1356926 - Update static analysis builds to clang 3.9

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Mike Hommey [:glandium]]

Despite bug 1331957 and bug 1331012, we still have static analysis builds using clang 3.8 in the tree: static-analysis-linux64-st-an/opt and static-analysis-linux64-st-an/debug.

Not only that, but they're using an old clang 3.8, that comes with libstdc++ 4.8. Even building a clang 3.8 from the current build-clang doesn't yield that (it then comes with libstdc++ 4.9), and doing static analysis builds with that freshly built clang 3.8 fails.

And more interestingly, it fails with the same error when using clang 3.9... so we might as well figure out why it fails and update to 3.9.

The error is the following:
Executing: /usr/bin/ccache ../../../clang/bin/clang++ -std=gnu++11 -Qunused-arguments -I/home/worker/workspace/build/src/clang/include -fPIC -static-libstdc++ -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wnon-virtual-dtor -std=c++11 -fcolor-diagnostics -O3 -DNDEBUG -fno-exceptions -fno-rtti -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DHAVE_NEW_ASTMATCHER_NAMES -DHAS_ACCEPTS_IGNORINGPARENIMPCASTS -fno-rtti -fno-exceptions -fno-omit-frame-pointer -Werror -fPIC -shared -Wl,-h,libclang-plugin.so -o libclang-plugin.so /home/worker/workspace/build/src/obj-firefox/build/clang-plugin/tmpRTK1nh.list -L/home/worker/workspace/build/src/clang/lib -lclangASTMatchers -Wl,-rpath-link,/home/worker/workspace/build/src/obj-firefox/dist/bin -Wl,-rpath-link,/usr/local/lib -ldl
/home/worker/workspace/build/src/obj-firefox/build/clang-plugin/tmpRTK1nh.list:
    INPUT("Unified_cpp_build_clang-plugin0.o")
    INPUT("Unified_cpp_build_clang-plugin1.o")
    INPUT("../unix/stdc++compat/stdc++compat.o")
/home/worker/workspace/build/src/clang/lib/libstdc++.a(functexcept.o): In function `std::__throw_out_of_range_fmt(char const*, ...)':
/home/worker/workspace/build/gcc-objdir/x86_64-unknown-linux-gnu/libstdc++-v3/src/c++11/../../../../../gcc-4.9.4/libstdc++-v3/src/c++11/functexcept.cc:91: multiple definition of `std::__throw_out_of_range_fmt(char const*, ...)'
../unix/stdc++compat/stdc++compat.o:/home/worker/workspace/build/src/build/unix/stdc++compat/stdc++compat.cpp:41: first defined here
/home/worker/workspace/build/src/clang/lib/libstdc++.a(eh_aux_runtime.o): In function `__cxa_throw_bad_array_new_length':
/home/worker/workspace/build/gcc-objdir/x86_64-unknown-linux-gnu/libstdc++-v3/libsupc++/../../../../gcc-4.9.4/libstdc++-v3/libsupc++/eh_aux_runtime.cc:42: multiple definition of `__cxa_throw_bad_array_new_length'
../unix/stdc++compat/stdc++compat.o:/home/worker/workspace/build/src/build/unix/stdc++compat/stdc++compat.cpp:62: first defined here
clang-3.9: error: linker command failed with exit code 1 (use -v to see invocation)
/home/worker/workspace/build/src/config/rules.mk:783: recipe for target 'libclang-plugin.so' failed
gmake[5]: *** [libclang-plugin.so] Error 1

Nathan, would you mind looking into this?

Comment 1

[Bobby Holley (:bholley)]

I'm going to add --disable-stylo to these builds to work around this issue. When we fix this. we should remove that from the appropriate mozconfigs.

Comment 2

[Mike Hommey [:glandium]]

Attachment: Bug 1356926 - Use the same clang as normal builds for static analysis.

In practice, this upgrades from clang 3.8 to 3.9.

Review commit: https://reviewboard.mozilla.org/r/163920/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163920/

Comment 3

[Mike Hommey [:glandium]]

Attachment: Bug 1356926 - Enable stylo on static analysis builds.

Review commit: https://reviewboard.mozilla.org/r/163922/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/163922/

Comment 4

[Mike Hommey [:glandium]]

Comment on attachment 8892898 [details]
Bug 1356926 - Enable stylo on static analysis builds.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163922/diff/1-2/

Comment 5

[Gregory Szorc [:gps]]

Comment on attachment 8892897 [details]
Bug 1356926 - Use the same clang as normal builds for static analysis.

https://reviewboard.mozilla.org/r/163920/#review169404

These changes are beautiful.

Comment 6

[Gregory Szorc [:gps]]

Comment on attachment 8892898 [details]
Bug 1356926 - Enable stylo on static analysis builds.

https://reviewboard.mozilla.org/r/163922/#review169406

Isn't it great how problems go away when you establish consistency?

Comment 7

[Mike Hommey [:glandium]]

Attachment: Bug 1356926 - Make all stdc++compat symbols weak.

In some cases, we can end up linking some things with
--static-libstdc++. The notable (only?) example of that is for the
clang-plugin, and that happens because it gets some of its flags from
llvm-config, which contains --static-libstdc++ because clang itself is
built that way.

When that happens, the combination of --static-libstdc++ and
stdc++compat breaks the build because they have conflicting symbols,
which is very much by design.

There are two ways out of this:
- avoiding either -static-libstdc++ or stdc++compat
- work around the symbol conflicts

The former is not totally reliable ; we'd have to accurately determine
if we're in a potentially conflicting case, and remove one of the two in
that case, and while we can do that for the cases we explicitly know
about, that's not future-proof, and might fail just as much in the
future.

So we go with the latter. The way we do this is by defining all the
std++compat symbols weak, such that at link time, they're overridden by
any symbol with the same name. When building with -static-libstdc++,
libstdc++.a provides those symbols so the linker eliminates the weak
ones. When not building with -static-libstdc++, the linker keeps the
symbols from stdc++compat. That last assertion is validated by the
long-standing CHECK_STDCXX test that we run when linking shared
libraries and programs.

That still leaves the symbols weak in the final shared
libraries/programs, which is a change from the current setup, but
shouldn't cause problems because when using versions of libstdc++.so
that do provide those symbols, it's fine to use the libstdc++.so version
anyways.

Review commit: https://reviewboard.mozilla.org/r/164218/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/164218/

Comment 8

[Mike Hommey [:glandium]]

Comment on attachment 8892897 [details]
Bug 1356926 - Use the same clang as normal builds for static analysis.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163920/diff/1-2/

Comment 9

[Mike Hommey [:glandium]]

Comment on attachment 8892898 [details]
Bug 1356926 - Enable stylo on static analysis builds.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/163922/diff/2-3/

Comment 10

[Nathan Froyd [:froydnj]]

Comment on attachment 8893190 [details]
Bug 1356926 - Make all stdc++compat symbols weak.

https://reviewboard.mozilla.org/r/164218/#review169780

Makes sense to me.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/721b953ccc76
Make all stdc++compat symbols weak. r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/2a497555fda7
Use the same clang as normal builds for static analysis. r=gps
https://hg.mozilla.org/integration/mozilla-inbound/rev/24041731b278
Enable stylo on static analysis builds. r=gps

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/721b953ccc76
https://hg.mozilla.org/mozilla-central/rev/2a497555fda7
https://hg.mozilla.org/mozilla-central/rev/24041731b278