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.
>
> 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
>
