On Tue, Mar 26, 2024 at 11:26 AM Hongtao Liu <crazy...@gmail.com> wrote:
>
> On Mon, Mar 25, 2024 at 8:51 PM Jakub Jelinek <ja...@redhat.com> wrote:
> >
> > On Tue, Mar 12, 2024 at 07:57:59PM +0800, liuhongt wrote:
> > > if alignb > ASAN_RED_ZONE_SIZE and offset[0] is not multiple of
> > > alignb. (base_align_bias - base_offset) may not aligned to alignb, and
> > > caused segement fault.
> > >
> > > Bootstrapped and regtested on x86_64-linux-gnu{-m32,}.
> > > Ok for trunk and backport to GCC13?
> > >
> > > gcc/ChangeLog:
> > >
> > > PR sanitizer/110027
> > > * cfgexpand.cc (expand_stack_vars): Align frame offset to
> > > MAX (alignb, ASAN_RED_ZONE_SIZE).
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > > * g++.dg/asan/pr110027.C: New test.
> > > ---
> > > gcc/cfgexpand.cc | 2 +-
> > > gcc/testsuite/g++.dg/asan/pr110027.C | 20 ++++++++++++++++++++
> > > 2 files changed, 21 insertions(+), 1 deletion(-)
> > > create mode 100644 gcc/testsuite/g++.dg/asan/pr110027.C
> > >
> > > diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
> > > index 0de299c62e3..92062378d8e 100644
> > > --- a/gcc/cfgexpand.cc
> > > +++ b/gcc/cfgexpand.cc
> > > @@ -1214,7 +1214,7 @@ expand_stack_vars (bool (*pred) (size_t), class
> > > stack_vars_data *data)
> > > {
> > > if (data->asan_vec.is_empty ())
> > > {
> > > - align_frame_offset (ASAN_RED_ZONE_SIZE);
> > > + align_frame_offset (MAX (alignb, ASAN_RED_ZONE_SIZE));
> > > prev_offset = frame_offset.to_constant ();
> > > }
> > > prev_offset = align_base (prev_offset,
> >
> > This doesn't look correct to me.
> > The above is done just once for the first var partition. And
> > var partitions are sorted by stack_var_cmp, which puts >
> > MAX_SUPPORTED_STACK_ALIGNMENT
> > alignment vars first (that should be none on x86, the above is quite huge
> > alignment), then on size decreasing and only after that on alignment
> > decreasing.
> >
> > So, try to add some other variable with larger size and smaller alignment
> > to the frame (and make sure it isn't optimized away).
> >
> > alignb above is the alignment of the first partition's var, if
> > align_frame_offset really needs to depend on the var alignment, it probably
> > should be the maximum alignment of all the vars with alignment
> > alignb * BITS_PER_UNIT <= MAX_SUPPORTED_STACK_ALIGNMENT
>
> In asan_emit_stack_protection, when it allocated fake stack, it assume
> bottom of stack is also aligned to alignb. And the place violated this
> is the first var partition. which is 32 bytes offsets, it should be
> MAX_SUPPORTED_STACK_ALIGNMENT / BITS_PER_UNIT.
> So I think we need to use MAX (MAX_SUPPORTED_STACK_ALIGNMENT /
> BITS_PER_UNIT, ASAN_RED_ZONE_SIZE) for the first var partition.
It should be MAX (BIGGEST_ALIGNMENT / BITS_PER_UNIT, ASAN_RED_ZONE_SIZE).
MAX_SUPPORTED_STACK_ALIGNMENT is huge.
>
> >
> > > diff --git a/gcc/testsuite/g++.dg/asan/pr110027.C
> > > b/gcc/testsuite/g++.dg/asan/pr110027.C
> > > new file mode 100644
> > > index 00000000000..0067781bc89
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/asan/pr110027.C
> > > @@ -0,0 +1,20 @@
> > > +/* PR sanitizer/110027 */
> > > +/* { dg-do run } */
> > > +/* { dg-require-effective-target avx512f_runtime } */
> > > +/* { dg-options "-std=gnu++23 -mavx512f -fsanitize=address -O0 -g
> > > -fstack-protector-strong" } */
> > > +
> > > +#include <cstddef>
> > > +#include <cstdint>
> > > +
> > > +template <ptrdiff_t W, typename T>
> > > +using Vec [[gnu::vector_size(W * sizeof(T))]] = T;
> > > +
> > > +auto foo() {
> > > + Vec<8, int64_t> ret{};
> > > + return ret;
> > > +}
> > > +
> > > +int main() {
> > > + foo();
> > > + return 0;
> > > +}
> > > --
> > > 2.31.1
> >
> > Jakub
> >
>
>
> --
> BR,
> Hongtao
--
BR,
Hongtao
