ec: fix `make coverage` code coverage reporting

Fixed problems that were preventing us from building the unit tests
with code coverage testing via `make coverage`.
* Changed test_util so that programs will cleanly exit on SIGTERM.
* Changed run_host_test to wait for the child process to exit, and only
  proc.kill() if it times out, so the child process will generate code
  coverage output files on exit.
* Changed Makefile.toolchain to use the --coverage flag for both compile
  and link.
* Changed build.mk and Makefile.rules to exclude certain tests from code
  coverage because they were causing failures either during the
  individual stage of code coverage, or generating the overall report.

BUG=b:143065231
BRANCH=none
TEST=`make coverage` produces results

Signed-off-by: Paul Fagerburg <pfagerburg@chromium.org>
Change-Id: I8575013551ce1dba3fd249cd933a3cf6d110db8d
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2186853
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>

5 files changed, 40 insertions, 12 deletions

diff --git a/Makefile.rules b/Makefile.rules
index 3e3ae48bc9..c40b6623a4 100644
--- a/Makefile.rules
+++ b/Makefile.rules
@@ -339,6 +339,7 @@ buildfuzztests: $(fuzz-test-targets)
 
 .PHONY: hosttests runhosttests runfuzztests runtests
 hosttests: $(host-test-targets)
+runhosttests: TEST_FLAG=TEST_HOSTTEST=y
 runhosttests: $(run-test-targets)
 runfuzztests: $(run-fuzz-test-targets)
 runtests: runhosttests runfuzztests
@@ -376,7 +377,7 @@ $(foreach b, $(cts_boards), \
 	) \
 )
 
-cov-test-targets=$(foreach t,$(test-list-host),build/host/$(t).info)
+cov-test-targets=$(foreach t,$(cov-test-list-host),build/host/$(t).info)
 bldversion=$(shell (./util/getversion.sh ; echo VERSION) | $(CPP) -P -)
 
 # lcov fails when multiple instances run at the same time.
@@ -727,6 +728,7 @@ help:
 	@echo "  tests [BOARD=]       - Build all unit tests for a specific board"
 	@echo "  hosttests            - Build all host unit tests"
 	@echo "  runhosttests         - Build and run all host unit tests"
+	@echo "  coverage             - Build and run all host unit tests for code coverage"
 	@echo "  buildfuzztests       - Build all host fuzzers"
 	@echo "  runfuzztests         - Build and run all host fuzzers for one round"
 	@echo ""
diff --git a/Makefile.toolchain b/Makefile.toolchain
index 9446db84df..01252ba14f 100644
--- a/Makefile.toolchain
+++ b/Makefile.toolchain
@@ -68,8 +68,9 @@ CFLAGS_TEST=$(if $(TEST_BUILD),-DTEST_BUILD=$(EMPTY) \
             $(if $(TEST_UBSAN),$(UBSAN_FLAGS)) \
             $(if $(TEST_FUZZ),-fsanitize=fuzzer-no-link \
                        -fno-experimental-new-pass-manager -DTEST_FUZZ=$(EMPTY))
-CFLAGS_COVERAGE=$(if $(TEST_COVERAGE),-fprofile-arcs -ftest-coverage \
+CFLAGS_COVERAGE=$(if $(TEST_COVERAGE),--coverage \
 				      -DTEST_COVERAGE=$(EMPTY),)
+CFLAGS_HOSTTEST=$(if $(TEST_HOSTTEST),-DTEST_HOSTTEST=$(EMPTY),)
 CFLAGS_DEFINE=-DOUTDIR=$(out)/$(BLD) -DCHIP=$(CHIP) -DBOARD_TASKFILE=$(_tsk_lst_file) \
               -DBOARD=$(BOARD) -DCORE=$(CORE) -DPROJECT=$(PROJECT) \
               -DCHIP_VARIANT=$(CHIP_VARIANT) -DCHIP_FAMILY=$(UC_CHIP_FAMILY) \
@@ -82,13 +83,13 @@ CFLAGS_DEFINE=-DOUTDIR=$(out)/$(BLD) -DCHIP=$(CHIP) -DBOARD_TASKFILE=$(_tsk_lst_
 	      -DPROTOBUF_MIN_PROTOC_VERSION=0 \
               $(CFLAGS_BASEBOARD)
 CPPFLAGS=$(CFLAGS_DEFINE) $(CFLAGS_INCLUDE) $(CFLAGS_TEST) \
-	 $(EXTRA_CFLAGS) $(CFLAGS_COVERAGE) $(LATE_CFLAGS_DEFINE) \
+	 $(EXTRA_CFLAGS) $(CFLAGS_COVERAGE) $(CFLAGS_HOSTTEST) $(LATE_CFLAGS_DEFINE) \
 	-DSECTION_IS_$(BLD)=$(EMPTY) -DSECTION=$(BLD) $(CPPFLAGS_$(BLD))
 BUILD_CPPFLAGS=$(CFLAGS_DEFINE) -Icore/host $(CFLAGS_INCLUDE) $(CFLAGS_TEST) \
-	 $(EXTRA_CFLAGS) $(CFLAGS_COVERAGE) $(LATE_CFLAGS_DEFINE) \
+	 $(EXTRA_CFLAGS) $(CFLAGS_COVERAGE) $(CFLAGS_HOSTTEST) $(LATE_CFLAGS_DEFINE) \
 	 -DSECTION_IS_$(BLD)=$(EMPTY) -DSECTION=$(BLD) $(CPPFLAGS_$(BLD))
 HOST_CPPFLAGS=$(CFLAGS_DEFINE) $(CFLAGS_INCLUDE) $(CFLAGS_TEST) \
-	 $(EXTRA_CFLAGS) $(CFLAGS_COVERAGE) $(LATE_CFLAGS_DEFINE) \
+	 $(EXTRA_CFLAGS) $(CFLAGS_COVERAGE) $(CFLAGS_HOSTTEST) $(LATE_CFLAGS_DEFINE) \
 	 -DSECTION_IS_$(BLD)=$(EMPTY) -DSECTION=$(BLD) $(CPPFLAGS_$(BLD))
 ifneq ($(BOARD),host)
 CPPFLAGS+=-ffreestanding -fno-builtin -nostdinc -nostdlib
@@ -142,7 +143,7 @@ BUILD_LDFLAGS=$(LIBFTDIUSB_LDLIBS)
 HOST_LDFLAGS=$(LIBFTDIUSB_LDLIBS)
 HOST_TEST_LDFLAGS=-Wl,-T core/host/host_exe.lds -lrt -pthread -rdynamic -lm\
 		  -fuse-ld=bfd \
-		  $(if $(TEST_COVERAGE),-fprofile-arcs,) \
+		  $(if $(TEST_COVERAGE), --coverage,) \
 		  $(if $(TEST_ASAN), -fsanitize=address) \
 		  $(if $(TEST_MSAN), -fsanitize=memory) \
 		  $(if $(TEST_UBSAN), ${UBSAN_FLAGS}) \
diff --git a/common/test_util.c b/common/test_util.c
index 2d2d3ba462..6ad1474700 100644
--- a/common/test_util.c
+++ b/common/test_util.c
@@ -5,7 +5,8 @@
  * Test utilities.
  */
 
-#ifdef TEST_COVERAGE
+#if defined(TEST_COVERAGE) || defined(TEST_HOSTTEST)
+/* We need signal() and exit() only when building to run on the host. */
 #include <signal.h>
 #include <stdlib.h>
 #endif
@@ -46,7 +47,14 @@ void emulator_flush(void)
 {
 	__gcov_flush();
 }
+#else
+void emulator_flush(void)
+{
+}
+#endif
 
+#if defined(TEST_HOSTTEST) || defined(TEST_COVERAGE)
+/* Host-based unit tests need to exit(0) when they receive a SIGTERM. */
 void test_end_hook(int sig)
 {
 	emulator_flush();
@@ -58,10 +66,6 @@ void register_test_end_hook(void)
 	signal(SIGTERM, test_end_hook);
 }
 #else
-void emulator_flush(void)
-{
-}
-
 void register_test_end_hook(void)
 {
 }
diff --git a/test/build.mk b/test/build.mk
index 0cc3f964da..850b48c4cf 100644
--- a/test/build.mk
+++ b/test/build.mk
@@ -95,6 +95,21 @@ test-list-host += x25519
 test-list-host += stillness_detector
 endif
 
+# Build up the list of coverage test targets based on test-list-host, but
+# with some tests excluded because they cause code coverage to fail.
+
+# is_enabled_error is a shell script that does not produce coverage results
+cov-dont-test = is_enabled_error
+# static_if_error is a shell script that does not produce coverage results
+cov-dont-test += static_if_error
+# fpsensor: genhtml looks for build/host/fpsensor/cryptoc/util.c
+cov-dont-test += fpsensor
+# fpsensor_crypto: genhtml looks for build/host/fpsensor_crypto/cryptoc/util.c
+cov-dont-test += fpsensor_crypto
+# fpsensor_state: genhtml looks for build/host/fpsensor_state/cryptoc/util.c
+cov-dont-test += fpsensor_state
+cov-test-list-host = $(filter-out $(cov-dont-test), $(test-list-host))
+
 accel_cal-y=accel_cal.o
 aes-y=aes.o
 base32-y=base32.o
diff --git a/util/run_host_test b/util/run_host_test
index 337e941b75..57b197b6e7 100755
--- a/util/run_host_test
+++ b/util/run_host_test
@@ -81,8 +81,14 @@ def run_test(path, timeout=10):
       if proc.poll():
         return TestResult.UNEXPECTED_TERMINATION, output_log
   finally:
+    # Check if the process has exited. If not, send it a SIGTERM, wait for it
+    # to exit, and if it times out, kill the process directly.
     if not proc.poll():
-      proc.kill()
+      try:
+        proc.terminate()
+        proc.wait(timeout)
+      except subprocess.TimeoutExpired:
+        proc.kill()
 
 
 def host_test(test_name):