[llvm] r308387 - [asan] Copy arguments passed by value into explicit allocas for ASan
George Karpenkov via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 9 11:10:35 PDT 2017
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 <mailto: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 <mailto: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 <mailto: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 <mailto: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 <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 <https://reviews.llvm.org/D34789>
>>
>> Added:
>> llvm/trunk/test/Instrumentation/AddressSanitizer/stack-poisoning-byval-args.ll
>> Modified:
>> llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
>>
>> Modified: llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp?rev=308387&r1=308386&r2=308387&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/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-poisoning-byval-args.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Instrumentation/AddressSanitizer/stack-poisoning-byval-args.ll?rev=308387&view=auto <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Instrumentation/AddressSanitizer/stack-poisoning-byval-args.ll?rev=308387&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-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128: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{{[^%]+}}[[copyBytePtr]]{{[^%]+}}[[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{{[^%]+}}[[copyBytePtr]]{{[^%]+}}[[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 <mailto:llvm-commits at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <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/0a49787b/attachment.html>
More information about the llvm-commits
mailing list