makefile: use separate directory for code coverage

When building host-based unit tests for code coverage, put the build
outputs in a different directory. Because the code coverage build
has calls into gcov library functions, a partial rebuild without code
coverage will result in undefined linker errors. The previous solution
was an inefficient cycle of `make clobber` and full rebuild when
switching between building with and without code coverage.

BUG=b:157091606
BRANCH=None
TEST=`make buildall -j ; make coverage -j`
Verify that build/host and build/coverage both exist, and that code
coverage data (*.gcno, *.gcda, *.info) is only in build/coverage.

Signed-off-by: Paul Fagerburg <pfagerburg@google.com>
Change-Id: Iac0b18068082d34546aa15b174f86efb6a7f41a7
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2242351
Tested-by: Paul Fagerburg <pfagerburg@chromium.org>
Reviewed-by: Jett Rink <jettrink@chromium.org>
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>
Commit-Queue: Paul Fagerburg <pfagerburg@chromium.org>

3 files changed, 32 insertions, 33 deletions

diff --git a/Makefile.rules b/Makefile.rules
index c40b6623a4..074564f051 100644
--- a/Makefile.rules
+++ b/Makefile.rules
@@ -112,7 +112,9 @@ cmd_host_test = $(MAKE) BOARD=host PROJECT=$* \
 	V=$(V) out=build/host/$* TEST_BUILD=y EMU_BUILD=y CROSS_COMPILE= \
 	$(if $(TEST_SCRIPT),TEST_SCRIPT=$(TEST_SCRIPT)) $(TEST_FLAG) \
 	build/host/$*/$*.exe
+cmd_coverage_test = $(subst build/host,build/coverage,$(cmd_host_test))
 cmd_run_host_test = ./util/run_host_test $* $(silent)
+cmd_run_coverage_test = ./util/run_host_test --coverage $* $(silent)
 # generate new version.h, compare if it changed and replace if so
 cmd_version = ./util/getversion.sh > $@.tmp && \
 	cmp -s $@.tmp $@ && rm -f $@.tmp || mv $@.tmp $@
@@ -303,6 +305,18 @@ $(run-test-targets): run-%: host-%
 	$(call quiet,run_host_test,TEST   )
 	@rm -f $(FAILED_BOARDS_DIR)/test-$*
 
+host-coverage-targets=$(foreach t,$(test-list-host),host-coverage-$(t))
+run-coverage-targets=$(foreach t,$(test-list-host),run-coverage-$(t))
+.PHONY: $(host-coverage-targets) $(run-coverage-targets)
+
+$(host-coverage-targets): host-coverage-%: | $(FAILED_BOARDS_DIR)
+	@touch $(FAILED_BOARDS_DIR)/test-$*
+	+$(call quiet,coverage_test,BUILD  )
+
+$(run-coverage-targets): run-coverage-%: host-coverage-%
+	$(call quiet,run_coverage_test,TEST   )
+	@rm -f $(FAILED_BOARDS_DIR)/test-$*
+
 .PHONY: print-host-tests
 print-host-tests:
 	$(call cmd_pretty_print_list, \
@@ -377,13 +391,13 @@ $(foreach b, $(cts_boards), \
 	) \
 )
 
-cov-test-targets=$(foreach t,$(cov-test-list-host),build/host/$(t).info)
+cov-test-targets=$(foreach t,$(cov-test-list-host),build/coverage/$(t).info)
 bldversion=$(shell (./util/getversion.sh ; echo VERSION) | $(CPP) -P -)
 
 # lcov fails when multiple instances run at the same time.
 # We need to run them sequentially by using flock
-cmd_lcov=flock /tmp/ec-lcov-lock -c "lcov -q -o $@ -c -d build/host/$*"
-cmd_report_cov=genhtml -q -o build/host/coverage_rpt -t \
+cmd_lcov=flock /tmp/ec-lcov-lock -c "lcov -q -o $@ -c -d build/coverage/$*"
+cmd_report_cov=genhtml -q -o build/coverage/coverage_rpt -t \
 	       "EC Unittest "$(bldversion) $^
 
 # Unless V is set to 0 we always want the 'size:' target to report its output,
@@ -420,7 +434,7 @@ cmd_stats= \
 	printf "%-10s: %6d\n" $$board $$size; \
   done
 
-build/host/%.info: run-%
+build/coverage/%.info: run-coverage-%
 	$(call quiet,lcov,COV    )
 
 .PHONY: coverage
diff --git a/docs/code_coverage.md b/docs/code_coverage.md
index 92f091e52d..458d87b7c5 100644
--- a/docs/code_coverage.md
+++ b/docs/code_coverage.md
@@ -21,26 +21,7 @@ This target will compile and link the unit tests with `--coverage` flag (which
 pulls in the `gcov` libraries), run the tests, and then process the profiling
 data into a code coverage report using the `lcov` and `genhtml` tools.
 
-The coverage report top-level page is `build/host/coverage_rpt/index.html`.
-
-### `make clobber` is required
-
-**Always** `make clobber` when switching from building with code coverage
-to building without code coverage, or from building without code coverage
-to building with code coverage. `make clean` is not sufficient.
-
-`make buildall -j ; make clobber ; make coverage -j`
-
-`make coverage -j ; make clobber ; make buildall -j`
-
-If you do not `make clobber`, you will get link-time errors such as:
-
-```
-core/host/task.c:558: undefined reference to `__gcov_init'
-build/host/online_calibration/RO/core/host/timer.o:(.data+0x5b0): undefined reference to `__gcov_merge_add'
-```
-
-Note that `make clobber` will delete the coverage report.
+The coverage report top-level page is `build/coverage/coverage_rpt/index.html`.
 
 ### Noise in the build output
 
diff --git a/util/run_host_test b/util/run_host_test
index 57b197b6e7..ce33722c3a 100755
--- a/util/run_host_test
+++ b/util/run_host_test
@@ -91,26 +91,30 @@ def run_test(path, timeout=10):
         proc.kill()
 
 
-def host_test(test_name):
-  exec_path = pathlib.Path('build', 'host', test_name, f'{test_name}.exe')
-  if not exec_path.is_file():
-    raise argparse.ArgumentTypeError(f'No test named {test_name} exists!')
-  return exec_path
-
-
 def parse_options(argv):
   parser = argparse.ArgumentParser()
   parser.add_argument('-t', '--timeout', type=float, default=60,
                       help='Timeout to kill test after.')
-  parser.add_argument('test_name', type=host_test)
+  parser.add_argument('--coverage', action='store_const', const='coverage',
+                      default='host', dest='test_target',
+                      help='Flag if this is a code coverage test.')
+  parser.add_argument('test_name', type=str)
   return parser.parse_args(argv)
 
 
 def main(argv):
   opts = parse_options(argv)
 
+  # Tests will be located in build/host, unless the --coverage flag was
+  # provided, in which case they will be in build/coverage.
+  exec_path = pathlib.Path('build', opts.test_target, opts.test_name,
+                           f'{opts.test_name}.exe')
+  if not exec_path.is_file():
+    print(f'No test named {opts.test_name} exists!')
+    return 1
+
   start_time = time.monotonic()
-  result, output = run_test(opts.test_name, timeout=opts.timeout)
+  result, output = run_test(exec_path, timeout=opts.timeout)
   elapsed_time = time.monotonic() - start_time
 
   print('{} {}! ({:.3f} seconds)'.format(