llvm-cov: Fix handling of line zero appearing in a line table

Reading line tables in llvm-cov was pretty broken, but would happen to
work as long as no line in the table was 0. It's not clear to me
whether a line of zero *should* show up in these tables, but deciding
to read a string in the middle of the line table is certainly the
wrong thing to do if it does.

I've also added some comments, as trying to figure out what this block
of code was doing was fairly unpleasant.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@207866 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/IR/GCOV.cpp b/lib/IR/GCOV.cpp
index 8f060d2ac90d117687c29b30718657340ebf2d63..522716309a944be158d2d0220f2e3410e681dd84 100644 (file)
--- a/lib/IR/GCOV.cpp
+++ b/lib/IR/GCOV.cpp
@@ -166,9 +166,11 @@ bool GCOVFunction::readGCNO(GCOVBuffer &Buff, GCOV::GCOVVersion Version) {
   // read line table.
   while (Buff.readLineTag()) {
     uint32_t LineTableLength;
+    // Read the length of this line table.
     if (!Buff.readInt(LineTableLength)) return false;
     uint32_t EndPos = Buff.getCursor() + LineTableLength*4;
     uint32_t BlockNo;
+    // Read the block number this table is associated with.
     if (!Buff.readInt(BlockNo)) return false;
     if (BlockNo >= BlockCount) {
       errs() << "Unexpected block number: " << BlockNo << " (in " << Name
@@ -176,24 +178,34 @@ bool GCOVFunction::readGCNO(GCOVBuffer &Buff, GCOV::GCOVVersion Version) {
       return false;
     }
     GCOVBlock &Block = *Blocks[BlockNo];
-    if (!Buff.readInt(Dummy)) return false; // flag
-    while (Buff.getCursor() != (EndPos - 4)) {
+    // Read the word that pads the beginning of the line table. This may be a
+    // flag of some sort, but seems to always be zero.
+    if (!Buff.readInt(Dummy)) return false;
+
+    // Line information starts here and continues up until the last word.
+    if (Buff.getCursor() != (EndPos - sizeof(uint32_t))) {
       StringRef F;
+      // Read the source file name.
       if (!Buff.readString(F)) return false;
       if (Filename != F) {
         errs() << "Multiple sources for a single basic block: " << Filename
                << " != " << F << " (in " << Name << ").\n";
         return false;
       }
-      if (Buff.getCursor() == (EndPos - 4)) break;
-      while (true) {
+      // Read lines up to, but not including, the null terminator.
+      while (Buff.getCursor() < (EndPos - 2 * sizeof(uint32_t))) {
         uint32_t Line;
         if (!Buff.readInt(Line)) return false;
-        if (!Line) break;
+        // Line 0 means this instruction was injected by the compiler. Skip it.
+        if (!Line) continue;
         Block.addLine(Line);
       }
+      // Read the null terminator.
+      if (!Buff.readInt(Dummy)) return false;
     }
-    if (!Buff.readInt(Dummy)) return false; // flag
+    // The last word is either a flag or padding, it isn't clear which. Skip
+    // over it.
+    if (!Buff.readInt(Dummy)) return false;
   }
   return true;
 }

diff --git a/test/tools/llvm-cov/Inputs/range_based_for.gcda b/test/tools/llvm-cov/Inputs/range_based_for.gcda
new file mode 100644 (file)
index 0000000..df51888
Binary files /dev/null and b/test/tools/llvm-cov/Inputs/range_based_for.gcda differ

diff --git a/test/tools/llvm-cov/Inputs/range_based_for.gcno b/test/tools/llvm-cov/Inputs/range_based_for.gcno
new file mode 100644 (file)
index 0000000..5f30acf
Binary files /dev/null and b/test/tools/llvm-cov/Inputs/range_based_for.gcno differ

diff --git a/test/tools/llvm-cov/lit.local.cfg b/test/tools/llvm-cov/lit.local.cfg
index f738810475fefc85b39907f057ef12675727acce..dc1f66537969e27da16f3f968ef184ba1967e1ca 100644 (file)
--- a/test/tools/llvm-cov/lit.local.cfg
+++ b/test/tools/llvm-cov/lit.local.cfg
@@ -1 +1 @@
-config.suffixes = ['.test', '.m']
+config.suffixes = ['.test', '.m', '.c', '.cpp']

diff --git a/test/tools/llvm-cov/range_based_for.cpp b/test/tools/llvm-cov/range_based_for.cpp
new file mode 100644 (file)
index 0000000..61f60f6
--- /dev/null
+++ b/test/tools/llvm-cov/range_based_for.cpp
@@ -0,0 +1,29 @@
+// Make sure that compiler-added local variables (whose line number is zero)
+// don't crash llvm-cov.
+
+// We need shell for cd
+// REQUIRES: shell
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+// RUN: cp %s %p/Inputs/range_based_for.gc* .
+
+// RUN: llvm-cov range_based_for.cpp | FileCheck %s --check-prefix=STDOUT
+// STDOUT: File 'range_based_for.cpp'
+// STDOUT: Lines executed:100.00% of 5
+// STDOUT: range_based_for.cpp:creating 'range_based_for.cpp.gcov'
+
+// RUN: FileCheck %s --check-prefix=GCOV < %t/range_based_for.cpp.gcov
+// GCOV: -:    0:Runs:1
+// GCOV: -:    0:Programs:1
+
+int main(int argc, const char *argv[]) { // GCOV: 1:    [[@LINE]]:int main(
+  int V[] = {1, 2};                      // GCOV: 1:    [[@LINE]]:  int V[]
+  for (int &I : V) {                     // GCOV: 10:   [[@LINE]]:  for (
+  }                                      // GCOV: 2:    [[@LINE]]:  }
+  return 0;                              // GCOV: 1:    [[@LINE]]:  return
+}                                        // GCOV: -:    [[@LINE]]:}
+
+// llvm-cov doesn't work on big endian yet
+// XFAIL: powerpc64, s390x, mips-, mips64-, sparc