Fixed a crash on replaying Preamble's PP conditional stack.

[ilya-biryukov]

Summary

The crash occurs when the first token after a preamble is a macro
expansion.
Fixed by moving replayPreambleConditionalStack from Parser into
Preprocessor. It is now called right after the predefines file is
processed.

[erikjv]

Can you check if the example in https://bugs.llvm.org/show_bug.cgi?id=33574 works correctly?

include/clang/Lex/Preprocessor.h

2029

Duplicate "private:"

[ilya-biryukov]

• Removed redundant 'private' specifier.
• Added a test for https://bugs.llvm.org/show_bug.cgi?id=33574

[ilya-biryukov]

In D36872#845364, @erikjv wrote:

Can you check if the example in https://bugs.llvm.org/show_bug.cgi?id=33574 works correctly?

Works correctly, added an extra test for it.

[bkramer]

This looks good to me.

[nik]

I see this in trunk/master submitted, but not in the release_50 branch. Could this be cherry-picked to 5.0?

[hans]

In D36872#847416, @nik wrote:

I see this in trunk/master submitted, but not in the release_50 branch. Could this be cherry-picked to 5.0?

I've replied on the commit message thread. Thanks!

include/clang/Lex/Preprocessor.h

public:
   /// \brief Create a new preprocessing record, which will keep track of
   /// all macro expansions, macro definitions, etc.
   void createPreprocessingRecord();
 
   /// \brief Enter the specified FileID as the main source file,
   /// which implicitly adds the builtin defines etc.
   void EnterMainSourceFile();
 
-  /// \brief After parser warm-up, initialize the conditional stack from
-  /// the preamble.
-  void replayPreambleConditionalStack();
-
   /// \brief Inform the preprocessor callbacks that processing is complete.
   void EndSourceFile();
 
   /// \brief Add a source file to the top of the include stack and
   /// start lexing tokens from it instead of the current buffer.
   ///
   /// Emits a diagnostic, doesn't enter the file, and returns true on error.
   bool EnterSourceFile(FileID CurFileID, const DirectoryLookup *Dir,
public:
   }
 
   void setReplayablePreambleConditionalStack(ArrayRef<PPConditionalInfo> s) {
     PreambleConditionalStack.startReplaying();
     PreambleConditionalStack.setStack(s);
   }
 
 private:
+  /// \brief After processing predefined file, initialize the conditional stack from
+  /// the preamble.
+  void replayPreambleConditionalStack();
+
+private:
   // Macro handling.
   void HandleDefineDirective(Token &Tok, bool ImmediatelyAfterTopLevelIfndef);
   void HandleUndefDirective();
 
   // Conditional Inclusion.
   void HandleIfdefDirective(Token &Tok, bool isIfndef,
                             bool ReadAnyTokensBeforeDirective);
   void HandleIfDirective(Token &Tok, bool ReadAnyTokensBeforeDirective);

lib/Lex/PPLexerChange.cpp

if (!isEndOfMacro && CurPPLexer &&
       // Notify SourceManager to record the number of FileIDs that were created
       // during lexing of the #include'd file.
       unsigned NumFIDs =
           SourceMgr.local_sloc_entry_size() -
           CurPPLexer->getInitialNumSLocEntries() + 1/*#include'd file*/;
       SourceMgr.setNumCreatedFIDsForFileID(CurPPLexer->getFileID(), NumFIDs);
     }
 
+    bool ExitedFromPredefinesFile = false;
     FileID ExitedFID;
-    if (Callbacks && !isEndOfMacro && CurPPLexer)
+    if (!isEndOfMacro && CurPPLexer) {
       ExitedFID = CurPPLexer->getFileID();
 
+      assert(PredefinesFileID.isValid() &&
+             "HandleEndOfFile is called before PredefinesFileId is set");
+      ExitedFromPredefinesFile = (PredefinesFileID == ExitedFID);
+    }
+
     if (LeavingSubmodule) {
       // We're done with this submodule.
       Module *M = LeaveSubmodule(/*ForPragma*/false);
 
       // Notify the parser that we've left the module.
       const char *EndPos = getCurLexerEndPos();
       Result.startToken();
       CurLexer->BufferPtr = EndPos;
if (!IncludeMacroStack.empty()) {
     // Notify the client, if desired, that we are in a new source file.
     if (Callbacks && !isEndOfMacro && CurPPLexer) {
       SrcMgr::CharacteristicKind FileType =
         SourceMgr.getFileCharacteristic(CurPPLexer->getSourceLocation());
       Callbacks->FileChanged(CurPPLexer->getSourceLocation(),
                              PPCallbacks::ExitFile, FileType, ExitedFID);
     }
 
+    // Restore conditional stack from the preamble right after exiting from the
+    // predefines file.
+    if (ExitedFromPredefinesFile)
+      replayPreambleConditionalStack();
+
     // Client should lex another token unless we generated an EOM.
     return LeavingSubmodule;
   }
 
   // If this is the end of the main file, form an EOF token.
   if (CurLexer) {
     const char *EndPos = getCurLexerEndPos();
     Result.startToken();

lib/Lex/Preprocessor.cpp

void Preprocessor::EnterMainSourceFile() {
 
   // Start parsing the predefines.
   EnterSourceFile(FID, nullptr, SourceLocation());
 }
 
 void Preprocessor::replayPreambleConditionalStack() {
   // Restore the conditional stack from the preamble, if there is one.
   if (PreambleConditionalStack.isReplaying()) {
+    assert(CurPPLexer &&
+           "CurPPLexer is null when calling replayPreambleConditionalStack.");
     CurPPLexer->setConditionalLevels(PreambleConditionalStack.getStack());
     PreambleConditionalStack.doneReplaying();
   }
 }
 
 void Preprocessor::EndSourceFile() {
   // Notify the client that we reached the end of the source file.
   if (Callbacks)

lib/Parse/Parser.cpp

if(getLangOpts().Borland) {
     PP.SetPoisonReason(Ident___abnormal_termination,diag::err_seh___finally_block);
     PP.SetPoisonReason(Ident_AbnormalTermination,diag::err_seh___finally_block);
   }
 
   Actions.Initialize();
 
   // Prime the lexer look-ahead.
   ConsumeToken();
-
-  PP.replayPreambleConditionalStack();
 }
 
 void Parser::LateTemplateParserCleanupCallback(void *P) {
   // While this RAII helper doesn't bracket any actual work, the destructor will
   // clean up annotations that were created during ActOnEndOfTranslationUnit
   // when incremental processing is enabled.
   DestroyTemplateIdAnnotationsRAIIObj CleanupRAII(((Parser *)P)->TemplateIds);
 }

test/Index/preamble-conditionals-crash.cpp

+#ifndef HEADER_GUARD
+
+#define FOO int aba;
+FOO
+
+#endif
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-load-source-reparse 5 \
+// RUN:                                       local -std=c++14 %s 2>&1 \
+// RUN: | FileCheck %s --implicit-check-not "libclang: crash detected" \
+// RUN:                --implicit-check-not "error:"
+// CHECK: macro expansion=FOO:3:9 Extent=[4:1 - 4:4]
+// CHECK: VarDecl=aba:4:1 (Definition) Extent=[4:1 - 4:4]