Thanks for poking!<div><br></div><div><div>diff --git lib/Lex/Pragma.cpp lib/Lex/Pragma.cpp</div><div>index 7b94d26..c9cc4ad 100644</div><div>--- lib/Lex/Pragma.cpp</div><div>+++ lib/Lex/Pragma.cpp</div><div>@@ -1010,6 +1010,10 @@ struct PragmaDebugHandler : public PragmaHandler {</div>
<div> llvm_unreachable("This is an assertion!");</div><div> } else if (II->isStr("crash")) {</div><div> *(volatile int*) 0x11 = 0;</div><div><br></div><div>Wait, what? __builtin_trap maybe?</div>
<div><br></div><div>+ } else if (II->isStr("parser_crash")) {</div><div>+ Token Crasher;</div><div>+ Crasher.setKind(tok::annot_pragma_parser_crash);</div><div>+ PP.EnterToken(Crasher);</div>
<div> } else if (II->isStr("llvm_fatal_error")) {</div><div> llvm::report_fatal_error("#pragma clang __debug llvm_fatal_error");</div><div> } else if (II->isStr("llvm_unreachable")) {</div>
<div>diff --git lib/Parse/ParseDecl.cpp lib/Parse/ParseDecl.cpp</div><div>index 489a46c..409f8e0 100644</div><div>--- lib/Parse/ParseDecl.cpp</div><div>+++ lib/Parse/ParseDecl.cpp</div><div>@@ -4252,6 +4252,8 @@ void Parser::ParseDirectDeclarator(Declarator &D) {</div>
<div> // portion is empty), if an abstract-declarator is allowed.</div><div> D.SetIdentifier(0, Tok.getLocation());</div><div> } else {</div><div>+ if (Tok.getKind() == tok::annot_pragma_parser_crash)</div><div>
+ *(volatile int*) 0x11 = 0;</div><div><br></div><div>Yea, here too.</div><div><br></div><div>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.</div>
</div><div><br></div><div><br></div><div><div>--- /dev/null</div><div>+++ test/Driver/crash-report.c</div><div>@@ -0,0 +1,6 @@</div><div>+// RUN: %clang -fsyntax-only %s 2>&1 | FileCheck %s</div><div>+// REQUIRES: crash-recovery</div>
<div>+</div><div>+#pragma clang __debug parser_crash</div><div>+// CHECK: Preprocessed source(s) and associated run script(s) are located at:</div><div>+// CHECK-NEXT: clang-3: note: diagnostic msg: {{.*}}.c</div></div><div>
<br></div><div>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?</div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jun 29, 2012 at 2:42 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">*vigorous ping*<br>
<div class="HOEnZb"><div class="h5"><br>
On Wed, Jun 27, 2012 at 10:10 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
> On Tue, Jun 26, 2012 at 2:09 PM, Eli Friedman <<a href="mailto:eli.friedman@gmail.com">eli.friedman@gmail.com</a>> wrote:<br>
>> On Tue, Jun 26, 2012 at 1:54 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
>>> On Fri, Jun 22, 2012 at 4:50 PM, Chandler Carruth <<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>> wrote:<br>
>>>> Doh, sorry for missing this last patch.<br>
>>>><br>
>>>> Looks pretty good. As discussed in person, you may want to sink the arg<br>
>>>> mutation below the PrintJob... Not sure.<br>
>>><br>
>>> We also discussed the possibility of avoiding mutating the original<br>
>>> args - did you/we end up dismissing that since we're mutating the jobs<br>
>>> anyway, so there's no real immutability invariant that we'd be<br>
>>> mantaining?<br>
>>><br>
>>> As for moving it, if it's all the same to you, I think I'll just leave<br>
>>> it where it is next to the point at which we raise the CCCIsCPP flag<br>
>>> as they seem related. (I could move all of that down below PrintJob I<br>
>>> suppose, though... )<br>
>>><br>
>>>> I'm concerned by the lack of test case modifications. If we don't have a<br>
>>>> test case yet for this, lets get one and check the filename. If we do, we<br>
>>>> should tighten it up to check the filename or figure out whats going on.<br>
>>><br>
>>> Agreed, but how exactly would I do that? The existing mechanism we<br>
>>> have for crashing clang is "#pragma clang __debug crash" which crashes<br>
>>> in the preprocessor. That means it crashes when crash recovery<br>
>>> attempts to produce the preprocessed source file. As we discussed<br>
>>> offline there's probably a couple of options here:<br>
>>><br>
>>> 1) Transform the pragma into a special token then look for that token<br>
>>> in the parser & crash there. But how do we do this without adding a<br>
>>> test to a very hot part of the parser?<br>
>><br>
>> We don't necessarily need to crash anywhere we see the token in<br>
>> question; we could say it's only "legal" in places where a top-level<br>
>> declaration would be allowed.<br>
><br>
> Ah, that certainly helps.<br>
><br>
> So I've implemented a basic version of this - all I did was find where<br>
> we handle the crash pragma, added another "parser_crash" and inserted<br>
> a token (annot_pragma_parser_crash) into the stream. Then I found<br>
> where that crashed in the parser & added an explicit check/crash for<br>
> it. This still means you'll get arbitrary behavior/crashes if you use<br>
> this pragma elsewhere - is that acceptable "invalid" behavior for such<br>
> an implementation detail, or should I make this more robust in some<br>
> way?<br>
><br>
> Attached is the parser_crash support, test case (testing only the file<br>
> extension mentioned in the crash report - is there some way I could<br>
> actually open that file from within lit so I could FileCheck it? I<br>
> guess that's just not worth the effort?) and the original<br>
> functionality I'd sent for review.<br>
><br>
> - David<br>
</div></div></blockquote></div><br></div>