(SPARC/LEON) fix incorrect array return value printed by "finish"
Consider the code in the gdb.ada/array_return.exp testcase, which
defines a function returning an array of 2 integers:
type Data_Small is array (1 .. 2) of Integer;
function Create_Small return Data_Small;
When doing a "finish" from inside function Create_Small, we expect
GDB to tell us that the return value was "(1, 1)". However, it currently
prints the wrong value:
(gdb) finish
Run till exit from #0 pck.create_small () at /[...]/pck.adb:5
p () at /[...]/p.adb:10
10 Large := Create_Large;
Value returned is $1 = (0, 0)
This is a regression which I traced back to the following commit...
| commit
1933fd8ee01ad2e74a9c6341bc40f54962a8f889
| Date: Fri May 19 03:06:19 2017 -0700
| Subject: gdb: fix TYPE_CODE_ARRAY handling in sparc targets
... which, despite what the subject says, is not really about
TYPE_CODE_ARRAY handling, which is a bit of an implementation detail,
but about the GNU vectors extension.
The author of the patch equated TYPE_CODE_ARRAY with vectors, which
is not correct. Vectors are TYPE_CODE_ARRAY types with the TYPE_VECTOR
flag set. So at the very minimum, the patch should have been checking
for both TYPE_CODE_ARRAY and TYPE_VECTOR.
But, that's not the only thing that did not seem right to me. When
looking at the ABI, and at the summary of the implementation in GCC
of the calling conventions for that architecture:
size argument return value
small integer <4 int. reg. int. reg.
word 4 int. reg. int. reg.
double word 8 int. reg. int. reg.
_Complex small integer <8 int. reg. int. reg.
_Complex word 8 int. reg. int. reg.
_Complex double word 16 memory int. reg.
vector integer <=8 int. reg. FP reg.
vector integer >8 memory memory
float 4 int. reg. FP reg.
double 8 int. reg. FP reg.
long double 16 memory memory
_Complex float 8 memory FP reg.
_Complex double 16 memory FP reg.
_Complex long double 32 memory FP reg.
vector float any memory memory
aggregate any memory memory
The nice thing about the patch above is that it nicely factorized
the code that determines how arguments are passed/returns. The bad
news is that the implementation, particularly for the handling of
arrays and vectors, doesn't seem to match the summary above. Hence,
the regression we observed.
So what I did was review and re-implement some of the predicate functions
according to the summary above. Because dejagnu crashes all our Solaris
machines real bad, I can't run the dejagnu testsuite there. So what I did
was test the patch with AdaCore's testsuite against leon3-elf, no
regression. I verified that this fixes the regression above while
at the same time still passing gdb.base/gnu_vector.exp (I transposed
that testcase to our testsuite), which is the testcase that was cited
in the commit above as seeing some FAIL->PASS improvements.
This patch also removes one assertion...
gdb_assert (sparc_integral_or_pointer_p (type)
|| (TYPE_CODE (type) == TYPE_CODE_ARRAY && len <= 8));
... because that assertion is really the "negative" of the other conditions
written in the same "if, else if, else [assert]" block in this function.
To me, this assertion forces us to maintain two versions of the same code,
and is an unnecessary burden. In particular, the above is not the
correct condition, and the ABI summary table above shows that we need
a more complex condition to describe the situations where we expect
arguments to be passed by register.
gdb/ChangeLog:
* sparc-tdep.c (sparc_structure_return_p): Re-implement to
match the ABI as summarized in GCC's gcc/config/sparc/sparc.c.
(sparc_arg_by_memory_p): Renamed from sparc_arg_on_registers_p.
Re-implement to match the ABI as summarized in GCC's
gcc/config/sparc/sparc.c. All callers updated.
(sparc32_store_arguments): Remove assertion.