Coverage for rust projects with clang 13 #6449

catenacyber · 2021-09-15T15:09:45Z

As discussed in https://reviews.llvm.org/D104556 since clang supports only one version of profraw files at a time, we need for llvm-based languages such as rust and swift, to use the right version of clang

For rust, this is now clang 13, (as clang 14 introduced some changes)

install_rust.sh now installs clang , using checkout_build_install_llvm.sh by giving it an argument of the revision to build (we need also clang13 on the building side and not only llvm-profdata-13, so that multiple languages project produce consistent profraw file)
compile script now copies llvm-profdata and llvm-covin $OUT if needed (as is done for llvm-symbolizer)
coverage script takes specific llvm-profdata if any

The same change likely needs to be done for swift, but with clang 12 I guess.

catenacyber · 2021-09-15T15:10:54Z

cc @jonathanmetzman @inferno-chromium
Test is to run

python3 infra/helper.py build_image --no-pull base-clang
python3 infra/helper.py build_image --no-pull base-builder
python3 infra/helper.py build_image --no-pull base-builder-rust
python3 infra/helper.py build_image --no-pull base-runner
export PROJECT=suricata
python3 infra/helper.py build_image --no-pull $PROJECT
python3 infra/helper.py build_fuzzers --sanitizer coverage $PROJECT
python3 infra/helper.py coverage --no-corpus-download $PROJECT

catenacyber · 2021-09-15T15:11:43Z

infra/base-images/base-clang/Dockerfile

 # Keep all steps in the same script to decrease the number of intermediate
 # layes in docker file.
 RUN /root/checkout_build_install_llvm.sh
-RUN rm /root/checkout_build_install_llvm.sh


checkout_build_install_llvm.sh is now reused by install_rust

catenacyber · 2021-09-15T15:12:07Z

infra/base-images/base-clang/checkout_build_install_llvm.sh

-apt-get remove --purge -y $LLVM_DEP_PACKAGES
-apt-get autoremove -y
+if [ $# -eq 0 ] ; then
+  apt-get remove --purge -y $LLVM_DEP_PACKAGES


do not remove git in base-builder-rust image

Please remove this change.
If it's just a matter of installing git, then install it in the install_rust.sh script. There's tons of other stuff here being installed that should be removed. As is this pretty hacky.

catenacyber · 2021-09-15T15:12:28Z

projects/suricata/project.yaml

@@ -1,5 +1,5 @@
 homepage: "https://suricata-ids.org"
-language: c++
+language: rust


so that we get rust llvm-cov

I'm not sure how this works.
Here's my understanding.
suricata's c++ code is compiled with clang 14. suricata's rust code is compiled with clang 13.
Are the profraw files produced by suricata coverage builds compatible with clang 13 but not clang 14?

catenacyber · 2021-09-16T15:12:13Z

CI is failing because it does not rebuild base-clang

inferno-chromium · 2021-09-19T04:25:24Z

This version mgmt is getting messy, can we spend time to make things work with clang 13

catenacyber · 2021-09-19T19:03:04Z

This version mgmt is getting messy, can we spend time to make things work with clang 13

Things (that is rust coverage) work with clang 13.
The purpose of this PR is to use clang 13 for rust, while keeping clang 14 for for C/C++

For swift, I think we will need clang 12
Each of these version produce a different version of profraw files (and has a different llvm-profdata which reads only one version)
For multi languages projects (suricata, grpc-swift, etc..), I think we need to use the right clang to have a consistent profraw file.

What would you suggest ?

jonathanmetzman · 2021-09-24T16:46:01Z

This version mgmt is getting messy, can we spend time to make things work with clang 13

C++ is use clang14, we should strive to get everything working for clang14.

Things (that is rust coverage) work with clang 13.

Does rust not work with clang 14?

The purpose of this PR is to use clang 13 for rust, while keeping clang 14 for for C/C++

For swift, I think we will need clang 12
Each of these version produce a different version of profraw files (and has a different llvm-profdata which reads only one version)

Can you explain why? How long would it take to fix this issue?

For multi languages projects (suricata, grpc-swift, etc..), I think we need to use the right clang to have a consistent profraw file.

What would you suggest ?

My preference is to get everything working for one version of clang and we only use that.
That version of clang probably shouldn't be clang12. It should probably as recent clang as possible. Maybe it can be clang 13 for now.

jonathanmetzman

I think this change is going to introduce complexity that we will regret.
I'd really like to be using the same clang version of everywhere to avoid this. How easy will that be to do?

jonathanmetzman · 2021-09-24T16:47:08Z

infra/base-images/base-builder/install_rust.sh

+unset CFLAGS
+unset CXXFLAGS
+# so that compile_libfuzzer can copy, without knowing the clang version
+rm /usr/local/lib/clang/*/lib/linux/libclang_rt.fuzzer-*


This uses an older version of libfuzzer for some of our projects right?
This can be a huge hassle for us and I don't think we want it.

jonathanmetzman · 2021-09-24T16:47:36Z

infra/base-images/base-builder/install_rust.sh

+(
+unset CFLAGS
+unset CXXFLAGS
+# so that compile_libfuzzer can copy, without knowing the clang version


Please begin comment with captial and end with peirod.

jonathanmetzman · 2021-09-24T16:49:47Z

infra/base-images/base-clang/checkout_build_install_llvm.sh

-apt-get remove --purge -y $LLVM_DEP_PACKAGES
-apt-get autoremove -y
+if [ $# -eq 0 ] ; then
+  apt-get remove --purge -y $LLVM_DEP_PACKAGES


Please remove this change.
If it's just a matter of installing git, then install it in the install_rust.sh script. There's tons of other stuff here being installed that should be removed. As is this pretty hacky.

jonathanmetzman · 2021-09-24T16:52:23Z

infra/base-images/base-runner/coverage

 cd $OUT

+# copy version/language-specific coverage tools if any
+cp $OUT/llvm-cov /usr/local/bin || true


Not a fan of this. We copy llvm-cov and llvm-profdata into the build for rust and swift and then copy those from build into /usr/local/bin if they exist?
Here's a way I like more (assuming we end up using multiple versions of clang): Copy llvm-cov and llvm-profdata into build for all fuzzers no matter the language, then instead of copying those into /usr/local/bin, use them directly.

jonathanmetzman · 2021-09-24T16:54:15Z

projects/suricata/project.yaml

@@ -1,5 +1,5 @@
 homepage: "https://suricata-ids.org"
-language: c++
+language: rust


I'm not sure how this works.
Here's my understanding.
suricata's c++ code is compiled with clang 14. suricata's rust code is compiled with clang 13.
Are the profraw files produced by suricata coverage builds compatible with clang 13 but not clang 14?

jonathanmetzman · 2021-09-24T16:56:59Z

This change makes rust projects use a different libfuzzer than c++. I think this is a bad idea.

catenacyber · 2021-09-24T16:59:07Z

This got replaced by #6517, forgot to close

catenacyber · 2021-09-24T17:00:42Z

Suricata needs to be labelled as rust project because we need rust std coverage
And there is a hack to get it for projects marked as rust
cf
https://github.com/google/oss-fuzz/blob/master/infra/base-images/base-builder/compile#L166

catenacyber · 2021-09-24T17:02:43Z

This version mgmt is getting messy, can we spend time to make things work with clang 13

C++ is use clang14, we should strive to get everything working for clang14.

#6517 does that

Things (that is rust coverage) work with clang 13.

Does rust not work with clang 14?

Nope
I can make a long explanation...

The purpose of this PR is to use clang 13 for rust, while keeping clang 14 for for C/C++
For swift, I think we will need clang 12
Each of these version produce a different version of profraw files (and has a different llvm-profdata which reads only one version)

Can you explain why? How long would it take to fix this issue?

For multi languages projects (suricata, grpc-swift, etc..), I think we need to use the right clang to have a consistent profraw file.
What would you suggest ?

My preference is to get everything working for one version of clang and we only use that.
That version of clang probably shouldn't be clang12. It should probably as recent clang as possible. Maybe it can be clang 13 for now.

#6517 makes it work with clang 14

jonathanmetzman · 2021-09-24T17:43:33Z

This version mgmt is getting messy, can we spend time to make things work with clang 13

C++ is use clang14, we should strive to get everything working for clang14.

#6517 does that

Things (that is rust coverage) work with clang 13.

Does rust not work with clang 14?

Nope
I can make a long explanation...

The purpose of this PR is to use clang 13 for rust, while keeping clang 14 for for C/C++
For swift, I think we will need clang 12
Each of these version produce a different version of profraw files (and has a different llvm-profdata which reads only one version)

Can you explain why? How long would it take to fix this issue?

For multi languages projects (suricata, grpc-swift, etc..), I think we need to use the right clang to have a consistent profraw file.
What would you suggest ?

My preference is to get everything working for one version of clang and we only use that.
That version of clang probably shouldn't be clang12. It should probably as recent clang as possible. Maybe it can be clang 13 for now.

#6517 makes it work with clang 14

Is it possible to briefly explain the issue with clang 14 and rust?

jonathanmetzman · 2021-09-24T17:43:51Z

I want to understand, but I don't want to waste your time explaining this to me.

catenacyber · 2021-09-24T18:21:41Z

Rust and Swift compiler use llvm (I do not fully understand how much)
For coverage information, they use in the end some llvm library (compiler-rt I guess) which generates the profraw file
Current Rust nightly uses llvm 13, upgrading to llvm 14 seems not trivial cf https://rustc-dev-guide.rust-lang.org/backend/updating-llvm.html
Current Swift compiler uses llvm 10 (it uses llvm 13 on MacOS, but 10 on linux)

llvm 12, 13 and 14 generate different profraw file formats, and their llvm-cov/llvm-profdata tools can only read their current file format

llvm 13 added binary ids, and bumped version from 5 to 7 : the script adds the field in the header (and says the length of these binary ids is zero)
llvm 14 changed some addresses to be relative offsets instead of absolute, and is keeping version at 7 (we go through the list and recompute the relative offsets instead of the absolute ones)

catenacyber added 8 commits September 15, 2021 14:24

rust: use specific clang-13 for coverage

d9b638a

fixu

3197ce3

Recompile

dc6c937

forget CFLAGS to compile clang

e5d87c7

do not remove git from base-builder-rust

d6e62c7

hack for compile_libfuzzer copy

6503805

Suricata as a rust project

9be75aa

comment

ecb3d51

catenacyber commented Sep 15, 2021

View reviewed changes

catenacyber mentioned this pull request Sep 24, 2021

Fix for rust and swift coverages #6517

Merged

jonathanmetzman requested changes Sep 24, 2021

View reviewed changes

catenacyber closed this Sep 24, 2021

Coverage for rust projects with clang 13 #6449

Coverage for rust projects with clang 13 #6449

Uh oh!

Conversation

catenacyber commented Sep 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

catenacyber commented Sep 15, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

catenacyber commented Sep 16, 2021

Uh oh!

inferno-chromium commented Sep 19, 2021

Uh oh!

catenacyber commented Sep 19, 2021

Uh oh!

jonathanmetzman commented Sep 24, 2021

Uh oh!

jonathanmetzman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonathanmetzman commented Sep 24, 2021

Uh oh!

catenacyber commented Sep 24, 2021

Uh oh!

catenacyber commented Sep 24, 2021

Uh oh!

catenacyber commented Sep 24, 2021

Uh oh!

jonathanmetzman commented Sep 24, 2021

Uh oh!

jonathanmetzman commented Sep 24, 2021

Uh oh!

catenacyber commented Sep 24, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

catenacyber commented Sep 15, 2021 •

edited

Loading