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

[Hans Wennborg via cfe-commits]

Nikolai suggested this should be merged to 5.0. Richard, what do you think?

On Mon, Aug 21, 2017 at 5:03 AM, Ilya Biryukov via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> Author: ibiryukov
> Date: Mon Aug 21 05:03:08 2017
> New Revision: 311330
>
> URL: http://llvm.org/viewvc/llvm-project?rev=311330&view=rev
> Log:
> Fixed a crash on replaying Preamble's PP conditional stack.
>
> 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.
>
> Reviewers: erikjv, bkramer, klimek, yvvan
>
> Reviewed By: bkramer
>
> Subscribers: cfe-commits
>
> Differential Revision: https://reviews.llvm.org/D36872
>
> Added:
>     cfe/trunk/test/Index/preamble-conditionals-crash.cpp
>     cfe/trunk/test/Index/preamble-conditionals.cpp
> Modified:
>     cfe/trunk/include/clang/Lex/Preprocessor.h
>     cfe/trunk/lib/Lex/PPLexerChange.cpp
>     cfe/trunk/lib/Lex/Preprocessor.cpp
>     cfe/trunk/lib/Parse/Parser.cpp
>
> Modified: cfe/trunk/include/clang/Lex/Preprocessor.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=311330&r1=311329&r2=311330&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Lex/Preprocessor.h (original)
> +++ cfe/trunk/include/clang/Lex/Preprocessor.h Mon Aug 21 05:03:08 2017
> @@ -1049,10 +1049,6 @@ public:
>    /// 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();
>
> @@ -2026,6 +2022,10 @@ public:
>    }
>
>  private:
> +  /// \brief After processing predefined file, initialize the conditional stack from
> +  /// the preamble.
> +  void replayPreambleConditionalStack();
> +
>    // Macro handling.
>    void HandleDefineDirective(Token &Tok, bool ImmediatelyAfterTopLevelIfndef);
>    void HandleUndefDirective();
>
> Modified: cfe/trunk/lib/Lex/PPLexerChange.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPLexerChange.cpp?rev=311330&r1=311329&r2=311330&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/PPLexerChange.cpp (original)
> +++ cfe/trunk/lib/Lex/PPLexerChange.cpp Mon Aug 21 05:03:08 2017
> @@ -458,10 +458,16 @@ bool Preprocessor::HandleEndOfFile(Token
>        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);
> @@ -489,6 +495,11 @@ bool Preprocessor::HandleEndOfFile(Token
>                               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;
>    }
>
> Modified: cfe/trunk/lib/Lex/Preprocessor.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Preprocessor.cpp?rev=311330&r1=311329&r2=311330&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/Preprocessor.cpp (original)
> +++ cfe/trunk/lib/Lex/Preprocessor.cpp Mon Aug 21 05:03:08 2017
> @@ -540,6 +540,8 @@ void Preprocessor::EnterMainSourceFile()
>  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();
>    }
>
> Modified: cfe/trunk/lib/Parse/Parser.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/Parser.cpp?rev=311330&r1=311329&r2=311330&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/Parser.cpp (original)
> +++ cfe/trunk/lib/Parse/Parser.cpp Mon Aug 21 05:03:08 2017
> @@ -516,8 +516,6 @@ void Parser::Initialize() {
>
>    // Prime the lexer look-ahead.
>    ConsumeToken();
> -
> -  PP.replayPreambleConditionalStack();
>  }
>
>  void Parser::LateTemplateParserCleanupCallback(void *P) {
>
> Added: cfe/trunk/test/Index/preamble-conditionals-crash.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/preamble-conditionals-crash.cpp?rev=311330&view=auto
> ==============================================================================
> --- cfe/trunk/test/Index/preamble-conditionals-crash.cpp (added)
> +++ cfe/trunk/test/Index/preamble-conditionals-crash.cpp Mon Aug 21 05:03:08 2017
> @@ -0,0 +1,12 @@
> +#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]
>
> Added: cfe/trunk/test/Index/preamble-conditionals.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/preamble-conditionals.cpp?rev=311330&view=auto
> ==============================================================================
> --- cfe/trunk/test/Index/preamble-conditionals.cpp (added)
> +++ cfe/trunk/test/Index/preamble-conditionals.cpp Mon Aug 21 05:03:08 2017
> @@ -0,0 +1,8 @@
> +// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-load-source local %s 2>&1 \
> +// RUN: | FileCheck %s --implicit-check-not "error:"
> +#ifndef FOO_H
> +#define FOO_H
> +
> +void foo();
> +
> +#endif
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits