<div dir="ltr">Hi Hal!<div><br></div><div>See Kostya's reply for r171107.</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Sat, Jul 6, 2013 at 9:46 PM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">----- Original Message -----<br>
> Author: samsonov<br>
> Date: Wed Jan 16 07:23:28 2013<br>
> New Revision: 172610<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=172610&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=172610&view=rev</a><br>
> Log:<br>
> ASan: wrap mapping scale and offset in a struct and make it a member<br>
> of ASan passes. Add test for non-default mapping scale and offset.<br>
> No functionality change<br>
><br>
> Added:<br>
> llvm/trunk/test/Instrumentation/AddressSanitizer/different_scale_and_offset.ll<br>
> Modified:<br>
> llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp<br>
><br>
> Modified:<br>
> llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp?rev=172610&r1=172609&r2=172610&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp?rev=172610&r1=172609&r2=172610&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp<br>
> (original)<br>
> +++ llvm/trunk/lib/Transforms/Instrumentation/AddressSanitizer.cpp<br>
> Wed Jan 16 07:23:28 2013<br>
> @@ -53,7 +53,7 @@<br>
> static const uint64_t kDefaultShadowScale = 3;<br>
> static const uint64_t kDefaultShadowOffset32 = 1ULL << 29;<br>
> static const uint64_t kDefaultShadowOffset64 = 1ULL << 44;<br>
> -static const uint64_t kDefaultShadowOffsetAndroid = 0;<br>
> +static const uint64_t kDefaultShadowOffsetPie = 0;<br>
><br>
> static const size_t kMaxStackMallocSize = 1 << 16; // 64K<br>
> static const uintptr_t kCurrentStackFrameMagic = 0x41B58AB3;<br>
> @@ -186,14 +186,38 @@<br>
> SmallSet<GlobalValue*, 32> DynInitGlobals;<br>
> };<br>
><br>
> -static int MappingScale() {<br>
> - return ClMappingScale ? ClMappingScale : kDefaultShadowScale;<br>
> +/// This struct defines the shadow mapping using the rule:<br>
> +/// shadow = (mem >> Scale) + Offset.<br>
> +struct ShadowMapping {<br>
> + int Scale;<br>
> + uint64_t Offset;<br>
> +};<br>
> +<br>
> +static ShadowMapping getShadowMapping(const Module &M, int LongSize)<br>
> {<br>
> + llvm::Triple targetTriple(M.getTargetTriple());<br>
> + bool isAndroid = targetTriple.getEnvironment() ==<br>
> llvm::Triple::Android;<br>
> +<br>
> + ShadowMapping Mapping;<br>
> +<br>
> + Mapping.Offset = isAndroid ? kDefaultShadowOffsetPie :<br>
> + (LongSize == 32 ? kDefaultShadowOffset32 :<br>
> kDefaultShadowOffset64);<br>
> + if (ClMappingOffsetLog >= 0) {<br>
> + // Zero offset log is the special case.<br>
> + Mapping.Offset = (ClMappingOffsetLog == 0) ? 0 : 1ULL <<<br>
> ClMappingOffsetLog;<br>
> + }<br>
> +<br>
> + Mapping.Scale = kDefaultShadowScale;<br>
> + if (ClMappingScale) {<br>
> + Mapping.Scale = ClMappingScale;<br>
> + }<br>
> +<br>
> + return Mapping;<br>
> }<br>
><br>
> -static size_t RedzoneSize() {<br>
> +static size_t RedzoneSizeForScale(int MappingScale) {<br>
> // Redzone used for stack and globals is at least 32 bytes.<br>
> // For scales 6 and 7, the redzone has to be 64 and 128 bytes<br>
> respectively.<br>
> - return std::max(32U, 1U << MappingScale());<br>
> + return std::max(32U, 1U << MappingScale);<br>
> }<br>
<br>
</div></div>This does not seem to work for me when MappingScale is large (meaning in my case, 5, and I'm working on a system with 32-byte-aligned stacks). In this case, unit tests like stack-overflow and stack-oob-frames don't trigger a fault as they should. If I change this to read:<br>
return std::max(32U, 4*(1U << MappingScale));<br>
(where I guessed that 4 would be a good multiplier because 32/8 == 4) then things seem to work as expected.<br>
<br>
Does this make sense?<br>
<br>
-Hal<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
> /// AddressSanitizer: instrument the code in module to find memory<br>
> bugs.<br>
> @@ -227,6 +251,7 @@<br>
> void createInitializerPoisonCalls(Module &M,<br>
> Value *FirstAddr, Value<br>
> *LastAddr);<br>
> bool maybeInsertAsanInitAtFunctionEntry(Function &F);<br>
> + void emitShadowMapping(Module &M, IRBuilder<> &IRB) const;<br>
> virtual bool doInitialization(Module &M);<br>
> static char ID; // Pass identification, replacement for typeid<br>
><br>
> @@ -242,9 +267,9 @@<br>
> bool CheckLifetime;<br>
> LLVMContext *C;<br>
> DataLayout *TD;<br>
> - uint64_t MappingOffset;<br>
> int LongSize;<br>
> Type *IntptrTy;<br>
> + ShadowMapping Mapping;<br>
> Function *AsanCtorFunction;<br>
> Function *AsanInitFunction;<br>
> Function *AsanHandleNoReturnFunc;<br>
> @@ -278,6 +303,9 @@<br>
> bool ShouldInstrumentGlobal(GlobalVariable *G);<br>
> void createInitializerPoisonCalls(Module &M, Value *FirstAddr,<br>
> Value *LastAddr);<br>
> + size_t RedzoneSize() const {<br>
> + return RedzoneSizeForScale(Mapping.Scale);<br>
> + }<br>
><br>
> bool CheckInitOrder;<br>
> SmallString<64> BlacklistFile;<br>
> @@ -286,6 +314,7 @@<br>
> Type *IntptrTy;<br>
> LLVMContext *C;<br>
> DataLayout *TD;<br>
> + ShadowMapping Mapping;<br>
> Function *AsanPoisonGlobals;<br>
> Function *AsanUnpoisonGlobals;<br>
> Function *AsanRegisterGlobals;<br>
> @@ -308,6 +337,7 @@<br>
> LLVMContext *C;<br>
> Type *IntptrTy;<br>
> Type *IntptrPtrTy;<br>
> + ShadowMapping Mapping;<br>
><br>
> SmallVector<AllocaInst*, 16> AllocaVec;<br>
> SmallVector<Instruction*, 8> RetVec;<br>
> @@ -332,7 +362,8 @@<br>
> FunctionStackPoisoner(Function &F, AddressSanitizer &ASan)<br>
> : F(F), ASan(ASan), DIB(*F.getParent()), C(ASan.C),<br>
> IntptrTy(ASan.IntptrTy),<br>
> IntptrPtrTy(PointerType::get(IntptrTy, 0)),<br>
> - TotalStackSize(0), StackAlignment(1 << MappingScale()) {}<br>
> + Mapping(ASan.Mapping),<br>
> + TotalStackSize(0), StackAlignment(1 << Mapping.Scale) {}<br>
><br>
> bool runOnFunction() {<br>
> if (!ClStack) return false;<br>
> @@ -411,6 +442,9 @@<br>
> AI.getAllocatedType()->isSized());<br>
> }<br>
><br>
> + size_t RedzoneSize() const {<br>
> + return RedzoneSizeForScale(Mapping.Scale);<br>
> + }<br>
> uint64_t getAllocaSizeInBytes(AllocaInst *AI) {<br>
> Type *Ty = AI->getAllocatedType();<br>
> uint64_t SizeInBytes = ASan.TD->getTypeAllocSize(Ty);<br>
> @@ -473,12 +507,12 @@<br>
><br>
> Value *AddressSanitizer::memToShadow(Value *Shadow, IRBuilder<><br>
> &IRB) {<br>
> // Shadow >> scale<br>
> - Shadow = IRB.CreateLShr(Shadow, MappingScale());<br>
> - if (MappingOffset == 0)<br>
> + Shadow = IRB.CreateLShr(Shadow, Mapping.Scale);<br>
> + if (Mapping.Offset == 0)<br>
> return Shadow;<br>
> // (Shadow >> scale) | offset<br>
> return IRB.CreateOr(Shadow, ConstantInt::get(IntptrTy,<br>
> - MappingOffset));<br>
> + Mapping.Offset));<br>
> }<br>
><br>
> void AddressSanitizer::instrumentMemIntrinsicParam(<br>
> @@ -614,7 +648,7 @@<br>
> Value *AddressSanitizer::createSlowPathCmp(IRBuilder<> &IRB, Value<br>
> *AddrLong,<br>
> Value *ShadowValue,<br>
> uint32_t TypeSize) {<br>
> - size_t Granularity = 1 << MappingScale();<br>
> + size_t Granularity = 1 << Mapping.Scale;<br>
> // Addr & (Granularity - 1)<br>
> Value *LastAccessedByte = IRB.CreateAnd(<br>
> AddrLong, ConstantInt::get(IntptrTy, Granularity - 1));<br>
> @@ -635,7 +669,7 @@<br>
> Value *AddrLong = IRB.CreatePointerCast(Addr, IntptrTy);<br>
><br>
> Type *ShadowTy = IntegerType::get(<br>
> - *C, std::max(8U, TypeSize >> MappingScale()));<br>
> + *C, std::max(8U, TypeSize >> Mapping.Scale));<br>
> Type *ShadowPtrTy = PointerType::get(ShadowTy, 0);<br>
> Value *ShadowPtr = memToShadow(AddrLong, IRB);<br>
> Value *CmpVal = Constant::getNullValue(ShadowTy);<br>
> @@ -644,7 +678,7 @@<br>
><br>
> Value *Cmp = IRB.CreateICmpNE(ShadowValue, CmpVal);<br>
> size_t AccessSizeIndex = TypeSizeToSizeIndex(TypeSize);<br>
> - size_t Granularity = 1 << MappingScale();<br>
> + size_t Granularity = 1 << Mapping.Scale;<br>
> TerminatorInst *CrashTerm = 0;<br>
><br>
> if (ClAlwaysSlowPath || (TypeSize < 8 * Granularity)) {<br>
> @@ -782,7 +816,9 @@<br>
> BL.reset(new BlackList(BlacklistFile));<br>
> if (BL->isIn(M)) return false;<br>
> C = &(M.getContext());<br>
> - IntptrTy = Type::getIntNTy(*C, TD->getPointerSizeInBits());<br>
> + int LongSize = TD->getPointerSizeInBits();<br>
> + IntptrTy = Type::getIntNTy(*C, LongSize);<br>
> + Mapping = getShadowMapping(M, LongSize);<br>
> initializeCallbacks(M);<br>
> DynamicallyInitializedGlobals.Init(M);<br>
><br>
> @@ -930,6 +966,28 @@<br>
> /*hasSideEffects=*/true);<br>
> }<br>
><br>
> +void AddressSanitizer::emitShadowMapping(Module &M, IRBuilder<><br>
> &IRB) const {<br>
> + // Tell the values of mapping offset and scale to the run-time if<br>
> they are<br>
> + // specified by command-line flags.<br>
> + if (ClMappingOffsetLog >= 0) {<br>
> + GlobalValue *asan_mapping_offset =<br>
> + new GlobalVariable(M, IntptrTy, true,<br>
> GlobalValue::LinkOnceODRLinkage,<br>
> + ConstantInt::get(IntptrTy, Mapping.Offset),<br>
> + kAsanMappingOffsetName);<br>
> + // Read the global, otherwise it may be optimized away.<br>
> + IRB.CreateLoad(asan_mapping_offset, true);<br>
> + }<br>
> +<br>
> + if (ClMappingScale) {<br>
> + GlobalValue *asan_mapping_scale =<br>
> + new GlobalVariable(M, IntptrTy, true,<br>
> GlobalValue::LinkOnceODRLinkage,<br>
> + ConstantInt::get(IntptrTy,<br>
> Mapping.Scale),<br>
> + kAsanMappingScaleName);<br>
> + // Read the global, otherwise it may be optimized away.<br>
> + IRB.CreateLoad(asan_mapping_scale, true);<br>
> + }<br>
> +}<br>
> +<br>
> // virtual<br>
> bool AddressSanitizer::doInitialization(Module &M) {<br>
> // Initialize the private fields. No one has accessed them before.<br>
> @@ -955,41 +1013,10 @@<br>
> AsanInitFunction->setLinkage(Function::ExternalLinkage);<br>
> IRB.CreateCall(AsanInitFunction);<br>
><br>
> - llvm::Triple targetTriple(M.getTargetTriple());<br>
> - bool isAndroid = targetTriple.getEnvironment() ==<br>
> llvm::Triple::Android;<br>
> -<br>
> - MappingOffset = isAndroid ? kDefaultShadowOffsetAndroid :<br>
> - (LongSize == 32 ? kDefaultShadowOffset32 :<br>
> kDefaultShadowOffset64);<br>
> - if (ClMappingOffsetLog >= 0) {<br>
> - if (ClMappingOffsetLog == 0) {<br>
> - // special case<br>
> - MappingOffset = 0;<br>
> - } else {<br>
> - MappingOffset = 1ULL << ClMappingOffsetLog;<br>
> - }<br>
> - }<br>
> -<br>
> -<br>
> - if (ClMappingOffsetLog >= 0) {<br>
> - // Tell the run-time the current values of mapping offset and<br>
> scale.<br>
> - GlobalValue *asan_mapping_offset =<br>
> - new GlobalVariable(M, IntptrTy, true,<br>
> GlobalValue::LinkOnceODRLinkage,<br>
> - ConstantInt::get(IntptrTy, MappingOffset),<br>
> - kAsanMappingOffsetName);<br>
> - // Read the global, otherwise it may be optimized away.<br>
> - IRB.CreateLoad(asan_mapping_offset, true);<br>
> - }<br>
> - if (ClMappingScale) {<br>
> - GlobalValue *asan_mapping_scale =<br>
> - new GlobalVariable(M, IntptrTy, true,<br>
> GlobalValue::LinkOnceODRLinkage,<br>
> - ConstantInt::get(IntptrTy,<br>
> MappingScale()),<br>
> - kAsanMappingScaleName);<br>
> - // Read the global, otherwise it may be optimized away.<br>
> - IRB.CreateLoad(asan_mapping_scale, true);<br>
> - }<br>
> + Mapping = getShadowMapping(M, LongSize);<br>
> + emitShadowMapping(M, IRB);<br>
><br>
> appendToGlobalCtors(M, AsanCtorFunction,<br>
> kAsanCtorAndCtorPriority);<br>
> -<br>
> return true;<br>
> }<br>
><br>
> @@ -1147,7 +1174,7 @@<br>
> void FunctionStackPoisoner::poisonRedZones(<br>
> const ArrayRef<AllocaInst*> &AllocaVec, IRBuilder<> IRB, Value<br>
> *ShadowBase,<br>
> bool DoPoison) {<br>
> - size_t ShadowRZSize = RedzoneSize() >> MappingScale();<br>
> + size_t ShadowRZSize = RedzoneSize() >> Mapping.Scale;<br>
> assert(ShadowRZSize >= 1 && ShadowRZSize <= 4);<br>
> Type *RZTy = Type::getIntNTy(*C, ShadowRZSize * 8);<br>
> Type *RZPtrTy = PointerType::get(RZTy, 0);<br>
> @@ -1178,13 +1205,13 @@<br>
> // Poison the partial redzone at right<br>
> Ptr = IRB.CreateAdd(<br>
> ShadowBase, ConstantInt::get(IntptrTy,<br>
> - (Pos >> MappingScale()) -<br>
> ShadowRZSize));<br>
> + (Pos >> Mapping.Scale) -<br>
> ShadowRZSize));<br>
> size_t AddressableBytes = RedzoneSize() - (AlignedSize -<br>
> SizeInBytes);<br>
> uint32_t Poison = 0;<br>
> if (DoPoison) {<br>
> PoisonShadowPartialRightRedzone((uint8_t*)&Poison,<br>
> AddressableBytes,<br>
> RedzoneSize(),<br>
> - 1ULL << MappingScale(),<br>
> + 1ULL << Mapping.Scale,<br>
> kAsanStackPartialRedzoneMagic);<br>
> }<br>
> Value *PartialPoison = ConstantInt::get(RZTy, Poison);<br>
> @@ -1193,7 +1220,7 @@<br>
><br>
> // Poison the full redzone at right.<br>
> Ptr = IRB.CreateAdd(ShadowBase,<br>
> - ConstantInt::get(IntptrTy, Pos >><br>
> MappingScale()));<br>
> + ConstantInt::get(IntptrTy, Pos >><br>
> Mapping.Scale));<br>
> bool LastAlloca = (i == AllocaVec.size() - 1);<br>
> Value *Poison = LastAlloca ? PoisonRight : PoisonMid;<br>
> IRB.CreateStore(Poison, IRB.CreateIntToPtr(Ptr, RZPtrTy));<br>
><br>
> Added:<br>
> llvm/trunk/test/Instrumentation/AddressSanitizer/different_scale_and_offset.ll<br>
> URL:<br>
> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Instrumentation/AddressSanitizer/different_scale_and_offset.ll?rev=172610&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Instrumentation/AddressSanitizer/different_scale_and_offset.ll?rev=172610&view=auto</a><br>
> ==============================================================================<br>
> ---<br>
> llvm/trunk/test/Instrumentation/AddressSanitizer/different_scale_and_offset.ll<br>
> (added)<br>
> +++<br>
> llvm/trunk/test/Instrumentation/AddressSanitizer/different_scale_and_offset.ll<br>
> Wed Jan 16 07:23:28 2013<br>
> @@ -0,0 +1,41 @@<br>
> +; Test non-default shadow mapping scale and offset.<br>
> +;<br>
> +; RUN: opt < %s -asan -asan-mapping-scale=2<br>
> -asan-mapping-offset-log=0 -S | FileCheck %s<br>
> +<br>
> +target datalayout =<br>
> "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"<br>
> +target triple = "x86_64-unknown-linux-gnu"<br>
> +<br>
> +; Test that ASan tells scale and offset to runtime.<br>
> +; CHECK: @__asan_mapping_offset = linkonce_odr constant i64 0<br>
> +; CHECK: @__asan_mapping_scale = linkonce_odr constant i64 2<br>
> +<br>
> +define i32 @test_load(i32* %a) address_safety {<br>
> +; CHECK: @test_load<br>
> +; CHECK-NOT: load<br>
> +; CHECK: %[[LOAD_ADDR:[^ ]*]] = ptrtoint i32* %a to i64<br>
> +; CHECK: lshr i64 %[[LOAD_ADDR]], 2<br>
> +<br>
> +; No need in shift for zero offset.<br>
> +; CHECK-NOT: or i64<br>
> +<br>
> +; CHECK: %[[LOAD_SHADOW_PTR:[^ ]*]] = inttoptr<br>
> +; CHECK: %[[LOAD_SHADOW:[^ ]*]] = load i8* %[[LOAD_SHADOW_PTR]]<br>
> +; CHECK: icmp ne i8<br>
> +; CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}<br>
> +<br>
> +; No need in slow path for i32 and mapping scale equal to 2.<br>
> +; CHECK-NOT: and i64 %[[LOAD_ADDR]]<br>
> +;<br>
> +; The crash block reports the error.<br>
> +; CHECK: call void @__asan_report_load4(i64 %[[LOAD_ADDR]])<br>
> +; CHECK: unreachable<br>
> +;<br>
> +; The actual load.<br>
> +; CHECK: %tmp1 = load i32* %a<br>
> +; CHECK: ret i32 %tmp1<br>
> +<br>
> +entry:<br>
> + %tmp1 = load i32* %a<br>
> + ret i32 %tmp1<br>
> +}<br>
> +<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
<br>
</div></div><span class="HOEnZb"><font color="#888888">--<br>
Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</font></span></blockquote></div><br><br clear="all"><div><br></div>-- <br><div>Alexey Samsonov, MSK</div>
</div>