This is the mail archive of the
gdb-cvs@sourceware.org
mailing list for the GDB project.
[binutils-gdb] gdb/riscv: Handle empty C++ structs during argument passing
- From: Andrew Burgess <aburgess at sourceware dot org>
- To: gdb-cvs at sourceware dot org
- Date: 11 Apr 2019 22:39:24 -0000
- Subject: [binutils-gdb] gdb/riscv: Handle empty C++ structs during argument passing
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=9f0272f8548164b024ff9fd151686b2b904a5d59
commit 9f0272f8548164b024ff9fd151686b2b904a5d59
Author: Andrew Burgess <andrew.burgess@embecosm.com>
Date: Fri Apr 5 13:50:19 2019 +0100
gdb/riscv: Handle empty C++ structs during argument passing
This commit resolves a large number of failures in the test script
gdb.base/infcall-nested-structs.exp which were caused by GDB (for
RISC-V) incorrectly handling empty C++ structures when preparing
arguments for a dummy call, or collecting a return value.
The issue is further complicated in that there was a bug in GCC, such
that in some cases GCC would generate incorrect code when passing a
small structure that contained empty sub-structures. This was fixed
in GCC trunk on 5-March-2019, so in order to see the best results with
this patch you'll need a recent version of GCC.
Anything that used to work should continue to work after this patch,
regardless of GCC version being used.
The fix in this commit is that GDB now pays more attention to the
offset of fields within a structure when preparing arguments as in C++
an empty structure has a non-zero size, this is an example:
struct s1 { struct s2 { } empty; int f; };
We previously assumed that 'f' was at offset 0 inside type 's1',
however this is not the case in C++ as 's2' has size 1, and with
alignment 'f' is likely at some even bigger offset inside 's1'.
gdb/ChangeLog:
* riscv-tdep.c (riscv_call_arg_complex_float): Fix offset of first
component to 0.
(riscv_struct_info::riscv_struct_info): Initialise m_offsets
member.
(riscv_struct_info::analyse): New implementation using new
analyse_inner member function.
(riscv_struct_info::field_offset): New member function.
(riscv_struct_info::m_offsets): New member variable.
(riscv_struct_info::analyse_inner): New private member function,
takes the old implementation of riscv_struct_info::analyse but
extended to track field offsets.
(riscv_call_arg_struct): Update the struct folding special cases
to handle cases where empty C++ structs, which are non-zero
length, are found.
(riscv_arg_location): Initialise the length of each location, a
non-zero length now indicates the location is in use.
(riscv_push_dummy_call): Allow for the first location having a
non-zero offset when setting up arguments.
(riscv_return_value): Likewise, but for return values.
Diff:
---
gdb/ChangeLog | 22 ++++++++
gdb/riscv-tdep.c | 153 +++++++++++++++++++++++++++++++++++++++----------------
2 files changed, 132 insertions(+), 43 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7428ed9..0be7538 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,25 @@
+2019-04-11 Andrew Burgess <andrew.burgess@embecosm.com>
+
+ * riscv-tdep.c (riscv_call_arg_complex_float): Fix offset of first
+ component to 0.
+ (riscv_struct_info::riscv_struct_info): Initialise m_offsets
+ member.
+ (riscv_struct_info::analyse): New implementation using new
+ analyse_inner member function.
+ (riscv_struct_info::field_offset): New member function.
+ (riscv_struct_info::m_offsets): New member variable.
+ (riscv_struct_info::analyse_inner): New private member function,
+ takes the old implementation of riscv_struct_info::analyse but
+ extended to track field offsets.
+ (riscv_call_arg_struct): Update the struct folding special cases
+ to handle cases where empty C++ structs, which are non-zero
+ length, are found.
+ (riscv_arg_location): Initialise the length of each location, a
+ non-zero length now indicates the location is in use.
+ (riscv_push_dummy_call): Allow for the first location having a
+ non-zero offset when setting up arguments.
+ (riscv_return_value): Likewise, but for return values.
+
2019-04-11 Tom Tromey <tromey@adacore.com>
* utils.c (internal_vproblem): Make "msg" const.
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index bd09ae6..fbf89ab 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -1993,7 +1993,7 @@ riscv_call_arg_complex_float (struct riscv_arg_info *ainfo,
int len = ainfo->length / 2;
result = riscv_assign_reg_location (&ainfo->argloc[0],
- &cinfo->float_regs, len, len);
+ &cinfo->float_regs, len, 0);
gdb_assert (result);
result = riscv_assign_reg_location (&ainfo->argloc[1],
@@ -2014,14 +2014,18 @@ class riscv_struct_info
public:
riscv_struct_info ()
: m_number_of_fields (0),
- m_types { nullptr, nullptr }
+ m_types { nullptr, nullptr },
+ m_offsets { 0, 0 }
{
/* Nothing. */
}
/* Analyse TYPE descending into nested structures, count the number of
scalar fields and record the types of the first two fields found. */
- void analyse (struct type *type);
+ void analyse (struct type *type)
+ {
+ analyse_inner (type, 0);
+ }
/* The number of scalar fields found in the analysed type. This is
currently only accurate if the value returned is 0, 1, or 2 as the
@@ -2041,6 +2045,16 @@ public:
return m_types[index];
}
+ /* Return the offset of scalar field INDEX within the analysed type. Will
+ return 0 if there is no field at that index. Only INDEX values 0 and
+ 1 can be requested as the RiscV ABI only has special cases for
+ structures with 1 or 2 fields. */
+ int field_offset (int index) const
+ {
+ gdb_assert (index < (sizeof (m_offsets) / sizeof (m_offsets[0])));
+ return m_offsets[index];
+ }
+
private:
/* The number of scalar fields found within the structure after recursing
into nested structures. */
@@ -2049,13 +2063,20 @@ private:
/* The types of the first two scalar fields found within the structure
after recursing into nested structures. */
struct type *m_types[2];
+
+ /* The offsets of the first two scalar fields found within the structure
+ after recursing into nested structures. */
+ int m_offsets[2];
+
+ /* Recursive core for ANALYSE, the OFFSET parameter tracks the byte
+ offset from the start of the top level structure being analysed. */
+ void analyse_inner (struct type *type, int offset);
};
-/* Analyse TYPE descending into nested structures, count the number of
- scalar fields and record the types of the first two fields found. */
+/* See description in class declaration. */
void
-riscv_struct_info::analyse (struct type *type)
+riscv_struct_info::analyse_inner (struct type *type, int offset)
{
unsigned int count = TYPE_NFIELDS (type);
unsigned int i;
@@ -2067,11 +2088,13 @@ riscv_struct_info::analyse (struct type *type)
struct type *field_type = TYPE_FIELD_TYPE (type, i);
field_type = check_typedef (field_type);
+ int field_offset
+ = offset + TYPE_FIELD_BITPOS (type, i) / TARGET_CHAR_BIT;
switch (TYPE_CODE (field_type))
{
case TYPE_CODE_STRUCT:
- analyse (field_type);
+ analyse_inner (field_type, field_offset);
break;
default:
@@ -2081,7 +2104,10 @@ riscv_struct_info::analyse (struct type *type)
structure we can special case, and pass the structure in
memory. */
if (m_number_of_fields < 2)
- m_types[m_number_of_fields] = field_type;
+ {
+ m_types[m_number_of_fields] = field_type;
+ m_offsets[m_number_of_fields] = field_offset;
+ }
m_number_of_fields++;
break;
}
@@ -2114,17 +2140,54 @@ riscv_call_arg_struct (struct riscv_arg_info *ainfo,
if (sinfo.number_of_fields () == 1
&& TYPE_CODE (sinfo.field_type (0)) == TYPE_CODE_COMPLEX)
{
- gdb_assert (TYPE_LENGTH (ainfo->type)
- == TYPE_LENGTH (sinfo.field_type (0)));
- return riscv_call_arg_complex_float (ainfo, cinfo);
+ /* The following is similar to RISCV_CALL_ARG_COMPLEX_FLOAT,
+ except we use the type of the complex field instead of the
+ type from AINFO, and the first location might be at a non-zero
+ offset. */
+ if (TYPE_LENGTH (sinfo.field_type (0)) <= (2 * cinfo->flen)
+ && riscv_arg_regs_available (&cinfo->float_regs) >= 2
+ && !ainfo->is_unnamed)
+ {
+ bool result;
+ int len = TYPE_LENGTH (sinfo.field_type (0)) / 2;
+ int offset = sinfo.field_offset (0);
+
+ result = riscv_assign_reg_location (&ainfo->argloc[0],
+ &cinfo->float_regs, len,
+ offset);
+ gdb_assert (result);
+
+ result = riscv_assign_reg_location (&ainfo->argloc[1],
+ &cinfo->float_regs, len,
+ (offset + len));
+ gdb_assert (result);
+ }
+ else
+ riscv_call_arg_scalar_int (ainfo, cinfo);
+ return;
}
if (sinfo.number_of_fields () == 1
&& TYPE_CODE (sinfo.field_type (0)) == TYPE_CODE_FLT)
{
- gdb_assert (TYPE_LENGTH (ainfo->type)
- == TYPE_LENGTH (sinfo.field_type (0)));
- return riscv_call_arg_scalar_float (ainfo, cinfo);
+ /* The following is similar to RISCV_CALL_ARG_SCALAR_FLOAT,
+ except we use the type of the first scalar field instead of
+ the type from AINFO. Also the location might be at a non-zero
+ offset. */
+ if (TYPE_LENGTH (sinfo.field_type (0)) > cinfo->flen
+ || ainfo->is_unnamed)
+ riscv_call_arg_scalar_int (ainfo, cinfo);
+ else
+ {
+ int offset = sinfo.field_offset (0);
+ int len = TYPE_LENGTH (sinfo.field_type (0));
+
+ if (!riscv_assign_reg_location (&ainfo->argloc[0],
+ &cinfo->float_regs,
+ len, offset))
+ riscv_call_arg_scalar_int (ainfo, cinfo);
+ }
+ return;
}
if (sinfo.number_of_fields () == 2
@@ -2134,17 +2197,14 @@ riscv_call_arg_struct (struct riscv_arg_info *ainfo,
&& TYPE_LENGTH (sinfo.field_type (1)) <= cinfo->flen
&& riscv_arg_regs_available (&cinfo->float_regs) >= 2)
{
- int len0, len1, offset;
-
- gdb_assert (TYPE_LENGTH (ainfo->type) <= (2 * cinfo->flen));
-
- len0 = TYPE_LENGTH (sinfo.field_type (0));
+ int len0 = TYPE_LENGTH (sinfo.field_type (0));
+ int offset = sinfo.field_offset (0);
if (!riscv_assign_reg_location (&ainfo->argloc[0],
- &cinfo->float_regs, len0, 0))
+ &cinfo->float_regs, len0, offset))
error (_("failed during argument setup"));
- len1 = TYPE_LENGTH (sinfo.field_type (1));
- offset = align_up (len0, riscv_type_alignment (sinfo.field_type (1)));
+ int len1 = TYPE_LENGTH (sinfo.field_type (1));
+ offset = sinfo.field_offset (1);
gdb_assert (len1 <= (TYPE_LENGTH (ainfo->type)
- TYPE_LENGTH (sinfo.field_type (0))));
@@ -2162,15 +2222,14 @@ riscv_call_arg_struct (struct riscv_arg_info *ainfo,
&& is_integral_type (sinfo.field_type (1))
&& TYPE_LENGTH (sinfo.field_type (1)) <= cinfo->xlen))
{
- int len0, len1, offset;
-
- len0 = TYPE_LENGTH (sinfo.field_type (0));
+ int len0 = TYPE_LENGTH (sinfo.field_type (0));
+ int offset = sinfo.field_offset (0);
if (!riscv_assign_reg_location (&ainfo->argloc[0],
- &cinfo->float_regs, len0, 0))
+ &cinfo->float_regs, len0, offset))
error (_("failed during argument setup"));
- len1 = TYPE_LENGTH (sinfo.field_type (1));
- offset = align_up (len0, riscv_type_alignment (sinfo.field_type (1)));
+ int len1 = TYPE_LENGTH (sinfo.field_type (1));
+ offset = sinfo.field_offset (1);
gdb_assert (len1 <= cinfo->xlen);
if (!riscv_assign_reg_location (&ainfo->argloc[1],
&cinfo->int_regs, len1, offset))
@@ -2185,19 +2244,18 @@ riscv_call_arg_struct (struct riscv_arg_info *ainfo,
&& TYPE_CODE (sinfo.field_type (1)) == TYPE_CODE_FLT
&& TYPE_LENGTH (sinfo.field_type (1)) <= cinfo->flen))
{
- int len0, len1, offset;
-
- len0 = TYPE_LENGTH (sinfo.field_type (0));
- len1 = TYPE_LENGTH (sinfo.field_type (1));
- offset = align_up (len0, riscv_type_alignment (sinfo.field_type (1)));
+ int len0 = TYPE_LENGTH (sinfo.field_type (0));
+ int len1 = TYPE_LENGTH (sinfo.field_type (1));
gdb_assert (len0 <= cinfo->xlen);
gdb_assert (len1 <= cinfo->flen);
+ int offset = sinfo.field_offset (0);
if (!riscv_assign_reg_location (&ainfo->argloc[0],
- &cinfo->int_regs, len0, 0))
+ &cinfo->int_regs, len0, offset))
error (_("failed during argument setup"));
+ offset = sinfo.field_offset (1);
if (!riscv_assign_reg_location (&ainfo->argloc[1],
&cinfo->float_regs,
len1, offset))
@@ -2233,6 +2291,8 @@ riscv_arg_location (struct gdbarch *gdbarch,
ainfo->align = riscv_type_alignment (ainfo->type);
ainfo->is_unnamed = is_unnamed;
ainfo->contents = nullptr;
+ ainfo->argloc[0].c_length = 0;
+ ainfo->argloc[1].c_length = 0;
switch (TYPE_CODE (ainfo->type))
{
@@ -2474,10 +2534,11 @@ riscv_push_dummy_call (struct gdbarch *gdbarch,
memset (tmp, -1, sizeof (tmp));
else
memset (tmp, 0, sizeof (tmp));
- memcpy (tmp, info->contents, info->argloc[0].c_length);
+ memcpy (tmp, (info->contents + info->argloc[0].c_offset),
+ info->argloc[0].c_length);
regcache->cooked_write (info->argloc[0].loc_data.regno, tmp);
second_arg_length =
- ((info->argloc[0].c_length < info->length)
+ (((info->argloc[0].c_length + info->argloc[0].c_offset) < info->length)
? info->argloc[1].c_length : 0);
second_arg_data = info->contents + info->argloc[1].c_offset;
}
@@ -2629,18 +2690,24 @@ riscv_return_value (struct gdbarch *gdbarch,
<= register_size (gdbarch, regnum));
if (readbuf)
- regcache->cooked_read_part (regnum, 0,
- info.argloc[0].c_length,
- readbuf);
+ {
+ gdb_byte *ptr = readbuf + info.argloc[0].c_offset;
+ regcache->cooked_read_part (regnum, 0,
+ info.argloc[0].c_length,
+ ptr);
+ }
if (writebuf)
- regcache->cooked_write_part (regnum, 0,
- info.argloc[0].c_length,
- writebuf);
+ {
+ const gdb_byte *ptr = writebuf + info.argloc[0].c_offset;
+ regcache->cooked_write_part (regnum, 0,
+ info.argloc[0].c_length,
+ ptr);
+ }
/* A return value in register can have a second part in a
second register. */
- if (info.argloc[0].c_length < info.length)
+ if (info.argloc[1].c_length > 0)
{
switch (info.argloc[1].loc_type)
{