<div dir="ltr">LG for branch.</div><div class="gmail_extra"><br><div class="gmail_quote">On 23 August 2017 at 11:04, Hans Wennborg via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Richard: ping?<br>
<div class="HOEnZb"><div class="h5"><br>
On Mon, Aug 21, 2017 at 1:00 PM, Hans Wennborg <<a href="mailto:hans@chromium.org">hans@chromium.org</a>> wrote:<br>
> Nikolai suggested this should be merged to 5.0. Richard, what do you think?<br>
><br>
> On Mon, Aug 21, 2017 at 5:03 AM, Ilya Biryukov via cfe-commits<br>
> <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br>
>> Author: ibiryukov<br>
>> Date: Mon Aug 21 05:03:08 2017<br>
>> New Revision: 311330<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=311330&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=311330&view=rev</a><br>
>> Log:<br>
>> Fixed a crash on replaying Preamble's PP conditional stack.<br>
>><br>
>> Summary:<br>
>> The crash occurs when the first token after a preamble is a macro<br>
>> expansion.<br>
>> Fixed by moving replayPreambleConditionalStack from Parser into<br>
>> Preprocessor. It is now called right after the predefines file is<br>
>> processed.<br>
>><br>
>> Reviewers: erikjv, bkramer, klimek, yvvan<br>
>><br>
>> Reviewed By: bkramer<br>
>><br>
>> Subscribers: cfe-commits<br>
>><br>
>> Differential Revision: <a href="https://reviews.llvm.org/D36872" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D36872</a><br>
>><br>
>> Added:<br>
>>     cfe/trunk/test/Index/preamble-<wbr>conditionals-crash.cpp<br>
>>     cfe/trunk/test/Index/preamble-<wbr>conditionals.cpp<br>
>> Modified:<br>
>>     cfe/trunk/include/clang/Lex/<wbr>Preprocessor.h<br>
>>     cfe/trunk/lib/Lex/<wbr>PPLexerChange.cpp<br>
>>     cfe/trunk/lib/Lex/<wbr>Preprocessor.cpp<br>
>>     cfe/trunk/lib/Parse/Parser.cpp<br>
>><br>
>> Modified: cfe/trunk/include/clang/Lex/<wbr>Preprocessor.h<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=311330&r1=311329&r2=311330&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/include/<wbr>clang/Lex/Preprocessor.h?rev=<wbr>311330&r1=311329&r2=311330&<wbr>view=diff</a><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- cfe/trunk/include/clang/Lex/<wbr>Preprocessor.h (original)<br>
>> +++ cfe/trunk/include/clang/Lex/<wbr>Preprocessor.h Mon Aug 21 05:03:08 2017<br>
>> @@ -1049,10 +1049,6 @@ public:<br>
>>    /// which implicitly adds the builtin defines etc.<br>
>>    void EnterMainSourceFile();<br>
>><br>
>> -  /// \brief After parser warm-up, initialize the conditional stack from<br>
>> -  /// the preamble.<br>
>> -  void replayPreambleConditionalStack<wbr>();<br>
>> -<br>
>>    /// \brief Inform the preprocessor callbacks that processing is complete.<br>
>>    void EndSourceFile();<br>
>><br>
>> @@ -2026,6 +2022,10 @@ public:<br>
>>    }<br>
>><br>
>>  private:<br>
>> +  /// \brief After processing predefined file, initialize the conditional stack from<br>
>> +  /// the preamble.<br>
>> +  void replayPreambleConditionalStack<wbr>();<br>
>> +<br>
>>    // Macro handling.<br>
>>    void HandleDefineDirective(Token &Tok, bool ImmediatelyAfterTopLevelIfndef<wbr>);<br>
>>    void HandleUndefDirective();<br>
>><br>
>> Modified: cfe/trunk/lib/Lex/<wbr>PPLexerChange.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPLexerChange.cpp?rev=311330&r1=311329&r2=311330&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/Lex/<wbr>PPLexerChange.cpp?rev=311330&<wbr>r1=311329&r2=311330&view=diff</a><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- cfe/trunk/lib/Lex/<wbr>PPLexerChange.cpp (original)<br>
>> +++ cfe/trunk/lib/Lex/<wbr>PPLexerChange.cpp Mon Aug 21 05:03:08 2017<br>
>> @@ -458,10 +458,16 @@ bool Preprocessor::HandleEndOfFile(<wbr>Token<br>
>>        SourceMgr.<wbr>setNumCreatedFIDsForFileID(<wbr>CurPPLexer->getFileID(), NumFIDs);<br>
>>      }<br>
>><br>
>> +    bool ExitedFromPredefinesFile = false;<br>
>>      FileID ExitedFID;<br>
>> -    if (Callbacks && !isEndOfMacro && CurPPLexer)<br>
>> +    if (!isEndOfMacro && CurPPLexer) {<br>
>>        ExitedFID = CurPPLexer->getFileID();<br>
>><br>
>> +      assert(PredefinesFileID.<wbr>isValid() &&<br>
>> +             "HandleEndOfFile is called before PredefinesFileId is set");<br>
>> +      ExitedFromPredefinesFile = (PredefinesFileID == ExitedFID);<br>
>> +    }<br>
>> +<br>
>>      if (LeavingSubmodule) {<br>
>>        // We're done with this submodule.<br>
>>        Module *M = LeaveSubmodule(/*ForPragma*/<wbr>false);<br>
>> @@ -489,6 +495,11 @@ bool Preprocessor::HandleEndOfFile(<wbr>Token<br>
>>                               PPCallbacks::ExitFile, FileType, ExitedFID);<br>
>>      }<br>
>><br>
>> +    // Restore conditional stack from the preamble right after exiting from the<br>
>> +    // predefines file.<br>
>> +    if (ExitedFromPredefinesFile)<br>
>> +      replayPreambleConditionalStack<wbr>();<br>
>> +<br>
>>      // Client should lex another token unless we generated an EOM.<br>
>>      return LeavingSubmodule;<br>
>>    }<br>
>><br>
>> Modified: cfe/trunk/lib/Lex/<wbr>Preprocessor.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Preprocessor.cpp?rev=311330&r1=311329&r2=311330&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/Lex/<wbr>Preprocessor.cpp?rev=311330&<wbr>r1=311329&r2=311330&view=diff</a><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- cfe/trunk/lib/Lex/<wbr>Preprocessor.cpp (original)<br>
>> +++ cfe/trunk/lib/Lex/<wbr>Preprocessor.cpp Mon Aug 21 05:03:08 2017<br>
>> @@ -540,6 +540,8 @@ void Preprocessor::<wbr>EnterMainSourceFile()<br>
>>  void Preprocessor::<wbr>replayPreambleConditionalStack<wbr>() {<br>
>>    // Restore the conditional stack from the preamble, if there is one.<br>
>>    if (PreambleConditionalStack.<wbr>isReplaying()) {<br>
>> +    assert(CurPPLexer &&<br>
>> +           "CurPPLexer is null when calling replayPreambleConditionalStack<wbr>.");<br>
>>      CurPPLexer-><wbr>setConditionalLevels(<wbr>PreambleConditionalStack.<wbr>getStack());<br>
>>      PreambleConditionalStack.<wbr>doneReplaying();<br>
>>    }<br>
>><br>
>> Modified: cfe/trunk/lib/Parse/Parser.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/Parser.cpp?rev=311330&r1=311329&r2=311330&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/Parse/<wbr>Parser.cpp?rev=311330&r1=<wbr>311329&r2=311330&view=diff</a><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- cfe/trunk/lib/Parse/Parser.cpp (original)<br>
>> +++ cfe/trunk/lib/Parse/Parser.cpp Mon Aug 21 05:03:08 2017<br>
>> @@ -516,8 +516,6 @@ void Parser::Initialize() {<br>
>><br>
>>    // Prime the lexer look-ahead.<br>
>>    ConsumeToken();<br>
>> -<br>
>> -  PP.<wbr>replayPreambleConditionalStack<wbr>();<br>
>>  }<br>
>><br>
>>  void Parser::<wbr>LateTemplateParserCleanupCallb<wbr>ack(void *P) {<br>
>><br>
>> Added: cfe/trunk/test/Index/preamble-<wbr>conditionals-crash.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/preamble-conditionals-crash.cpp?rev=311330&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/Index/<wbr>preamble-conditionals-crash.<wbr>cpp?rev=311330&view=auto</a><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- cfe/trunk/test/Index/preamble-<wbr>conditionals-crash.cpp (added)<br>
>> +++ cfe/trunk/test/Index/preamble-<wbr>conditionals-crash.cpp Mon Aug 21 05:03:08 2017<br>
>> @@ -0,0 +1,12 @@<br>
>> +#ifndef HEADER_GUARD<br>
>> +<br>
>> +#define FOO int aba;<br>
>> +FOO<br>
>> +<br>
>> +#endif<br>
>> +// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-load-source-reparse 5 \<br>
>> +// RUN:                                       local -std=c++14 %s 2>&1 \<br>
>> +// RUN: | FileCheck %s --implicit-check-not "libclang: crash detected" \<br>
>> +// RUN:                --implicit-check-not "error:"<br>
>> +// CHECK: macro expansion=FOO:3:9 Extent=[4:1 - 4:4]<br>
>> +// CHECK: VarDecl=aba:4:1 (Definition) Extent=[4:1 - 4:4]<br>
>><br>
>> Added: cfe/trunk/test/Index/preamble-<wbr>conditionals.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/preamble-conditionals.cpp?rev=311330&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/Index/<wbr>preamble-conditionals.cpp?rev=<wbr>311330&view=auto</a><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- cfe/trunk/test/Index/preamble-<wbr>conditionals.cpp (added)<br>
>> +++ cfe/trunk/test/Index/preamble-<wbr>conditionals.cpp Mon Aug 21 05:03:08 2017<br>
>> @@ -0,0 +1,8 @@<br>
>> +// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-load-source local %s 2>&1 \<br>
>> +// RUN: | FileCheck %s --implicit-check-not "error:"<br>
>> +#ifndef FOO_H<br>
>> +#define FOO_H<br>
>> +<br>
>> +void foo();<br>
>> +<br>
>> +#endif<br>
>><br>
>><br>
>> ______________________________<wbr>_________________<br>
>> cfe-commits mailing list<br>
>> <a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br></div>