From 2bb5d8148b2c15e9573a7f041ef631bdf4429b4b Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 15 May 2019 08:27:02 -0400 Subject: [PATCH 1/5] In coverage builds, avoid basic-block complexity in log_debug Ordinarily we skip calling log_fn(LOG_DEBUG,...) if debug logging is completely disabled. However, in coverage builds, this means that we get spurious complaints about partially covered basic blocks, in a way that makes our coverage determinism harder to check. --- src/lib/log/log.h | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/lib/log/log.h b/src/lib/log/log.h index a381220af0..4a3ea0ad55 100644 --- a/src/lib/log/log.h +++ b/src/lib/log/log.h @@ -194,6 +194,11 @@ void tor_log_get_logfile_names(struct smartlist_t *out); extern int log_global_min_severity_; +#ifdef TOR_COVERAGE +/* For coverage builds, we try to avoid our log_debug optimization, since it + * can have weird effects on internal macro coverage. */ +#define debug_logging_enabled() (1) +#else static inline bool debug_logging_enabled(void); /** * Return true iff debug logging is enabled for at least one domain. @@ -202,6 +207,7 @@ static inline bool debug_logging_enabled(void) { return PREDICT_UNLIKELY(log_global_min_severity_ == LOG_DEBUG); } +#endif void log_fn_(int severity, log_domain_mask_t domain, const char *funcname, const char *format, ...) From b88281024579d5f207d15d1a2cc54c113f8a2bde Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 15 May 2019 08:28:25 -0400 Subject: [PATCH 2/5] In coverage builds, use branch-free timeradd() and timersub() The ordinary definitions of timeradd() and timersub() contain a branch. However, in coverage builds, this means that we get spurious complaints about partially covered basic blocks, in a way that makes our coverage determinism harder to check. --- src/lib/wallclock/timeval.h | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/lib/wallclock/timeval.h b/src/lib/wallclock/timeval.h index 4967e939bf..33076adc8b 100644 --- a/src/lib/wallclock/timeval.h +++ b/src/lib/wallclock/timeval.h @@ -20,6 +20,27 @@ #include #endif +#ifdef TOR_COVERAGE +/* For coverage builds, we use a slower definition of these macros without + * branches, to make coverage consistent. */ +#undef timeradd +#undef timersub +#define timeradd(tv1,tv2,tvout) \ + do { \ + (tvout)->tv_sec = (tv1)->tv_sec + (tv2)->tv_sec; \ + (tvout)->tv_usec = (tv1)->tv_usec + (tv2)->tv_usec; \ + (tvout)->tv_sec += (tvout)->tv_usec / 1000000; \ + (tvout)->tv_usec %= 1000000; \ + } while (0) +#define timersub(tv1,tv2,tvout) \ + do { \ + (tvout)->tv_sec = (tv1)->tv_sec - (tv2)->tv_sec - 1; \ + (tvout)->tv_usec = (tv1)->tv_usec - (tv2)->tv_usec + 1000000; \ + (tvout)->tv_sec += (tvout)->tv_usec / 1000000; \ + (tvout)->tv_usec %= 1000000; \ + } while (0) +#endif + #ifndef timeradd /** Replacement for timeradd on platforms that do not have it: sets tvout to * the sum of tv1 and tv2. */ From 07ccffa989a8c6ac6ac216b6e729daed14372620 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 15 May 2019 14:58:46 -0400 Subject: [PATCH 3/5] Coverage: do not include test-rebind in coverage builds. Because it invokes the Tor mainloop, it does unpredictable things to test coverage of a lot of code that it doesn't actually test at all. (It is more an integration test than anything else.) --- src/test/include.am | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/test/include.am b/src/test/include.am index 824089bc47..85f9c9f880 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -32,8 +32,15 @@ endif if USEPYTHON TESTSCRIPTS += src/test/test_ntor.sh src/test/test_hs_ntor.sh src/test/test_bt.sh + +if COVERAGE_ENABLED +# ... +else +# Only do this when coverage is not on, since it invokes lots of code +# in a kind of unpredictable way. TESTSCRIPTS += src/test/test_rebind.sh endif +endif TESTS += src/test/test src/test/test-slow src/test/test-memwipe \ src/test/test_workqueue \ From 7893f2cd73310aaf20283f72dc05167a23584ac6 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Thu, 23 May 2019 14:36:01 -0400 Subject: [PATCH 4/5] cov-test-determinism: use the same RNG seed as in travis.yml We added this facility so that we could get deterministic PRNG behavior for coverage testing on tests that use a replaced PRNG. We need to have our coverage determinism tool test for this as well. --- scripts/test/cov-test-determinism.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/test/cov-test-determinism.sh b/scripts/test/cov-test-determinism.sh index 3b4f372e04..3458f96968 100755 --- a/scripts/test/cov-test-determinism.sh +++ b/scripts/test/cov-test-determinism.sh @@ -25,6 +25,9 @@ else fi if test "$run" = 1; then + # same seed as in travis.yml + TOR_TEST_RNG_SEED="636f766572616765" + export TOR_TEST_RNG_SEED while true; do make reset-gcov CD=coverage-raw/coverage-$(date +%s) From 6d9e47702fe52b0817a593117a7f4a3eecf06ad7 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 24 May 2019 07:42:59 -0400 Subject: [PATCH 5/5] changes file for test coverage --- changes/ticket30519 | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changes/ticket30519 diff --git a/changes/ticket30519 b/changes/ticket30519 new file mode 100644 index 0000000000..efb25b9294 --- /dev/null +++ b/changes/ticket30519 @@ -0,0 +1,4 @@ + o Minor features (testing): + - When running tests in coverage mode, take additional care to make + our coverage deterministic, so that we can accurately track changes in + code coverage. Closes ticket 30519.