On Thu, Nov 29, 2018 at 04:03:42PM +0100, Martin Liška wrote: > > Two problems, it uses unconditionally unaligned stores, without > > checking if the target supports them at all (in this case it does). > > And, it doesn't check if it wouldn't be more efficient to use > > 32-bit stores. > > Ok, so now we reduced the alignment of stack variables to > ASAN_MIN_RED_ZONE_SIZE (16). > What options do wehave when we emit the red zones? The only guarantee we have > is > that in shadow memory we are aligned to at least 2. Should byte-based > emission used > for STRICT_ALIGNMENT targets?
While individual variables can have smaller alignment, why we can't as before ensure the whole block of all the vars is 32-byte aligned and (see below) also padded to 32 bytes? Doesn't __asan_stack_malloc* align those as before and if not doing use after return sanitization, has your patch changed anything in how the whole block is aligned on the stack? Of course, on non-strict alignment targets we might not care that much. > > It isn't that we want the gaps to have whatever > > value the shadow memory contained before, we want it to be 0 and just rely > > on it being zero from earlier. > > Another question is if it wouldn't be worth to ensure the variable area is > > always a multiple of 32 bytes (thus the corresponding shadow always multiple > > of 4 bytes). Then, for this testcase, you'd store: > > $-235802127, 2147450880(%rbp) // 0xf1f1f1f1 > > $-234687999, 2147450884(%rbp) // 0xf202f201 > > $-218041852, 2147450888(%rbp) // 0xf300f204 > > $-202116109, 2147450892(%rbp) // 0xf3f3f3f3 > > For the single char/short/int/long var in a function case you'd still emit > > just f1 f1 f1 f1 0[1240] f3 f3 f3 > > and the number of final f3 bytes would be from 3 to 6 (or 2 to 5), 1 is > > probably too few. Here, the padding to 32 bytes. Jakub