1013341 - "make package" fails in ASan build using GCC 4.8.2

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Georg Koppen]

Building Fx 29 with ASan enabled and GCC 4.8.2 I got (in the packaging step):

"Error: /path/to/build/obj-x86_64-unknown-linux-gnu/browser/installer/package-manifest:549: Missing files(s): bin/llvm-symbolizer"

Seems this got introduced by bug 917242.

Comment 1

[Trevor Saunders (:tbsaunde)]

you can probably just add to the #ifdef MOZ_ASAN a check we're using clang.  I'm not sure off hand if we already have a define for that, but if not adding one shouldn't be that hard.

Comment 2

[Georg Koppen]

(In reply to Trevor Saunders (:tbsaunde) from comment #1)
> you can probably just add to the #ifdef MOZ_ASAN a check we're using clang. 
> I'm not sure off hand if we already have a define for that, but if not
> adding one shouldn't be that hard.

That is one solution, yes. But maybe the smarter one is to have an option whether to add a symbolizer in the packaging step at all given that at least GCC 4.8.x has no own symbolizer yet and one probably can use clang's instead: http://tsdgeos.blogspot.de/2014/03/asan-and-gcc-how-to-get-line-numbers-in.html

Comment 3

[Christian Holler (:decoder)]

Leaving the fact aside that we should fix this, why are you building with GCC anyway? As far as I know, the Clang support for ASan is much better and there's no reason not to use Clang, since we support it (and we even create release builds with it).

Comment 4

[Georg Koppen]

(In reply to Christian Holler (:decoder) from comment #3)
> Leaving the fact aside that we should fix this, why are you building with
> GCC anyway?

Because the browser we ship (Tor Browser) is just one part of a bundle containing a bunch of other software that is currently built with GCC. So it felt quite natural to not introduce an additional compiler in the build process but just upgrade the GCC version used. Especially as these ASan builds are currently only test builds for us.

Comment 5

[Christian Holler (:decoder)]

Especially for testing I can highly recommend Clang. ASan isn't the only thing available there, you also get TSan, UBSan, LSan and some other checkers that GCC lacks. Not all of these are usable on Firefox, since our codebase has quite a few races and undefined behavior, but smaller programs can be tested quite well.

Comment 6

[Trevor Saunders (:tbsaunde)]

(In reply to Georg Koppen from comment #2)
> (In reply to Trevor Saunders (:tbsaunde) from comment #1)
> > you can probably just add to the #ifdef MOZ_ASAN a check we're using clang. 
> > I'm not sure off hand if we already have a define for that, but if not
> > adding one shouldn't be that hard.
> 
> That is one solution, yes. But maybe the smarter one is to have an option
> whether to add a symbolizer in the packaging step at all given that at least
> GCC 4.8.x has no own symbolizer yet and one probably can use clang's
> instead:
> http://tsdgeos.blogspot.de/2014/03/asan-and-gcc-how-to-get-line-numbers-in.
> html

judging by https://gcc.gnu.org/ml/gcc-patches/2013-12/msg00558.html I'd guess after gcc 4.8 your not going to need a symbolizer, so it hardly seems worth bothering.

(In reply to Christian Holler (:decoder) from comment #3)
> Leaving the fact aside that we should fix this, why are you building with
> GCC anyway? As far as I know, the Clang support for ASan is much better and

its certainly not going to get better if nobody uses it ;) but its kind of off topic.

Comment 7

[Georg Koppen]

Attachment: Adding a CLANG_CXX check.

Trying to follow the idea in comment 1: Including the llvm-symbolizer only if CLANG_CXX is defined. I tested this with my GCC setup and it worked.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/98644e8a0e12

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/98644e8a0e12

Comment 10

[Georg Koppen]

Comment on attachment 8428597 [details] [diff] [review]
Adding a CLANG_CXX check.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 917242
User impact if declined: Downstream builds based on ESR can't be hardened with GCC >= 4.8.x and get deployed
Testing completed (on m-c, etc.): nothing besides landing the patch on mozilla-inbound and m-c
Risk to taking this patch (and alternatives if risky): none
String or IDL/UUID changes made by this patch: none

Comment 11

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8428597 [details] [diff] [review]
Adding a CLANG_CXX check.

Aurora approval granted.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/780cfe21c209