[cfe-commits] [patch] rewrite-includes crash support

[Chandler Carruth]

Thanks for poking!

diff --git lib/Lex/Pragma.cpp lib/Lex/Pragma.cpp
index 7b94d26..c9cc4ad 100644
--- lib/Lex/Pragma.cpp
+++ lib/Lex/Pragma.cpp
@@ -1010,6 +1010,10 @@ struct PragmaDebugHandler : public PragmaHandler {
       llvm_unreachable("This is an assertion!");
     } else if (II->isStr("crash")) {
       *(volatile int*) 0x11 = 0;

Wait, what? __builtin_trap maybe?

+    } else if (II->isStr("parser_crash")) {
+      Token Crasher;
+      Crasher.setKind(tok::annot_pragma_parser_crash);
+      PP.EnterToken(Crasher);
     } else if (II->isStr("llvm_fatal_error")) {
       llvm::report_fatal_error("#pragma clang __debug llvm_fatal_error");
     } else if (II->isStr("llvm_unreachable")) {
diff --git lib/Parse/ParseDecl.cpp lib/Parse/ParseDecl.cpp
index 489a46c..409f8e0 100644
--- lib/Parse/ParseDecl.cpp
+++ lib/Parse/ParseDecl.cpp
@@ -4252,6 +4252,8 @@ void Parser::ParseDirectDeclarator(Declarator &D) {
     // portion is empty), if an abstract-declarator is allowed.
     D.SetIdentifier(0, Tok.getLocation());
   } else {
+    if (Tok.getKind() == tok::annot_pragma_parser_crash)
+      *(volatile int*) 0x11 = 0;

Yea, here too.

Don't fix this in this patch -- I expect we'll need to try a few times to
get a portable incantation of __builtin_trap, likely involving a Compiler.h
bit of magic to do the volatile pointer thwack when the compiler doesn't
have __builtin_trap.


--- /dev/null
+++ test/Driver/crash-report.c
@@ -0,0 +1,6 @@
+// RUN: %clang -fsyntax-only %s 2>&1 | FileCheck %s
+// REQUIRES: crash-recovery
+
+#pragma clang __debug parser_crash
+// CHECK: Preprocessed source(s) and associated run script(s) are located
at:
+// CHECK-NEXT: clang-3: note: diagnostic msg: {{.*}}.c

Isn't this going to litter 'tmp' with crash reductions? I think our bots
will be sad. Is there a tool to redirect the output to a specific file or
directory so we can put it in the expected %t-related place fro the
testsuite?


On Fri, Jun 29, 2012 at 2:42 PM, David Blaikie <dblaikie at gmail.com> wrote:

> *vigorous ping*
>
> On Wed, Jun 27, 2012 at 10:10 AM, David Blaikie <dblaikie at gmail.com>
> wrote:
> > On Tue, Jun 26, 2012 at 2:09 PM, Eli Friedman <eli.friedman at gmail.com>
> wrote:
> >> On Tue, Jun 26, 2012 at 1:54 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >>> On Fri, Jun 22, 2012 at 4:50 PM, Chandler Carruth <
> chandlerc at google.com> wrote:
> >>>> Doh, sorry for missing this last patch.
> >>>>
> >>>> Looks pretty good. As discussed in person, you may want to sink the
> arg
> >>>> mutation below the PrintJob... Not sure.
> >>>
> >>> We also discussed the possibility of avoiding mutating the original
> >>> args - did you/we end up dismissing that since we're mutating the jobs
> >>> anyway, so there's no real immutability invariant that we'd be
> >>> mantaining?
> >>>
> >>> As for moving it, if it's all the same to you, I think I'll just leave
> >>> it where it is next to the point at which we raise the CCCIsCPP flag
> >>> as they seem related. (I could move all of that down below PrintJob I
> >>> suppose, though... )
> >>>
> >>>> I'm concerned by the lack of test case modifications. If we don't
> have a
> >>>> test case yet for this, lets get one and check the filename. If we
> do, we
> >>>> should tighten it up to check the filename or figure out whats going
> on.
> >>>
> >>> Agreed, but how exactly would I do that? The existing mechanism we
> >>> have for crashing clang is "#pragma clang __debug crash" which crashes
> >>> in the preprocessor. That means it crashes when crash recovery
> >>> attempts to produce the preprocessed source file. As we discussed
> >>> offline there's probably a couple of options here:
> >>>
> >>> 1) Transform the pragma into a special token then look for that token
> >>> in the parser & crash there. But how do we do this without adding a
> >>> test to a very hot part of the parser?
> >>
> >> We don't necessarily need to crash anywhere we see the token in
> >> question; we could say it's only "legal" in places where a top-level
> >> declaration would be allowed.
> >
> > Ah, that certainly helps.
> >
> > So I've implemented a basic version of this - all I did was find where
> > we handle the crash pragma, added another "parser_crash" and inserted
> > a token (annot_pragma_parser_crash) into the stream. Then I found
> > where that crashed in the parser & added an explicit check/crash for
> > it. This still means you'll get arbitrary behavior/crashes if you use
> > this pragma elsewhere - is that acceptable "invalid" behavior for such
> > an implementation detail, or should I make this more robust in some
> > way?
> >
> > Attached is the parser_crash support, test case (testing only the file
> > extension mentioned in the crash report - is there some way I could
> > actually open that file from within lit so I could FileCheck it? I
> > guess that's just not worth the effort?) and the original
> > functionality I'd sent for review.
> >
> > - David
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120629/ed4cb376/attachment.html>