[RFA] (riscv/ada) fix error when calling functions with range argument
Commit Message
* Tom Tromey <tom@tromey.com> [2019-02-17 07:25:57 -0700]:
> >>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:
>
> Joel> Attached is a patch which does just that.
>
> Joel> gdb/ChangeLog:
>
> Joel> * gdbtypes.c (type_align): Handle TYPE_CODE_RANGE the same as
> Joel> integers and enumeration types.
>
> Joel> Tested on x86_64-linux. Also tested on a variety of platforms
> Joel> (with CPUs being ARM, AArch64, Leon3 (SPARC-like), PowerPC,
> Joel> PowerPC64, RV64, Visium, x86, x86_64).
>
> Joel> OK to push?
>
> Yes, thanks for doing this.
>
> I still have a to-do item to convert the various arch-specific
> type-alignment functions to use type_align. In some cases I'm not sure
> how I'd test this though...
So I took a look at doing this switch for RISC-V, and the problem is
the handling of vectors.
The RISC-V alignment code contains this:
case TYPE_CODE_ARRAY:
if (TYPE_VECTOR (t))
return std::min (TYPE_LENGTH (t), (unsigned) BIGGEST_ALIGNMENT);
/* FALLTHROUGH */
case TYPE_CODE_COMPLEX:
return riscv_type_alignment (TYPE_TARGET_TYPE (t));
While the generic type_align just does this:
case TYPE_CODE_ARRAY:
case TYPE_CODE_COMPLEX:
case TYPE_CODE_TYPEDEF:
align = type_align (TYPE_TARGET_TYPE (type));
break;
So, identical in the non-vector array case, but different in the
vector case. The RISC-V vector handling is required, it fixes an
issue on RV64 in the gdb.base/gnu_vector.exp case.
I can see that at least ARM and AARCH64 also have different rules for
vectors than for arrays.
I wonder if we should change the relationship between 'type_align' and
'gdbarch_type_align'?
Currently 'gdbarch_type_align' is only called for "base" types, int,
float, etc. But not compound types, arrays, structs, complex, etc.
What if instead, we always called 'gdbarch_type_align' from
'type_align' and we had 'default_type_align' return 0 for compound
types, and only provide an alignment for the "base" types. Then
'type_align' would inspect the value returned from
'gdbarch_type_align', and if it sees 0, it breaks the compound type
up, and calls itself on the component type.
Below is a rough, untested patch of this idea, feedback welcome.
Thanks,
Andrew
Comments
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
Andrew> So I took a look at doing this switch for RISC-V, and the problem is
Andrew> the handling of vectors.
[...]
Andrew> I wonder if we should change the relationship between 'type_align' and
Andrew> 'gdbarch_type_align'?
[...]
Andrew> Below is a rough, untested patch of this idea, feedback welcome.
I think it is a good idea. Thanks for looking into this.
I think it would be good to update the type_align documentation in
gdbarch.sh to explain what is expected, and in particular what returning
0 means.
Tom
@@ -993,7 +993,35 @@ default_in_indirect_branch_thunk (gdbarch *gdbarch, CORE_ADDR pc)
ULONGEST
default_type_align (struct gdbarch *gdbarch, struct type *type)
{
- return type_length_units (check_typedef (type));
+ ULONGEST align;
+
+ switch (TYPE_CODE (type))
+ {
+ case TYPE_CODE_PTR:
+ case TYPE_CODE_FUNC:
+ case TYPE_CODE_FLAGS:
+ case TYPE_CODE_INT:
+ case TYPE_CODE_RANGE:
+ case TYPE_CODE_FLT:
+ case TYPE_CODE_ENUM:
+ case TYPE_CODE_REF:
+ case TYPE_CODE_RVALUE_REF:
+ case TYPE_CODE_CHAR:
+ case TYPE_CODE_BOOL:
+ case TYPE_CODE_DECFLOAT:
+ case TYPE_CODE_METHODPTR:
+ case TYPE_CODE_MEMBERPTR:
+ align = type_length_units (check_typedef (type));
+ break;
+ default:
+ /* Return 0 for more compound types, the type_align function in
+ gdbtypes.c will take care of breaking these types up and calling
+ this method again on the component types. */
+ align = 0;
+ break;
+ }
+
+ return align;
}
void
@@ -2992,11 +2992,17 @@ type_raw_align (struct type *type)
unsigned
type_align (struct type *type)
{
+ /* Check alignment provided in the debug information. */
unsigned raw_align = type_raw_align (type);
if (raw_align != 0)
return raw_align;
- ULONGEST align = 0;
+ /* Allow the architecture to provide an alignment. */
+ struct gdbarch *arch = get_type_arch (type);
+ ULONGEST align = gdbarch_type_align (arch, type);
+ if (align != 0)
+ return align;
+
switch (TYPE_CODE (type))
{
case TYPE_CODE_PTR:
@@ -3013,10 +3019,7 @@ type_align (struct type *type)
case TYPE_CODE_DECFLOAT:
case TYPE_CODE_METHODPTR:
case TYPE_CODE_MEMBERPTR:
- {
- struct gdbarch *arch = get_type_arch (type);
- align = gdbarch_type_align (arch, type);
- }
+ /* We have no further way to compute an alignment for this type. */
break;
case TYPE_CODE_ARRAY: