[PATCH] D78649: [clang] Make sure argument expansion locations are correct in presence of predefined buffer

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Macro argument expansion logic relies on skipping file IDs that created
as a result of an include. Unfortunately it fails to do that for
predefined buffer since it doesn't have a valid insertion location.

As a result of that any file ID created for an include inside the
predefined buffers breaks the traversal logic in
SourceManager::computeMacroArgsCache.

To fix this issue we first record number of created FIDs for predefined
buffer, and then skip them explicitly in source manager.

Another solution would be to just give predefined buffers a valid source
location, but it is unclear where that should be..


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78649

Files:
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Lex/PPLexerChange.cpp
  clang/unittests/Basic/SourceManagerTest.cpp
  clang/unittests/Lex/LexerTest.cpp


Index: clang/unittests/Lex/LexerTest.cpp
===================================================================
--- clang/unittests/Lex/LexerTest.cpp
+++ clang/unittests/Lex/LexerTest.cpp
@@ -556,4 +556,17 @@
   EXPECT_THAT(GeneratedByNextToken, ElementsAre("abcd", "=", "0", ";", "int",
                                                 "xyz", "=", "abcd", ";"));
 }
+
+TEST_F(LexerTest, CreatedFIDCountForPredefinedBuffer) {
+  TrivialModuleLoader ModLoader;
+  auto PP = CreatePP("", ModLoader);
+  while (1) {
+    Token tok;
+    PP->Lex(tok);
+    if (tok.is(tok::eof))
+      break;
+  }
+  EXPECT_EQ(SourceMgr.getNumCreatedFIDsForFileID(PP->getPredefinesFileID()),
+            1U);
+}
 } // anonymous namespace
Index: clang/unittests/Basic/SourceManagerTest.cpp
===================================================================
--- clang/unittests/Basic/SourceManagerTest.cpp
+++ clang/unittests/Basic/SourceManagerTest.cpp
@@ -294,10 +294,16 @@
   TrivialModuleLoader ModLoader;
   HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
                           Diags, LangOpts, &*Target);
+
   Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
                   SourceMgr, HeaderInfo, ModLoader,
                   /*IILookup =*/nullptr,
                   /*OwnsHeaderSearch =*/false);
+  // Ensure we can get expanded locations in presence of implicit includes.
+  // These are different than normal includes since predefines buffer doesn't
+  // have a valid insertion location.
+  PP.setPredefines("#include \"/implicit-header.h\"");
+  FileMgr.getVirtualFile("/implicit-header.h", 0, 0);
   PP.Initialize(*Target);
   PP.EnterMainSourceFile();
 
Index: clang/lib/Lex/PPLexerChange.cpp
===================================================================
--- clang/lib/Lex/PPLexerChange.cpp
+++ clang/lib/Lex/PPLexerChange.cpp
@@ -415,7 +415,9 @@
     }
 
     if (!isEndOfMacro && CurPPLexer &&
-        SourceMgr.getIncludeLoc(CurPPLexer->getFileID()).isValid()) {
+        (SourceMgr.getIncludeLoc(CurPPLexer->getFileID()).isValid() ||
+         // Predefines file doesn't have a valid include location.
+         CurPPLexer->FID == getPredefinesFileID())) {
       // Notify SourceManager to record the number of FileIDs that were created
       // during lexing of the #include'd file.
       unsigned NumFIDs =
Index: clang/lib/Basic/SourceManager.cpp
===================================================================
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -1799,6 +1799,14 @@
     if (Invalid)
       return;
     if (Entry.isFile()) {
+      // Predefined header doesn't have a valid include location in main faile,
+      // but any files created by it should still be skipped when computing
+      // macro args expanded in the main file.
+      if (Entry.getFile().Filename == "<built-in>" && FID == getMainFileID()) {
+        if (Entry.getFile().NumCreatedFIDs)
+          ID += Entry.getFile().NumCreatedFIDs - 1 /*because of next ++ID*/;
+        continue;
+      }
       SourceLocation IncludeLoc = Entry.getFile().getIncludeLoc();
       if (IncludeLoc.isInvalid())
         continue;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D78649.259310.patch
Type: text/x-patch
Size: 3205 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200422/8fcae0d1/attachment-0001.bin>