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

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.

Reviewed-by: akyrtzi

Differential Revision: http://reviews.llvm.org/D20401

llvm-svn: 270160

clang/lib/Lex/TokenLexer.cpp

@@ -787,6 +787,9 @@ static void updateConsecutiveMacroArgTokens(SourceManager &SM,
     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.

clang/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__);
+
+// Note: M2 stresses vararg macro functions with macro arguments. The spelling
+// locations of the args used to be set to the expansion site, leading to
+// crashes (region LineEnd < LineStart). The regression test requires M2's line
+// number to be greater than the line number containing the expansion.
+#define M2(a, ...) M1(a, helper1, ##__VA_ARGS__);

clang/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

clang/unittests/Lex/LexerTest.cpp

@@ -58,8 +58,7 @@ protected:
     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 @@ protected:
       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,21 @@ TEST_F(LexerTest, LexAPI) {
   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\"); }");
+
+  // Check the file corresponding to the "helper1" macro arg in M2.
+  //
+  // The lexer used to report its size as 31, meaning that the end of the
+  // expansion would be on the *next line* (just past `M2("a", "b")`). Make
+  // sure that we get the correct end location (the comma after "helper1").
+  SourceLocation helper1ArgLoc = toks[20].getLocation();
+  EXPECT_EQ(SourceMgr.getFileIDSize(SourceMgr.getFileID(helper1ArgLoc)), 8U);
+}
+
 } // anonymous namespace