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