Browse Source

Merge remote-tracking branch 'public/lcov_excl'

Nick Mathewson 8 years ago
parent
commit
94e3555187
6 changed files with 64 additions and 10 deletions
  1. 7 0
      changes/lcov_excl
  2. 13 0
      doc/HACKING/WritingTests.md
  3. 2 2
      scripts/test/cov-diff
  4. 28 0
      scripts/test/cov-exclude
  5. 10 6
      src/common/crypto.c
  6. 4 2
      src/common/crypto_s2k.c

+ 7 - 0
changes/lcov_excl

@@ -0,0 +1,7 @@
+  o Minor features (testing):
+    - Use the lcov convention for marking lines as unreachable, so that
+      we don't count them when we're generating test coverage data.
+      Update our coverage tools to understand this convention.
+      Closes ticket #16792.
+
+

+ 13 - 0
doc/HACKING/WritingTests.md

@@ -109,6 +109,19 @@ To count new or modified uncovered lines in D2, you can run:
 
     ./scripts/test/cov-diff ${D1} ${D2}" | grep '^+ *\#' | wc -l
 
+### Marking lines as unreachable by tests
+
+You can mark a specific line as unreachable by using the special
+string LCOV_EXCL_LINE.  You can mark a range of lines as unreachable
+with LCOV_EXCL_START... LCOV_EXCL_STOP.  Note that older versions of
+lcov don't understand these lines.
+
+You can post-process .gcov files to make these lines 'unreached' by
+running ./scripts/test/cov-exclude on them.
+
+Note: you should never do this unless the line is meant to 100%
+unreachable by actual code.
+
 
 What kinds of test should I write?
 ----------------------------------

+ 2 - 2
scripts/test/cov-diff

@@ -9,8 +9,8 @@ DIRB="$2"
 
 for A in $DIRA/*; do
   B=$DIRB/`basename $A`
-  perl -pe 's/^\s*\d+:/        1:/; s/^([^:]+:)[\d\s]+:/$1/; s/^ *-:(Runs|Programs):.*//;' "$A" > "$A.tmp"
-  perl -pe 's/^\s*\d+:/        1:/; s/^([^:]+:)[\d\s]+:/$1/; s/^ *-:(Runs|Programs):.*//;' "$B" > "$B.tmp"
+  perl -pe 's/^\s*\!*\d+:/        1:/; s/^([^:]+:)[\d\s]+:/$1/; s/^ *-:(Runs|Programs):.*//;' "$A" > "$A.tmp"
+  perl -pe 's/^\s*\!*\d+:/        1:/; s/^([^:]+:)[\d\s]+:/$1/; s/^ *-:(Runs|Programs):.*//;' "$B" > "$B.tmp"
   diff -u "$A.tmp" "$B.tmp"
   rm "$A.tmp" "$B.tmp"
 done

+ 28 - 0
scripts/test/cov-exclude

@@ -0,0 +1,28 @@
+#!/usr/bin/perl -p -i
+
+use warnings;
+use strict;
+our $excluding;
+
+# This script is meant to post-process a .gcov file for an input source
+# that was annotated with LCOV_EXCL_START, LCOV_EXCL_STOP, and LCOV_EXCL_LINE
+# entries.  It doesn't understand the LCOV_EXCL_BR* variations.
+#
+# It replaces unreached reached lines with x:, and reached excluded lines
+# with !!!num:.
+
+BEGIN { our $excluding = 0; }
+
+if (m/LCOV_EXCL_START/) {
+  $excluding = 1;
+}
+if ($excluding and m/LCOV_EXCL_STOP/) {
+  $excluding = 0;
+}
+
+my $exclude_this = (m/LCOV_EXCL_LINE/);
+
+if ($excluding or $exclude_this) {
+  s{^\s*\#\#+:}{        x:};
+  s{^   (\s*)(\d+):}{$1!!!$2:};
+}

+ 10 - 6
src/common/crypto.c

@@ -134,7 +134,7 @@ crypto_get_rsa_padding_overhead(int padding)
   switch (padding)
     {
     case RSA_PKCS1_OAEP_PADDING: return PKCS1_OAEP_PADDING_OVERHEAD;
-    default: tor_assert(0); return -1;
+    default: tor_assert(0); return -1; // LCOV_EXCL_LINE
     }
 }
 
@@ -146,7 +146,7 @@ crypto_get_rsa_padding(int padding)
   switch (padding)
     {
     case PK_PKCS1_OAEP_PADDING: return RSA_PKCS1_OAEP_PADDING;
-    default: tor_assert(0); return -1;
+    default: tor_assert(0); return -1; // LCOV_EXCL_LINE
     }
 }
 
@@ -1739,8 +1739,8 @@ crypto_digest_algorithm_get_length(digest_algorithm_t alg)
     case DIGEST_SHA3_512:
       return DIGEST512_LEN;
     default:
-      tor_assert(0);
-      return 0; /* Unreachable */
+      tor_assert(0);              // LCOV_EXCL_LINE
+      return 0; /* Unreachable */ // LCOV_EXCL_LINE
   }
 }
 
@@ -1783,8 +1783,8 @@ crypto_digest_alloc_bytes(digest_algorithm_t alg)
     case DIGEST_SHA3_512:
       return END_OF_FIELD(d.sha3);
     default:
-      tor_assert(0);
-      return 0;
+      tor_assert(0); // LCOV_EXCL_LINE
+      return 0;      // LCOV_EXCL_LINE
   }
 #undef END_OF_FIELD
 #undef STRUCT_FIELD_SIZE
@@ -1914,6 +1914,7 @@ crypto_digest_get_digest(crypto_digest_t *digest,
     case DIGEST_SHA512:
       SHA512_Final(r, &tmpenv.d.sha512);
       break;
+//LCOV_EXCL_START
     case DIGEST_SHA3_256: /* FALLSTHROUGH */
     case DIGEST_SHA3_512:
       log_warn(LD_BUG, "Handling unexpected algorithm %d", digest->algorithm);
@@ -1921,6 +1922,7 @@ crypto_digest_get_digest(crypto_digest_t *digest,
     default:
       tor_assert(0); /* Unreachable. */
       break;
+//LCOV_EXCL_STOP
   }
   memcpy(out, r, out_len);
   memwipe(r, 0, sizeof(r));
@@ -2760,10 +2762,12 @@ crypto_strongest_rand(uint8_t *out, size_t out_len)
   while (out_len) {
     crypto_rand((char*) inp, DLEN);
     if (crypto_strongest_rand_raw(inp+DLEN, DLEN) < 0) {
+      // LCOV_EXCL_START
       log_err(LD_CRYPTO, "Failed to load strong entropy when generating an "
               "important key. Exiting.");
       /* Die with an assertion so we get a stack trace. */
       tor_assert(0);
+      // LCOV_EXCL_STOP
     }
     if (out_len >= DLEN) {
       SHA512(inp, sizeof(inp), out);

+ 4 - 2
src/common/crypto_s2k.c

@@ -57,7 +57,8 @@
 #define SCRYPT_KEY_LEN 32
 
 /** Given an algorithm ID (one of S2K_TYPE_*), return the length of the
- * specifier part of it, without the prefix type byte. */
+ * specifier part of it, without the prefix type byte.  Return -1 if it is not
+ * a valid algorithm ID. */
 static int
 secret_to_key_spec_len(uint8_t type)
 {
@@ -86,7 +87,8 @@ secret_to_key_key_len(uint8_t type)
     case S2K_TYPE_SCRYPT:
       return DIGEST256_LEN;
     default:
-      return -1;
+      tor_fragile_assert(); // LCOV_EXCL_LINE
+      return -1;            // LCOV_EXCL_LINE
   }
 }