[PATCH] D20401: [Lexer] Don't merge macro args from different macro files

[Vedant Kumar via cfe-commits]

vsk created this revision.
vsk added reviewers: akyrtzi, doug.gregor.
vsk added a subscriber: cfe-commits.

The lexer sets the end location of macro arguments incorrectly *if*,
while merging consecutive args to fit into a single SLocEntry, it finds
args which come from different macro files.

Fix the issue by using separate SLocEntries in this situation.

This fixes a code coverage crasher (rdar://problem/26181005). Because
the lexer reported end locations for certain macro args incorrectly, we
would generate bogus coverage mappings with negative line offsets.

http://reviews.llvm.org/D20401

Files:
  lib/Lex/TokenLexer.cpp
  test/CoverageMapping/Inputs/macros.h
  test/CoverageMapping/include-macros.c
  unittests/Lex/LexerTest.cpp

Index: unittests/Lex/LexerTest.cpp
===================================================================
--- unittests/Lex/LexerTest.cpp
+++ unittests/Lex/LexerTest.cpp
@@ -58,8 +58,7 @@
     Target = TargetInfo::CreateTargetInfo(Diags, TargetOpts);
   }
 
-  std::vector<Token> CheckLex(StringRef Source,
-                              ArrayRef<tok::TokenKind> ExpectedTokens) {
+  std::vector<Token> Lex(StringRef Source) {
     std::unique_ptr<llvm::MemoryBuffer> Buf =
         llvm::MemoryBuffer::getMemBuffer(Source);
     SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));
@@ -82,6 +81,12 @@
       toks.push_back(tok);
     }
 
+    return toks;
+  }
+
+  std::vector<Token> CheckLex(StringRef Source,
+                              ArrayRef<tok::TokenKind> ExpectedTokens) {
+    auto toks = Lex(Source);
     EXPECT_EQ(ExpectedTokens.size(), toks.size());
     for (unsigned i = 0, e = ExpectedTokens.size(); i != e; ++i) {
       EXPECT_EQ(ExpectedTokens[i], toks[i].getKind());
@@ -358,4 +363,16 @@
   EXPECT_EQ("N", Lexer::getImmediateMacroName(idLoc4, SourceMgr, LangOpts));
 }
 
+TEST_F(LexerTest, DontMergeMacroArgsFromDifferentMacroFiles) {
+  std::vector<Token> toks =
+      Lex("#define helper1 0\n"
+          "void helper2(const char *, ...);\n"
+          "#define M1(a, ...) helper2(a, ##__VA_ARGS__)\n"
+          "#define M2(a, ...) M1(a, helper1, ##__VA_ARGS__)\n"
+          "void f1() { M2(\"a\", \"b\"); }");
+
+  SourceLocation helper1ArgLoc = toks[20].getLocation();
+  EXPECT_EQ(SourceMgr.getFileIDSize(SourceMgr.getFileID(helper1ArgLoc)), 8U);
+}
+
 } // anonymous namespace
Index: test/CoverageMapping/include-macros.c
===================================================================
--- /dev/null
+++ test/CoverageMapping/include-macros.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fprofile-instrument=clang -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -main-file-name include-macros.c %s | FileCheck %s
+
+#include "Inputs/macros.h"
+
+void f1() {
+  M2("a", "b");
+}
+
+// CHECK-LABEL: f1:
+// CHECK-NEXT:   File 0, 5:11 -> 7:2 = #0
+// CHECK-NEXT:   Expansion,File 0, 6:3 -> 6:5 = #0 (Expanded file = 1)
+// CHECK-NEXT:   File 1, 13:20 -> 13:50 = #0
+// CHECK-NEXT:   Expansion,File 1, 13:20 -> 13:22 = #0 (Expanded file = 2)
+// CHECK-NEXT:   File 2, 7:20 -> 7:46 = #0
+// CHECK-NEXT:   Expansion,File 2, 7:33 -> 7:44 = #0 (Expanded file = 3)
+// CHECK-NEXT:   File 3, 13:26 -> 13:34 = #0
+// CHECK-NEXT:   Expansion,File 3, 13:26 -> 13:33 = #0 (Expanded file = 4)
+// CHECK-NEXT:   File 4, 3:17 -> 3:18 = #0
Index: test/CoverageMapping/Inputs/macros.h
===================================================================
--- /dev/null
+++ test/CoverageMapping/Inputs/macros.h
@@ -0,0 +1,13 @@
+// Assorted macros to help test #include behavior across file boundaries.
+
+#define helper1 0
+
+void helper2(const char *, ...);
+
+#define M1(a, ...) helper2(a, ##__VA_ARGS__);
+
+// Important: The line number on which M2 is defined must be greater than the
+// line number on which one of the expansions of M2 occurs. This is done to
+// test support for regions which start in a header and end in the file which
+// contains the expansion.
+#define M2(a, ...) M1(a, helper1, ##__VA_ARGS__);
Index: lib/Lex/TokenLexer.cpp
===================================================================
--- lib/Lex/TokenLexer.cpp
+++ lib/Lex/TokenLexer.cpp
@@ -787,6 +787,9 @@
     if (CurLoc.isFileID() != NextLoc.isFileID())
       break; // Token from different kind of FileID.
 
+    if (CurLoc.isMacroID() && !SM.isWrittenInSameFile(CurLoc, NextLoc))
+      break; // Token from a different macro.
+
     int RelOffs;
     if (!SM.isInSameSLocAddrSpace(CurLoc, NextLoc, &RelOffs))
       break; // Token from different local/loaded location.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D20401.57710.patch
Type: text/x-patch
Size: 3795 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160518/18ebfa48/attachment-0001.bin>