From: Justin Bogner <mail@justinbogner.com>
Date: Fri, 2 May 2014 20:01:24 +0000 (+0000)
Subject: llvm-cov: Fix handling of line zero appearing in a line table
X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=688567e91cf42fcffec70eb8abf2c6c31b7add4a;p=oota-llvm.git

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 8f060d2ac90..522716309a9 100644
--- 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
index 00000000000..df51888d48d
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
index 00000000000..5f30acf12f8
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 f738810475f..dc1f6653796 100644
--- 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
index 00000000000..61f60f6b0d6
--- /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