<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jul 8, 2016, at 2:44 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Tue, Jun 28, 2016 at 3:28 PM, Yunzhong Gao via llvm-commits <span dir="ltr" class=""><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">ygao created this revision.<br class="">
ygao added reviewers: aprantl, echristo, dblaikie.<br class="">
ygao added a subscriber: llvm-commits.<br class="">
Herald added a subscriber: mehdi_amini.<br class="">
<br class="">
Hi,<br class="">
The compiler crashed with the following test program:<br class="">
<br class="">
/* test.c<br class="">
* To reproduce:<br class="">
* $ clang -c -g -fstack-protector-strong test.c<br class="">
* (fatal error: error in backend: Broken function found, compilation aborted!)<br class="">
* (need an assertion-enabled build of the compiler)<br class="">
*/<br class="">
void __stack_chk_fail(void) { return; }<br class="">
<br class="">
void foo() {<br class="">
char bar[5];<br class="">
}<br class="">
/* end of test.c */<br class="">
<br class="">
This is not a legitimate input C file because __stack_chk_fail is a reserved function name.<br class=""></blockquote><div class=""><br class=""></div><div class="">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 class=""><br class="">If there isn't, then perhaps we should fix the original issue (allowing this invalid construct) rather than adding downstream workarounds?</div></div></div></div></div></blockquote><div><br class=""></div><div>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.</div><div><br class=""></div><div>-- adrian</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
On the other hand, it seems that the compiler should be able to give a nicer diagnostic or<br class="">
otherwise fail more gracefully instead of having to crash on invalid input (in the spirit of all<br class="">
"internal compiler error"s are compiler bugs).<br class="">
<br class="">
The proposed patch here attempts to add artificial debug information when the compiler<br class="">
generates the call to __stack_chk_fail. Does this look like a reasonable thing to do?<br class="">
<br class="">
Many thanks in advance,<br class="">
- Gao<br class="">
<br class="">
<a href="http://reviews.llvm.org/D21818" rel="noreferrer" target="_blank" class="">http://reviews.llvm.org/D21818</a><br class="">
<br class="">
Files:<br class="">
lib/CodeGen/StackProtector.cpp<br class="">
test/CodeGen/X86/stack-protector.ll<br class="">
<br class="">
Index: test/CodeGen/X86/stack-protector.ll<br class="">
===================================================================<br class="">
--- test/CodeGen/X86/stack-protector.ll<br class="">
+++ test/CodeGen/X86/stack-protector.ll<br class="">
@@ -3880,6 +3880,17 @@<br class="">
ret i32 %call<br class="">
}<br class="">
<br class="">
+define void @__stack_chk_fail() #1 !dbg !6 {<br class="">
+entry:<br class="">
+ ret void<br class="">
+}<br class="">
+<br class="">
+define void @test32() #1 !dbg !7 {<br class="">
+entry:<br class="">
+ %0 = alloca [5 x i8], align 1<br class="">
+ ret void<br class="">
+}<br class="">
+<br class="">
declare double @testi_aux()<br class="">
declare i8* @strcpy(i8*, i8*)<br class="">
declare i32 @printf(i8*, ...)<br class="">
@@ -3899,3 +3910,16 @@<br class="">
attributes #3 = { ssp "stack-protector-buffer-size"="33" }<br class="">
attributes #4 = { ssp "stack-protector-buffer-size"="5" }<br class="">
attributes #5 = { ssp "stack-protector-buffer-size"="6" }<br class="">
+<br class="">
+!<a href="http://llvm.dbg.cu/" rel="noreferrer" target="_blank" class="">llvm.dbg.cu</a> = !{!0}<br class="">
+!llvm.module.flags = !{!3, !4}<br class="">
+!llvm.ident = !{!5}<br class="">
+<br class="">
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1)<br class="">
+!1 = !DIFile(filename: "test.c", directory: "/tmp")<br class="">
+!2 = !{}<br class="">
+!3 = !{i32 2, !"Dwarf Version", i32 4}<br class="">
+!4 = !{i32 2, !"Debug Info Version", i32 3}<br class="">
+!5 = !{!"clang version x.y.z"}<br class="">
+!6 = distinct !DISubprogram(name: "__stack_chk_fail", scope: !1, unit: !0)<br class="">
+!7 = distinct !DISubprogram(name: "foo", scope: !1, unit: !0)<br class="">
Index: lib/CodeGen/StackProtector.cpp<br class="">
===================================================================<br class="">
--- lib/CodeGen/StackProtector.cpp<br class="">
+++ lib/CodeGen/StackProtector.cpp<br class="">
@@ -25,6 +25,7 @@<br class="">
#include "llvm/IR/Attributes.h"<br class="">
#include "llvm/IR/Constants.h"<br class="">
#include "llvm/IR/DataLayout.h"<br class="">
+#include "llvm/IR/DebugInfo.h"<br class="">
#include "llvm/IR/DerivedTypes.h"<br class="">
#include "llvm/IR/Function.h"<br class="">
#include "llvm/IR/GlobalValue.h"<br class="">
@@ -448,6 +449,7 @@<br class="">
LLVMContext &Context = F->getContext();<br class="">
BasicBlock *FailBB = BasicBlock::Create(Context, "CallStackCheckFailBlk", F);<br class="">
IRBuilder<> B(FailBB);<br class="">
+ B.SetCurrentDebugLocation(DebugLoc::get(0, 0, F->getSubprogram()));<br class="">
if (Trip.isOSOpenBSD()) {<br class="">
Constant *StackChkFail =<br class="">
M->getOrInsertFunction("__stack_smash_handler",<br class="">
<br class="">
<br class="">
<br class="">_______________________________________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="">
<br class=""></blockquote></div><br class=""></div></div>
</div></blockquote></div><br class=""></body></html>