Improve Native Coverage latency

* The current implementation works by sending signal 37 to
the testing process and busy-looping for 60 seconds or until it
has witnessed the testing process act upon the signal. Sometimes
we miss the signal completely which leads to a 60 second overhead
to the test runtime. Changing the deadline is infeasible as we
don't have any estimation on how long does it take to dump the coverage.

* This CL improves the current implementation by completely eliminating both
problems, we don't busy loop instead we call directly the method to dump
the profile data to the disk and then we exit, there is an introduced
latency for loading the native library but that should be negligble.

* Maintained backward compatibility with previous use-case by falling
back to the old implementation if we fail to load the native library.

* The next step should be migrating everyone over to the new way and
incorprating this change directly into Soong so that it would add
the native library + java dependency to all of the targets.

Bug: 329680904
Test: atest NetHttpCoverageTests && confirmed that coverage did not change
Change-Id: I966f4e2923b75258831857af3d68bd72685d93f6

3 files changed, 114 insertions, 1 deletions

diff --git a/java/com/android/modules/utils/testing/NativeCoverageHackInstrumentationListener.java b/java/com/android/modules/utils/testing/NativeCoverageHackInstrumentationListener.java
index 438789a..5b3ee94 100644
--- a/java/com/android/modules/utils/testing/NativeCoverageHackInstrumentationListener.java
+++ b/java/com/android/modules/utils/testing/NativeCoverageHackInstrumentationListener.java
@@ -35,6 +35,11 @@ import java.util.Map;
  *
  * This is necessary as native coverage is not dumped automatically after Java/JNI tests
  * (b/185074329) and should be replaced by more generic solutions (b/185822084).
+ *
+ * Improve the current state of CLANG coverage by directly calling the __llvm_profile_write_data
+ * methods through JNI instead of sending signal (37) and monitoring the SIG status of the process
+ * which can sometimes read to a race condition also the app never knows when it finishes dumping
+ * the profile data.
  */
 public class NativeCoverageHackInstrumentationListener extends InstrumentationRunListener {
     private static final String LOG_TAG =
@@ -45,10 +50,36 @@ public class NativeCoverageHackInstrumentationListener extends InstrumentationRu
 
     @Override
     public void testRunFinished(Result result) throws Exception {
-        maybeDumpNativeCoverage();
+        // This will first try to dump coverage through the deterministic way of loading
+        // a shared library and executing a JNI method. If that fails, we don't want to crash
+        // as some modules might not be bundling the nativecoverage.so yet!, fallback to
+        // the old signal method even if it is lses efficient.
+        // The fallback should be removed when everyone has bundled nativecoverage.so as part
+        // of their testing APK.
+        try {
+            System.loadLibrary("nativecoverage");
+            dumpCoverage();
+        } catch (UnsatisfiedLinkError e) {
+            // TODO(b/329680904): Remove fallback once the main method has been well-tested.
+            Log.w(
+                    LOG_TAG,
+                    "Failed to load libnativecoverage.so! Falling back to old way,"
+                            + " exception: %s",
+                    e);
+            maybeDumpNativeCoverage();
+        } catch (Exception e) {
+            // TODO(b/329680904): Remove fallback once the main method has been well-tested.
+            Log.w(
+                    LOG_TAG,
+                    "Failed to dump coverage through JNI. Falling back to old way, exception: %s",
+                    e);
+            maybeDumpNativeCoverage();
+        }
         super.testRunFinished(result);
     }
 
+    public native void dumpCoverage();
+
     /**
      * If this test process is instrumented for native coverage, then trigger a dump
      * of the coverage data and wait until either we detect the dumping has finished or 60 seconds,
diff --git a/java/com/android/modules/utils/testing/coverage_wrapper/Android.bp b/java/com/android/modules/utils/testing/coverage_wrapper/Android.bp
new file mode 100644
index 0000000..1821e35
--- /dev/null
+++ b/java/com/android/modules/utils/testing/coverage_wrapper/Android.bp
@@ -0,0 +1,31 @@
+//
+// Copyright (C) 2024 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+cc_library_shared {
+    name: "libnativecoverage",
+    cflags: [
+        "-Wall",
+        "-Werror",
+        "-Wno-format",
+        "-Wno-unused-parameter",
+    ],
+    shared_libs: ["liblog"],
+    header_libs: ["jni_headers"],
+    // To be compatible with R devices, the min_sdk_version must be 30.
+    sdk_version: "current",
+    min_sdk_version: "30",
+    srcs: ["coverage_signal_executor.c"],
+    visibility: ["//visibility:public"],
+}
diff --git a/java/com/android/modules/utils/testing/coverage_wrapper/coverage_signal_executor.c b/java/com/android/modules/utils/testing/coverage_wrapper/coverage_signal_executor.c
new file mode 100644
index 0000000..0d043a9
--- /dev/null
+++ b/java/com/android/modules/utils/testing/coverage_wrapper/coverage_signal_executor.c
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2024 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <jni.h>
+#include <signal.h>
+#include <stdlib.h>
+
+#include "android/log.h"
+
+#define COVERAGE_FLUSH_SIGNAL (__SIGRTMIN + 5)
+const char *TAG = "CoverageDumper";
+
+// Retrieve the method registered for SIG COVERAGE_FLUSH_SIGNAL(37) and
+// execute it. This method can only be executed once in the lifetime of a
+// process and MUST BE executed before the process exits. Successive executions
+// will result in noops (No operations).
+//
+// More context:
+// https://cs.android.com/android/platform/superproject/main/+/main:system/extras/toolchain-extras/profile-clang-extras.cpp;l=52
+// Everytime a piece of code (eg: shared_library gets loaded, a process starts),
+// `init_profile_extras` gets executed, that method chains coverge dump methods.
+//
+// What we do here is that we pick the head of the chain and execute it.
+JNIEXPORT void
+Java_com_android_modules_utils_testing_NativeCoverageHackInstrumentationListener_dumpCoverage(
+    JNIEnv *env) {
+  sighandler_t ret = signal(COVERAGE_FLUSH_SIGNAL, SIG_IGN);
+  if (ret != SIG_ERR && ret != SIG_IGN && ret != SIG_DFL) {
+    __android_log_print(ANDROID_LOG_INFO, TAG, "Coverage dumped.");
+    // The signum is unused.
+    (ret)(/* signum */ COVERAGE_FLUSH_SIGNAL);
+  } else {
+    // Clang did not register its signal handler which means that the code
+    // was not compiled under coverage variant.
+    __android_log_print(ANDROID_LOG_INFO, TAG,
+                        "No coverage signal registered! No-op");
+  }
+}
\ No newline at end of file