Closed Bug 1428158 Opened 2 years ago Closed 10 months ago

run static analysis on Android builds

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox59 wontfix, firefox67 fixed)

RESOLVED FIXED
mozilla67
Tracking Status
firefox59 --- wontfix
firefox67 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

References

(Blocks 1 open bug)

Details

Attachments

(10 files, 7 obsolete files)

1.04 KB, patch
nalexander
: review+
Details | Diff | Splinter Review
6.31 KB, patch
nalexander
: review+
Details | Diff | Splinter Review
2.44 KB, patch
nalexander
: review+
Details | Diff | Splinter Review
1.27 KB, patch
Details | Diff | Splinter Review
3.12 KB, patch
glandium
: review-
Details | Diff | Splinter Review
1.31 KB, patch
nalexander
: review+
Details | Diff | Splinter Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
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.
For the mean time i think this can be avoidable by running the clang-tidy based static analysis within the mach.
Depends on: 1435409
Product: Core → Firefox Build System
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.
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.
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.
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.
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.
Attachment #8958895 - Attachment is obsolete: true
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.
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!
Flags: needinfo?(bpostelnicu)
Blocks: 1223932
Attached patch part6.diff (obsolete) — Splinter Review
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
Flags: needinfo?(bpostelnicu)
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?
Flags: needinfo?(bpostelnicu)
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.
Sorry, I think comment 11 was based on some local changes in my tree.
Andi, can you file a separate bug for comment 9 so we can get that patch reviewed and into the tree?
Depends on: 1454667
(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
Flags: needinfo?(bpostelnicu)
Attachment #8961303 - Attachment is obsolete: true
Attachment #8958893 - Attachment is obsolete: true
Depends on: 1463847
Thanks to the weirdness that is our host shared library build situation.
Attachment #8980055 - Flags: review?(core-build-config-reviews)
Attachment #8958892 - Attachment is obsolete: true
Attachment #8958894 - Attachment is obsolete: true
Attachment #8958949 - Attachment is obsolete: true
Attachment #8959195 - Attachment is obsolete: true
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.
Attachment #8980056 - Flags: review?(core-build-config-reviews)
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.
Attachment #8980057 - Flags: review?(core-build-config-reviews)
...and remove some outdated relaxed requirements for libstdc++ in the
process, since we now require 3.4.15.
Attachment #8980058 - Flags: review?(core-build-config-reviews)
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?
Attachment #8980059 - Flags: review?(core-build-config-reviews)
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.
Attachment #8980060 - Flags: review?(core-build-config-reviews)
Assignee: nobody → nfroyd
Attachment #8980055 - Flags: review?(core-build-config-reviews) → review+
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?
Attachment #8980056 - Flags: review?(core-build-config-reviews) → review+
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...
Attachment #8980057 - Flags: review?(core-build-config-reviews) → review+
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+.
Attachment #8980058 - Flags: review?(core-build-config-reviews) → review+
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 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!
Attachment #8980060 - Flags: review?(core-build-config-reviews) → review+
Attachment #8980059 - Flags: review?(core-build-config-reviews) → review?(mh+mozilla)
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)
Attachment #8980058 - Flags: review+
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.
Attachment #8980059 - Flags: review?(mh+mozilla) → review-
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).
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.
(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.
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?
> ../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?
(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.
(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?
Oh, I see, those builds are still using GCC 4.9, so of course they haven't gotten the newer headers yet.
Oh, clang *does* have the 4.9 headers, so the problem is that they're actually not used.
Depends on: 1470502, 1470452, 1451104

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.

Blocks: 1500941
Flags: needinfo?(nfroyd)
Flags: needinfo?(mh+mozilla)

Seems fair game to me.

Flags: needinfo?(mh+mozilla)

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

Depends on D20400

Pushed by kgupta@mozilla.com:
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
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
Flags: needinfo?(kats)

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

Flags: needinfo?(kats) → needinfo?(VYV03354)

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.

Flags: needinfo?(VYV03354)
Pushed by VYV03354@nifty.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/179ed9bc4e22
Follow-up to fix beta simulation bustage. r=kats
You need to log in before you can comment on or make changes to this bug.