[PATCH] D18846: [safestack] Add canary to unsafe stack frames

Peter Collingbourne via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 7 17:06:04 PDT 2016


pcc accepted this revision.
pcc added a comment.
This revision is now accepted and ready to land.

LGTM


================
Comment at: test/CodeGen/X86/safestack_ssp.ll:2
@@ +1,3 @@
+; RUN: llc -mtriple=i386-linux < %s -o - | FileCheck --check-prefix=LINUX-I386 %s
+; RUN: llc -mtriple=x86_64-linux < %s -o - | FileCheck --check-prefix=LINUX-X64 %s
+
----------------
eugenis wrote:
> pcc wrote:
> > Well, I mainly wanted you to tell me why you made this a backend test :) We don't normally test passes this way.
> > 
> > Anyway, if you're just trying to provide test coverage for the codegen pipeline, it looks like the existing test `test/CodeGen/X86/safestack.ll` is already covering that, so unless you have another reason to keep this test, it seems redundant with the opt tests you added.
> test/CodeGen/X86/safestack.ll does not cover the stack protector cookie code.
> 
> No other tests verify that StackProtector and SafeStack passes do not apply to the same function.
> 
Okay, seems reasonable enough to me. Please add a comment explaining why this is a backend test.


Repository:
  rL LLVM

http://reviews.llvm.org/D18846





More information about the llvm-commits mailing list