[llvm] r308387 - [asan] Copy arguments passed by value into explicit allocas for ASan

Matt Morehouse via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 9 11:12:55 PDT 2017


A line was added to
llvm/test/Instrumentation/AddressSanitizer/stack-poisoning-byval-args.ll
testing correct behavior with -asan-force-dynamic-shadow.

On Wed, Aug 9, 2017 at 11:10 AM, George Karpenkov <ekarpenkov at apple.com>
wrote:

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


More information about the llvm-commits mailing list