This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/3] RISC-V: gdb.base/gnu_vector fixes.
* Jim Wilson <jimw@sifive.com> [2018-11-06 13:44:03 -0800]:
> Ensure that stack slots are always the same size as XLEN by rounding up arg
> sizes when computing the address of the next stack slot.
>
> gdb/
> * riscv-tdep.c (riscv_assign_stack_location): New arg slot_align.
> Use with align_up when setting arg_offset.
> (riscv_call_arg_scalar_int): Call riscv_assign_stack_location with
> cinfo->xlen as new arg.
> ---
> gdb/riscv-tdep.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index db372e2163..ac4f2533f4 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -1868,13 +1868,13 @@ riscv_assign_reg_location (struct riscv_arg_info::location *loc,
> static void
> riscv_assign_stack_location (struct riscv_arg_info::location *loc,
> struct riscv_memory_offsets *memory,
> - int length, int align)
> + int length, int align, int slot_align)
I struggled to understand what the difference between align and
slot_align is in this patch, it feels like....
> {
> loc->loc_type = riscv_arg_info::location::on_stack;
> memory->arg_offset
> = align_up (memory->arg_offset, align);
> loc->loc_data.offset = memory->arg_offset;
> - memory->arg_offset += length;
> + memory->arg_offset += align_up (length, slot_align);
> loc->c_length = length;
>
> /* Offset is always 0, either we're the first location part, in which
> @@ -1919,7 +1919,7 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo,
> cinfo->xlen, 0))
> riscv_assign_stack_location (&ainfo->argloc[1],
> &cinfo->memory, cinfo->xlen,
> - cinfo->xlen);
> + cinfo->xlen, cinfo->xlen);
> }
> else
> {
> @@ -1928,7 +1928,8 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo,
> if (!riscv_assign_reg_location (&ainfo->argloc[0],
> &cinfo->int_regs, len, 0))
> riscv_assign_stack_location (&ainfo->argloc[0],
> - &cinfo->memory, len, ainfo->align);
> + &cinfo->memory, len, ainfo->align,
> + cinfo->xlen);
.... you might be better off just passing a better value of align
through here. Testing on gdb.base/gnu_vector.exp shows that doing
this fixes the same problem that your original patch fixes.
My revision is below, but I've only just kicked off a test run, so
there might be problems I'm not seeing.
Thanks,
Andrew
>
> if (len < ainfo->length)
> {
> @@ -1937,7 +1938,8 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo,
> &cinfo->int_regs, len,
> cinfo->xlen))
> riscv_assign_stack_location (&ainfo->argloc[1],
> - &cinfo->memory, len, cinfo->xlen);
> + &cinfo->memory, len, cinfo->xlen,
> + cinfo->xlen);
> }
> }
> }
> --
> 2.17.1
>
---
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index a0b2d1f5d7..243db12999 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -1951,12 +1951,13 @@ riscv_call_arg_scalar_int (struct riscv_arg_info *ainfo,
}
else
{
- int len = (ainfo->length > cinfo->xlen) ? cinfo->xlen : ainfo->length;
+ int len = std::min (ainfo->length, cinfo->xlen);
+ int align = std::max (ainfo->align, cinfo->xlen);
if (!riscv_assign_reg_location (&ainfo->argloc[0],
&cinfo->int_regs, len, 0))
riscv_assign_stack_location (&ainfo->argloc[0],
- &cinfo->memory, len, ainfo->align);
+ &cinfo->memory, len, align);
if (len < ainfo->length)
{