1471339 - Enable Rust coverage in Linux builds

Status: RESOLVED FIXED

(Testing :: Code Coverage, enhancement)

Description

[Marco Castelluccio [:marco]]

We have two options:
1) Switch the Linux ccov builds to use Clang;
2) Keep using GCC, manually link to clang_rt.profile-x86_64.

Locally the GCC build works with clang_rt.profile-x86_64 (after removing the __gcov_flush symbol from this library, as otherwise the linking fails as the symbol is defined twice). It might be the fasteset avenue, compared to fighting with the test failures in the Clang build.

Comment 1

[Marco Castelluccio [:marco]]

Attachment: Update Rust Nightly to 2018-06-23

Comment 2

[Marco Castelluccio [:marco]]

Attachment: Use Nightly Rust for coverage builds on Linux

Comment 3

[Marco Castelluccio [:marco]]

Attachment: Introduce clang 7 toolchain build

Comment 4

[Marco Castelluccio [:marco]]

Attachment: Use clang 7 for Linux ccov builds

Comment 5

[Marco Castelluccio [:marco]]

Attachment: Rename LLVM's __gcov_flush function to __llvm_gcov_flush to avoid naming clashes with GCC profiling library

Comment 6

[Marco Castelluccio [:marco]]

Attachment: Link with clang_rt.profile-x86_64 library in code coverage builds

Comment 7

[Marco Castelluccio [:marco]]

Attachment: Add RUSTFLAGS to Linux coverage build to enable gcov profiling for Rust

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8991281 [details] [diff] [review]
Use Nightly Rust for coverage builds on Linux

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

will we switch back?  if so we should have a bug on file and maybe a comment here referencing the bug.

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8991284 [details] [diff] [review]
Use clang 7 for Linux ccov builds

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

I am assuming this is not causing other problems in the builds or tests.

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8991287 [details] [diff] [review]
Add RUSTFLAGS to Linux coverage build to enable gcov profiling for Rust

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

big old rubber stamp

Comment 11

[Mike Hommey [:glandium]]

Comment on attachment 8991280 [details] [diff] [review]
Update Rust Nightly to 2018-06-23

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

::: taskcluster/ci/toolchain/linux.yml
@@ +454,5 @@
>      run:
>          using: toolchain-script
>          script: repack_rust.py
>          arguments: [
> +            '--channel', 'nightly-2018-06-23',

might as well update the the last version available when you land, which will get you a version with llvm 7.

Comment 12

[Mike Hommey [:glandium]]

Comment on attachment 8991285 [details] [diff] [review]
Rename LLVM's __gcov_flush function to __llvm_gcov_flush to avoid naming clashes with GCC profiling library

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

Why would we have naming clashes? Because of bug 1409276?

Comment 13

[Mike Hommey [:glandium]]

Comment on attachment 8991286 [details] [diff] [review]
Link with clang_rt.profile-x86_64 library in code coverage builds

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

::: toolkit/library/moz.build
@@ +350,5 @@
> +
> +# This library contains functions needed for gcov profiling, which Rust uses
> +# when compiled with -Zprofile.
> +if CONFIG['MOZ_CODE_COVERAGE']:
> +    OS_LIBS += ['clang_rt.profile-x86_64']

the windows builds do that through LDFLAGS in the mozconfig. Which sounds better because libxul is not the only thing we link, although at the moment it happens to be the only one that links rust, but that's not going to stay true.

Comment 14

[Marco Castelluccio [:marco]]

(In reply to Mike Hommey [:glandium] from comment #13)
> Comment on attachment 8991286 [details] [diff] [review]
> Link with clang_rt.profile-x86_64 library in code coverage builds
> 
> Review of attachment 8991286 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/library/moz.build
> @@ +350,5 @@
> > +
> > +# This library contains functions needed for gcov profiling, which Rust uses
> > +# when compiled with -Zprofile.
> > +if CONFIG['MOZ_CODE_COVERAGE']:
> > +    OS_LIBS += ['clang_rt.profile-x86_64']
> 
> the windows builds do that through LDFLAGS in the mozconfig. Which sounds
> better because libxul is not the only thing we link, although at the moment
> it happens to be the only one that links rust, but that's not going to stay
> true.

The problem is that adding it to LDFLAGS doesn't add it at the end of the linker command, so the gcov symbols get removed.
Any suggestion how to work around this?

Comment 15

[Marco Castelluccio [:marco]]

(In reply to Mike Hommey [:glandium] from comment #12)
> Comment on attachment 8991285 [details] [diff] [review]
> Rename LLVM's __gcov_flush function to __llvm_gcov_flush to avoid naming
> clashes with GCC profiling library
> 
> Review of attachment 8991285 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Why would we have naming clashes? Because of bug 1409276?

Both GCC and LLVM are providing __gcov_flush. We are compiling the C++ code with GCC and the Rust code with LLVM, so in the end we end up with two __gcov_flush symbols.

Comment 16

[Mike Hommey [:glandium]]

Why not do the C++ compilation for those coverage builds with clang? Wouldn't clang even add clang_rt.profile-x86_64 to its linker invocation on its own?

Comment 17

[Marco Castelluccio [:marco]]

(In reply to Mike Hommey [:glandium] from comment #16)
> Why not do the C++ compilation for those coverage builds with clang?
> Wouldn't clang even add clang_rt.profile-x86_64 to its linker invocation on
> its own?

We can do it but it will require more time to get it fully working (there are many tests failing or timing out when switching to Clang, so we need to tweak their timeouts, disable the ones we can't fix, etc.). Also, GCC currently has more precise gcov coverage information.

Comment 18

[Mike Hommey [:glandium]]

(In reply to Marco Castelluccio [:marco] from comment #14)
> The problem is that adding it to LDFLAGS doesn't add it at the end of the
> linker command, so the gcov symbols get removed.
> Any suggestion how to work around this?

Try setting LIBS instead of LDFLAGS (from the mozconfig).

Comment 19

[Marco Castelluccio [:marco]]

Attachment: link_clang_profile

(In reply to Mike Hommey [:glandium] from comment #18)
> (In reply to Marco Castelluccio [:marco] from comment #14)
> > The problem is that adding it to LDFLAGS doesn't add it at the end of the
> > linker command, so the gcov symbols get removed.
> > Any suggestion how to work around this?
> 
> Try setting LIBS instead of LDFLAGS (from the mozconfig).

Thanks, it worked!

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4bd5799f38ea
Update Rust Nightly to 2018-07-18. r=glandium
https://hg.mozilla.org/integration/mozilla-inbound/rev/a2c059da18d6
Use Nightly Rust for coverage builds on Linux. r=jmaher
https://hg.mozilla.org/integration/mozilla-inbound/rev/8c07f4beab64
Introduce clang 7 toolchain build. r=glandium
https://hg.mozilla.org/integration/mozilla-inbound/rev/4506b20cab42
Use clang 7 for Linux ccov builds. r=jmaher
https://hg.mozilla.org/integration/mozilla-inbound/rev/a4e32af75247
Rename LLVM's __gcov_flush function to __llvm_gcov_flush to avoid naming clashes with GCC profiling library. r=glandium
https://hg.mozilla.org/integration/mozilla-inbound/rev/3424dbb3eaa5
Link with clang_rt.profile-x86_64 library in Linux code coverage builds. r=glandium
https://hg.mozilla.org/integration/mozilla-inbound/rev/76540384ad3d
Add RUSTFLAGS to Linux coverage build to enable gcov profiling for Rust. r=jmaher

Comment 21

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/4bd5799f38ea
https://hg.mozilla.org/mozilla-central/rev/a2c059da18d6
https://hg.mozilla.org/mozilla-central/rev/8c07f4beab64
https://hg.mozilla.org/mozilla-central/rev/4506b20cab42
https://hg.mozilla.org/mozilla-central/rev/a4e32af75247
https://hg.mozilla.org/mozilla-central/rev/3424dbb3eaa5
https://hg.mozilla.org/mozilla-central/rev/76540384ad3d

Comment 22

[Marco Castelluccio [:marco]]

The patches worked this morning (https://treeherder.mozilla.org/#/jobs?repo=try&revision=a8293a63025fe3664108614cbe3ae94d5cb1850e), but are broken now (https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&revision=690cb3015db6645b335ac4835a50073cb6a3e23c&filter-searchStr=ccov) for failures while linking dump_syms.
Something must have changed really recently.

Comment 23

[Bogdan Tara[:bogdan_tara | bogdant]]

Backed out changeset 76540384ad3d (bug 1471339) by marco's request.
https://hg.mozilla.org/mozilla-central/rev/4aa8eb6e5ca7

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f065635c180e
Use Clang 7 for opt Linux ccov builds too. r=me

Comment 25

[Tiberius Oros[:tiberius_oros]]

https://hg.mozilla.org/mozilla-central/rev/f065635c180e

Comment 26

[Marco Castelluccio [:marco]]

All the prerequisites landed, but the patch to actually enable it is backed out (errors while linking dump_syms).

Comment 27

[Intermittent Failures Robot]

1 failures in 653 pushes (0.002 failures/push) were associated with this bug in the last 7 days.    

Repository breakdown:
* mozilla-central: 1

Platform breakdown:
* linux64: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1471339&startday=2018-07-16&endday=2018-07-22&tree=trunk

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5cd3b13d5eb9
Add RUSTFLAGS to Linux coverage build to enable gcov profiling for Rust. r=jmaher

Comment 29

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/5cd3b13d5eb9