[PATCH] D21818: Add artificial debug information to avoid compiler crash

Gao, Yunzhong via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 8 16:05:36 PDT 2016

Hi Adrian, David,
I can add CHECK lines to the test just like what was added for the other functions in the same file.
If there is a better way to test verifier errors or a better place to add such test, could you advise?
I can understand what David is saying, but I would need help in order to make the test more specific,
- Gao

From: aprantl at apple.com [aprantl at apple.com]
Sent: Friday, July 08, 2016 2:47 PM
To: David Blaikie
Cc: reviews+D21818+public+b40f0be86340e0e5 at reviews.llvm.org; Gao, Yunzhong; Eric Christopher; llvm-commits
Subject: Re: [PATCH] D21818: Add artificial debug information to avoid compiler crash

On Jul 8, 2016, at 2:44 PM, David Blaikie <dblaikie at gmail.com<mailto:dblaikie at gmail.com>> wrote:

On Tue, Jun 28, 2016 at 3:28 PM, Yunzhong Gao via llvm-commits <llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>> wrote:
ygao created this revision.
ygao added reviewers: aprantl, echristo, dblaikie.
ygao added a subscriber: llvm-commits.
Herald added a subscriber: mehdi_amini.

The compiler crashed with the following test program:

/* test.c
 * To reproduce:
 * $ clang -c -g -fstack-protector-strong test.c
 * (fatal error: error in backend: Broken function found, compilation aborted!)
 * (need  an assertion-enabled build of the compiler)
void __stack_chk_fail(void) { return; }

void foo() {
  char bar[5];
/* end of test.c */

This is not a legitimate input C file because __stack_chk_fail is a reserved function name.

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.

If there isn't, then perhaps we should fix the original issue (allowing this invalid construct) rather than adding downstream workarounds?

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.

-- adrian

On the other hand, it seems that the compiler should be able to give a nicer diagnostic or
otherwise fail more gracefully instead of having to crash on invalid input (in the spirit of all
"internal compiler error"s are compiler bugs).

The proposed patch here attempts to add artificial debug information when the compiler
generates the call to __stack_chk_fail. Does this look like a reasonable thing to do?

Many thanks in advance,
- Gao



Index: test/CodeGen/X86/stack-protector.ll
--- test/CodeGen/X86/stack-protector.ll
+++ test/CodeGen/X86/stack-protector.ll
@@ -3880,6 +3880,17 @@
   ret i32 %call

+define void @__stack_chk_fail() #1 !dbg !6 {
+  ret void
+define void @test32() #1 !dbg !7 {
+  %0 = alloca [5 x i8], align 1
+  ret void
 declare double @testi_aux()
 declare i8* @strcpy(i8*, i8*)
 declare i32 @printf(i8*, ...)
@@ -3899,3 +3910,16 @@
 attributes #3 = { ssp "stack-protector-buffer-size"="33" }
 attributes #4 = { ssp "stack-protector-buffer-size"="5" }
 attributes #5 = { ssp "stack-protector-buffer-size"="6" }
+!llvm.dbg.cu<http://llvm.dbg.cu/> = !{!0}
+!llvm.module.flags = !{!3, !4}
+!llvm.ident = !{!5}
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1)
+!1 = !DIFile(filename: "test.c", directory: "/tmp")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{!"clang version x.y.z"}
+!6 = distinct !DISubprogram(name: "__stack_chk_fail", scope: !1, unit: !0)
+!7 = distinct !DISubprogram(name: "foo", scope: !1, unit: !0)
Index: lib/CodeGen/StackProtector.cpp
--- lib/CodeGen/StackProtector.cpp
+++ lib/CodeGen/StackProtector.cpp
@@ -25,6 +25,7 @@
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
+#include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalValue.h"
@@ -448,6 +449,7 @@
   LLVMContext &Context = F->getContext();
   BasicBlock *FailBB = BasicBlock::Create(Context, "CallStackCheckFailBlk", F);
   IRBuilder<> B(FailBB);
+  B.SetCurrentDebugLocation(DebugLoc::get(0, 0, F->getSubprogram()));
   if (Trip.isOSOpenBSD()) {
     Constant *StackChkFail =

llvm-commits mailing list
llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: test.patch
Type: text/x-patch
Size: 1048 bytes
Desc: test.patch
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160708/8da084e9/attachment.bin>

More information about the llvm-commits mailing list