diff mbox

[RFA] (riscv/ada) fix error when calling functions with range argument

Message ID 20190217124708.GA25164@adacore.com
State New
Headers show

Commit Message

Joel Brobecker Feb. 17, 2019, 12:47 p.m. UTC 
> > Tom> I don't have any issue with this, but I do wonder if
> > Tom> riscv_type_alignment can be removed and/or simplified in favor
> > Tom> type_align and the gdbarch method.
> > 
> > Also now I wonder whether the TYPE_CODE_RANGE case in
> > gdbtypes.c:type_align ought to be changed.  Right now it just returns 0,
> > which means "cannot be determined".
> 
> Makes sense to me. I don't see this as being any problem at all, but
> just in case, Let's make the change internally at AdaCore, and
> have it be evaluated on all our platforms.

Attached is a patch which does just that.

gdb/ChangeLog:

        * gdbtypes.c (type_align): Handle TYPE_CODE_RANGE the same as
        integers and enumeration types.

Tested on x86_64-linux. Also tested on a variety of platforms
(with CPUs being ARM, AArch64, Leon3 (SPARC-like), PowerPC,
PowerPC64, RV64, Visium, x86, x86_64).

OK to push?

Comments

Tom Tromey Feb. 17, 2019, 2:25 p.m. UTC | #1 
>>>>> "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...

Tom
Joel Brobecker Feb. 17, 2019, 3:08 p.m. UTC | #2 
> 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.

Thank you. Pushed to master.

> 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...

Besides the build bots that exist, the only architectures I think
you can reasonably test are the architectures that we have at AdaCore.
Testing the change with AdaCore's testsuite isn't going to guaranty
no regression, but it's better than nothing. If there are specific
maintainers for those arches, we can ask them too. For the others,
maybe what we could do is just compare the two implementations as
best we can, and see if their code has any deviation, and if they
do, check the ABI...
diff mbox

Patch

From c6cec5f43eefc87120ae148fc3edf695a345438d Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Sun, 17 Feb 2019 07:40:43 -0500
Subject: [PATCH] type_align: handle range types the same as ints and enums

This commit enhances type_align to handle TYPE_CODE_RANGE types
the same as integers and enums, rather than returning zero,
which means for this function that it could not determine its
alignment.

gdb/ChangeLog:

	* gdbtypes.c (type_align): Handle TYPE_CODE_RANGE the same as
        integers and enumeration types.

Tested on x86_64-linux. Also tested on a variety of platforms
(with CPUs being ARM, AArch64, Leon3 (SPARC-like), PowerPC,
PowerPC64, RV64, Visium, x86, x86_64).
---
 gdb/gdbtypes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index d1ca304..6758783 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -3003,6 +3003,7 @@  type_align (struct type *type)
     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:
@@ -3047,7 +3048,6 @@  type_align (struct type *type)
       break;
 
     case TYPE_CODE_SET:
-    case TYPE_CODE_RANGE:
     case TYPE_CODE_STRING:
       /* Not sure what to do here, and these can't appear in C or C++
 	 anyway.  */
-- 
2.1.4