Reland "Enable continuous mode/runtime counter relocation for coverag…

…e builds" This is a reland of commit 0265879 The original change was reverted because this enabled the continuous mode on PGO bots, causing them to fail. The continuous mode was enabled everywhere by the change made in the original CL to test_env.py. It is believed that the continuous mode is incompatible with pgo builds because PGO builds make manual calls to compiler-rt to flush the counters. To address this, this reland wil omit the changes made to test_env.py. Instead, continuous mode will be enabled on the bots later by updating the recipe, which is in a separate repository. Original change's description: > Enable continuous mode/runtime counter relocation for coverage builds > > Enabling the continuous mode for coverage builds allows us to recover > coverage data for test processes run in sandboxes while avoiding the CQ > failures that we see with the noncontinuous mode that flushes all the > counters to disk at process shutdown. This should also be expected to > recover coverage data for tests that exit abnormally. > > Enabling the continuous mode is also the first step in landing several > changes that will potentially improve instrumented test performance. > > > Cq-Include-Trybots: luci.chromium.try:chromeos-amd64-generic-siso-rel > Cq-Include-Trybots: luci.chromium.try:ios-simulator-siso > Cq-Include-Trybots: luci.chromium.try:linux_chromium_asan_siso_rel_ng > Cq-Include-Trybots: luci.chromium.try:mac-siso-rel > Change-Id: Iab1177b2e8f5a24887a33417d1556c90f1002363 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4953786 > Reviewed-by: Junji Watanabe <jwata@google.com> > Reviewed-by: danakj <danakj@chromium.org> > Reviewed-by: Ben Pastene <bpastene@chromium.org> > Reviewed-by: Hans Wennborg <hans@chromium.org> > Commit-Queue: Alan Zhao <ayzhao@google.com> > Reviewed-by: Dominic Farolino <dom@chromium.org> > Reviewed-by: Prakhar Asthana <pasthana@google.com> > Cr-Commit-Position: refs/heads/main@{#1235959} Bug: 1462187,1468343,1510916 Change-Id: I8a065f0a5886a53f5509491796501dd5b288c99a Cq-Include-Trybots: luci.chromium.try:chromeos-amd64-generic-siso-rel Cq-Include-Trybots: luci.chromium.try:ios-simulator-siso Cq-Include-Trybots: luci.chromium.try:linux_chromium_asan_siso_rel_ng Cq-Include-Trybots: luci.chromium.try:mac-siso-rel Cq-Include-Trybots: chrome/try:linux-pgo,mac-pgo,win64-pgo Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5122344 Commit-Queue: Prakhar Asthana <pasthana@google.com> Reviewed-by: Junji Watanabe <jwata@google.com> Reviewed-by: Prakhar Asthana <pasthana@google.com> Reviewed-by: Dominic Farolino <dom@chromium.org> Reviewed-by: danakj <danakj@chromium.org> Reviewed-by: Hans Wennborg <hans@chromium.org> Commit-Queue: Alan Zhao <ayzhao@google.com> Cr-Commit-Position: refs/heads/main@{#1245683}
JAJAJAwe · Jan 11, 2024 · f8fa313 · f8fa313
1 parent c2c6349
commit f8fa313
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 5 deletions.
diff --git a/base/test/clang_profiling.cc b/base/test/clang_profiling.cc
@@ -23,9 +23,13 @@ void WriteClangProfilingProfile() {
   base::AutoLock auto_lock(*lock);
 
 // Fuchsia's profile runtime does not handle profile dumping.
-#if !BUILDFLAG(IS_FUCHSIA)
+// Coverage builds are built with runtime counter relocation and are expected to
+// be run under continuous coverage mode (enabled by adding %c to the
+// LLVM_PROFILE_FILE environment variable), which updates counters in real time,
+// so __llvm_profile_dump() is not needed.
+#if !BUILDFLAG(IS_FUCHSIA) && !BUILDFLAG(USE_CLANG_COVERAGE)
   __llvm_profile_dump();
-#endif  // !BUILDFLAG(IS_FUCHSIA)
+#endif  // !BUILDFLAG(IS_FUCHSIA) && !BUILDFLAG(USE_CLANG_COVERAGE)
 }
 
 }  // namespace base
diff --git a/build/config/coverage/BUILD.gn b/build/config/coverage/BUILD.gn
@@ -29,6 +29,8 @@ config("default_coverage") {
     cflags = [
       "-fprofile-instr-generate",
       "-fcoverage-mapping",
+      "-mllvm",
+      "-runtime-counter-relocation=true",
 
       # Following experimental flags removes unused header functions from the
       # coverage mapping data embedded in the test binaries, and the reduction
@@ -43,7 +45,10 @@ config("default_coverage") {
     # tools that will be used to process the coverage output. This is because
     # the coverage file format is not stable.
     if (use_chromium_rust_toolchain) {
-      rustflags += [ "-Cinstrument-coverage" ]
+      rustflags += [
+        "-Cinstrument-coverage",
+        "-Cllvm-args=-runtime-counter-relocation",
+      ]
     }
 
     if (is_linux || is_chromeos) {

diff --git a/build/config/siso/clang_code_coverage_wrapper.star b/build/config/siso/clang_code_coverage_wrapper.star
@@ -17,6 +17,8 @@ load("@builtin//struct.star", "module")
 _COVERAGE_FLAGS = [
     "-fprofile-instr-generate",
     "-fcoverage-mapping",
+    "-mllvm",
+    "-runtime-counter-relocation=true",
     # Following experimental flags remove unused header functions from the
     # coverage mapping data embedded in the test binaries, and the reduction
     # of binary size enables building Chrome's large unit test targets on

diff --git a/build/toolchain/clang_code_coverage_wrapper.py b/build/toolchain/clang_code_coverage_wrapper.py
@@ -63,6 +63,8 @@
 _COVERAGE_FLAGS = [
     '-fprofile-instr-generate',
     '-fcoverage-mapping',
+    '-mllvm',
+    '-runtime-counter-relocation=true',
     # Following experimental flags remove unused header functions from the
     # coverage mapping data embedded in the test binaries, and the reduction
     # of binary size enables building Chrome's large unit test targets on

diff --git a/content/child/child_thread_impl.cc b/content/child/child_thread_impl.cc
@@ -398,7 +398,7 @@ class ChildThreadImpl::IOThreadState
 #if BUILDFLAG(IS_POSIX)
     // Take the file descriptor so that |file| does not close it.
     base::ScopedFD fd(file.TakePlatformFile());
-#if BUILDFLAG(CLANG_PGO)
+#if BUILDFLAG(CLANG_PGO) || BUILDFLAG(USE_CLANG_COVERAGE)
     FILE* f = fdopen(fd.release(), "r+b");
     __llvm_profile_set_file_object(f, 1);
 #else

diff --git a/docs/testing/code_coverage.md b/docs/testing/code_coverage.md
@@ -186,8 +186,15 @@ mode, where `N` is the number of raw profiles. With `N = 4`, the total size of
 the raw profiles are limited to a few gigabytes. (If working on Android, the
 .profraw files will be located in ./out/coverage/coverage by default.)
 
+Additionally, we also recommend enabling the continuous mode by adding the `%c`
+pattern to `LLVM_PROFILE_FILE`. The continuous mode updates counters in real
+time instead of flushing to disk at process exit. This recovers coverage data
+from tests that exit abnormally (e.g. death tests). Furthermore, the continuous
+mode is required to recover coverage data for tests that run in sandboxed
+processes. For more information, see crbug.com/1468343.
+
 ```
-$ export LLVM_PROFILE_FILE="out/report/crypto_unittests.%4m.profraw"
+$ export LLVM_PROFILE_FILE="out/report/crypto_unittests.%4m%c.profraw"
 $ ./out/coverage/crypto_unittests
 $ ls out/report/
 crypto_unittests.3657994905831792357_0.profraw