[PATCH] D36458: Fix crash when current lexer is nullptr
ilya-biryukov added a comment. Here's an alternative fix that moves the `replay...` call into `Preprocessor`: https://reviews.llvm.org/D36872. It also fixes the crash and doesn't add invalid compiler errors about non-matching `#ifdefs`. https://reviews.llvm.org/D36458 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36458: Fix crash when current lexer is nullptr
ilya-biryukov added inline comments. Comment at: lib/Parse/Parser.cpp:519 ConsumeToken(); - - PP.replayPreambleConditionalStack(); + if (!PP.isCurrentLexer(nullptr)) +PP.replayPreambleConditionalStack(); ilya-biryukov wrote: > This certainly fixes the crash, but aren't we breaking other things if we > don't run `replayConditionalStack`? > I.e. won't we get spurious errors on `#endif`? I get a spurious `#endif without #if` error with your fix. We need to figure out the right place to run `replayPreambleConditionalStack`. @erikjv, any ideas? https://reviews.llvm.org/D36458 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36458: Fix crash when current lexer is nullptr
ilya-biryukov added a comment.
I've also stumbled upon this crash. Here is an even simpler repro that crashes
for me (does not involve includes):
#ifndef HEADER_GUARD
#define FOO(X) int fhjdfhsdjkfhjkshfjk;
FOO(base)
struct X {
int a;
};
int test() {
X v;
v.
}
#endif
Comment at: lib/Parse/Parser.cpp:519
ConsumeToken();
-
- PP.replayPreambleConditionalStack();
+ if (!PP.isCurrentLexer(nullptr))
+PP.replayPreambleConditionalStack();
This certainly fixes the crash, but aren't we breaking other things if we don't
run `replayConditionalStack`?
I.e. won't we get spurious errors on `#endif`?
https://reviews.llvm.org/D36458
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36458: Fix crash when current lexer is nullptr
yvvan updated this revision to Diff 110978.
yvvan added a comment.
I have minimized the crash case and added simple test for it.
https://reviews.llvm.org/D36458
Files:
lib/Parse/Parser.cpp
test/Index/std-begin-error.cpp
Index: test/Index/std-begin-error.cpp
===
--- test/Index/std-begin-error.cpp
+++ test/Index/std-begin-error.cpp
@@ -0,0 +1,9 @@
+#ifndef RC_INVOKED
+#include
+_STD_BEGIN
+}
+#endif /* RC_INVOKED */
+
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-load-source-reparse 5
local -std=c++14 %S\std-begin-error.cpp 2> %t.err
+// RUN: not FileCheck < %t.err %s
+// CHECK: Failure: libclang crashed
Index: lib/Parse/Parser.cpp
===
--- lib/Parse/Parser.cpp
+++ lib/Parse/Parser.cpp
@@ -516,8 +516,8 @@
// Prime the lexer look-ahead.
ConsumeToken();
-
- PP.replayPreambleConditionalStack();
+ if (!PP.isCurrentLexer(nullptr))
+PP.replayPreambleConditionalStack();
}
void Parser::LateTemplateParserCleanupCallback(void *P) {
Index: test/Index/std-begin-error.cpp
===
--- test/Index/std-begin-error.cpp
+++ test/Index/std-begin-error.cpp
@@ -0,0 +1,9 @@
+#ifndef RC_INVOKED
+#include
+_STD_BEGIN
+}
+#endif /* RC_INVOKED */
+
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-load-source-reparse 5 local -std=c++14 %S\std-begin-error.cpp 2> %t.err
+// RUN: not FileCheck < %t.err %s
+// CHECK: Failure: libclang crashed
Index: lib/Parse/Parser.cpp
===
--- lib/Parse/Parser.cpp
+++ lib/Parse/Parser.cpp
@@ -516,8 +516,8 @@
// Prime the lexer look-ahead.
ConsumeToken();
-
- PP.replayPreambleConditionalStack();
+ if (!PP.isCurrentLexer(nullptr))
+PP.replayPreambleConditionalStack();
}
void Parser::LateTemplateParserCleanupCallback(void *P) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36458: Fix crash when current lexer is nullptr
yvvan added a comment. But I don't know the source of that issue, I only know how to fix it because I caught that one in debugger and added a check to prevent it. How can I write testcase for such fix? https://reviews.llvm.org/D36458 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36458: Fix crash when current lexer is nullptr
bkramer added a comment. Changes like this should come with a small, c-index-test based, test case so we don't reintroduce the same bug in the future. https://reviews.llvm.org/D36458 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36458: Fix crash when current lexer is nullptr
yvvan added a comment. And if you just meant how to reproduce: You need to parse and reparse standard header (my current one is C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\memory). I can provide the options and a command line that I have if you can't reproduce it with default ones... https://reviews.llvm.org/D36458 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36458: Fix crash when current lexer is nullptr
yvvan added a comment. Do you mean the testcase that runs only for windows with ms-extensions flag and msvc includes? I don't know if clang has that kind of tests. Does it? https://reviews.llvm.org/D36458 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36458: Fix crash when current lexer is nullptr
bkramer added a comment. Test case? https://reviews.llvm.org/D36458 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
