1428158 - run static analysis on Android builds

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Nathan Froyd [:froydnj]]

We would like to do this because it moves our Android builds closer to parity with our desktop builds and means we can *really* depend on static analysis to enforce things on all our platforms.

The current status is that if you add --enable-clang-plugin to Android builds, you get:

checking for wget... (cached) wget
checking for fdatasync... (cached) yes
checking for valid C compiler optimization flags... yes
checking for llvm-config... not found
configure: error: Cannot find an llvm-config binary for building a clang plugin

The current checks in build/autoconf/clang-plugin.m4 want LLVMCONFIG, rather than the LLVM_CONFIG that things like Stylo's configury want.  It may be worth switching clang-plugin.m4 to use LLVM_CONFIG instead; it's not clear to me how difficult it would be to port clang-plugin.m4 to moz.configure.

OK, so we point LLVMCONFIG at a suitable llvm-config (the r15c NDK does not distribute llvm-config, though it *does* distribute all the clang headers...):

export LLVMCONFIG=/home/froydnj/.mozbuild/clang/bin/llvm-config

This is the same toolchain that I use for building Stylo.  That gets us through configure and into an actual build, where we fail with:

/home/froydnj/.mozbuild/android-ndk-r15c/toolchains/llvm/prebuilt/linux-x86_64/bin/clang++ -std=gnu++14 --target=arm-linux-androideabi -o Unified_cpp_build_clang-plugin0.o -c -DNDEBUG=1 -DTRIMMED=1 -I/home/froydnj/src/gecko-dev.git/build/clang-plugin -I/opt/build/froydnj/build-android/build/clang-plugin -I/opt/build/froydnj/build-android/dist/include -I/opt/build/froydnj/build-android/dist/include/nspr -I/opt/build/froydnj/build-android/dist/include/nss -fPIC -Qunused-arguments -isystem /home/froydnj/.mozbuild/android-ndk-r15c/platforms/android-9/arch-arm/usr/include -gcc-toolchain /home/froydnj/.mozbuild/android-ndk-r15c/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64 -I/home/froydnj/.mozbuild/clang/include -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Werror=date-time -std=c++11 -ffunction-sections -fdata-sections -O3 -DNDEBUG -fno-exceptions -fno-rtti -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -fno-rtti -fno-exceptions -funwind-tables -Werror  -MD -MP -MF .deps/Unified_cpp_build_clang-plugin0.o.pp   /opt/build/froydnj/build-android/build/clang-plugin/Unified_cpp_build_clang-plugin0.cpp
In file included from /opt/build/froydnj/build-android/build/clang-plugin/Unified_cpp_build_clang-plugin0.cpp:2:
In file included from /home/froydnj/src/gecko-dev.git/build/clang-plugin/ArithmeticArgChecker.cpp:5:
In file included from /home/froydnj/src/gecko-dev.git/build/clang-plugin/ArithmeticArgChecker.h:8:
In file included from /home/froydnj/src/gecko-dev.git/build/clang-plugin/plugin.h:8:
In file included from /home/froydnj/.mozbuild/clang/include/clang/AST/ASTConsumer.h:17:
In file included from /home/froydnj/.mozbuild/clang/include/llvm/ADT/StringRef.h:13:
/home/froydnj/.mozbuild/clang/include/llvm/ADT/iterator_range.h:22:10: fatal error: 'utility' file not found
#include <utility>

Notice that we're trying to build the plugin (which is really a *host* thing) with the *target* compiler, which isn't going to turn out very well, since the target compiler isn't producing code for the host and doesn't know where its C++ standard library headers live.

build/clang-plugin/Makefile.in contains a bit of hackery for cross-Darwin compilation, so let's try (ab)using that.  We'll need to set HOST_{CC,CXX} appropriately as well, which may not be so great for users.

...except that doesn't work, because the target compile flags use things like -isystem, which *also* render the host compile unable to find C++ standard library headers.  So we have to strip those out somewhere along the way.

It's also not even clear that the NDK clang would be able to load the resulting plugin, since the plugin would be using the host clang's headers, and the two clangs are probably not compatible.  So we'd need to either:

1) Use the NDK clang's headers for the plugin; or
2) Just use our custom clang for compiling everything.

In any event, there's a non-trivial amount of work to be done here.

Comment 1

[Andi-Bogdan Postelnicu [:andi]]

For the mean time i think this can be avoidable by running the clang-tidy based static analysis within the mach.

Comment 2

[Nathan Froyd [:froydnj]]

Attachment: part 1 - use HOST_CXX for building the clang-plugin for Android builds

These upcoming patches at least let me build things on my Linux machine.  But
running the clang-plugin tests--that is, compiling something with the clang
plugin active--just falls over without any kind of error message.

Comment 3

[Nathan Froyd [:froydnj]]

Attachment: part 2 - clear out CPPFLAGS before compiling test files for the clang plugin

We want to compile the clang-plugin as a host thing, but CPPFLAGS
contains a bunch of target-specific stuff, which we don't want.  We
already modify CXXFLAGS for the same reason; let's modify CPPFLAGS also.

Comment 4

[Nathan Froyd [:froydnj]]

Attachment: part 3 - make a modifiable version of OS_LIBS

The clang static analysis plugin is, for reasons, built as a target
library, even though it's really a host library.  This decision
necessitates various hacks when building the clang plugin, most of which
are confined to the clang-plugin build directory.

For target code on Android, however, we need to link in several target
libraries, most notably the C++ standard library and various supporting
bits.  Trying to link a host library while using these target-specific
libraries does not work very well.

Thus this hack.  We need to remove the target-specific libraries when
linking the clang plugin, and only the clang plugin.  But we can't
modify OS_LIBS, as moz.build knows about OS_LIBS and will complain if
Makefile.in files touch OS_LIBS.  So instead we need a level of
indirection, a variable whose sole purpose is to wrap OS_LIBS so the
clang plugin Makefile.in can remove various bits from it.

Comment 5

[Nathan Froyd [:froydnj]]

Attachment: part 4 - make the clang plugin buildable on Android builds

We need to do a couple of things.

1. We need to avoid linking the Android-specific libraries.
2. We need to filter out -static-libstdc++ when compiling plugin files.
3. We need to avoid passing -isystem when compiling plugin files.

It's not obvious to me that the patch for the last two is actually the way I
should be doing thing.

Comment 6

[Nathan Froyd [:froydnj]]

Attachment: part 4 - make the clang plugin buildable on Android builds

Turns out the -static-libstdc++ thing can be made to work if we are sure to
pass -Qunused-arguments, so compilations don't complain that we're using
-static-libstdc++.  But that doesn't actually fix the plugin loading issues.

Comment 7

[Nathan Froyd [:froydnj]]

Attachment: part 5 - deal with libstdc++ incompatibilities

Loading the clang plugin was failing completely without any useful error
messages.  LD_DEBUG showed me that it was failing to resolve several
symbols, presumably because said methods aren't defined in the
libstdc++.a files that come with the clang packaged up on infra (because
we built the whole thing with an older compiler there?).  This is going
to be super-annoying for people who want to make the plugin work
locally, but we can cross that bridge when we come to it; maybe by using
the clang on their system instead of one downloaded from `mach artifact
toolchain` will magically resolve this.

Anyway, some basic_string methods were taken care of by using the old
libstdc++ ABI.  Out-of-line definitions for error_category's virtual
methods were also necessary.

These steps are really similar to what we need for
--enable-stdcxx-compat, but I was hesistant to go down that rabbit hole
on Android, since that could be a whole heap of fun to deal with.  I'd
rather get everything working, and then sort of what the best way from
"working locally" to "working on infra" is.

Comment 8

[Nathan Froyd [:froydnj]]

With the above stack of patches, I can build the clang plugin successfully and run all of the self tests.  The snprintf/vsnprintf tests fail; I attempted to talk with Andi about them over IRC, which was really helpful, but it got to the point where Andi can debug things more effectively on a local machine rather than having a high-latency connection to the failing system (i.e. me).

My mozconfig, not all of which may be necessary, looks like:

mk_add_options MOZ_OBJDIR=/opt/build/froydnj/build-android
mk_add_options MOZ_ANDROID_ANR_REPORTER=1
mk_add_options AUTOCLOBBER=1

NDK_DIR=/home/froydnj/.mozbuild/android-ndk-linux

export CC=/home/froydnj/.mozbuild/clang/bin/clang
export CXX=/home/froydnj/.mozbuild/clang/bin/clang++
export HOST_CC=/home/froydnj/.mozbuild/clang/bin/clang
export HOST_CXX=/home/froydnj/.mozbuild/clang/bin/clang++

ac_add_options --with-android-ndk=$NDK_DIR
ac_add_options --with-android-sdk=/home/froydnj/.mozbuild/android-sdk-linux
ac_add_options --with-android-version=9
ac_add_options --enable-application=mobile/android
ac_add_options --target=arm-linux-androideabi
ac_add_options --enable-optimize
ac_add_options --enable-debug-symbols
ac_add_options --enable-tests
ac_add_options --disable-crashreporter
ac_add_options --enable-release
ac_add_options --enable-warnings-as-errors
ac_add_options --enable-clang-plugin
export LLVMCONFIG=/home/froydnj/.mozbuild/clang/bin/llvm-config

ac_add_options --with-android-min-sdk=16

ac_add_options --enable-elf-hack

`mach bootstrap` can provide the pieces that you need.  I was using infra's 3.9 static analysis clang, installed locally.  Andi, please let me know if you need anything else, and thanks for helping out!

Comment 9

[Andi-Bogdan Postelnicu [:andi]]

Attachment: part6.diff

This patch should fix the sprintf literal checker. Also i think it would be a good idea to migrate to clang 6 on our static analysis try builds. , by doing this we will get rid of some obsolete code and a bunch of ifdefs and re-implementations like polymorphic checkers as: isInSystemHeader

Comment 10

[Nathan Froyd [:froydnj]]

I can confirm attachment 8961303 [details] [diff] [review] fixes snprintf literal checking, and I can now start compiling Android with the clang plugin! \o/

...which leads to the next peculiar error: many, many complaints about things like:

/opt/build/froydnj/build-android/dist/include/nsIEventTarget.h:60:23: error: variable of type 'already_AddRefed<nsIRunnable>' is only valid as a temporary
  NS_IMETHOD Dispatch(already_AddRefed<nsIRunnable> event, uint32_t flags = DISPATCH_NORMAL) = 0;

/opt/build/froydnj/build-android/dist/include/nsIThread.h:51:27: error: variable of type 'already_AddRefed<nsIRunnable>' is only valid as a temporary
  NS_IMETHOD IdleDispatch(already_AddRefed<nsIRunnable> event) = 0;

...and so forth (I won't enumerate them all here).  This seems peculiar to me, as we compile this code without complain on other platforms with the plugin.  Andi, can you take a look?

Comment 11

[Nathan Froyd [:froydnj]]

There are also errors like:

/opt/build/froydnj/build-android/dist/include/nsProxyRelease.h:280:7: error: Unused "kungFuDeathGrip" 'RefPtr<nsIOpenSignedAppFileCallback>' objects constructed from temporary values are prohibited
      RefPtr<T> local(mozilla::Move(mPtr));
      ^
/opt/build/froydnj/build-android/dist/include/nsProxyRelease.h:280:23: note: Please switch all accesses to this value to go through 'local', or explicitly pass 'local' to `mozilla::Unused`
      RefPtr<T> local(mozilla::Move(mPtr));
                      ^
/opt/build/froydnj/build-android/dist/include/nsProxyRelease.h:280:7: error: Unused "kungFuDeathGrip" 'RefPtr<nsIVerifySignedDirectoryCallback>' objects constructed from temporary values are prohibited
      RefPtr<T> local(mozilla::Move(mPtr));
      ^
/opt/build/froydnj/build-android/dist/include/nsProxyRelease.h:280:23: note: Please switch all accesses to this value to go through 'local', or explicitly pass 'local' to `mozilla::Unused`
      RefPtr<T> local(mozilla::Move(mPtr));

which seems peculiar as well.

Comment 12

[Nathan Froyd [:froydnj]]

Sorry, I think comment 11 was based on some local changes in my tree.

Comment 13

[Nathan Froyd [:froydnj]]

Andi, can you file a separate bug for comment 9 so we can get that patch reviewed and into the tree?

Comment 14

[Andi-Bogdan Postelnicu [:andi]]

(In reply to Nathan Froyd [:froydnj] from comment #13)
> Andi, can you file a separate bug for comment 9 so we can get that patch
> reviewed and into the tree?
Sorry for the delay, opened: https://bugzilla.mozilla.org/show_bug.cgi?id=1454667

Comment 15

[Nathan Froyd [:froydnj]]

Attachment: part 1 - use HOST_CXX for building the clang-plugin for Android builds

Thanks to the weirdness that is our host shared library build situation.

Comment 16

[Nathan Froyd [:froydnj]]

Attachment: part 2 - make a modifiable version of OS_LIBS

The clang static analysis plugin is, for reasons, built as a target
library, even though it's really a host library.  This decision
necessitates various hacks when building the clang plugin, most of which
are confined to the clang-plugin build directory.

For target code on Android, however, we need to link in several target
libraries, most notably the C++ standard library and various supporting
bits.  Trying to link a host library while using these target-specific
libraries does not work very well.

Thus this hack.  We need to remove the target-specific libraries when
linking the clang plugin, and only the clang plugin.  But we can't
modify OS_LIBS, as moz.build knows about OS_LIBS and will complain if
Makefile.in files touch OS_LIBS.  So instead we need a level of
indirection, a variable whose sole purpose is to wrap OS_LIBS so the
clang plugin Makefile.in can remove various bits from it.

Comment 17

[Nathan Froyd [:froydnj]]

Attachment: part 3 - make the clang plugin buildable on Android builds

We need to do a couple of things.

1. We need to avoid linking the Android-specific libraries.
2. We need to avoid passing -isystem when compiling plugin files.

Comment 18

[Nathan Froyd [:froydnj]]

Attachment: part 4 - relax glibc requirements for the clang plugin

...and remove some outdated relaxed requirements for libstdc++ in the
process, since we now require 3.4.15.

Comment 19

[Nathan Froyd [:froydnj]]

Attachment: part 5 - deal with libstdc++ incompatibilities

Loading the clang plugin was failing completely without any useful error
messages.  LD_DEBUG showed me that it was failing to resolve several
symbols, presumably because said methods aren't defined in the
libstdc++.a files that come with the clang packaged up on infra (because
we built the whole thing with an older compiler there?).  This is going
to be super-annoying for people who want to make the plugin work
locally, but we can cross that bridge when we come to it; maybe by using
the clang on their system instead of one downloaded from `mach artifact
toolchain` will magically resolve this.

Anyway, some basic_string methods were taken care of by using the old
libstdc++ ABI.  Out-of-line definitions for error_category's virtual
methods were also necessary.

These steps are really similar to what we need for
--enable-stdcxx-compat, but I was hesistant to go down that rabbit hole
on Android, since that could be a whole heap of fun to deal with.  I'd
rather get everything working, and then sort of what the best way from
"working locally" to "working on infra" is.

...except that this works great for Android, and not so much for our other
static analysis builds.  I'm not sure what to do here, short of #ifdef'ing all
of this code MOZ_AUTOMATION + Android.  Ideas?

Comment 20

[Nathan Froyd [:froydnj]]

Attachment: part 6 - enable static analysis on Android

With all the previous patches, we can now build and run the static analysis
plugin, and with all of the dependent bugs fixed, we have green builds on try.

Comment 21

[Nick Alexander :nalexander [he/him] (parental leave until 2020!)]

Comment on attachment 8980056 [details] [diff] [review]
part 2 - make a modifiable version of OS_LIBS

Review of attachment 8980056 [details] [diff] [review]:
-----------------------------------------------------------------

This is horrendous, but I'm willing to assume that it's not radically worse than what we're already doing in build/clang-plugin, which includes this gem already: https://searchfox.org/mozilla-central/source/build/clang-plugin/Makefile.in#7-8.

::: config/config.mk
@@ +200,5 @@
>  include $(MOZILLA_DIR)/config/static-checking-config.mk
>  
>  LDFLAGS		= $(COMPUTED_LDFLAGS) $(PGO_LDFLAGS) $(MK_LDFLAGS)
>  
> +LINK_OS_LIBS = $(OS_LIBS)

Can we get a comment about this?

Comment 22

[Nick Alexander :nalexander [he/him] (parental leave until 2020!)]

Comment on attachment 8980057 [details] [diff] [review]
part 3 - make the clang plugin buildable on Android builds

Review of attachment 8980057 [details] [diff] [review]:
-----------------------------------------------------------------

::: build/clang-plugin/moz.build
@@ +100,5 @@
>      LINK_FLAGS['FIX_LINK_PATHS'] = []
> +
> +if CONFIG['OS_TARGET'] == 'Android':
> +    # Remove all of the target flags that point into the NDK.
> +    for v in ('OS_CPPFLAGS',):

If you don't actually grow a second var, please drop this outer loop.  And consider s/v/var/, 'cuz I had to read this twice to figure out what you were doing.

This all seems very fragile -- your comment suggests that you're stripping NDK flags but there's no reference to the NDK (directly) in your actual code; that is, you're whitelisting not blacklisting.

Again, I'm willing to accept that this is the best way to proceed...

Comment 23

[Nick Alexander :nalexander [he/him] (parental leave until 2020!)]

Comment on attachment 8980058 [details] [diff] [review]
part 4 - relax glibc requirements for the clang plugin

Review of attachment 8980058 [details] [diff] [review]:
-----------------------------------------------------------------

Based on https://searchfox.org/mozilla-central/rev/dc6d85680539cb7e5fe2843d38a30a0581bfefe1/config/config.mk#447, we now require 3.4.17+.  I'm assuming these are refreshes of old patches, hence the versions are incorrect in non-critical parts like comments/commit comments?

::: build/clang-plugin/Makefile.in
@@ +35,5 @@
>  # correctly.  Note that the binary produced here is a host tool and doesn't need
>  # to be distributed.
>  MACOSX_DEPLOYMENT_TARGET :=
>  
> +# Relax the requirements for glibc symbol versions for the static analyis plugin.

It really seems that you're strengthening the requirements for glibc: https://searchfox.org/mozilla-central/rev/dc6d85680539cb7e5fe2843d38a30a0581bfefe1/config/config.mk#448 requires 3+ or 2.13+; you're requiring 3+ or 2.15+.

Comment 24

[Nick Alexander :nalexander [he/him] (parental leave until 2020!)]

Comment on attachment 8980059 [details] [diff] [review]
part 5 - deal with libstdc++ incompatibilities

Review of attachment 8980059 [details] [diff] [review]:
-----------------------------------------------------------------

Sadly, I can't review this.  glandium?  snorp?

Comment 25

[Nick Alexander :nalexander [he/him] (parental leave until 2020!)]

Comment on attachment 8980060 [details] [diff] [review]
part 6 - enable static analysis on Android

Review of attachment 8980060 [details] [diff] [review]:
-----------------------------------------------------------------

If it's green for you, it's good for me!

Comment 26

[Mike Hommey [:glandium] (high latency)]

Comment on attachment 8980058 [details] [diff] [review]
part 4 - relax glibc requirements for the clang plugin

Review of attachment 8980058 [details] [diff] [review]:
-----------------------------------------------------------------

::: build/clang-plugin/Makefile.in
@@ +37,5 @@
>  MACOSX_DEPLOYMENT_TARGET :=
>  
> +# Relax the requirements for glibc symbol versions for the static analyis plugin.
> +ifdef CHECK_GLIBC
> +CHECK_GLIBC = $(call CHECK_SYMBOLS,$(1),GLIBC,libc,v[1] > 2 || (v[1] == 2 && v[2] > 14))

The real problem here is that we *are* building android builds with --enable-stdcxx-compat, because they used to be built on centos, and was necessary for host things there when building with our toolchains. However, now that we're building on Debian stable, we actually don't need that at all, so it would be better to unset MOZ_STDCXX_COMPAT after the mozconfig.stdcxx include in mobile/android/config/mozconfigs/common and not touch this (although, you /can/ remove the CHECK_STDCXX override)

Comment 27

[Mike Hommey [:glandium] (high latency)]

Comment on attachment 8980059 [details] [diff] [review]
part 5 - deal with libstdc++ incompatibilities

Review of attachment 8980059 [details] [diff] [review]:
-----------------------------------------------------------------

So the real problem is that we end up using the *system* libstdc++ headers, which, on the build environment are those of gcc 6.3, while linking against the libstdc++ that comes along clang, which comes from the old gcc it's built against. That's a *very* fragile setup to keep around, and I'd rather not hack our way around it. The right thing to do here is to ensure we use the same libstdc++ library and headers. By either not shipping libstdc++ with the clang toolchain or shippint the libstdc++ headers with the clang toolchain.

Comment 28

[Mike Hommey [:glandium] (high latency)]

FWIW, if I remove clang/lib/libstdc++.a, the std::_V2::error_category::~error_category() symbol is not missing anymore. I do, however, still end up with a build failure because of a missing *llvm* symbol (llvm::DisableABIBreakingChecks).

Comment 29

[Mike Hommey [:glandium] (high latency)]

And that last one is because the clang-plugin is built for the clang toolchain, while the plugin is used with the clang from the android ndk.

Comment 30

[Nathan Froyd [:froydnj]]

(In reply to Mike Hommey [:glandium] from comment #29)
> And that last one is because the clang-plugin is built for the clang
> toolchain, while the plugin is used with the clang from the android ndk.

The idea is that this wouldn't be the case (at least on automation) after bug 1435409.

Comment 31

[Nathan Froyd [:froydnj]]

Unsetting MOZ_STDCXX_COMPAT on its own runs into problems, because it renders elfhack unable to run:

[task 2018-06-01T20:38:07.250Z] 20:38:07     INFO -  package> /builds/worker/workspace/build/src/obj-firefox/build/unix/elfhack/elfhack: /builds/worker/workspace/build/src/clang/lib/libstdc++.so.6: version `GLIBCXX_3.4.21' not found (required by /builds/worker/workspace/build/src/obj-firefox/build/unix/elfhack/elfhack)
[task 2018-06-01T20:38:07.250Z] 20:38:07     INFO -  package> Traceback (most recent call last):
[task 2018-06-01T20:38:07.251Z] 20:38:07     INFO -  package>   File "/builds/worker/workspace/build/src/toolkit/mozapps/installer/packager.py", line 339, in <module>
[task 2018-06-01T20:38:07.251Z] 20:38:07     INFO -  package>     main()
[task 2018-06-01T20:38:07.251Z] 20:38:07     INFO -  package>   File "/builds/worker/workspace/build/src/toolkit/mozapps/installer/packager.py", line 333, in main
[task 2018-06-01T20:38:07.251Z] 20:38:07     INFO -  package>     copier.copy(args.destination)
[task 2018-06-01T20:38:07.251Z] 20:38:07     INFO -  package>   File "/builds/worker/workspace/build/src/python/mozbuild/mozpack/copier.py", line 431, in copy
[task 2018-06-01T20:38:07.251Z] 20:38:07     INFO -  package>     copy_results.append((destfile, f.copy(destfile, skip_if_older)))
[task 2018-06-01T20:38:07.252Z] 20:38:07     INFO -  package>   File "/builds/worker/workspace/build/src/python/mozbuild/mozpack/files.py", line 296, in copy
[task 2018-06-01T20:38:07.252Z] 20:38:07     INFO -  package>     elfhack(dest)
[task 2018-06-01T20:38:07.252Z] 20:38:07     INFO -  package>   File "/builds/worker/workspace/build/src/python/mozbuild/mozpack/executables.py", line 124, in elfhack
[task 2018-06-01T20:38:07.252Z] 20:38:07     INFO -  package>     errors.fatal('Error executing ' + ' '.join(cmd))
[task 2018-06-01T20:38:07.252Z] 20:38:07     INFO -  package>   File "/builds/worker/workspace/build/src/python/mozbuild/mozpack/errors.py", line 103, in fatal
[task 2018-06-01T20:38:07.252Z] 20:38:07     INFO -  package>     self._handle(self.FATAL, msg)
[task 2018-06-01T20:38:07.253Z] 20:38:07     INFO -  package>   File "/builds/worker/workspace/build/src/python/mozbuild/mozpack/errors.py", line 98, in _handle
[task 2018-06-01T20:38:07.253Z] 20:38:07     INFO -  package>     raise ErrorMessage(msg)
[task 2018-06-01T20:38:07.253Z] 20:38:07     INFO -  package> mozpack.errors.ErrorMessage: Error: Error executing /builds/worker/workspace/build/src/obj-firefox/build/unix/elfhack/elfhack ../../../dist/geckoview/assets/armeabi-v7a/liblgpllibs.so
[task 2018-06-01T20:38:07.253Z] 20:38:07     INFO -  package> /builds/worker/workspace/build/src/toolkit/mozapps/installer/packager.mk:22: recipe for target 'stage-package' failed

Presumably if we got the libstdc++ situation straightened out, as comment 27 suggests, this problem would go away?

Comment 32

[Mike Hommey [:glandium] (high latency)]

> ../elfhack: /builds/worker/workspace/build/src/clang/lib/libstdc++.so.6: version `GLIBCXX_3.4.21' not found

haha! what if you remove the clang/lib directory from LD_LIBRARY_PATH?

Comment 33

[Nathan Froyd [:froydnj]]

(In reply to Mike Hommey [:glandium] from comment #32)
> > ../elfhack: /builds/worker/workspace/build/src/clang/lib/libstdc++.so.6: version `GLIBCXX_3.4.21' not found
> 
> haha! what if you remove the clang/lib directory from LD_LIBRARY_PATH?

Removing the mozconfig.stdcxx include entirely (rather than just unsetting MOZ_STDCXX_COMPAT) at least lets the build complete.  But that's not with the plugin, just testing those bits on their own.  Will have to see about the plugin next.

Comment 34

[Nathan Froyd [:froydnj]]

(In reply to Mike Hommey [:glandium] from comment #27)
> So the real problem is that we end up using the *system* libstdc++ headers,
> which, on the build environment are those of gcc 6.3, while linking against
> the libstdc++ that comes along clang, which comes from the old gcc it's
> built against. That's a *very* fragile setup to keep around, and I'd rather
> not hack our way around it. The right thing to do here is to ensure we use
> the same libstdc++ library and headers. By either not shipping libstdc++
> with the clang toolchain or shippint the libstdc++ headers with the clang
> toolchain.

Why does the clang package even still have the 4.9 headers? Shouldn't we have triggered a rebuild of clang when we switched over to GCC 6.x?

In any event, I *think* that not shipping the libstdc++ headers will break all sorts of things, so we should just make the clang package ship the correct headers?  Or additionally, more correctly, we should have the host clang in this case build against the packaged headers, rather than the system ones?

Comment 35

[Nathan Froyd [:froydnj]]

Oh, I see, those builds are still using GCC 4.9, so of course they haven't gotten the newer headers yet.

Comment 36

[Mike Hommey [:glandium] (high latency)]

Oh, clang *does* have the 4.9 headers, so the problem is that they're actually not used.

Comment 37

[Kartikaya Gupta (email:[e-mail]) (away until Feb-2020)]

Using the magic of --gcc-toolchain I got static analysis working on Android: try push with patches. If this approach is acceptable I can put the patches up for review.

Comment 38

[Mike Hommey [:glandium] (high latency)]

Seems fair game to me.

Comment 39

[Kartikaya Gupta (email:[e-mail]) (away until Feb-2020)]

Attachment: Bug 1428158 - Fix static analysis errors in Android widgetry code. r?snorp

Just a few implicit constructors that need to be made explicit.

Comment 40

[Kartikaya Gupta (email:[e-mail]) (away until Feb-2020)]

Attachment: Bug 1428158 - Fix static analysis errors in arm64 JIT code. r?nbp

A few instances of snprintf are replaced with SprintfLiteral, and a
implicit constructor is made explicit.

Depends on D20400

Comment 41

[Kartikaya Gupta (email:[e-mail]) (away until Feb-2020)]

Attachment: Bug 1428158 - Enable static analysis on Android. r?glandium,froydnj

Depends on D20401

Comment 42

[Kartikaya Gupta (email:[e-mail]) (away until Feb-2020)]

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=04ab16c0d346825768875bf4825cf4a700f05f3f is a try push with all the android build jobs green

Comment 43

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1b8f3906f9cc
Fix static analysis errors in Android widgetry code. r=snorp
https://hg.mozilla.org/integration/autoland/rev/da84c1d96656
Fix static analysis errors in arm64 JIT code. r=nbp
https://hg.mozilla.org/integration/autoland/rev/89815abe4ecd
Enable static analysis on Android. r=glandium

Comment 44

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/1b8f3906f9cc
https://hg.mozilla.org/mozilla-central/rev/da84c1d96656
https://hg.mozilla.org/mozilla-central/rev/89815abe4ecd

Comment 45

[Masatoshi Kimura [:emk]]

This patch causes beta simulation failure:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=231229865&repo=try&lineNumber=5915

Comment 46

[Kartikaya Gupta (email:[e-mail]) (away until Feb-2020)]

Most likely that file needs a #include "mozilla/Sprintf.h" - can you try that? If you need I can write a proper patch.

Comment 47

[Masatoshi Kimura [:emk]]

Yes, adding #include "mozilla/Sprintf.h" fixed the issue:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=24e838e7e4e2e8724fa8b26ce3184c4a74bc28ec

Now I'm setting up Phabricator, so it would take some time to write a review-able patch.

Comment 48

[Masatoshi Kimura [:emk]]

Attachment: Bug 1428158 - Follow-up to fix beta simulation bustage. r?kats

Comment 49

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/179ed9bc4e22
Follow-up to fix beta simulation bustage. r=kats

Comment 50

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/179ed9bc4e22