This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Aarch64 SVE: Fix stack smashing when calling functions
> >> 2018-09-17 Alan Hayward <alan.hayward@arm.com>
> >>
> >> * aarch64-tdep.c (pass_in_v): Use register size.
> >> (aarch64_extract_return_value): Likewise.
> >> (aarch64_store_return_value): Likewise.
> >
> > Do we have a testcase already that demonstrates the problem?
> > Otherwise, it would be nice to add one.
>
> I should have mentioned that in the description. I can add:
> “This fixes gdb.base/callfuncs.exp for Aarch64 SVE."
I was hoping for something like that. Nice :).
> >> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c
> >> index 6993e9061e..516eb138dc 100644
> >> --- a/gdb/aarch64-tdep.c
> >> +++ b/gdb/aarch64-tdep.c
> >> @@ -1358,7 +1358,10 @@ pass_in_v (struct gdbarch *gdbarch,
> >> if (info->nsrn < 8)
> >> {
> >> int regnum = AARCH64_V0_REGNUM + info->nsrn;
> >> - gdb_byte reg[V_REGISTER_SIZE];
> >> + /* Enough space for a full vector register. */
> >> + gdb_byte reg[register_size (gdbarch, regnum)];
> >> + gdb_static_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM);
> >> + gdb_assert (len <= sizeof (reg));
> >
> > Could you explain the relationship between making the buffer large
> > enough, which is the purpose of this patch, and the assertion that
> > AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM?
> >
> > I don't see a problem with that assertion, but for archeology
> > purposes, it is better to decorelate changes that are independent.
> > It helps better document why we introduced changes.
>
> Thinking about it, that assert should be removed.
> I was reusing three lines from aarch64_pseudo_read_value_1, which
> passed AARCH64_V0_REGNUM into register_size. There the assert made
> sense. When I switched to use regnum I didn’t rethink the assert.
>
> I’ll remove the three new instances of that assert from this patch.
>
> Are you happy with those those changes?
Yes; pre-approved with the addition of gdb.base/callfuncs.exp in
the revision log and the 3 asserts removed.
Thank you,
--
Joel