On Mon, Jul 13, 2026 at 4:53 PM Jakub Jelinek <[email protected]> wrote: > > On Sat, Jul 11, 2026 at 08:24:08AM +0800, H.J. Lu wrote: > > All stack variables do have the proper alignments. But when > > we allocate ASAN red zone aligned with ASAN alignment on > > stack, we don't take alignments of stack variables into account. > > When a stack variable alignment > ASAN alignment, it has the > > wrong alignment at run-time since ASAN red zone size on the > > stack is only aligned to ASAN alignment. My patch pads ASAN > > red zone so that ASAN red zone size is aligned to > > > > MAX (ASAN alignment, alignments of stack variables) > > This makes no sense. > > All the variables tracked by ASAN have their proper > stack_vars[i].alignb (which better should be accurate) > and are allocated such that they have in between them > appropriate red zone and the alignments are taken into > accoiunt. > > Now, in the PR120201 testcase, it seems alignb of the > only VAR_DECL in there (transform) is 16 bytes, but > stack_alignment_needed got increased to 512 bits (i.e. > 64 bytes) not because of that var, but because of > a SSA_NAME with a V64QI type through > expand_one_ssa_partition -> record_alignment_for_reg_var (512). > > So, that extra alignment is not something needed for every single > var in the expand_stack_vars layout, it is for other stack > allocations done later on, right? > > From this your first hunk definitely looks completely unnecessary. > If you have a stack frame with 64 single byte vars and > stack_alignment_needed 512 because there is some V64QI SSA_NAME, > why should every single of those 64 vars be aligned to 64 bytes? > One should be able to align them solely because of their alignment. > If you need the extra alignment for later allocations, all that > is needed is to make sure the whole thing is aligned. > So, perhaps somewhere around your second hunk, make > data.asan_alignb max of its former value and > stack_alignment_needed / BITS_PER_UNIT, such that > if (data.asan_alignb > ASAN_RED_ZONE_SIZE > && data.asan_alignb <= 4096 > && sz + ASAN_RED_ZONE_SIZE >= (int) data.asan_alignb) > redzonesz = ((sz + ASAN_RED_ZONE_SIZE + data.asan_alignb - 1) > & ~(data.asan_alignb - HOST_WIDE_INT_1)) - sz; > will take it into account? >
Have you run your proposed change with the test included in my patch? -- H.J.
