On 9/25/18 12:10 PM, Martin Liška wrote: > On 9/25/18 11:24 AM, Jakub Jelinek wrote: >> On Tue, Sep 25, 2018 at 11:05:30AM +0200, Martin Liška wrote: >>> As requested in PR81715, GCC emits bigger middle redzones for small >>> variables. >>> It's analyzed in following comment: >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715#c28 >> >> First of all, does LLVM make the variable sized red zone size only for >> automatic variables, or also for global/local statics, or for alloca? > > Yes, definitely for global variables, as seen here: > > lib/Transforms/Instrumentation/AddressSanitizer.cpp: > 2122 Type *Ty = G->getValueType(); > 2123 uint64_t SizeInBytes = DL.getTypeAllocSize(Ty); > 2124 uint64_t MinRZ = MinRedzoneSizeForGlobal(); > 2125 // MinRZ <= RZ <= kMaxGlobalRedzone > 2126 // and trying to make RZ to be ~ 1/4 of SizeInBytes. > 2127 uint64_t RZ = std::max( > 2128 MinRZ, std::min(kMaxGlobalRedzone, (SizeInBytes / MinRZ / 4) > * MinRZ)); > 2129 uint64_t RightRedzoneSize = RZ; > 2130 // Round up to MinRZ > 2131 if (SizeInBytes % MinRZ) RightRedzoneSize += MinRZ - (SizeInBytes > % MinRZ); > 2132 assert(((RightRedzoneSize + SizeInBytes) % MinRZ) == 0); > 2133 Type *RightRedZoneTy = ArrayType::get(IRB.getInt8Ty(), > RightRedzoneSize); > > So roughly to SizeInBytes / 4. Confirmed: > > 0x000001180a7c is located 4 bytes to the left of global variable 'b' defined > in 'main.c:2:6' (0x1180a80) of size 4096 > 0x000001180a7c is located 1020 bytes to the right of global variable 'a' > defined in 'main.c:1:6' (0x117f680) of size 4096 > > size == 4096, rz_size == 1024.
... For allocas, red zones are constant if I see correctly. Martin > >> >> Have you considered also making the red zones larger for very large >> variables? > > I was mainly focused on shrinking as that's limiting usage of asan-stack in > KASAN. > But I'm open for it. Would you follow what LLVM does, or do you have a > specific idea how > to growth? > >> >>> For now I'm suggesting to shrink shadow memory for variables <= 16B to 32B >>> (including variable storage). >>> LLVM is more aggressive as they allocate just 16B of shadow memory for >>> variables <= 4B. That would >>> require bigger code refactoring in asan.c and I would like to avoid that. >> >> What exactly would need changing to support the 12-15 bytes long red zones >> for 4-1 bytes long automatic vars? >> Just asan_emit_stack_protection or something other? > > Primarily this function, that would need a generalization. Apart from that > we're > also doing alignment to ASAN_RED_ZONE_SIZE: > > prev_offset = align_base (prev_offset, > MAX (alignb, ASAN_RED_ZONE_SIZE), > !FRAME_GROWS_DOWNWARD); > > Maybe it has consequences I don't see right now? > >> >>> + poly_uint64 size = stack_vars[i].size; >>> + /* For small variables shrink middle redzone (including >>> + * variable store) just to ASAN_RED_ZONE_SIZE. */ >> >> We don't use this comment style (* at start of comment continuation lines). > > Sure, easy to fix. > >> Otherwise it looks reasonable, but I wouldn't stop here. > > First feedback is positive: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715#c30 > > It's questionable whether handling of variables 1-4B wide worth further > changes. > > Martin > >> >> Jakub >> >