<div dir="ltr">If it's really just a case of the verifier erroneously failing on this input and the input isn't unique/interesting in any other way (ie: all optimizations, codegen, etc, wouldn't see it as 'different' or interesting at all) then perhaps that's sufficient to have the verifier test that just says "doesn't consider this invalid".</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jul 8, 2016 at 4:05 PM, Gao, Yunzhong <span dir="ltr"><<a href="mailto:Yunzhong.Gao@sony.com" target="_blank">Yunzhong.Gao@sony.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Adrian, David,<br>
I can add CHECK lines to the test just like what was added for the other functions in the same file.<br>
If there is a better way to test verifier errors or a better place to add such test, could you advise?<br>
I can understand what David is saying, but I would need help in order to make the test more specific,<br>
- Gao<br>
<br>
<br>
<br>
________________________________________<br>
From: <a href="mailto:aprantl@apple.com">aprantl@apple.com</a> [<a href="mailto:aprantl@apple.com">aprantl@apple.com</a>]<br>
Sent: Friday, July 08, 2016 2:47 PM<br>
To: David Blaikie<br>
Cc: <a href="mailto:reviews%2BD21818%2Bpublic%2Bb40f0be86340e0e5@reviews.llvm.org">reviews+D21818+public+b40f0be86340e0e5@reviews.llvm.org</a>; Gao, Yunzhong; Eric Christopher; llvm-commits<br>
<span class="">Subject: Re: [PATCH] D21818: Add artificial debug information to avoid compiler crash<br>
<br>
</span>On Jul 8, 2016, at 2:44 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a><mailto:<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>>> wrote:<br>
<div><div class="h5"><br>
<br>
<br>
On Tue, Jun 28, 2016 at 3:28 PM, Yunzhong Gao via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><mailto:<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>>> wrote:<br>
ygao created this revision.<br>
ygao added reviewers: aprantl, echristo, dblaikie.<br>
ygao added a subscriber: llvm-commits.<br>
Herald added a subscriber: mehdi_amini.<br>
<br>
Hi,<br>
The compiler crashed with the following test program:<br>
<br>
/* test.c<br>
* To reproduce:<br>
* $ clang -c -g -fstack-protector-strong test.c<br>
* (fatal error: error in backend: Broken function found, compilation aborted!)<br>
* (need an assertion-enabled build of the compiler)<br>
*/<br>
void __stack_chk_fail(void) { return; }<br>
<br>
void foo() {<br>
char bar[5];<br>
}<br>
/* end of test.c */<br>
<br>
This is not a legitimate input C file because __stack_chk_fail is a reserved function name.<br>
<br>
Is there a legitimate case this fix addresses? Might be good to have such a test case so we don't lose the coverage if/when this issue is fixed.<br>
<br>
If there isn't, then perhaps we should fix the original issue (allowing this invalid construct) rather than adding downstream workarounds?<br>
<br>
IIRC, this patch fixed an IR Verifier error triggered by the StackProtector pass which used to insert a function call without attaching a valid DebugLoc to the call.<br>
<br>
-- adrian<br>
<br>
<br>
On the other hand, it seems that the compiler should be able to give a nicer diagnostic or<br>
otherwise fail more gracefully instead of having to crash on invalid input (in the spirit of all<br>
"internal compiler error"s are compiler bugs).<br>
<br>
The proposed patch here attempts to add artificial debug information when the compiler<br>
generates the call to __stack_chk_fail. Does this look like a reasonable thing to do?<br>
<br>
Many thanks in advance,<br>
- Gao<br>
<br>
<a href="http://reviews.llvm.org/D21818" rel="noreferrer" target="_blank">http://reviews.llvm.org/D21818</a><br>
<br>
Files:<br>
lib/CodeGen/StackProtector.cpp<br>
test/CodeGen/X86/stack-protector.ll<br>
<br>
Index: test/CodeGen/X86/stack-protector.ll<br>
===================================================================<br>
--- test/CodeGen/X86/stack-protector.ll<br>
+++ test/CodeGen/X86/stack-protector.ll<br>
@@ -3880,6 +3880,17 @@<br>
ret i32 %call<br>
}<br>
<br>
+define void @__stack_chk_fail() #1 !dbg !6 {<br>
+entry:<br>
+ ret void<br>
+}<br>
+<br>
+define void @test32() #1 !dbg !7 {<br>
+entry:<br>
+ %0 = alloca [5 x i8], align 1<br>
+ ret void<br>
+}<br>
+<br>
declare double @testi_aux()<br>
declare i8* @strcpy(i8*, i8*)<br>
declare i32 @printf(i8*, ...)<br>
@@ -3899,3 +3910,16 @@<br>
attributes #3 = { ssp "stack-protector-buffer-size"="33" }<br>
attributes #4 = { ssp "stack-protector-buffer-size"="5" }<br>
attributes #5 = { ssp "stack-protector-buffer-size"="6" }<br>
+<br>
</div></div>+!<a href="http://llvm.dbg.cu" rel="noreferrer" target="_blank">llvm.dbg.cu</a><<a href="http://llvm.dbg.cu/" rel="noreferrer" target="_blank">http://llvm.dbg.cu/</a>> = !{!0}<br>
<div><div class="h5">+!llvm.module.flags = !{!3, !4}<br>
+!llvm.ident = !{!5}<br>
+<br>
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1)<br>
+!1 = !DIFile(filename: "test.c", directory: "/tmp")<br>
+!2 = !{}<br>
+!3 = !{i32 2, !"Dwarf Version", i32 4}<br>
+!4 = !{i32 2, !"Debug Info Version", i32 3}<br>
+!5 = !{!"clang version x.y.z"}<br>
+!6 = distinct !DISubprogram(name: "__stack_chk_fail", scope: !1, unit: !0)<br>
+!7 = distinct !DISubprogram(name: "foo", scope: !1, unit: !0)<br>
Index: lib/CodeGen/StackProtector.cpp<br>
===================================================================<br>
--- lib/CodeGen/StackProtector.cpp<br>
+++ lib/CodeGen/StackProtector.cpp<br>
@@ -25,6 +25,7 @@<br>
#include "llvm/IR/Attributes.h"<br>
#include "llvm/IR/Constants.h"<br>
#include "llvm/IR/DataLayout.h"<br>
+#include "llvm/IR/DebugInfo.h"<br>
#include "llvm/IR/DerivedTypes.h"<br>
#include "llvm/IR/Function.h"<br>
#include "llvm/IR/GlobalValue.h"<br>
@@ -448,6 +449,7 @@<br>
LLVMContext &Context = F->getContext();<br>
BasicBlock *FailBB = BasicBlock::Create(Context, "CallStackCheckFailBlk", F);<br>
IRBuilder<> B(FailBB);<br>
+ B.SetCurrentDebugLocation(DebugLoc::get(0, 0, F->getSubprogram()));<br>
if (Trip.isOSOpenBSD()) {<br>
Constant *StackChkFail =<br>
M->getOrInsertFunction("__stack_smash_handler",<br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
</div></div><a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><mailto:<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
<br>
<br>
<br>
</blockquote></div><br></div>