<div dir="ltr">A line was added to llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-byval-args.ll testing correct behavior with -asan-force-dynamic-shadow.</div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Aug 9, 2017 at 11:10 AM, George Karpenkov <span dir="ltr"><<a href="mailto:ekarpenkov@apple.com" target="_blank">ekarpenkov@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">Great! Can we also have a test checking this?<div><div class="h5"><br><div><blockquote type="cite"><div>On Aug 9, 2017, at 11:09 AM, Matt Morehouse <<a href="mailto:mascasa@google.com" target="_blank">mascasa@google.com</a>> wrote:</div><br class="m_-6453747921213568126Apple-interchange-newline"><div><div dir="ltr">Fixed and re-enabled for dynamic shadow in r310503.</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 7, 2017 at 12:46 AM, Vitaly Buka <span dir="ltr"><<a href="mailto:vitalybuka@google.com" target="_blank">vitalybuka@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Disabled for -asan-force-dynamic-shadow with r310241<br>Matt, could you please investigate if we can make dynamic-shadow compatible?</div><div class="m_-6453747921213568126HOEnZb"><div class="m_-6453747921213568126h5"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Aug 4, 2017 at 10:07 PM, Vitaly Buka <span dir="ltr"><<a href="mailto:vitalybuka@google.com" target="_blank">vitalybuka@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I'll take a look</div><div class="m_-6453747921213568126m_2880810380710851788HOEnZb"><div class="m_-6453747921213568126m_2880810380710851788h5"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Aug 4, 2017 at 9:28 PM, Kuba Mracek <span dir="ltr"><<a href="mailto:mracek@apple.com" target="_blank">mracek@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space">Hi Vitaly!<div><br></div><div>This commit seems to be causing compile-time failures when using "-asan-force-dynamic-shadow":</div><div><br></div><blockquote style="margin:0 0 0 40px;border:none;padding:0px"><div><div>$ cat a.c </div><div>typedef struct {</div><div> unsigned char buf[16];</div><div>} my_struct_t;</div><div><br></div><div>void fn1(int p1, int p2, int p3, my_struct_t p4, my_struct_t p5)</div><div>{</div><div>}</div><div>$ ~/llvm-ninja-debug/bin/clang -fsanitize=address -mllvm -asan-force-dynamic-shadow -c a.c</div><div>Instruction does not dominate all uses!</div><div> %28 = load i64, i64* @__asan_shadow_memory_dynamic_<wbr>address</div><div> %21 = add i64 %20, %28</div><div>fatal error: error in backend: Broken function found, compilation aborted!</div></div><div><br></div></blockquote>It looks like when using dynamic shadow, we will emit the load of the shadow offset into the beginning of the first BB, but copyArgsPassedByValToAlloc<wbr>as also tries to emit instructions into the beginning of the entry block. In the end, we just end up emitting in the wrong order.<div><br></div><div>Do you think you could take a look at it?</div><div><br></div><div>Thanks!</div><div><span class="m_-6453747921213568126m_2880810380710851788m_-610277782734447166HOEnZb"><font color="#888888">Kuba</font></span><div><div class="m_-6453747921213568126m_2880810380710851788m_-610277782734447166h5"><br><div><div><br><blockquote type="cite"><div>On 18 Jul 2017, at 15:28, Vitaly Buka via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:</div><br class="m_-6453747921213568126m_2880810380710851788m_-610277782734447166m_6257883121503790812Apple-interchange-newline"><div><div>Author: vitalybuka<br>Date: Tue Jul 18 15:28:03 2017<br>New Revision: 308387<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=308387&view=rev" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=308387&view=rev</a><br>Log:<br>[asan] Copy arguments passed by value into explicit allocas for ASan<br><br> Summary:<br> ASan determines the stack layout from alloca instructions. Since<br>arguments marked as "byval" do not have an explicit alloca instruction, ASan<br>does not produce red zones for them. This commit produces an explicit alloca<br>instruction and copies the byval argument into the allocated memory so that red<br>zones are produced.<br><br> Submitted on behalf of @morehouse (Matt Morehouse)<br><br> Reviewers: eugenis, vitalybuka<br><br> Reviewed By: eugenis<br><br> Subscribers: hiraditya, llvm-commits<br><br> Differential Revision: <a href="https://reviews.llvm.org/D34789" target="_blank">https://reviews.llvm.org/D3478<wbr>9</a><br><br>Added:<br> llvm/trunk/test/Instrumenta<wbr>tion/AddressSanitizer/stack-po<wbr>isoning-byval-args.ll<br>Modified:<br> llvm/trunk/lib/Transforms/I<wbr>nstrumentation/AddressSanitize<wbr>r.cpp<br><br>Modified: llvm/trunk/lib/Transforms/Inst<wbr>rumentation/AddressSanitizer.c<wbr>pp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp?rev=308387&r1=308386&r2=308387&view=diff" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/lib/Transform<wbr>s/Instrumentation/AddressSanit<wbr>izer.cpp?rev=308387&r1=308386&<wbr>r2=308387&view=diff</a><br>==============================<wbr>==============================<wbr>==================<br>--- llvm/trunk/lib/Transforms/Inst<wbr>rumentation/AddressSanitizer.c<wbr>pp (original)<br>+++ llvm/trunk/lib/Transforms/Inst<wbr>rumentation/AddressSanitizer.c<wbr>pp Tue Jul 18 15:28:03 2017<br>@@ -22,9 +22,11 @@<br> #include "llvm/ADT/Statistic.h"<br> #include "llvm/ADT/StringExtras.h"<br> #include "llvm/ADT/Triple.h"<br>+#include "llvm/ADT/Twine.h"<br> #include "llvm/Analysis/MemoryBuiltins.<wbr>h"<br> #include "llvm/Analysis/TargetLibraryIn<wbr>fo.h"<br> #include "llvm/Analysis/ValueTracking.h<wbr>"<br>+#include "llvm/IR/Argument.h"<br> #include "llvm/IR/CallSite.h"<br> #include "llvm/IR/DIBuilder.h"<br> #include "llvm/IR/DataLayout.h"<br>@@ -43,6 +45,7 @@<br> #include "llvm/Support/DataTypes.h"<br> #include "llvm/Support/Debug.h"<br> #include "llvm/Support/Endian.h"<br>+#include "llvm/Support/ScopedPrinter.h"<br> #include "llvm/Support/SwapByteOrder.h"<br> #include "llvm/Support/raw_ostream.h"<br> #include "llvm/Transforms/Instrumentati<wbr>on.h"<br>@@ -192,6 +195,11 @@ static cl::opt<uint32_t> ClMaxInlinePois<br> static cl::opt<bool> ClUseAfterReturn("asan-use-aft<wbr>er-return",<br> <wbr> cl::desc("Check stack-use-after-return"),<br> <wbr> cl::Hidden, cl::init(true));<br>+static cl::opt<bool> ClRedzoneByvalArgs("asan-redzo<wbr>ne-byval-args",<br>+ <wbr> cl::desc("Create redzones for byval "<br>+ <wbr> "arguments (extra copy "<br>+ <wbr> "required)")<wbr>, cl::Hidden,<br>+ <wbr> cl::init(true));<br> static cl::opt<bool> ClUseAfterScope("asan-use-afte<wbr>r-scope",<br> <wbr> cl::desc("Check stack-use-after-scope"),<br> <wbr> cl::Hidden, cl::init(false));<br>@@ -747,6 +755,9 @@ struct FunctionStackPoisoner : public In<br><br> bool runOnFunction() {<br> if (!ClStack) return false;<br>+<br>+ if (ClRedzoneByvalArgs) copyArgsPassedByValToAllocas()<wbr>;<br>+<br> // Collect alloca, ret, lifetime instructions etc.<br> for (BasicBlock *BB : depth_first(&F.getEntryBlock()<wbr>)) visit(*BB);<br><br>@@ -763,6 +774,11 @@ struct FunctionStackPoisoner : public In<br> return true;<br> }<br><br>+ // Arguments marked with the "byval" attribute are implicitly copied without<br>+ // using an alloca instruction. To produce redzones for those arguments, we<br>+ // copy them a second time into memory allocated with an alloca instruction.<br>+ void copyArgsPassedByValToAllocas()<wbr>;<br>+<br> // Finds all Alloca instructions and puts<br> // poisoned red zones around all of them.<br> // Then unpoison everything back before the function returns.<br>@@ -2528,6 +2544,28 @@ static int StackMallocSizeClass(uint64_t<br> llvm_unreachable("impossible LocalStackSize");<br> }<br><br>+void FunctionStackPoisoner::copyArg<wbr>sPassedByValToAllocas() {<br>+ BasicBlock &FirstBB = *F.begin();<br>+ IRBuilder<> IRB(&FirstBB, FirstBB.getFirstInsertionPt())<wbr>;<br>+ const DataLayout &DL = F.getParent()->getDataLayout()<wbr>;<br>+ for (Argument &Arg : F.args()) {<br>+ if (Arg.hasByValAttr()) {<br>+ Type *Ty = Arg.getType()->getPointerEleme<wbr>ntType();<br>+ unsigned Align = Arg.getParamAlignment();<br>+ if (Align == 0) Align = DL.getABITypeAlignment(Ty);<br>+<br>+ const std::string &Name = Arg.hasName() ? Arg.getName().str() :<br>+ "Arg" + llvm::to_string(Arg.getArgNo()<wbr>);<br>+ AllocaInst *AI = IRB.CreateAlloca(Ty, nullptr, Twine(Name) + ".byval");<br>+ AI->setAlignment(Align);<br>+ Arg.replaceAllUsesWith(AI<wbr>);<br>+<br>+ uint64_t AllocSize = DL.getTypeAllocSize(Ty);<br>+ IRB.CreateMemCpy(AI, &Arg, AllocSize, Align);<br>+ }<br>+ }<br>+}<br>+<br> PHINode *FunctionStackPoisoner::create<wbr>PHI(IRBuilder<> &IRB, Value *Cond,<br> <wbr> Value *ValueIfTrue,<br> <wbr> Instruction *ThenTerm,<br><br>Added: llvm/trunk/test/Instrumentatio<wbr>n/AddressSanitizer/stack-poiso<wbr>ning-byval-args.ll<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Instrumentation/AddressSanitizer/stack-poisoning-byval-args.ll?rev=308387&view=auto" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/Instrume<wbr>ntation/AddressSanitizer/stack<wbr>-poisoning-byval-args.ll?rev=3<wbr>08387&view=auto</a><br>==============================<wbr>==============================<wbr>==================<br>--- llvm/trunk/test/Instrumentatio<wbr>n/AddressSanitizer/stack-poiso<wbr>ning-byval-args.ll (added)<br>+++ llvm/trunk/test/Instrumentatio<wbr>n/AddressSanitizer/stack-poiso<wbr>ning-byval-args.ll Tue Jul 18 15:28:03 2017<br>@@ -0,0 +1,48 @@<br>+; This check verifies that arguments passed by value get redzones.<br>+; RUN: opt < %s -asan -asan-realign-stack=32 -S | FileCheck %s<br>+<br>+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i1<wbr>6:16:16-i32:32:32-i64:64:64-f3<wbr>2:32:32-f64:64:64-v64:64:64-v1<wbr>28:128:128-a0:0:64-s0:64:64-f8<wbr>0:128:128-n8:16:32:64"<br>+target triple = "x86_64-unknown-linux-gnu"<br>+<br>+%struct.A = type { [8 x i32] }<br>+<br>+declare i32 @bar(%struct.A*)<br>+<br>+; Test behavior for named argument with explicit alignment. The memcpy and<br>+; alloca alignments should match the explicit alignment of 64.<br>+define void @foo(%struct.A* byval align 64 %a) sanitize_address {<br>+entry:<br>+; CHECK-LABEL: foo<br>+; CHECK: call i64 @__asan_stack_malloc<br>+; CHECK: alloca i8, i64 {{.*}} align 64<br>+; CHECK: [[copyPtr:%[^ \t]+]] = inttoptr i64 %{{[^ \t]+}} to %struct.A*<br>+; CHECK: [[copyBytePtr:%[^ \t]+]] = bitcast %struct.A* [[copyPtr]]<br>+; CHECK: [[aBytePtr:%[^ \t]+]] = bitcast %struct.A* %a<br>+; CHECK: call void @llvm.memcpy{{[^%]+}}[[copyByt<wbr>ePtr]]{{[^%]+}}[[aBytePtr]],{{<wbr>[^,]+}}, i32 64<br>+; CHECK: call i32 @bar(%struct.A* [[copyPtr]])<br>+; CHECK: ret void<br>+<br>+ %call = call i32 @bar(%struct.A* %a)<br>+ ret void<br>+}<br>+<br>+; Test behavior for unnamed argument without explicit alignment. In this case,<br>+; the first argument is referenced by the identifier %0 and the ABI requires a<br>+; minimum alignment of 4 bytes since struct.A contains i32s which have 4-byte<br>+; alignment. However, the alloca alignment will be 32 since that is the value<br>+; passed via the -asan-realign-stack option, which is greater than 4.<br>+define void @baz(%struct.A* byval) sanitize_address {<br>+entry:<br>+; CHECK-LABEL: baz<br>+; CHECK: call i64 @__asan_stack_malloc<br>+; CHECK: alloca i8, i64 {{.*}} align 32<br>+; CHECK: [[copyPtr:%[^ \t]+]] = inttoptr i64 %{{[^ \t]+}} to %struct.A*<br>+; CHECK: [[copyBytePtr:%[^ \t]+]] = bitcast %struct.A* [[copyPtr]]<br>+; CHECK: [[aBytePtr:%[^ \t]+]] = bitcast %struct.A* %0<br>+; CHECK: call void @llvm.memcpy{{[^%]+}}[[copyByt<wbr>ePtr]]{{[^%]+}}[[aBytePtr]],{{<wbr>[^,]+}}, i32 4<br>+; CHECK: call i32 @bar(%struct.A* [[copyPtr]])<br>+; CHECK: ret void<br>+<br>+ %call = call i32 @bar(%struct.A* %0)<br>+ ret void<br>+}<br><br><br>______________________________<wbr>_________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br></div></div></blockquote></div><br></div></div></div></div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></blockquote></div><br></div></div></div></blockquote></div><br></div>