From: Tom de Vries Date: Fri, 17 Oct 2025 06:02:02 +0000 (+0200) Subject: [gdb/tdep] Fix inferior call return of small char array for ppc64 v1 abi some more X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=0e4fd060ee7244d00c76f0d4815063dfcc9877f1;p=thirdparty%2Fbinutils-gdb.git [gdb/tdep] Fix inferior call return of small char array for ppc64 v1 abi some more PR tdep/33534 reports a regression due to commit 13f1820106c ("[gdb/tdep] Fix inferior call return of small char array for ppc64 v1 abi"). The regression can be reproduced with the test-case introduced in the commit: gdb.ada/return-small-char-array.exp, on a ppc64-linux setup with v1 elf abi (cfarm121). The commit contains two changes to a piece of code in ppc64_sysv_abi_return_value: ... /* Small character arrays are returned, right justified, in r3. */ - if (valtype->code () == TYPE_CODE_ARRAY + if (tdep->elf_abi == POWERPC_ELF_V1 + && valtype->code () == TYPE_CODE_ARRAY && !valtype->is_vector () && valtype->length () <= 8 - && valtype->target_type ()->code () == TYPE_CODE_INT + && (valtype->target_type ()->code () == TYPE_CODE_INT + || valtype->target_type ()->code () == TYPE_CODE_CHAR) && valtype->target_type ()->length () == 1) ... The first change limits the effect of the if clause to the v1 elf abi. This change doesn't affect the regression, since it's on a ppc64-linux setup with v1 elf abi. Furthermore, it's correct in the sense that the v2 elf abi doesn't have this kind of special treatment of small character arrays. The second change is the part that causes the regression. The code itself seems correct, in the sense that it enables gdb to recognize small char arrays in ada. The regression stems from the following discrepancy. The comment in gdb states that "small character arrays are returned, right justified, in r3". This matches the v1 ABI [1]. OTOH, gcc produces code that is not in agreement with this. Instead, it passes the small character arrays in memory, in a caller-allocated storage buffer pointed at by r3. This turns out to be an gcc bug [2]. Fix this by treating this as an abi spec bug, and replacing the code handling the "Small character arrays" case with a comment. Doing so reveals that there are two problems in the test-case: - missing fvar-tracking, and - the "step 2" command doesn't land at the intended line. Fix these by: - adding fvar-tracking, and - setting a breakpoint at the intended line, and continuing to it. Tested on ppc64-linux (v1 abi), ppc64le-linux (v2 abi), and x86_64-linux. Approved-By: Tom Tromey Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33534 [1] https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi-1.9.html. [2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=122282 --- diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c index f872f73e414..cc300cd0f6b 100644 --- a/gdb/ppc-sysv-tdep.c +++ b/gdb/ppc-sysv-tdep.c @@ -2059,26 +2059,13 @@ ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, return RETURN_VALUE_REGISTER_CONVENTION; } - /* Small character arrays are returned, right justified, in r3. */ - if (tdep->elf_abi == POWERPC_ELF_V1 - && valtype->code () == TYPE_CODE_ARRAY - && !valtype->is_vector () - && valtype->length () <= 8 - && (valtype->target_type ()->code () == TYPE_CODE_INT - || valtype->target_type ()->code () == TYPE_CODE_CHAR) - && valtype->target_type ()->length () == 1) - { - int regnum = tdep->ppc_gp0_regnum + 3; - int offset = (register_size (gdbarch, regnum) - valtype->length ()); - - if (writebuf != NULL) - regcache->cooked_write_part (regnum, offset, valtype->length (), - writebuf); - if (readbuf != NULL) - regcache->cooked_read_part (regnum, offset, valtype->length (), - readbuf); - return RETURN_VALUE_REGISTER_CONVENTION; - } + /* Small character arrays are returned, right justified, in r3, according to + the v1 ELF ABI spec [1]. But GCC doesn't implement this (PR gcc/122282), + so we treat this as an ABI spec bug, and use + RETURN_VALUE_STRUCT_CONVENTION. We don't handle this case explicly, but + let it be handled by the default return. + [1] https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi-1.9.html. + */ /* In the ELFv2 ABI, homogeneous floating-point or vector aggregates are returned in registers. */ diff --git a/gdb/testsuite/gdb.ada/return-small-char-array.exp b/gdb/testsuite/gdb.ada/return-small-char-array.exp index 1f6412edff4..63009bed2df 100644 --- a/gdb/testsuite/gdb.ada/return-small-char-array.exp +++ b/gdb/testsuite/gdb.ada/return-small-char-array.exp @@ -19,7 +19,13 @@ require allow_ada_tests standard_ada_testfile proc -if { [gdb_compile_ada $srcfile $binfile executable debug] != "" } { +set opts {} +lappend opts debug +if { [ada_fvar_tracking] } { + lappend opts "additional_flags=-fvar-tracking" +} + +if { [gdb_compile_ada $srcfile $binfile executable $opts] != "" } { return } @@ -31,9 +37,13 @@ runto "proc.adb:$bp_location" gdb_test "print Value.Name(My_Value)" \ { = "abcd"} -# Step into the function. -gdb_test "step 2" \ - "return Of_Value;" +# Step into the function. Don't do this by stepping, but by setting a +# breakpoint and continuing to it, to avoid the problem that varying line info +# may require us to step a different amount of times to land at the same +# location. +set bp_location_2 [gdb_get_line_number "STOP" $testdir/value.adb] +gdb_breakpoint value.adb:$bp_location_2 +gdb_continue_to_breakpoint "STOP" # and finish. gdb_test "finish" \ diff --git a/gdb/testsuite/gdb.ada/return-small-char-array/value.adb b/gdb/testsuite/gdb.ada/return-small-char-array/value.adb index 2dd9faacc1b..351e67aa27a 100644 --- a/gdb/testsuite/gdb.ada/return-small-char-array/value.adb +++ b/gdb/testsuite/gdb.ada/return-small-char-array/value.adb @@ -16,6 +16,6 @@ package body Value is function Name (Of_Value : T) return T is begin - return Of_Value; + return Of_Value; -- STOP end Name; end Value;