* [PATCH] PowerPC, add support for printing non-trivial C++ object for the finish command. @ 2022-03-11 22:26 Carl Love 2022-03-13 5:28 ` Joel Brobecker ` (2 more replies) 0 siblings, 3 replies; 44+ messages in thread From: Carl Love @ 2022-03-11 22:26 UTC (permalink / raw) To: gdb-patches, Ulrich Weigand; +Cc: Rogerio Alves, Will Schmidt, cel GDB maintainers: The following patch fixes an issue on Powerpc where the function return value for a C++ program prints garbage when the function return type is a class object. The fix is based on an x86-64 patch for the same issue. The patch has been tested on Power 10 with no regressions. Please let me know if this patch is acceptable for mainline. Thanks. Carl Love --------------------------------------------------- PowerPC, add support for printing non-trivial C++ object for the finish command. This patch fixes five testcase failures in gdb/cpp/non-trival-retval.exp. The testcases that fail were added by commit: commit b1718fcdd1d2a5c514f8ee504ba07fb3f42b8608 Author: Andrew Burgess <aburgess@redhat.com> Date: Mon Dec 13 16:56:16 2021 +0000 gdb: on x86-64 non-trivial C++ objects are returned in memory Fixes PR gdb/28681. It was observed that after using the `finish` command an incorrect value was displayed in some cases. Specifically, this behaviour was observed on an x86-64 target. The commit adds new tests for printing the return value of a C++ function. The return value of the function is a C++ Class. The current PowerPC code is looking for the return value in register r3. The return value is actually in memory as described in the commit message for x86-64 above. The fix for Powerpc is based on the fix in the above commit plus examining how x86-64 handles the value in function amd64_return_value in file gdb/amd64-tdep.c. --- gdb/ppc-sysv-tdep.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c index 9a3b02f028d..50277e7bdd1 100644 --- a/gdb/ppc-sysv-tdep.c +++ b/gdb/ppc-sysv-tdep.c @@ -2035,6 +2035,20 @@ ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, return RETURN_VALUE_REGISTER_CONVENTION; } + /* If the object is a non-trivial C++ object the object is in memory. */ + if (!language_pass_by_reference (valtype).trivially_copyable) + { + if (readbuf) + { + ULONGEST addr; + int regnum = tdep->ppc_gp0_regnum + 3; + + regcache_raw_read_unsigned (regcache, regnum, &addr); + read_memory (addr, readbuf, TYPE_LENGTH (valtype)); + } + return RETURN_VALUE_ABI_RETURNS_ADDRESS; + } + /* In the ELFv2 ABI, aggregate types of up to 16 bytes are returned in registers r3:r4. */ if (tdep->elf_abi == POWERPC_ELF_V2 -- 2.32.0 ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] PowerPC, add support for printing non-trivial C++ object for the finish command. 2022-03-11 22:26 [PATCH] PowerPC, add support for printing non-trivial C++ object for the finish command Carl Love @ 2022-03-13 5:28 ` Joel Brobecker 2022-03-14 10:43 ` Luis Machado 2022-03-14 13:40 ` Tom Tromey 2 siblings, 0 replies; 44+ messages in thread From: Joel Brobecker @ 2022-03-13 5:28 UTC (permalink / raw) To: Carl Love via Gdb-patches; +Cc: Ulrich Weigand, Rogerio Alves, Joel Brobecker Hello, > The following patch fixes an issue on Powerpc where the function return > value for a C++ program prints garbage when the function return type is > a class object. The fix is based on an x86-64 patch for the same > issue. > > The patch has been tested on Power 10 with no regressions. Please let > me know if this patch is acceptable for mainline. Thanks. Thanks for the patch. I don't know the precise specifications regarding returning C++ objects on PowerPC targets, but I think we can trust you on that. For the rest, the patch looks OK to me. Maybe one small teeny weeny wittle nitpick, erm, I mean, suggestion. Which lead me to realize that there was a small GNU Coding Style issue. See below: > > Carl Love > --------------------------------------------------- > PowerPC, add support for printing non-trivial C++ object for the finish command. > > This patch fixes five testcase failures in gdb/cpp/non-trival-retval.exp. > The testcases that fail were added by commit: > > commit b1718fcdd1d2a5c514f8ee504ba07fb3f42b8608 > Author: Andrew Burgess <aburgess@redhat.com> > Date: Mon Dec 13 16:56:16 2021 +0000 > > gdb: on x86-64 non-trivial C++ objects are returned in memory > > Fixes PR gdb/28681. It was observed that after using the `finish` > command an incorrect value was displayed in some cases. Specifically, > this behaviour was observed on an x86-64 target. > > The commit adds new tests for printing the return value of a C++ function. > The return value of the function is a C++ Class. The current PowerPC code > is looking for the return value in register r3. The return value is actually > in memory as described in the commit message for x86-64 above. > > The fix for Powerpc is based on the fix in the above commit plus examining > how x86-64 handles the value in function amd64_return_value in file > gdb/amd64-tdep.c. > --- > gdb/ppc-sysv-tdep.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c > index 9a3b02f028d..50277e7bdd1 100644 > --- a/gdb/ppc-sysv-tdep.c > +++ b/gdb/ppc-sysv-tdep.c > @@ -2035,6 +2035,20 @@ ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, > return RETURN_VALUE_REGISTER_CONVENTION; > } > > + /* If the object is a non-trivial C++ object the object is in memory. */ nitpick: I would add a coma after "non-trivial C++ object". Also, can you add a second space after the period, before the "*/"? (it's a GNU Coding Style requirement) Thank you! > + if (!language_pass_by_reference (valtype).trivially_copyable) > + { > + if (readbuf) > + { > + ULONGEST addr; > + int regnum = tdep->ppc_gp0_regnum + 3; > + > + regcache_raw_read_unsigned (regcache, regnum, &addr); > + read_memory (addr, readbuf, TYPE_LENGTH (valtype)); > + } > + return RETURN_VALUE_ABI_RETURNS_ADDRESS; > + } > + > /* In the ELFv2 ABI, aggregate types of up to 16 bytes are > returned in registers r3:r4. */ > if (tdep->elf_abi == POWERPC_ELF_V2 > -- > 2.32.0 > > -- Joel ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] PowerPC, add support for printing non-trivial C++ object for the finish command. 2022-03-11 22:26 [PATCH] PowerPC, add support for printing non-trivial C++ object for the finish command Carl Love 2022-03-13 5:28 ` Joel Brobecker @ 2022-03-14 10:43 ` Luis Machado 2022-03-14 13:40 ` Tom Tromey 2 siblings, 0 replies; 44+ messages in thread From: Luis Machado @ 2022-03-14 10:43 UTC (permalink / raw) To: Carl Love, gdb-patches, Ulrich Weigand; +Cc: Rogerio Alves On 3/11/22 22:26, Carl Love via Gdb-patches wrote: > > GDB maintainers: > > The following patch fixes an issue on Powerpc where the function return > value for a C++ program prints garbage when the function return type is > a class object. The fix is based on an x86-64 patch for the same > issue. > > The patch has been tested on Power 10 with no regressions. Please let > me know if this patch is acceptable for mainline. Thanks. > > Carl Love > --------------------------------------------------- > PowerPC, add support for printing non-trivial C++ object for the finish command. > > This patch fixes five testcase failures in gdb/cpp/non-trival-retval.exp. > The testcases that fail were added by commit: > > commit b1718fcdd1d2a5c514f8ee504ba07fb3f42b8608 > Author: Andrew Burgess <aburgess@redhat.com> > Date: Mon Dec 13 16:56:16 2021 +0000 > > gdb: on x86-64 non-trivial C++ objects are returned in memory > > Fixes PR gdb/28681. It was observed that after using the `finish` > command an incorrect value was displayed in some cases. Specifically, > this behaviour was observed on an x86-64 target. > > The commit adds new tests for printing the return value of a C++ function. > The return value of the function is a C++ Class. The current PowerPC code > is looking for the return value in register r3. The return value is actually > in memory as described in the commit message for x86-64 above. > > The fix for Powerpc is based on the fix in the above commit plus examining > how x86-64 handles the value in function amd64_return_value in file > gdb/amd64-tdep.c. > --- > gdb/ppc-sysv-tdep.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c > index 9a3b02f028d..50277e7bdd1 100644 > --- a/gdb/ppc-sysv-tdep.c > +++ b/gdb/ppc-sysv-tdep.c > @@ -2035,6 +2035,20 @@ ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, > return RETURN_VALUE_REGISTER_CONVENTION; > } > > + /* If the object is a non-trivial C++ object the object is in memory. */ > + if (!language_pass_by_reference (valtype).trivially_copyable) > + { > + if (readbuf) > + { > + ULONGEST addr; > + int regnum = tdep->ppc_gp0_regnum + 3; > + > + regcache_raw_read_unsigned (regcache, regnum, &addr); > + read_memory (addr, readbuf, TYPE_LENGTH (valtype)); > + } > + return RETURN_VALUE_ABI_RETURNS_ADDRESS; > + } > + > /* In the ELFv2 ABI, aggregate types of up to 16 bytes are > returned in registers r3:r4. */ > if (tdep->elf_abi == POWERPC_ELF_V2 Carl, Just a heads-up that I pushed a new test (gdb.base/retval-large-struct.exp) that exercises returning a large struct (> 16 bytes) from a function. You might want to validate that test on ppc as well, given this particular scenario wasn't being validated properly in the testsuite. ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] PowerPC, add support for printing non-trivial C++ object for the finish command. 2022-03-11 22:26 [PATCH] PowerPC, add support for printing non-trivial C++ object for the finish command Carl Love 2022-03-13 5:28 ` Joel Brobecker 2022-03-14 10:43 ` Luis Machado @ 2022-03-14 13:40 ` Tom Tromey 2022-03-14 13:45 ` Luis Machado 2022-03-14 14:58 ` Ulrich Weigand 2 siblings, 2 replies; 44+ messages in thread From: Tom Tromey @ 2022-03-14 13:40 UTC (permalink / raw) To: Carl Love via Gdb-patches; +Cc: Ulrich Weigand, Carl Love, Rogerio Alves >>>>> "Carl" == Carl Love via Gdb-patches <gdb-patches@sourceware.org> writes: Carl> The following patch fixes an issue on Powerpc where the function return Carl> value for a C++ program prints garbage when the function return type is Carl> a class object. The fix is based on an x86-64 patch for the same Carl> issue. Carl> The patch has been tested on Power 10 with no regressions. Please let Carl> me know if this patch is acceptable for mainline. Thanks. FWIW, here at AdaCore, we carry a very similar patch for ppc-linux-tdep.c. I've appended it. I haven't really submitted this yet since I wanted to investigate the ABI difference that lead to the change. Anyway I tend to think your patch is fine. Tom commit 693bb3df1fdf2de96e5a6ce338712d6ba370d0c9 Author: Joel Brobecker <brobecker@adacore.com> Date: Tue Jan 7 06:07:41 2020 -0500 GDB on ppc-linux is unable to print return value when value is struct/union Consider the following C code: | struct S2 { short s1, s2; }; | struct S2 function_2 (void) | { | struct S2 x = { 27, 28 }; | return x; | } When using the "finish" command from this function, GDB currently says it cannot print the return value: | (gdb) finish | Run till exit from #0 function_2 () at finish-struct.c:13 | 0x1000055c in main () at finish-struct.c:20 | 20 function_2 (); | Value returned has type: struct S2. Cannot determine contents As it happens, the ABI being used on PowerPC Linux is such that the address of the returned value is returned via register r3. So we can print its contents. gdb/ChangeLog: * ppc-linux-tdep.c (ppc_linux_return_value): Fix handling of return values which are either structs, unions, or vectors with a size different from 8 or 16. gdb/testsuite/ChangeLog: * gdb.base/finish-struct.c: New file. * gdb.base/finish-struct.exp: New files. Change-Id: I0b336739f6a1929c1b8910aeb87f2e4dfa504bef TN: SC20-032 diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c index 901b938f257..9e0d4902595 100644 --- a/gdb/ppc-linux-tdep.c +++ b/gdb/ppc-linux-tdep.c @@ -254,7 +254,19 @@ ppc_linux_return_value (struct gdbarch *gdbarch, struct value *function, || TYPE_CODE (valtype) == TYPE_CODE_UNION) && !((TYPE_LENGTH (valtype) == 16 || TYPE_LENGTH (valtype) == 8) && TYPE_VECTOR (valtype))) - return RETURN_VALUE_STRUCT_CONVENTION; + { + if (readbuf) + { + struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); + ULONGEST addr; + + regcache_raw_read_unsigned (regcache, tdep->ppc_gp0_regnum + 3, + &addr); + read_memory (addr, readbuf, TYPE_LENGTH (valtype)); + } + + return RETURN_VALUE_ABI_RETURNS_ADDRESS; + } else return ppc_sysv_abi_return_value (gdbarch, function, valtype, regcache, readbuf, writebuf); diff --git a/gdb/testsuite/gdb.base/finish-struct.c b/gdb/testsuite/gdb.base/finish-struct.c new file mode 100644 index 00000000000..2af02a1a1a3 --- /dev/null +++ b/gdb/testsuite/gdb.base/finish-struct.c @@ -0,0 +1,40 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2020 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +struct S1 { char c1, c2, c3, c4; }; + +struct S2 { short s1, s2; }; + +struct S1 +function_1 (void) +{ + struct S1 x = { 'A', 'B', 'C', 'D' }; + return x; +} + +struct S2 +function_2 (void) +{ + struct S2 x = { 27, 28 }; + return x; +} + +int main () +{ + function_1 (); + function_2 (); +} diff --git a/gdb/testsuite/gdb.base/finish-struct.exp b/gdb/testsuite/gdb.base/finish-struct.exp new file mode 100644 index 00000000000..b4e6875e5a7 --- /dev/null +++ b/gdb/testsuite/gdb.base/finish-struct.exp @@ -0,0 +1,47 @@ +# Copyright 2020 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see <http://www.gnu.org/licenses/>. + +# This file is part of the gdb testsuite. It is intended to test that +# gdb can correctly print arrays with indexes for each element of the +# array. + +standard_testfile .c + +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { + untested "failed to compile" + return -1 +} + +clean_restart ${binfile} + +gdb_breakpoint function_1 +gdb_breakpoint function_2 + +# Run the program until we reach the first breakpoint, which should be +# in function_1, and then test the "finish" command from there. + +gdb_run_cmd +gdb_test "" "Breakpoint \[0-9\]+,.*function_1.*" "run to function_1" + +gdb_test "finish" \ + " = {c1 = 65 'A', c2 = 66 'B', c3 = 67 'C', c4 = 68 'D'}" \ + "finish from function_1" + +# Continue the program's execution until reaching the breakpoint inside +# function_2, and then test the "finish" command from there. + +gdb_test "continue" "Breakpoint \[0-9\]+,.*function_2.*" "run to function_2" + +gdb_test "finish" " = {s1 = 27, s2 = 28}" "finish from function_2" ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] PowerPC, add support for printing non-trivial C++ object for the finish command. 2022-03-14 13:40 ` Tom Tromey @ 2022-03-14 13:45 ` Luis Machado 2022-03-14 14:58 ` Ulrich Weigand 1 sibling, 0 replies; 44+ messages in thread From: Luis Machado @ 2022-03-14 13:45 UTC (permalink / raw) To: Tom Tromey, Carl Love via Gdb-patches; +Cc: Ulrich Weigand, Rogerio Alves On 3/14/22 13:40, Tom Tromey via Gdb-patches wrote: >>>>>> "Carl" == Carl Love via Gdb-patches <gdb-patches@sourceware.org> writes: > > Carl> The following patch fixes an issue on Powerpc where the function return > Carl> value for a C++ program prints garbage when the function return type is > Carl> a class object. The fix is based on an x86-64 patch for the same > Carl> issue. > > Carl> The patch has been tested on Power 10 with no regressions. Please let > Carl> me know if this patch is acceptable for mainline. Thanks. > > FWIW, here at AdaCore, we carry a very similar patch for > ppc-linux-tdep.c. I've appended it. > > I haven't really submitted this yet since I wanted to investigate the > ABI difference that lead to the change. I wonder if the test I pushed, gdb.base/retval-large-struct.exp, exercises the same case your attached testcase. There may be more targets running into these issues, but it seems the testsuite wasn't exercising this particular case properly (or something else changed along the way). > > Anyway I tend to think your patch is fine. > > Tom > > commit 693bb3df1fdf2de96e5a6ce338712d6ba370d0c9 > Author: Joel Brobecker <brobecker@adacore.com> > Date: Tue Jan 7 06:07:41 2020 -0500 > > GDB on ppc-linux is unable to print return value when value is struct/union > > Consider the following C code: > > | struct S2 { short s1, s2; }; > | struct S2 function_2 (void) > | { > | struct S2 x = { 27, 28 }; > | return x; > | } > > When using the "finish" command from this function, GDB currently > says it cannot print the return value: > > | (gdb) finish > | Run till exit from #0 function_2 () at finish-struct.c:13 > | 0x1000055c in main () at finish-struct.c:20 > | 20 function_2 (); > | Value returned has type: struct S2. Cannot determine contents > > As it happens, the ABI being used on PowerPC Linux is such that > the address of the returned value is returned via register r3. > So we can print its contents. > > gdb/ChangeLog: > > * ppc-linux-tdep.c (ppc_linux_return_value): Fix handling > of return values which are either structs, unions, or vectors > with a size different from 8 or 16. > > gdb/testsuite/ChangeLog: > > * gdb.base/finish-struct.c: New file. > * gdb.base/finish-struct.exp: New files. > > Change-Id: I0b336739f6a1929c1b8910aeb87f2e4dfa504bef > TN: SC20-032 > > diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c > index 901b938f257..9e0d4902595 100644 > --- a/gdb/ppc-linux-tdep.c > +++ b/gdb/ppc-linux-tdep.c > @@ -254,7 +254,19 @@ ppc_linux_return_value (struct gdbarch *gdbarch, struct value *function, > || TYPE_CODE (valtype) == TYPE_CODE_UNION) > && !((TYPE_LENGTH (valtype) == 16 || TYPE_LENGTH (valtype) == 8) > && TYPE_VECTOR (valtype))) > - return RETURN_VALUE_STRUCT_CONVENTION; > + { > + if (readbuf) > + { > + struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch); > + ULONGEST addr; > + > + regcache_raw_read_unsigned (regcache, tdep->ppc_gp0_regnum + 3, > + &addr); > + read_memory (addr, readbuf, TYPE_LENGTH (valtype)); > + } > + > + return RETURN_VALUE_ABI_RETURNS_ADDRESS; > + } > else > return ppc_sysv_abi_return_value (gdbarch, function, valtype, regcache, > readbuf, writebuf); > diff --git a/gdb/testsuite/gdb.base/finish-struct.c b/gdb/testsuite/gdb.base/finish-struct.c > new file mode 100644 > index 00000000000..2af02a1a1a3 > --- /dev/null > +++ b/gdb/testsuite/gdb.base/finish-struct.c > @@ -0,0 +1,40 @@ > +/* This testcase is part of GDB, the GNU debugger. > + > + Copyright 2020 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see <http://www.gnu.org/licenses/>. */ > + > +struct S1 { char c1, c2, c3, c4; }; > + > +struct S2 { short s1, s2; }; > + > +struct S1 > +function_1 (void) > +{ > + struct S1 x = { 'A', 'B', 'C', 'D' }; > + return x; > +} > + > +struct S2 > +function_2 (void) > +{ > + struct S2 x = { 27, 28 }; > + return x; > +} > + > +int main () > +{ > + function_1 (); > + function_2 (); > +} > diff --git a/gdb/testsuite/gdb.base/finish-struct.exp b/gdb/testsuite/gdb.base/finish-struct.exp > new file mode 100644 > index 00000000000..b4e6875e5a7 > --- /dev/null > +++ b/gdb/testsuite/gdb.base/finish-struct.exp > @@ -0,0 +1,47 @@ > +# Copyright 2020 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see <http://www.gnu.org/licenses/>. > + > +# This file is part of the gdb testsuite. It is intended to test that > +# gdb can correctly print arrays with indexes for each element of the > +# array. > + > +standard_testfile .c > + > +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { > + untested "failed to compile" > + return -1 > +} > + > +clean_restart ${binfile} > + > +gdb_breakpoint function_1 > +gdb_breakpoint function_2 > + > +# Run the program until we reach the first breakpoint, which should be > +# in function_1, and then test the "finish" command from there. > + > +gdb_run_cmd > +gdb_test "" "Breakpoint \[0-9\]+,.*function_1.*" "run to function_1" > + > +gdb_test "finish" \ > + " = {c1 = 65 'A', c2 = 66 'B', c3 = 67 'C', c4 = 68 'D'}" \ > + "finish from function_1" > + > +# Continue the program's execution until reaching the breakpoint inside > +# function_2, and then test the "finish" command from there. > + > +gdb_test "continue" "Breakpoint \[0-9\]+,.*function_2.*" "run to function_2" > + > +gdb_test "finish" " = {s1 = 27, s2 = 28}" "finish from function_2" ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] PowerPC, add support for printing non-trivial C++ object for the finish command. 2022-03-14 13:40 ` Tom Tromey 2022-03-14 13:45 ` Luis Machado @ 2022-03-14 14:58 ` Ulrich Weigand [not found] ` <ce6c71356c4a58fcdfb655a6e50c3a24f812e66c.camel@us.ibm.com> 1 sibling, 1 reply; 44+ messages in thread From: Ulrich Weigand @ 2022-03-14 14:58 UTC (permalink / raw) To: gdb-patches, tromey; +Cc: Rogerio Alves Cardoso, cel Hi Carl, hi Tom, >Carl> The following patch fixes an issue on Powerpc where the function return >Carl> value for a C++ program prints garbage when the function return type is >Carl> a class object. The fix is based on an x86-64 patch for the same >Carl> issue. > >Carl> The patch has been tested on Power 10 with no regressions. Please let >Carl> me know if this patch is acceptable for mainline. Thanks. > >FWIW, here at AdaCore, we carry a very similar patch for >ppc-linux-tdep.c. I've appended it. > >I haven't really submitted this yet since I wanted to investigate the >ABI difference that lead to the change. As to my understanding of the ELFv2 ABI, while r3 contains a pointer to the struct return buffer *on function entry*, the ABI does *not* guarantee that this will remain live throughout the whole function, or be returned in r3. Of course, in many simple examples the compiler will not bother allocating anything else to r3, in particular if it stores the return value shortly before returning. But it's not guaranteed, and therefore either of your patches is not really correct IMO. (A possible always-correct fix would be to use the on-entry value of r3 if we have the relevant on-entry DWARF info.) Note that this is actually the same on ppc as on many (most?) other platform ABIs. Most test case differences are simply due to the fact that the test case uses a struct that happens to be returned in registers on x86_64, but in a buffer on ppc64le. Bye, Ulrich ^ permalink raw reply [flat|nested] 44+ messages in thread
[parent not found: <ce6c71356c4a58fcdfb655a6e50c3a24f812e66c.camel@us.ibm.com>]
[parent not found: <d9f17525c9a03c20b54015a6a71c36cae50fe4e3.camel@de.ibm.com>]
[parent not found: <6ca2276426343756e103995e07ff951d6e26837b.camel@us.ibm.com>]
[parent not found: <939797b94ab71f3f7356747d84a1515939cb3dcc.camel@de.ibm.com>]
[parent not found: <fccc34c438fda9a35b8a8565e0f5026237e7eab9.camel@us.ibm.com>]
[parent not found: <bb5b9b137fc11886964113d4b524526b3c733f4a.camel@us.ibm.com>]
[parent not found: <1edb818bd2873a3fa5278f28131089d228a0a4f6.camel@de.ibm.com>]
[parent not found: <7c884a865d06890cb325225c65d7a52fdfbd20d2.camel@us.ibm.com>]
[parent not found: <846ca96309d2732d3db0e4c323a81105c098fa5f.camel@de.ibm.com>]
[parent not found: <5a858dd7b957ecf45cf5b00ffc140a839c8ef023.camel@us.ibm.com>]
[parent not found: <b634fecae5e33a3d1a278191c37f306a3b8622f2.camel@de.ibm.com>]
[parent not found: <25f2380ced176f58a8e3ea9b70c7e7786988d650.camel@us.ibm.com>]
[parent not found: <2b0481466e9ecc33d52c74c3a3b4babb05435f47.camel@de.ibm.com>]
[parent not found: <df3b049416ff666e7bd3e3a91e4ea90d34256ea5.camel@us.ibm.com>]
[parent not found: <71370ce02bd57827d3b7958772b1594d3591bd16.camel@de.ibm.com>]
[parent not found: <ec9dafb1671699b03b28ee4be528711c6988eaa5.camel@us.ibm.com>]
[parent not found: <148d8d3efcc8d110119e566027bfd0c65dd02525.camel@de.ibm.com>]
[parent not found: <eef62b295e97fc464c22f9d748ff818860137de9.camel@us.ibm.com>]
[parent not found: <afd6fa576f479359618b1ee50b08be8932735da8.camel@de.ibm.com>]
[parent not found: <cb6b19e9d287d2bae4b72627791f2a00af062c48.camel@us.ibm.com>]
[parent not found: <ee7101f86b5c8581905c53347fa603dc23ddc2bd.camel@de.ibm.com>]
[parent not found: <8decd662134d57e8caf43960a1cdc47723e2bfe3.camel@us.ibm.com>]
[parent not found: <f7cad695cf64540bad8c95cf5fd31691711d0eeb.camel@de.ibm.com>]
[parent not found: <79d82ed277308ed5ce312bff398e770ab234390a.camel@us.ibm.com>]
[parent not found: <63f21a897f452d81a73fb386cb99110a359ef0b7.camel@de.ibm.com>]
[parent not found: <be178bc4f356d7f1937458290cb5883eeee9eee1.camel@us.ibm.com>]
[parent not found: <dfd935e9414d3dd2c27d1e877d3718ae7510aa07.camel@de.ibm.com>]
[parent not found: <97275f61ef101a12cde8e5a45008ed8e479424eb.camel@us.ibm.com>]
[parent not found: <b629440707165f46fb466e48b0c95de3bfa334d2.camel@de.ibm.com>]
[parent not found: <191f5826b228a7614c084c9704b086851d418c78.camel@us.ibm.com>]
[parent not found: <5405a79ecd6ed34646ad77eed0779063ee222d37.camel@de.ibm.com>]
* [PATCH 0/2] PowerPC, fix support for printing the function return value for non-trivial values. [not found] ` <5405a79ecd6ed34646ad77eed0779063ee222d37.camel@de.ibm.com> @ 2022-10-06 16:36 ` Carl Love 2022-10-18 18:55 ` [PATCH 0/2 version 2] " Carl Love 2022-10-06 16:37 ` [PATCH 2/2] " Carl Love 1 sibling, 1 reply; 44+ messages in thread From: Carl Love @ 2022-10-06 16:36 UTC (permalink / raw) To: Ulrich Weigand, gdb-patches GDB maintainers: This patch set addresses the five test failures in gdb.cp/non-trivial- retval.exp on PowerPC. The failures started with commit b1718fcdd1d2a5c514f8ee504ba07fb3f42b8608 Author: Andrew Burgess <aburgess@redhat.com> Date: Mon Dec 13 16:56:16 2021 +0000 gdb: on x86-64 non-trivial C++ objects are returned in memory Fixes PR gdb/28681. It was observed that after using the `finish` command an incorrect value was displayed in some cases. Specifically, this behaviour was observed on an x86-64 target. The return value from the function is not correct due to multiple problems. The PowerPC specific code does not return the correct return value convention. This is fixed by the first patach. However due to the PowerPC ABI, GDB is still not able to reliably determine the function return value. The second patch addresses the issue of not being able to reliably determine the return buffer address on PowerPC. The patch adds a new GDB ABI to use the DW_OP_entryvalue for the DWARF entries to reliably obtain the return buffer address stored in register r3 on entry to the function. These two patches fix the five testsuite failures in gdb.cp/non- trivial-retval.exp on PowerPC. Carl Love ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 0/2 version 2] PowerPC, fix support for printing the function return value for non-trivial values. 2022-10-06 16:36 ` [PATCH 0/2] PowerPC, fix support for printing the function return value for non-trivial values Carl Love @ 2022-10-18 18:55 ` Carl Love 0 siblings, 0 replies; 44+ messages in thread From: Carl Love @ 2022-10-18 18:55 UTC (permalink / raw) To: Ulrich Weigand, gdb-patches GDB maintainers: Version 2: The patch set has been updated per the review comments from Kevin Buettner and Bruno Larsen. Thank you to both for your help with the pathes. This patch set addresses the five test failures in gdb.cp/non-trivial- retval.exp on PowerPC. The failures started with commit b1718fcdd1d2a5c514f8ee504ba07fb3f42b8608 Author: Andrew Burgess <aburgess@redhat.com> Date: Mon Dec 13 16:56:16 2021 +0000 gdb: on x86-64 non-trivial C++ objects are returned in memory Fixes PR gdb/28681. It was observed that after using the `finish` command an incorrect value was displayed in some cases. Specifically, this behaviour was observed on an x86-64 target. The return value from the function is not correct due to multiple problems. The PowerPC specific code does not return the correct return value convention. This is fixed by the first patach. However due to the PowerPC ABI, GDB is still not able to reliably determine the function return value. The second patch addresses the issue of not being able to reliably determine the return buffer address on PowerPC. The patch adds a new GDB ABI to use the DW_OP_entryvalue for the DWARF entries to reliably obtain the return buffer address stored in register r3 on entry to the function. These two patches fix the five testsuite failures in gdb.cp/non- trivial-retval.exp on PowerPC. Carl Love ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 2/2] PowerPC, fix support for printing the function return value for non-trivial values. [not found] ` <5405a79ecd6ed34646ad77eed0779063ee222d37.camel@de.ibm.com> 2022-10-06 16:36 ` [PATCH 0/2] PowerPC, fix support for printing the function return value for non-trivial values Carl Love @ 2022-10-06 16:37 ` Carl Love 2022-10-08 4:36 ` Kevin Buettner ` (2 more replies) 1 sibling, 3 replies; 44+ messages in thread From: Carl Love @ 2022-10-06 16:37 UTC (permalink / raw) To: Ulrich Weigand, gdb-patches GDB maintainers: On PowerPC, a non-trivial return value for a function cannot be reliably determined with the current PowerPC ABI. The issue is the address of the return buffer for a non-trivial value is passed to the function in register r3. However, at the end of the function the ABI does not guarantee r3 will not be changed. Hence it cannot be reliably used to get the address of the return buffer. This patch adds a new GDB ABI to allow GDB to obtain the input value of r3 when the function returns. This is done by using the DW_OP_entry value for the DWARF entries to obtain the value of r3 on entry to the function. The ABI allows GDB to get the correct address for the return buffer on exit from the function. This patch fixes the five test failures in gdb.cp/non-trivial- retval.exp. The patch has been tested on PowerPC and X86-64 with no regression failures. Please let me know if this patch is acceptable for GDB mainline. Thanks. Carl Love -------------------------------------- PowerPC, fix support for printing the function return value for non-trivial values. Currently, a non-trivial return value from a function cannot currently be reliably determined on PowerPC. This is due to the fact that the PowerPC ABI uses register r3 to store the address of the buffer containing the non-trivial return value when the function is called. The PowerPC ABI does not guarantee the value in register r3 is not modified in the function. Thus the value in r3 cannot be reliably used to obtain the return addreses on exit from the function. This patch adds a new gdb ABI to allow PowerPC to access the value of r3 on entry to a function. The new gdb ABI attempts to use the DW_OP_entry value for the DWARF entries, when exiting the function, to determine the value of r3 on entry to the function. This requires the use of the -fvar-tracking compiler option to compile the user application thus generating the DW_OP_entry_value in the binary. The DW_OP_entries in the binary file enables GDB can resolve the DW_TAG_call_sites. This new API is used to get the return buffer address, in the case of a function returning a nontrivial data type, on exit from the function. The GDB function should_stop checks to see if RETURN_BUF is non-zero. By default, RETURN_BUF will be set to zero by the new ABI call for all architectures but PowerPC. The get_return_value function will be used to obtain the return value on these architectures as is currently being done if RETURN_BUF is zero. On PowerPC, the new ABI will return a nonzero address in RETURN_BUF if the value can be determined. The value_at function uses the return buffer address to get the return value. This patch fixes five testcase failures in gdb.cp/non-trivial-retval.exp. The correct function return values are now reported. Note this patch is dependent on patch: "PowerPC, function ppc64_sysv_abi_return_value add missing return value convention". This patch has been tested on Power 10 and x86-64 with no regressions. --- gdb/arch-utils.c | 6 +++ gdb/arch-utils.h | 4 ++ gdb/dwarf2/loc.c | 2 +- gdb/dwarf2/loc.h | 11 ++++++ gdb/gdbarch-components.py | 15 ++++++++ gdb/gdbarch-gen.h | 11 ++++++ gdb/gdbarch.c | 23 ++++++++++++ gdb/infcmd.c | 41 +++++++++++++++++++-- gdb/ppc-sysv-tdep.c | 37 +++++++++++++++++++ gdb/ppc-tdep.h | 2 + gdb/rs6000-tdep.c | 6 ++- gdb/testsuite/gdb.cp/non-trivial-retval.exp | 9 ++++- gdb/testsuite/lib/gdb.exp | 8 ++++ 13 files changed, 168 insertions(+), 7 deletions(-) diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c index 5943965bd75..12d6d1cdd75 100644 --- a/gdb/arch-utils.c +++ b/gdb/arch-utils.c @@ -1093,6 +1093,12 @@ default_read_core_file_mappings { } +CORE_ADDR +default_get_return_buf_addr (struct type *val_type, frame_info *cur_frame) +{ + return 0; +} + /* Non-zero if we want to trace architecture code. */ #ifndef GDBARCH_DEBUG diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h index f850e5fd6e7..a1944ff9c3c 100644 --- a/gdb/arch-utils.h +++ b/gdb/arch-utils.h @@ -300,4 +300,8 @@ extern void default_read_core_file_mappings struct bfd *cbfd, read_core_file_mappings_pre_loop_ftype pre_loop_cb, read_core_file_mappings_loop_ftype loop_cb); + +/* Default implementation of gdbarch default_get_return_buf_addr method. */ +extern CORE_ADDR default_get_return_buf_addr (struct type *val_typegdbarch, + frame_info *cur_frame); #endif /* ARCH_UTILS_H */ diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c index ad45d57a654..25a7499fe9b 100644 --- a/gdb/dwarf2/loc.c +++ b/gdb/dwarf2/loc.c @@ -1332,7 +1332,7 @@ static const struct lval_funcs entry_data_value_funcs = Function always returns non-NULL value. It throws NO_ENTRY_VALUE_ERROR if it cannot resolve the parameter for any reason. */ -static struct value * +extern struct value * value_of_dwarf_reg_entry (struct type *type, struct frame_info *frame, enum call_site_parameter_kind kind, union call_site_parameter_u kind_u) diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h index a9834d32066..f2cf93ba28e 100644 --- a/gdb/dwarf2/loc.h +++ b/gdb/dwarf2/loc.h @@ -296,4 +296,15 @@ extern struct value *indirect_synthetic_pointer dwarf2_per_objfile *per_objfile, struct frame_info *frame, struct type *type, bool resolve_abstract_p = false); +/* Read parameter of TYPE at (callee) FRAME's function entry. KIND and KIND_U + are used to match DW_AT_location at the caller's + DW_TAG_call_site_parameter. + + Function always returns non-NULL value. It throws NO_ENTRY_VALUE_ERROR if + it cannot resolve the parameter for any reason. */ + +extern struct value *value_of_dwarf_reg_entry (struct type *type, + struct frame_info *frame, + enum call_site_parameter_kind kind, + union call_site_parameter_u kind_u); #endif /* DWARF2LOC_H */ diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py index ad71bf754de..6c623d668c5 100644 --- a/gdb/gdbarch-components.py +++ b/gdb/gdbarch-components.py @@ -845,6 +845,21 @@ for instance). invalid=True, ) +Function( + comment=""" +Return the address at which the value being returned from +the current function will be stored. This routine is only +called if the current function uses the the "struct return +convention". + +May return 0 when unable to determine that address.""", + type="CORE_ADDR", + name="get_return_buf_addr", + params=[("struct type *", "val_type"), ("frame_info *", "cur_frame")], + predefault="default_get_return_buf_addr", + invalid=False, +) + Method( comment=""" Return true if the return value of function is stored in the first hidden diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h index c7a24704c7c..3e363b3a069 100644 --- a/gdb/gdbarch-gen.h +++ b/gdb/gdbarch-gen.h @@ -435,6 +435,17 @@ typedef enum return_value_convention (gdbarch_return_value_ftype) (struct gdbarc extern enum return_value_convention gdbarch_return_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf); extern void set_gdbarch_return_value (struct gdbarch *gdbarch, gdbarch_return_value_ftype *return_value); +/* Return the address at which the value being returned from + the current function will be stored. This routine is only + called if the current function uses the the "struct return + convention". + + May return 0 when unable to determine that address. */ + +typedef CORE_ADDR (gdbarch_get_return_buf_addr_ftype) (struct type *val_type, frame_info *cur_frame); +extern CORE_ADDR gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type *val_type, frame_info *cur_frame); +extern void set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, gdbarch_get_return_buf_addr_ftype *get_return_buf_addr); + /* Return true if the return value of function is stored in the first hidden parameter. In theory, this feature should be language-dependent, specified by language and its ABI, such as C++. Unfortunately, compiler may diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c index 18d46a39d7a..da3e2ad0e36 100644 --- a/gdb/gdbarch.c +++ b/gdb/gdbarch.c @@ -116,6 +116,7 @@ struct gdbarch gdbarch_address_to_pointer_ftype *address_to_pointer = nullptr; gdbarch_integer_to_address_ftype *integer_to_address = nullptr; gdbarch_return_value_ftype *return_value = nullptr; + gdbarch_get_return_buf_addr_ftype *get_return_buf_addr = nullptr; gdbarch_return_in_first_hidden_param_p_ftype *return_in_first_hidden_param_p = nullptr; gdbarch_skip_prologue_ftype *skip_prologue = nullptr; gdbarch_skip_main_prologue_ftype *skip_main_prologue = nullptr; @@ -313,6 +314,7 @@ gdbarch_alloc (const struct gdbarch_info *info, gdbarch->value_from_register = default_value_from_register; gdbarch->pointer_to_address = unsigned_pointer_to_address; gdbarch->address_to_pointer = unsigned_address_to_pointer; + gdbarch->get_return_buf_addr = default_get_return_buf_addr; gdbarch->return_in_first_hidden_param_p = default_return_in_first_hidden_param_p; gdbarch->breakpoint_from_pc = default_breakpoint_from_pc; gdbarch->sw_breakpoint_from_kind = NULL; @@ -465,6 +467,7 @@ verify_gdbarch (struct gdbarch *gdbarch) /* Skip verify of address_to_pointer, invalid_p == 0 */ /* Skip verify of integer_to_address, has predicate. */ /* Skip verify of return_value, has predicate. */ + /* Skip verify of get_return_buf_addr, invalid_p == 0 */ /* Skip verify of return_in_first_hidden_param_p, invalid_p == 0 */ if (gdbarch->skip_prologue == 0) log.puts ("\n\tskip_prologue"); @@ -877,6 +880,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file) gdb_printf (file, "gdbarch_dump: return_value = <%s>\n", host_address_to_string (gdbarch->return_value)); + gdb_printf (file, + "gdbarch_dump: get_return_buf_addr = <%s>\n", + host_address_to_string (gdbarch->get_return_buf_addr)); gdb_printf (file, "gdbarch_dump: return_in_first_hidden_param_p = <%s>\n", host_address_to_string (gdbarch->return_in_first_hidden_param_p)); @@ -2680,6 +2686,23 @@ set_gdbarch_return_value (struct gdbarch *gdbarch, gdbarch->return_value = return_value; } +CORE_ADDR +gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type *val_type, frame_info *cur_frame) +{ + gdb_assert (gdbarch != NULL); + gdb_assert (gdbarch->get_return_buf_addr != NULL); + if (gdbarch_debug >= 2) + gdb_printf (gdb_stdlog, "gdbarch_get_return_buf_addr called\n"); + return gdbarch->get_return_buf_addr (val_type, cur_frame); +} + +void +set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, + gdbarch_get_return_buf_addr_ftype get_return_buf_addr) +{ + gdbarch->get_return_buf_addr = get_return_buf_addr; +} + int gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch, struct type *type) { diff --git a/gdb/infcmd.c b/gdb/infcmd.c index b788f454432..7964baa7f92 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -55,6 +55,7 @@ #include "gdbsupport/gdb_optional.h" #include "source.h" #include "cli/cli-style.h" +#include "dwarf2/loc.h" /* Local functions: */ @@ -1598,6 +1599,12 @@ struct finish_command_fsm : public thread_fsm return value. */ struct return_value_info return_value_info {}; + /* If the current function uses the "struct return convention", + this holds the address at which the value being returned will + be stored, or zero if that address could not be determined or + the "struct return convention" is not being used. */ + CORE_ADDR return_buf; + explicit finish_command_fsm (struct interp *cmd_interp) : thread_fsm (cmd_interp) { @@ -1636,7 +1643,13 @@ finish_command_fsm::should_stop (struct thread_info *tp) struct value *func; func = read_var_value (function, NULL, get_current_frame ()); - rv->value = get_return_value (function, func); + + if (return_buf != 0) + /* Retrieve return value from the buffer where it was saved. */ + rv->value = value_at (rv->type, return_buf); + else + rv->value = get_return_value (function, func); + if (rv->value != NULL) rv->value_history_index = record_latest_value (rv->value); } @@ -1851,8 +1864,28 @@ finish_command (const char *arg, int from_tty) } /* Find the function we will return from. */ - - sm->function = find_pc_function (get_frame_pc (get_selected_frame (NULL))); + frame_info *callee_frame = get_selected_frame (NULL); + sm->function = find_pc_function (get_frame_pc (callee_frame)); + + /* Determine the return convention. If it is RETURN_VALUE_STRUCT_CONVENTION, + attempt to determine the address of the return buffer. */ + enum return_value_convention return_value; + struct gdbarch *gdbarch = get_frame_arch (callee_frame); + + struct type * val_type + = check_typedef (sm->function->type()->target_type ()); + + return_value = gdbarch_return_value (gdbarch, + read_var_value (sm->function, NULL, + callee_frame), + val_type, NULL, NULL, NULL); + + if (return_value == RETURN_VALUE_STRUCT_CONVENTION + && val_type->code() != TYPE_CODE_VOID) + sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type, + callee_frame); + else + sm->return_buf = 0; /* Print info on the selected frame, including level number but not source. */ @@ -1870,7 +1903,7 @@ finish_command (const char *arg, int from_tty) gdb_printf (_("Run till exit from ")); } - print_stack_frame (get_selected_frame (NULL), 1, LOCATION, 0); + print_stack_frame (callee_frame, 1, LOCATION, 0); } if (execution_direction == EXEC_REVERSE) diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c index 14effb93210..59145f9f166 100644 --- a/gdb/ppc-sysv-tdep.c +++ b/gdb/ppc-sysv-tdep.c @@ -28,6 +28,7 @@ #include "objfiles.h" #include "infcall.h" #include "dwarf2.h" +#include "dwarf2/loc.h" #include "target-float.h" #include <algorithm> @@ -2156,3 +2157,39 @@ ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, return RETURN_VALUE_STRUCT_CONVENTION; } +CORE_ADDR ppc64_sysv_get_return_buf_addr (struct type *val_type, + frame_info *cur_frame) +{ + /* On PowerPC, the ABI specifies r3 is the address where an argument + and the return value is stored. The PowerPC ABI does not guarantee + r3 will be preserved when the function returns. The function returns + the address of the buffer stored in r3. + + Attempt to determine the value of r3 on entry to the function using the + DW_OP_entry_value DWARF entries. This requires compiling the user + program with -fvar-tracking to resolve the DW_TAG_call_sites in the + binary file. */ + union call_site_parameter_u kind_u; + enum call_site_parameter_kind kind; + CORE_ADDR return_val = 0; + + kind_u.dwarf_reg = 3; /* First passed arg/return value is in r3. */ + kind = CALL_SITE_PARAMETER_DWARF_REG; + + /* val_type is the type of the return value. Need the pointer type + to the return value. */ + val_type = lookup_pointer_type (val_type); + + try + { + return_val = value_as_address (value_of_dwarf_reg_entry (val_type, + cur_frame, + kind, kind_u)); + } + catch (const gdb_exception_error &e) + { + warning ("Cannot determine the function return value.\n" + "Try compiling with -fvar-tracking."); + } + return return_val; +} diff --git a/gdb/ppc-tdep.h b/gdb/ppc-tdep.h index 44f63b145c6..299c87a169d 100644 --- a/gdb/ppc-tdep.h +++ b/gdb/ppc-tdep.h @@ -175,6 +175,8 @@ extern void ppc_collect_vsxregset (const struct regset *regset, const struct regcache *regcache, int regnum, void *vsxregs, size_t len); +extern CORE_ADDR ppc64_sysv_get_return_buf_addr (type*, frame_info*); + /* Private data that this module attaches to struct gdbarch. */ /* ELF ABI version used by the inferior. */ diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c index 87b2faad36a..0a4f4458845 100644 --- a/gdb/rs6000-tdep.c +++ b/gdb/rs6000-tdep.c @@ -8235,7 +8235,11 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) set_gdbarch_ps_regnum (gdbarch, tdep->ppc_ps_regnum); if (wordsize == 8) - set_gdbarch_return_value (gdbarch, ppc64_sysv_abi_return_value); + { + set_gdbarch_return_value (gdbarch, ppc64_sysv_abi_return_value); + set_gdbarch_get_return_buf_addr (gdbarch, + ppc64_sysv_get_return_buf_addr); + } else set_gdbarch_return_value (gdbarch, ppc_sysv_abi_return_value); diff --git a/gdb/testsuite/gdb.cp/non-trivial-retval.exp b/gdb/testsuite/gdb.cp/non-trivial-retval.exp index 21c390bc937..93a3a6832f7 100644 --- a/gdb/testsuite/gdb.cp/non-trivial-retval.exp +++ b/gdb/testsuite/gdb.cp/non-trivial-retval.exp @@ -15,11 +15,18 @@ # This file is part of the gdb testsuite +set additional_flags "" + if {[skip_cplus_tests]} { continue } standard_testfile .cc -if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} { +if {[have_fvar_tracking]} { + set additional_flags "additional_flags= -fvar-tracking" +} + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile [list debug c++ $additional_flags]]} { + return -1 } diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index 5ab4df1bcf3..a9ebc11dfea 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -8468,6 +8468,14 @@ gdb_caching_proc have_fuse_ld_gold { return [gdb_simple_compile $me $src executable $flags] } +# Return 1 if compiler supports fvar-tracking, otherwise return 0. +gdb_caching_proc have_fvar_tracking { + set me "have_fvar_tracking" + set flags "additional_flags=-fvar-tracking" + set src { int main() { return 0; } } + return [gdb_simple_compile $me $src executable $flags] +} + # Return 1 if linker supports -Ttext-segment, otherwise return 0. gdb_caching_proc linker_supports_Ttext_segment_flag { set me "linker_supports_Ttext_segment_flag" -- 2.31.1 ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/2] PowerPC, fix support for printing the function return value for non-trivial values. 2022-10-06 16:37 ` [PATCH 2/2] " Carl Love @ 2022-10-08 4:36 ` Kevin Buettner 2022-10-12 17:01 ` Carl Love 2022-10-14 2:49 ` Kevin Buettner 2022-10-18 18:55 ` [PATCH 2/2 ver 2] " Carl Love 2 siblings, 1 reply; 44+ messages in thread From: Kevin Buettner @ 2022-10-08 4:36 UTC (permalink / raw) To: Carl Love via Gdb-patches; +Cc: Carl Love, Ulrich Weigand Hi Carl, I'm still working through the patch, but I noticed what I believe to be incorrect terminology while reading the intro / commit log... On Thu, 06 Oct 2022 09:37:40 -0700 Carl Love via Gdb-patches <gdb-patches@sourceware.org> wrote: ... > This patch adds a new gdb ABI to allow PowerPC to access the value of r3 > on entry to a function. The new gdb ABI attempts to use the DW_OP_entry ... Rather than saying "gdb ABI", I think you meant to say "gdbarch method". Also, I'd say something like "On PowerPC, the new gdbarch method attempts to use DW_OP_entry...". (I.e., this new method doesn't attempt to access r3 on all architectures.) Kevin ^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH 2/2] PowerPC, fix support for printing the function return value for non-trivial values. 2022-10-08 4:36 ` Kevin Buettner @ 2022-10-12 17:01 ` Carl Love 0 siblings, 0 replies; 44+ messages in thread From: Carl Love @ 2022-10-12 17:01 UTC (permalink / raw) To: Kevin Buettner, gdb-patches; +Cc: Ulrich Weigand, Will Schmidt, cel Kevin: On Fri, 2022-10-07 at 21:36 -0700, Kevin Buettner wrote: > Hi Carl, > > I'm still working through the patch, but I noticed what I believe > to be incorrect terminology while reading the intro / commit log... > > On Thu, 06 Oct 2022 09:37:40 -0700 > Carl Love via Gdb-patches <gdb-patches@sourceware.org> wrote: > > ... > > This patch adds a new gdb ABI to allow PowerPC to access the value > > of r3 > > on entry to a function. The new gdb ABI attempts to use the > > DW_OP_entry > ... > > Rather than saying "gdb ABI", I think you meant to say "gdbarch > method". > > Also, I'd say something like "On PowerPC, the new gdbarch method > attempts > to use DW_OP_entry...". (I.e., this new method doesn't attempt to > access r3 > on all architectures.) Thanks for the feedback on the terminology. I was wondering if you had finished working thru the patch? Do you have any additional thoughts on the patch, particularly on the functional part of the patch before I post an updated version of the patch. Thanks for looking at the patch. Carl Love ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/2] PowerPC, fix support for printing the function return value for non-trivial values. 2022-10-06 16:37 ` [PATCH 2/2] " Carl Love 2022-10-08 4:36 ` Kevin Buettner @ 2022-10-14 2:49 ` Kevin Buettner 2022-10-14 7:36 ` Bruno Larsen 2022-10-14 23:23 ` Carl Love 2022-10-18 18:55 ` [PATCH 2/2 ver 2] " Carl Love 2 siblings, 2 replies; 44+ messages in thread From: Kevin Buettner @ 2022-10-14 2:49 UTC (permalink / raw) To: Carl Love via Gdb-patches; +Cc: Carl Love, Ulrich Weigand Hi Carl, While running "git am" to apply your patch, I ran into the following problems... 1) The patch didn't apply cleanly due to some changes in gdb/dwarf2/loc.c. It seemed to me that it was likely due to this commit: bd2b40ac129 Change GDB to use frame_info_ptr After checking out bd2b40ac129~, I was able to apply your patch via git am, though I did see some whitespace complaints... 2) Whitespace complaints: Applying: PowerPC, fix support for printing the function return value for non-trivial values. ...patch:161: indent with spaces. "gdbarch_dump: get_return_buf_addr = <%s>\n", ...patch:162: indent with spaces. host_address_to_string (gdbarch->get_return_buf_addr)); ...patch:182: indent with spaces. gdbarch_get_return_buf_addr_ftype get_return_buf_addr) warning: 3 lines add whitespace errors. (I substituted some long paths local to my machines w/ '...'.) See my remaining remarks inline below... Kevin On Thu, 06 Oct 2022 09:37:40 -0700 Carl Love via Gdb-patches <gdb-patches@sourceware.org> wrote: > PowerPC, fix support for printing the function return value for non-trivial values. > > Currently, a non-trivial return value from a function cannot currently be > reliably determined on PowerPC. This is due to the fact that the PowerPC > ABI uses register r3 to store the address of the buffer containing the > non-trivial return value when the function is called. The PowerPC ABI > does not guarantee the value in register r3 is not modified in the > function. Thus the value in r3 cannot be reliably used to obtain the > return addreses on exit from the function. > > This patch adds a new gdb ABI to allow PowerPC to access the value of r3 I already mentioned this last week, but... s/GDB ABI/gdbarch method/ I guess you could also refer to it as an API which you did later on in your commit remarks. (What I really object to is calling it an "ABI", which it definitely is not - that said it's easy to mis-type API as ABI, so I do understand this mistake.) > on entry to a function. The new gdb ABI attempts to use the DW_OP_entry s/The new/On PowerPC, the new/ s/GDB ABI/gdbarch method/ > value for the DWARF entries, when exiting the function, to determine the Missing '_' between "DW_OP_entry" and "value". I.e. it's "DW_OP_entry_value", not "DW_OP_entry value". > value of r3 on entry to the function. This requires the use of the > -fvar-tracking compiler option to compile the user application thus > generating the DW_OP_entry_value in the binary. The DW_OP_entries in the I don't think that "DW_OP_entries" is correct here. Maybe "DW_OP_entry_value entries"? > binary file enables GDB can resolve the DW_TAG_call_sites. This new API Maybe instead of "enables GDB can resolve the DW_TAG_call_sites", say "allow GDB to resolve the DW_TAG_call_site entries". > is used to get the return buffer address, in the case of a function > returning a nontrivial data type, on exit from the function. The GDB > function should_stop checks to see if RETURN_BUF is non-zero. By default, > RETURN_BUF will be set to zero by the new ABI call for all architectures s/ABI/API/ > but PowerPC. The get_return_value function will be used to obtain the > return value on these architectures as is currently being done if > RETURN_BUF is zero. On PowerPC, the new ABI will return a nonzero address > in RETURN_BUF if the value can be determined. The value_at function uses > the return buffer address to get the return value. > > This patch fixes five testcase failures in gdb.cp/non-trivial-retval.exp. > The correct function return values are now reported. > > Note this patch is dependent on patch: "PowerPC, function > ppc64_sysv_abi_return_value add missing return value convention". > > This patch has been tested on Power 10 and x86-64 with no regressions. > --- > gdb/arch-utils.c | 6 +++ > gdb/arch-utils.h | 4 ++ > gdb/dwarf2/loc.c | 2 +- > gdb/dwarf2/loc.h | 11 ++++++ > gdb/gdbarch-components.py | 15 ++++++++ > gdb/gdbarch-gen.h | 11 ++++++ > gdb/gdbarch.c | 23 ++++++++++++ > gdb/infcmd.c | 41 +++++++++++++++++++-- > gdb/ppc-sysv-tdep.c | 37 +++++++++++++++++++ > gdb/ppc-tdep.h | 2 + > gdb/rs6000-tdep.c | 6 ++- > gdb/testsuite/gdb.cp/non-trivial-retval.exp | 9 ++++- > gdb/testsuite/lib/gdb.exp | 8 ++++ > 13 files changed, 168 insertions(+), 7 deletions(-) > > diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c > index 5943965bd75..12d6d1cdd75 100644 > --- a/gdb/arch-utils.c > +++ b/gdb/arch-utils.c > @@ -1093,6 +1093,12 @@ default_read_core_file_mappings > { > } > > +CORE_ADDR > +default_get_return_buf_addr (struct type *val_type, frame_info *cur_frame) > +{ > + return 0; > +} > + > /* Non-zero if we want to trace architecture code. */ > > #ifndef GDBARCH_DEBUG > diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h > index f850e5fd6e7..a1944ff9c3c 100644 > --- a/gdb/arch-utils.h > +++ b/gdb/arch-utils.h > @@ -300,4 +300,8 @@ extern void default_read_core_file_mappings > struct bfd *cbfd, > read_core_file_mappings_pre_loop_ftype pre_loop_cb, > read_core_file_mappings_loop_ftype loop_cb); > + > +/* Default implementation of gdbarch default_get_return_buf_addr method. */ > +extern CORE_ADDR default_get_return_buf_addr (struct type *val_typegdbarch, > + frame_info *cur_frame); > #endif /* ARCH_UTILS_H */ > diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c > index ad45d57a654..25a7499fe9b 100644 > --- a/gdb/dwarf2/loc.c > +++ b/gdb/dwarf2/loc.c > @@ -1332,7 +1332,7 @@ static const struct lval_funcs entry_data_value_funcs = > Function always returns non-NULL value. It throws NO_ENTRY_VALUE_ERROR if it > cannot resolve the parameter for any reason. */ It appears that the doc above was moved to dwarf2/loc.h. I think it might be better to remove the above comment and replace it with "/* See dwarf2/loc.h. */". > > -static struct value * > +extern struct value * Remove 'extern ' here. > value_of_dwarf_reg_entry (struct type *type, struct frame_info *frame, > enum call_site_parameter_kind kind, > union call_site_parameter_u kind_u) > diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h > index a9834d32066..f2cf93ba28e 100644 > --- a/gdb/dwarf2/loc.h > +++ b/gdb/dwarf2/loc.h > @@ -296,4 +296,15 @@ extern struct value *indirect_synthetic_pointer > dwarf2_per_objfile *per_objfile, struct frame_info *frame, > struct type *type, bool resolve_abstract_p = false); > > +/* Read parameter of TYPE at (callee) FRAME's function entry. KIND and KIND_U > + are used to match DW_AT_location at the caller's > + DW_TAG_call_site_parameter. > + > + Function always returns non-NULL value. It throws NO_ENTRY_VALUE_ERROR if > + it cannot resolve the parameter for any reason. */ > + > +extern struct value *value_of_dwarf_reg_entry (struct type *type, > + struct frame_info *frame, > + enum call_site_parameter_kind kind, > + union call_site_parameter_u kind_u); > #endif /* DWARF2LOC_H */ > diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py > index ad71bf754de..6c623d668c5 100644 > --- a/gdb/gdbarch-components.py > +++ b/gdb/gdbarch-components.py > @@ -845,6 +845,21 @@ for instance). > invalid=True, > ) > > +Function( > + comment=""" > +Return the address at which the value being returned from > +the current function will be stored. This routine is only > +called if the current function uses the the "struct return > +convention". > + > +May return 0 when unable to determine that address.""", > + type="CORE_ADDR", > + name="get_return_buf_addr", > + params=[("struct type *", "val_type"), ("frame_info *", "cur_frame")], > + predefault="default_get_return_buf_addr", > + invalid=False, > +) > + > Method( > comment=""" > Return true if the return value of function is stored in the first hidden > diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h > index c7a24704c7c..3e363b3a069 100644 > --- a/gdb/gdbarch-gen.h > +++ b/gdb/gdbarch-gen.h > @@ -435,6 +435,17 @@ typedef enum return_value_convention (gdbarch_return_value_ftype) (struct gdbarc > extern enum return_value_convention gdbarch_return_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf); > extern void set_gdbarch_return_value (struct gdbarch *gdbarch, gdbarch_return_value_ftype *return_value); > > +/* Return the address at which the value being returned from > + the current function will be stored. This routine is only > + called if the current function uses the the "struct return > + convention". > + > + May return 0 when unable to determine that address. */ > + > +typedef CORE_ADDR (gdbarch_get_return_buf_addr_ftype) (struct type *val_type, frame_info *cur_frame); > +extern CORE_ADDR gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type *val_type, frame_info *cur_frame); > +extern void set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, gdbarch_get_return_buf_addr_ftype *get_return_buf_addr); > + > /* Return true if the return value of function is stored in the first hidden > parameter. In theory, this feature should be language-dependent, specified > by language and its ABI, such as C++. Unfortunately, compiler may > diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c > index 18d46a39d7a..da3e2ad0e36 100644 > --- a/gdb/gdbarch.c > +++ b/gdb/gdbarch.c > @@ -116,6 +116,7 @@ struct gdbarch > gdbarch_address_to_pointer_ftype *address_to_pointer = nullptr; > gdbarch_integer_to_address_ftype *integer_to_address = nullptr; > gdbarch_return_value_ftype *return_value = nullptr; > + gdbarch_get_return_buf_addr_ftype *get_return_buf_addr = nullptr; > gdbarch_return_in_first_hidden_param_p_ftype *return_in_first_hidden_param_p = nullptr; > gdbarch_skip_prologue_ftype *skip_prologue = nullptr; > gdbarch_skip_main_prologue_ftype *skip_main_prologue = nullptr; > @@ -313,6 +314,7 @@ gdbarch_alloc (const struct gdbarch_info *info, > gdbarch->value_from_register = default_value_from_register; > gdbarch->pointer_to_address = unsigned_pointer_to_address; > gdbarch->address_to_pointer = unsigned_address_to_pointer; > + gdbarch->get_return_buf_addr = default_get_return_buf_addr; > gdbarch->return_in_first_hidden_param_p = default_return_in_first_hidden_param_p; > gdbarch->breakpoint_from_pc = default_breakpoint_from_pc; > gdbarch->sw_breakpoint_from_kind = NULL; > @@ -465,6 +467,7 @@ verify_gdbarch (struct gdbarch *gdbarch) > /* Skip verify of address_to_pointer, invalid_p == 0 */ > /* Skip verify of integer_to_address, has predicate. */ > /* Skip verify of return_value, has predicate. */ > + /* Skip verify of get_return_buf_addr, invalid_p == 0 */ > /* Skip verify of return_in_first_hidden_param_p, invalid_p == 0 */ > if (gdbarch->skip_prologue == 0) > log.puts ("\n\tskip_prologue"); > @@ -877,6 +880,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file) > gdb_printf (file, > "gdbarch_dump: return_value = <%s>\n", > host_address_to_string (gdbarch->return_value)); > + gdb_printf (file, > + "gdbarch_dump: get_return_buf_addr = <%s>\n", > + host_address_to_string (gdbarch->get_return_buf_addr)); > gdb_printf (file, > "gdbarch_dump: return_in_first_hidden_param_p = <%s>\n", > host_address_to_string (gdbarch->return_in_first_hidden_param_p)); > @@ -2680,6 +2686,23 @@ set_gdbarch_return_value (struct gdbarch *gdbarch, > gdbarch->return_value = return_value; > } > > +CORE_ADDR > +gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type *val_type, frame_info *cur_frame) > +{ > + gdb_assert (gdbarch != NULL); > + gdb_assert (gdbarch->get_return_buf_addr != NULL); > + if (gdbarch_debug >= 2) > + gdb_printf (gdb_stdlog, "gdbarch_get_return_buf_addr called\n"); > + return gdbarch->get_return_buf_addr (val_type, cur_frame); > +} > + > +void > +set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, > + gdbarch_get_return_buf_addr_ftype get_return_buf_addr) > +{ > + gdbarch->get_return_buf_addr = get_return_buf_addr; > +} > + > int > gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch, struct type *type) > { > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index b788f454432..7964baa7f92 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -55,6 +55,7 @@ > #include "gdbsupport/gdb_optional.h" > #include "source.h" > #include "cli/cli-style.h" > +#include "dwarf2/loc.h" > > /* Local functions: */ > > @@ -1598,6 +1599,12 @@ struct finish_command_fsm : public thread_fsm > return value. */ > struct return_value_info return_value_info {}; > > + /* If the current function uses the "struct return convention", > + this holds the address at which the value being returned will > + be stored, or zero if that address could not be determined or > + the "struct return convention" is not being used. */ > + CORE_ADDR return_buf; > + > explicit finish_command_fsm (struct interp *cmd_interp) > : thread_fsm (cmd_interp) > { > @@ -1636,7 +1643,13 @@ finish_command_fsm::should_stop (struct thread_info *tp) > struct value *func; > > func = read_var_value (function, NULL, get_current_frame ()); > - rv->value = get_return_value (function, func); > + > + if (return_buf != 0) > + /* Retrieve return value from the buffer where it was saved. */ > + rv->value = value_at (rv->type, return_buf); > + else > + rv->value = get_return_value (function, func); > + > if (rv->value != NULL) > rv->value_history_index = record_latest_value (rv->value); > } > @@ -1851,8 +1864,28 @@ finish_command (const char *arg, int from_tty) > } > > /* Find the function we will return from. */ > - > - sm->function = find_pc_function (get_frame_pc (get_selected_frame (NULL))); > + frame_info *callee_frame = get_selected_frame (NULL); > + sm->function = find_pc_function (get_frame_pc (callee_frame)); > + > + /* Determine the return convention. If it is RETURN_VALUE_STRUCT_CONVENTION, > + attempt to determine the address of the return buffer. */ > + enum return_value_convention return_value; > + struct gdbarch *gdbarch = get_frame_arch (callee_frame); > + > + struct type * val_type > + = check_typedef (sm->function->type()->target_type ()); > + > + return_value = gdbarch_return_value (gdbarch, > + read_var_value (sm->function, NULL, > + callee_frame), > + val_type, NULL, NULL, NULL); > + > + if (return_value == RETURN_VALUE_STRUCT_CONVENTION > + && val_type->code() != TYPE_CODE_VOID) > + sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type, > + callee_frame); > + else > + sm->return_buf = 0; > > /* Print info on the selected frame, including level number but not > source. */ > @@ -1870,7 +1903,7 @@ finish_command (const char *arg, int from_tty) > gdb_printf (_("Run till exit from ")); > } > > - print_stack_frame (get_selected_frame (NULL), 1, LOCATION, 0); > + print_stack_frame (callee_frame, 1, LOCATION, 0); > } > > if (execution_direction == EXEC_REVERSE) > diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c > index 14effb93210..59145f9f166 100644 > --- a/gdb/ppc-sysv-tdep.c > +++ b/gdb/ppc-sysv-tdep.c > @@ -28,6 +28,7 @@ > #include "objfiles.h" > #include "infcall.h" > #include "dwarf2.h" > +#include "dwarf2/loc.h" > #include "target-float.h" > #include <algorithm> > > @@ -2156,3 +2157,39 @@ ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, > return RETURN_VALUE_STRUCT_CONVENTION; > } > > +CORE_ADDR ppc64_sysv_get_return_buf_addr (struct type *val_type, > + frame_info *cur_frame) > +{ > + /* On PowerPC, the ABI specifies r3 is the address where an argument s/specifies/specifies that/ I'm not sure about this phrase: "where an argument and". If you mean to say that r3 is sometimes used for arguments (when it's not used for the address of the return value buffer), then that's true, but it's not relevant here. So, I guess that I'm asking that this comment be reworded somewhat. > + and the return value is stored. The PowerPC ABI does not guarantee > + r3 will be preserved when the function returns. The function returns > + the address of the buffer stored in r3. I find this part confusing. If the function returns the address of the buffer, doesn't it do so by returning it in r3? (And, if so, it seems to me that this code would be unnecessary - so, clearly, I must be missing something...) > + > + Attempt to determine the value of r3 on entry to the function using the > + DW_OP_entry_value DWARF entries. This requires compiling the user > + program with -fvar-tracking to resolve the DW_TAG_call_sites in the > + binary file. */ > + union call_site_parameter_u kind_u; > + enum call_site_parameter_kind kind; > + CORE_ADDR return_val = 0; > + > + kind_u.dwarf_reg = 3; /* First passed arg/return value is in r3. */ > + kind = CALL_SITE_PARAMETER_DWARF_REG; > + > + /* val_type is the type of the return value. Need the pointer type > + to the return value. */ > + val_type = lookup_pointer_type (val_type); > + > + try > + { > + return_val = value_as_address (value_of_dwarf_reg_entry (val_type, > + cur_frame, > + kind, kind_u)); > + } > + catch (const gdb_exception_error &e) > + { > + warning ("Cannot determine the function return value.\n" > + "Try compiling with -fvar-tracking."); Will this warning be printed a lot? (I'm wondering if there should be a way to shut it off...) > + } > + return return_val; > +} > diff --git a/gdb/ppc-tdep.h b/gdb/ppc-tdep.h > index 44f63b145c6..299c87a169d 100644 > --- a/gdb/ppc-tdep.h > +++ b/gdb/ppc-tdep.h > @@ -175,6 +175,8 @@ extern void ppc_collect_vsxregset (const struct regset *regset, > const struct regcache *regcache, > int regnum, void *vsxregs, size_t len); > > +extern CORE_ADDR ppc64_sysv_get_return_buf_addr (type*, frame_info*); > + > /* Private data that this module attaches to struct gdbarch. */ > > /* ELF ABI version used by the inferior. */ > diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c > index 87b2faad36a..0a4f4458845 100644 > --- a/gdb/rs6000-tdep.c > +++ b/gdb/rs6000-tdep.c > @@ -8235,7 +8235,11 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) > set_gdbarch_ps_regnum (gdbarch, tdep->ppc_ps_regnum); > > if (wordsize == 8) > - set_gdbarch_return_value (gdbarch, ppc64_sysv_abi_return_value); > + { > + set_gdbarch_return_value (gdbarch, ppc64_sysv_abi_return_value); > + set_gdbarch_get_return_buf_addr (gdbarch, > + ppc64_sysv_get_return_buf_addr); > + } > else > set_gdbarch_return_value (gdbarch, ppc_sysv_abi_return_value); > > diff --git a/gdb/testsuite/gdb.cp/non-trivial-retval.exp b/gdb/testsuite/gdb.cp/non-trivial-retval.exp > index 21c390bc937..93a3a6832f7 100644 > --- a/gdb/testsuite/gdb.cp/non-trivial-retval.exp > +++ b/gdb/testsuite/gdb.cp/non-trivial-retval.exp > @@ -15,11 +15,18 @@ > > # This file is part of the gdb testsuite > > +set additional_flags "" > + > if {[skip_cplus_tests]} { continue } > > standard_testfile .cc > > -if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} { > +if {[have_fvar_tracking]} { > + set additional_flags "additional_flags= -fvar-tracking" > +} > + > +if {[prepare_for_testing "failed to prepare" $testfile $srcfile [list debug c++ $additional_flags]]} { > + > return -1 > } > > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp > index 5ab4df1bcf3..a9ebc11dfea 100644 > --- a/gdb/testsuite/lib/gdb.exp > +++ b/gdb/testsuite/lib/gdb.exp > @@ -8468,6 +8468,14 @@ gdb_caching_proc have_fuse_ld_gold { > return [gdb_simple_compile $me $src executable $flags] > } > > +# Return 1 if compiler supports fvar-tracking, otherwise return 0. > +gdb_caching_proc have_fvar_tracking { > + set me "have_fvar_tracking" > + set flags "additional_flags=-fvar-tracking" > + set src { int main() { return 0; } } > + return [gdb_simple_compile $me $src executable $flags] > +} > + > # Return 1 if linker supports -Ttext-segment, otherwise return 0. > gdb_caching_proc linker_supports_Ttext_segment_flag { > set me "linker_supports_Ttext_segment_flag" > -- > 2.31.1 > > ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/2] PowerPC, fix support for printing the function return value for non-trivial values. 2022-10-14 2:49 ` Kevin Buettner @ 2022-10-14 7:36 ` Bruno Larsen 2022-10-14 23:25 ` Carl Love 2022-10-14 23:23 ` Carl Love 1 sibling, 1 reply; 44+ messages in thread From: Bruno Larsen @ 2022-10-14 7:36 UTC (permalink / raw) To: Kevin Buettner, Carl Love via Gdb-patches; +Cc: Ulrich Weigand On 14/10/2022 04:49, Kevin Buettner via Gdb-patches wrote: > Hi Carl, > > While running "git am" to apply your patch, I ran into the following > problems... > > 1) The patch didn't apply cleanly due to some changes in gdb/dwarf2/loc.c. > It seemed to me that it was likely due to this commit: > > bd2b40ac129 Change GDB to use frame_info_ptr To expand on this, wherever you used frame_info *, you should switch to frame_info_ptr, unless you are dealing with guile. As far as I could see in your patch, that simple change should be enough. Cheers, Bruno ^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH 2/2] PowerPC, fix support for printing the function return value for non-trivial values. 2022-10-14 7:36 ` Bruno Larsen @ 2022-10-14 23:25 ` Carl Love 0 siblings, 0 replies; 44+ messages in thread From: Carl Love @ 2022-10-14 23:25 UTC (permalink / raw) To: Bruno Larsen, Kevin Buettner, Carl Love via Gdb-patches Cc: Ulrich Weigand, cel On Fri, 2022-10-14 at 09:36 +0200, Bruno Larsen via Gdb-patches wrote: > On 14/10/2022 04:49, Kevin Buettner via Gdb-patches wrote: > > Hi Carl, > > > > While running "git am" to apply your patch, I ran into the > > following > > problems... > > > > 1) The patch didn't apply cleanly due to some changes in > > gdb/dwarf2/loc.c. > > It seemed to me that it was likely due to this commit: > > > > bd2b40ac129 Change GDB to use frame_info_ptr > To expand on this, wherever you used frame_info *, you should switch > to > frame_info_ptr, unless you are dealing with guile. As far as I could > see > in your patch, that simple change should be enough. Yup, made the change you mentioned. Sorry, I forgot to explicitly add you to my reply to Kevin. Carl ^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH 2/2] PowerPC, fix support for printing the function return value for non-trivial values. 2022-10-14 2:49 ` Kevin Buettner 2022-10-14 7:36 ` Bruno Larsen @ 2022-10-14 23:23 ` Carl Love 2022-10-18 1:06 ` Kevin Buettner 1 sibling, 1 reply; 44+ messages in thread From: Carl Love @ 2022-10-14 23:23 UTC (permalink / raw) To: Kevin Buettner, gdb-patches; +Cc: Ulrich Weigand, cel On Thu, 2022-10-13 at 19:49 -0700, Kevin Buettner wrote: > Hi Carl, > > While running "git am" to apply your patch, I ran into the following > problems... > > 1) The patch didn't apply cleanly due to some changes in > gdb/dwarf2/loc.c. > It seemed to me that it was likely due to this commit: > > bd2b40ac129 Change GDB to use frame_info_ptr > > After checking out bd2b40ac129~, I was able to apply your patch > via git am, though I did see some whitespace complaints... Yes, mainline has moved a bit. I refreshed the patch. I made the change mentioned by Bruno c/frame_info */frame_info_ptr/. I was then able to get gdb to compile. > > 2) Whitespace complaints: > > Applying: PowerPC, fix support for printing the function return value > for non-trivial values. > ...patch:161: indent with spaces. > "gdbarch_dump: get_return_buf_addr = <%s>\n", > ...patch:162: indent with spaces. > host_address_to_string (gdbarch- > >get_return_buf_addr)); > ...patch:182: indent with spaces. > gdbarch_get_return_buf_addr_ftype > get_return_buf_addr) > warning: 3 lines add whitespace errors. > > (I substituted some long paths local to my machines w/ '...'.) Yes, I have seen the complaints about the white spaces. The whitespace complaints come from file gdb/gdbarch.c which is a generated file. As I assume you know, the file was regenerated by the gdb/gdbarch.py based on the changes to gdb/gdbarch-components.py. The regenerated file is set to read only. Not sure what I can do to rectify the white spaces? Any ideas? > <snip> > I already mentioned this last week, but... > > s/GDB ABI/gdbarch method/ > > I guess you could also refer to it as an API which you did later on > in your commit remarks. (What I really object to is calling it an > "ABI", which it definitely is not - that said it's easy to mis-type > API > as ABI, so I do understand this mistake.) Yea, I think referring to it as a new gdbarch method is probably better than GDB API/ABI. I have changed the GDB API/ABI references to gdbarch method. I will need to update that in the message to the GDB maintainers as well when I send the next version of the patch. > > > on entry to a function. The new gdb ABI attempts to use the > > DW_OP_entry > > s/The new/On PowerPC, the new/ > s/GDB ABI/gdbarch method/ > > > value for the DWARF entries, when exiting the function, to > > determine the > > Missing '_' between "DW_OP_entry" and "value". I.e. it's > "DW_OP_entry_value", > not "DW_OP_entry value". Fixed. > > > value of r3 on entry to the function. This requires the use of the > > -fvar-tracking compiler option to compile the user application thus > > generating the DW_OP_entry_value in the binary. The DW_OP_entries > > in the > > I don't think that "DW_OP_entries" is correct here. Maybe > "DW_OP_entry_value > entries"? > > binary file enables GDB can resolve the DW_TAG_call_sites. This > > new API > > Maybe instead of "enables GDB can resolve the DW_TAG_call_sites", say > "allow GDB to resolve the DW_TAG_call_site entries". > > > > is used to get the return buffer address, in the case of a function > > returning a nontrivial data type, on exit from the function. The > > GDB > > function should_stop checks to see if RETURN_BUF is non-zero. By > > default, > > RETURN_BUF will be set to zero by the new ABI call for all > > architectures > > s/ABI/API/ > > > but PowerPC. The get_return_value function will be used to obtain > > the > > return value on these architectures as is currently being done if > > RETURN_BUF is zero. On PowerPC, the new ABI will return a nonzero > > address > > in RETURN_BUF if the value can be determined. The value_at > > function uses > > the return buffer address to get the return value. So, I tried to address all of the above comments about ABI, "DW_OP_entries" etc. The paragraph now reads: This patch adds a new gdbarch method to allow PowerPC to access the value of r3 on entry to a function. On PowerPC, the new gdb arch method attempts to use the DW_OP_entry_value for the DWARF entries, when exiting the function, to determine the value of r3 on entry to the function. This requires the use of the -fvar-tracking compiler option to compile the user application thus generating the DW_OP_entry_value in the binary. The DW_OP_entry_value entries in the binary file allows GDB to resolve the DW_TAG_call_site entries. This new gdbarch method is used to get the return buffer address, in the case of a function returning a nontrivial data type, on exit from the function. The GDB function should_stop checks to see if RETURN_BUF is non-zero. By default, RETURN_BUF will be set to zero by the new gdbarch method call for all architectures except PowerPC. The get_return_value function will be used to obtain the return value on all other architectures as is currently being done if RETURN_BUF is zero. On PowerPC, the new gdbarch method will return a nonzero address in RETURN_BUF if the value can be determined. The value_at function uses the return buffer address to get the return value. Which I think covers all of the comments above. > > > > This patch fixes five testcase failures in gdb.cp/non-trivial- > > retval.exp. > > < snip > > > frame_info *cur_frame); > > #endif /* ARCH_UTILS_H */ > > diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c > > index ad45d57a654..25a7499fe9b 100644 > > --- a/gdb/dwarf2/loc.c > > +++ b/gdb/dwarf2/loc.c > > @@ -1332,7 +1332,7 @@ static const struct lval_funcs > > entry_data_value_funcs = > > Function always returns non-NULL value. It throws > > NO_ENTRY_VALUE_ERROR if it > > cannot resolve the parameter for any reason. */ > > It appears that the doc above was moved to dwarf2/loc.h. I think it > might be > better to remove the above comment and replace it with "/* See > dwarf2/loc.h. */". > Fixed. > > > > -static struct value * > > +extern struct value * > > Remove 'extern ' here. Fixed. > <snip> > > if (execution_direction == EXEC_REVERSE) > > diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c > > index 14effb93210..59145f9f166 100644 > > --- a/gdb/ppc-sysv-tdep.c > > +++ b/gdb/ppc-sysv-tdep.c > > @@ -28,6 +28,7 @@ > > #include "objfiles.h" > > #include "infcall.h" > > #include "dwarf2.h" > > +#include "dwarf2/loc.h" > > #include "target-float.h" > > #include <algorithm> > > > > @@ -2156,3 +2157,39 @@ ppc64_sysv_abi_return_value (struct gdbarch > > *gdbarch, struct value *function, > > return RETURN_VALUE_STRUCT_CONVENTION; > > } > > > > +CORE_ADDR ppc64_sysv_get_return_buf_addr (struct type *val_type, > > + frame_info *cur_frame) > > +{ > > + /* On PowerPC, the ABI specifies r3 is the address where an > > argument > > s/specifies/specifies that/ > > I'm not sure about this phrase: "where an argument and". If you > mean > to say that r3 is sometimes used for arguments (when it's not used > for > the address of the return value buffer), then that's true, but it's > not relevant here. > > So, I guess that I'm asking that this comment be reworded somewhat. > > > + and the return value is stored. The PowerPC ABI does not > > guarantee > > + r3 will be preserved when the function returns. The function > > returns > > + the address of the buffer stored in r3. > > I find this part confusing. If the function returns the address of > the buffer, doesn't it do so by returning it in r3? (And, if so, it > seems to me that this code would be unnecessary - so, clearly, I must > be missing something...) > > + > > + Attempt to determine the value of r3 on entry to the function > > using the > > + DW_OP_entry_value DWARF entries. This requires compiling the > > user > > + program with -fvar-tracking to resolve the DW_TAG_call_sites > > in the > > + binary file. */ OK, so some rework here to make things a bit better. I changed the header comment for function ppc64_sysv_get_return_buf_addr to the following. Hopefully this addresses the various questions and comments. /* The PowerPC ABI specifies aggregates that are not returned by value are returned in a storage buffer provided by the caller. The address of the storage buffer is provided as a hidden first input arguement in register r3. The PowerPC ABI does not guarantee that register r3 will not be changed while executing the function. Hence, it cannot be assumed that r3 will still contain the address of the storage buffer when execution reaches the end of the function. This function attempts to determine the value of r3 on entry to the function using the DW_OP_entry_value DWARF entries. This requires compiling the user program with -fvar-tracking to resolve the DW_TAG_call_sites in the binary file. */ Hopefully that is clearer and clears up any confusion as to the role of r3. > > + union call_site_parameter_u kind_u; > > + enum call_site_parameter_kind kind; > > + CORE_ADDR return_val = 0; > > + > > + kind_u.dwarf_reg = 3; /* First passed arg/return value is in > > r3. */ > > + kind = CALL_SITE_PARAMETER_DWARF_REG; > > + > > + /* val_type is the type of the return value. Need the pointer > > type > > + to the return value. */ > > + val_type = lookup_pointer_type (val_type); > > + > > + try > > + { > > + return_val = value_as_address (value_of_dwarf_reg_entry > > (val_type, > > + cur_fram > > e, > > + kind, > > kind_u)); > > + } > > + catch (const gdb_exception_error &e) > > + { > > + warning ("Cannot determine the function return value.\n" > > + "Try compiling with -fvar-tracking."); > > Will this warning be printed a lot? (I'm wondering if there should > be a > way to shut it off...) Hmm, hard to say how often it will get printed. It is going to be up to what the user is trying to do. If they are stopping on the return of functions that return non-trivial structures, they will see the message for every function. Otherwise, the message should not get printed. Not aware of anyway we could prompt the user to ask them if they want to suppress this message from the command line. If there is, can you give me a pointer to an example? We could do something like: catch (const gdb_exception_error &e) { static int warned = 0; if (warned++ < 6) warning ("Cannot determine the function return value.\n" "Try compiling with -fvar-tracking."); } So the warning is only printed the first five or whatever number of times we want before suppressing the message. Not sure what the right number of times should be? Would adding something like this be agreeable? Carl ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/2] PowerPC, fix support for printing the function return value for non-trivial values. 2022-10-14 23:23 ` Carl Love @ 2022-10-18 1:06 ` Kevin Buettner 2022-10-18 18:26 ` Carl Love 0 siblings, 1 reply; 44+ messages in thread From: Kevin Buettner @ 2022-10-18 1:06 UTC (permalink / raw) To: Carl Love; +Cc: gdb-patches, Ulrich Weigand On Fri, 14 Oct 2022 16:23:58 -0700 Carl Love <cel@us.ibm.com> wrote: > > 2) Whitespace complaints: > > > > Applying: PowerPC, fix support for printing the function return value > > for non-trivial values. > > ...patch:161: indent with spaces. > > "gdbarch_dump: get_return_buf_addr = <%s>\n", > > ...patch:162: indent with spaces. > > host_address_to_string (gdbarch- > > >get_return_buf_addr)); > > ...patch:182: indent with spaces. > > gdbarch_get_return_buf_addr_ftype > > get_return_buf_addr) > > warning: 3 lines add whitespace errors. > > > > (I substituted some long paths local to my machines w/ '...'.) > > Yes, I have seen the complaints about the white spaces. The whitespace > complaints come from file gdb/gdbarch.c which is a generated file. As > I assume you know, the file was regenerated by the gdb/gdbarch.py based > on the changes to gdb/gdbarch-components.py. The regenerated file is > set to read only. Not sure what I can do to rectify the white spaces? > Any ideas? If the unwanted white space is generated by gdbarch.py,maybe fix that script? That said, if those files already have this problem, I'm okay with it going in with the whitespace complaints. (It can be fixed in a later patch addressing just that problem.) > > I already mentioned this last week, but... > > > > s/GDB ABI/gdbarch method/ > > > > I guess you could also refer to it as an API which you did later on > > in your commit remarks. (What I really object to is calling it an > > "ABI", which it definitely is not - that said it's easy to mis-type > > API > > as ABI, so I do understand this mistake.) > > Yea, I think referring to it as a new gdbarch method is probably better > than GDB API/ABI. I have changed the GDB API/ABI references to gdbarch > method. I will need to update that in the message to the GDB > maintainers as well when I send the next version of the patch. Sounds good. > > > value of r3 on entry to the function. This requires the use of the > > > -fvar-tracking compiler option to compile the user application thus > > > generating the DW_OP_entry_value in the binary. The DW_OP_entries > > > in the > > > > I don't think that "DW_OP_entries" is correct here. Maybe > > "DW_OP_entry_value > > entries"? > > > > binary file enables GDB can resolve the DW_TAG_call_sites. This > > > new API > > > > Maybe instead of "enables GDB can resolve the DW_TAG_call_sites", say > > "allow GDB to resolve the DW_TAG_call_site entries". > > > > > > > is used to get the return buffer address, in the case of a function > > > returning a nontrivial data type, on exit from the function. The > > > GDB > > > function should_stop checks to see if RETURN_BUF is non-zero. By > > > default, > > > RETURN_BUF will be set to zero by the new ABI call for all > > > architectures > > > > s/ABI/API/ > > > > > but PowerPC. The get_return_value function will be used to obtain > > > the > > > return value on these architectures as is currently being done if > > > RETURN_BUF is zero. On PowerPC, the new ABI will return a nonzero > > > address > > > in RETURN_BUF if the value can be determined. The value_at > > > function uses > > > the return buffer address to get the return value. > > So, I tried to address all of the above comments about ABI, > "DW_OP_entries" etc. The paragraph now reads: > > This patch adds a new gdbarch method to allow PowerPC to access the value > of r3 on entry to a function. On PowerPC, the new gdb arch method attempts > to use the DW_OP_entry_value for the DWARF entries, when exiting the > function, to determine the value of r3 on entry to the function. This > requires the use of the -fvar-tracking compiler option to compile the > user application thus generating the DW_OP_entry_value in the binary. The > DW_OP_entry_value entries in the binary file allows GDB to resolve the > DW_TAG_call_site entries. This new gdbarch method is used to get the > return buffer address, in the case of a function returning a nontrivial > data type, on exit from the function. The GDB function should_stop checks > to see if RETURN_BUF is non-zero. By default, RETURN_BUF will be set to > zero by the new gdbarch method call for all architectures except PowerPC. > The get_return_value function will be used to obtain the return value on > all other architectures as is currently being done if RETURN_BUF is zero. > On PowerPC, the new gdbarch method will return a nonzero address in > RETURN_BUF if the value can be determined. The value_at function uses the > return buffer address to get the return value. > > Which I think covers all of the comments above. Yes, this looks good, though I'd use 'gdbarch' in place of 'gdb arch'. This is a small nit though; if you'd prefer to make them separate words, I won't argue about it. > > > if (execution_direction == EXEC_REVERSE) > > > diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c > > > index 14effb93210..59145f9f166 100644 > > > --- a/gdb/ppc-sysv-tdep.c > > > +++ b/gdb/ppc-sysv-tdep.c > > > @@ -28,6 +28,7 @@ > > > #include "objfiles.h" > > > #include "infcall.h" > > > #include "dwarf2.h" > > > +#include "dwarf2/loc.h" > > > #include "target-float.h" > > > #include <algorithm> > > > > > > @@ -2156,3 +2157,39 @@ ppc64_sysv_abi_return_value (struct gdbarch > > > *gdbarch, struct value *function, > > > return RETURN_VALUE_STRUCT_CONVENTION; > > > } > > > > > > +CORE_ADDR ppc64_sysv_get_return_buf_addr (struct type *val_type, > > > + frame_info *cur_frame) > > > +{ > > > + /* On PowerPC, the ABI specifies r3 is the address where an > > > argument > > > > s/specifies/specifies that/ > > > > I'm not sure about this phrase: "where an argument and". If you > > mean > > to say that r3 is sometimes used for arguments (when it's not used > > for > > the address of the return value buffer), then that's true, but it's > > not relevant here. > > > > So, I guess that I'm asking that this comment be reworded somewhat. > > > > > + and the return value is stored. The PowerPC ABI does not > > > guarantee > > > + r3 will be preserved when the function returns. The function > > > returns > > > + the address of the buffer stored in r3. > > > > I find this part confusing. If the function returns the address of > > the buffer, doesn't it do so by returning it in r3? (And, if so, it > > seems to me that this code would be unnecessary - so, clearly, I must > > be missing something...) > > > > + > > > + Attempt to determine the value of r3 on entry to the function > > > using the > > > + DW_OP_entry_value DWARF entries. This requires compiling the > > > user > > > + program with -fvar-tracking to resolve the DW_TAG_call_sites > > > in the > > > + binary file. */ > > OK, so some rework here to make things a bit better. I changed the > header comment for function ppc64_sysv_get_return_buf_addr to the > following. Hopefully this addresses the various questions and > comments. > > /* The PowerPC ABI specifies aggregates that are not returned by value > are returned in a storage buffer provided by the caller. The > address of the storage buffer is provided as a hidden first input > arguement in register r3. The PowerPC ABI does not guarantee that > register r3 will not be changed while executing the function. Hence, it > cannot be assumed that r3 will still contain the address of the storage > buffer when execution reaches the end of the function. > > This function attempts to determine the value of r3 on entry to the > function using the DW_OP_entry_value DWARF entries. This requires > compiling the user program with -fvar-tracking to resolve the > DW_TAG_call_sites in the binary file. */ > > Hopefully that is clearer and clears up any confusion as to the role of r3. Thanks for reworking that comment. > > > + catch (const gdb_exception_error &e) > > > + { > > > + warning ("Cannot determine the function return value.\n" > > > + "Try compiling with -fvar-tracking."); > > > > Will this warning be printed a lot? (I'm wondering if there should > > be a > > way to shut it off...) > > Hmm, hard to say how often it will get printed. It is going to be up > to what the user is trying to do. If they are stopping on the return > of functions that return non-trivial structures, they will see the > message for every function. Otherwise, the message should not get > printed. > > Not aware of anyway we could prompt the user to ask them if they want > to suppress this message from the command line. If there is, can you > give me a pointer to an example? > > We could do something like: > > catch (const gdb_exception_error &e) > { > static int warned = 0; > > if (warned++ < 6) > warning ("Cannot determine the function return value.\n" > "Try compiling with -fvar-tracking."); > } > > So the warning is only printed the first five or whatever number of > times we want before suppressing the message. Not sure what the right > number of times should be? > > Would adding something like this be agreeable? Maybe. If we go that route, I'd recommend printing an additional message along side the last "Cannot determine the function return value." message indicating that these messages are now being suppressed. We have several mechanisms in gdb for suppressing messages already. Probably the most used one is the "complaints" facility for supressing warnings while reading symbols. (See gdb/complaints.{h,c}.) I recently added a bfd error supression mechanism in gdb_bfd.c. Look for gdb_bfd_error_handler() in that file. There's also lim_warning() in ada-lang.c along with an old FIXME remark indicating that it's doing something similar to that of complaint(). There may be others, but I don't know for sure. None of them prompt the user about whether to continue to print warnings; I'd prefer not to do it that way. Hard coding it to six (or something) is okay. Making it user-settable with some default is also okay. That said, this is just one message. It seems like overkill to add a setting for this one message. After thinking about this a bit more, I think we should go with your original code which doesn't suppress any messages. If we find that it's too noisy, we can add something along the lines of what you suggested above. Kevin ^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH 2/2] PowerPC, fix support for printing the function return value for non-trivial values. 2022-10-18 1:06 ` Kevin Buettner @ 2022-10-18 18:26 ` Carl Love 0 siblings, 0 replies; 44+ messages in thread From: Carl Love @ 2022-10-18 18:26 UTC (permalink / raw) To: Kevin Buettner, blarsen; +Cc: gdb-patches, Ulrich Weigand, cel On Mon, 2022-10-17 at 18:06 -0700, Kevin Buettner wrote: > On Fri, 14 Oct 2022 16:23:58 -0700 > Carl Love <cel@us.ibm.com> wrote: > > > > 2) Whitespace complaints: > > > > > > Applying: PowerPC, fix support for printing the function return > > > value > > > for non-trivial values. > > > ...patch:161: indent with spaces. > > > "gdbarch_dump: get_return_buf_addr = > > > <%s>\n", > > > ...patch:162: indent with spaces. > > > host_address_to_string (gdbarch- > > > > get_return_buf_addr)); > > > ...patch:182: indent with spaces. > > > gdbarch_get_return_buf_addr_ftyp > > > e > > > get_return_buf_addr) > > > warning: 3 lines add whitespace errors. > > > > > > (I substituted some long paths local to my machines w/ '...'.) > > > > Yes, I have seen the complaints about the white spaces. The > > whitespace > > complaints come from file gdb/gdbarch.c which is a generated > > file. As > > I assume you know, the file was regenerated by the gdb/gdbarch.py > > based > > on the changes to gdb/gdbarch-components.py. The regenerated file > > is > > set to read only. Not sure what I can do to rectify the white > > spaces? > > Any ideas? > > If the unwanted white space is generated by gdbarch.py,maybe fix that > script? > > That said, if those files already have this problem, I'm okay with > it going in with the whitespace complaints. (It can be fixed in > a later patch addressing just that problem.) I see the same white space issues through out the file. The script generates the spaces so yes, it probably needs fixing and then cleanup the white spaces in the file. However, that is beyond the scope of this patch. <snip ? > > So, I tried to address all of the above comments about ABI, > > "DW_OP_entries" etc. The paragraph now reads: > > > > This patch adds a new gdbarch method to allow PowerPC to access the > > value > > of r3 on entry to a function. On PowerPC, the new gdb arch method > > attempts > > to use the DW_OP_entry_value for the DWARF entries, when exiting > > the > > function, to determine the value of r3 on entry to the > > function. This > > requires the use of the -fvar-tracking compiler option to compile > > the > > user application thus generating the DW_OP_entry_value in the > > binary. The > > DW_OP_entry_value entries in the binary file allows GDB to resolve > > the > > DW_TAG_call_site entries. This new gdbarch method is used to get > > the > > return buffer address, in the case of a function returning a > > nontrivial > > data type, on exit from the function. The GDB function should_stop > > checks > > to see if RETURN_BUF is non-zero. By default, RETURN_BUF will be > > set to > > zero by the new gdbarch method call for all architectures except > > PowerPC. > > The get_return_value function will be used to obtain the return > > value on > > all other architectures as is currently being done if RETURN_BUF is > > zero. > > On PowerPC, the new gdbarch method will return a nonzero address in > > RETURN_BUF if the value can be determined. The value_at function > > uses the > > return buffer address to get the return value. > > > > Which I think covers all of the comments above. > > Yes, this looks good, though I'd use 'gdbarch' in place of 'gdb > arch'. > This is a small nit though; if you'd prefer to make them separate > words, I won't argue about it. I intended to use "gdbarch" not "gdb arch". Fixed. <snip> > > Thanks for reworking that comment. > > > > > + catch (const gdb_exception_error &e) > > > > + { > > > > + warning ("Cannot determine the function return value.\n" > > > > + "Try compiling with -fvar-tracking."); > > > > > > Will this warning be printed a lot? (I'm wondering if there > > > should > > > be a > > > way to shut it off...) > > > > Hmm, hard to say how often it will get printed. It is going to be > > up > > to what the user is trying to do. If they are stopping on the > > return > > of functions that return non-trivial structures, they will see the > > message for every function. Otherwise, the message should not get > > printed. > > > > Not aware of anyway we could prompt the user to ask them if they > > want > > to suppress this message from the command line. If there is, can > > you > > give me a pointer to an example? > > > > We could do something like: > > > > catch (const gdb_exception_error &e) > > { > > static int warned = 0; > > > > if (warned++ < 6) > > warning ("Cannot determine the function return value.\n" > > "Try compiling with -fvar-tracking."); > > } > > > > So the warning is only printed the first five or whatever number of > > times we want before suppressing the message. Not sure what the > > right > > number of times should be? > > > > Would adding something like this be agreeable? > > Maybe. If we go that route, I'd recommend printing an additional > message along side the last "Cannot determine the function return > value." > message indicating that these messages are now being suppressed. > > We have several mechanisms in gdb for suppressing messages already. > Probably the most used one is the "complaints" facility for > supressing warnings while reading symbols. (See > gdb/complaints.{h,c}.) I recently added a bfd error supression > mechanism in gdb_bfd.c. Look for gdb_bfd_error_handler() in that > file. There's also lim_warning() in ada-lang.c along with an old > FIXME remark indicating that it's doing something similar to that of > complaint(). There may be others, but I don't know for sure. > None of them prompt the user about whether to continue to print > warnings; I'd prefer not to do it that way. Hard coding it to six > (or > something) is okay. Making it user-settable with some default is > also okay. That said, this is just one message. It seems like > overkill > to add a setting for this one message. > > After thinking about this a bit more, I think we should go with your > original code which doesn't suppress any messages. If we find that > it's too noisy, we can add something along the lines of what you > suggested above. I think that sounds reasonable. We can always look at suppressing the messages in the future if it looks like an issue. I am leaving the code as it was originally. Thanks again for the feedback. Take care. Carl ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 2/2 ver 2] PowerPC, fix support for printing the function return value for non-trivial values. 2022-10-06 16:37 ` [PATCH 2/2] " Carl Love 2022-10-08 4:36 ` Kevin Buettner 2022-10-14 2:49 ` Kevin Buettner @ 2022-10-18 18:55 ` Carl Love 2022-10-31 16:07 ` Carl Love ` (2 more replies) 2 siblings, 3 replies; 44+ messages in thread From: Carl Love @ 2022-10-18 18:55 UTC (permalink / raw) To: Ulrich Weigand, gdb-patches GDB maintainers: Version 2, updated comments per Kevin's comments. Changed "frame_info *" to frame_info_ptr per Bruno's comments. Discussed supressing the new warning after printing it with Kevin. In the end we decided to not suppress the warnings. Suppression can be added later if deemed necessary. The white space introduced by the gdb/gdbarch-components.py needs to be addressed by a future patch to fix the script and the generated files. On PowerPC, a non-trivial return value for a function cannot be reliably determined with the current PowerPC ABI. The issue is the address of the return buffer for a non-trivial value is passed to the function in register r3. However, at the end of the function the ABI does not guarantee r3 will not be changed. Hence it cannot be reliably used to get the address of the return buffer. This patch adds a new GDB ABI to allow GDB to obtain the input value of r3 when the function returns. This is done by using the DW_OP_entry value for the DWARF entries to obtain the value of r3 on entry to the function. The ABI allows GDB to get the correct address for the return buffer on exit from the function. This patch fixes the five test failures in gdb.cp/non-trivial- retval.exp. The patch has been re-tested on PowerPC and X86-64 with no regression failures. Please let me know if this version of the patch is acceptable for GDB mainline. Thanks. Carl Love ----------------------------------- PowerPC, fix support for printing the function return value for non-trivial values. Currently, a non-trivial return value from a function cannot currently be reliably determined on PowerPC. This is due to the fact that the PowerPC ABI uses register r3 to store the address of the buffer containing the non-trivial return value when the function is called. The PowerPC ABI does not guarantee the value in register r3 is not modified in the function. Thus the value in r3 cannot be reliably used to obtain the return addreses on exit from the function. This patch adds a new gdbarch method to allow PowerPC to access the value of r3 on entry to a function. On PowerPC, the new gdbarch method attempts to use the DW_OP_entry_value for the DWARF entries, when exiting the function, to determine the value of r3 on entry to the function. This requires the use of the -fvar-tracking compiler option to compile the user application thus generating the DW_OP_entry_value in the binary. The DW_OP_entry_value entries in the binary file allows GDB to resolve the DW_TAG_call_site entries. This new gdbarch method is used to get the return buffer address, in the case of a function returning a nontrivial data type, on exit from the function. The GDB function should_stop checks to see if RETURN_BUF is non-zero. By default, RETURN_BUF will be set to zero by the new gdbarch method call for all architectures except PowerPC. The get_return_value function will be used to obtain the return value on all other architectures as is currently being done if RETURN_BUF is zero. On PowerPC, the new gdbarch method will return a nonzero address in RETURN_BUF if the value can be determined. The value_at function uses the return buffer address to get the return value. This patch fixes five testcase failures in gdb.cp/non-trivial-retval.exp. The correct function return values are now reported. Note this patch is dependent on patch: "PowerPC, function ppc64_sysv_abi_return_value add missing return value convention". This patch has been tested on Power 10 and x86-64 with no regressions. Signed-off-by: Carl Love <cel@us.ibm.com> --- gdb/arch-utils.c | 6 +++ gdb/arch-utils.h | 4 ++ gdb/dwarf2/loc.c | 10 +---- gdb/dwarf2/loc.h | 11 ++++++ gdb/gdbarch-components.py | 15 ++++++++ gdb/gdbarch-gen.h | 11 ++++++ gdb/gdbarch.c | 23 ++++++++++++ gdb/infcmd.c | 41 +++++++++++++++++++-- gdb/ppc-sysv-tdep.c | 41 +++++++++++++++++++++ gdb/ppc-tdep.h | 2 + gdb/rs6000-tdep.c | 6 ++- gdb/testsuite/gdb.cp/non-trivial-retval.exp | 9 ++++- gdb/testsuite/lib/gdb.exp | 8 ++++ 13 files changed, 173 insertions(+), 14 deletions(-) diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c index 6a72b3f5efd..1358987c02f 100644 --- a/gdb/arch-utils.c +++ b/gdb/arch-utils.c @@ -1092,6 +1092,12 @@ default_read_core_file_mappings { } +CORE_ADDR +default_get_return_buf_addr (struct type *val_type, frame_info_ptr cur_frame) +{ + return 0; +} + /* Non-zero if we want to trace architecture code. */ #ifndef GDBARCH_DEBUG diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h index f6229f434fd..46018a6fbbb 100644 --- a/gdb/arch-utils.h +++ b/gdb/arch-utils.h @@ -300,4 +300,8 @@ extern void default_read_core_file_mappings struct bfd *cbfd, read_core_file_mappings_pre_loop_ftype pre_loop_cb, read_core_file_mappings_loop_ftype loop_cb); + +/* Default implementation of gdbarch default_get_return_buf_addr method. */ +extern CORE_ADDR default_get_return_buf_addr (struct type *val_typegdbarch, + frame_info_ptr cur_frame); #endif /* ARCH_UTILS_H */ diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c index 791648d6e7e..b88226db610 100644 --- a/gdb/dwarf2/loc.c +++ b/gdb/dwarf2/loc.c @@ -1325,14 +1325,8 @@ static const struct lval_funcs entry_data_value_funcs = entry_data_value_free_closure }; -/* Read parameter of TYPE at (callee) FRAME's function entry. KIND and KIND_U - are used to match DW_AT_location at the caller's - DW_TAG_call_site_parameter. - - Function always returns non-NULL value. It throws NO_ENTRY_VALUE_ERROR if it - cannot resolve the parameter for any reason. */ - -static struct value * +/* See dwarf2/loc.h. */ +struct value * value_of_dwarf_reg_entry (struct type *type, frame_info_ptr frame, enum call_site_parameter_kind kind, union call_site_parameter_u kind_u) diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h index d6709f2e342..9156e1ee533 100644 --- a/gdb/dwarf2/loc.h +++ b/gdb/dwarf2/loc.h @@ -296,4 +296,15 @@ extern struct value *indirect_synthetic_pointer dwarf2_per_objfile *per_objfile, frame_info_ptr frame, struct type *type, bool resolve_abstract_p = false); +/* Read parameter of TYPE at (callee) FRAME's function entry. KIND and KIND_U + are used to match DW_AT_location at the caller's + DW_TAG_call_site_parameter. + + Function always returns non-NULL value. It throws NO_ENTRY_VALUE_ERROR if + it cannot resolve the parameter for any reason. */ + +extern struct value *value_of_dwarf_reg_entry (struct type *type, + struct frame_info_ptr frame, + enum call_site_parameter_kind kind, + union call_site_parameter_u kind_u); #endif /* DWARF2LOC_H */ diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py index 46e7565f293..325801256a2 100644 --- a/gdb/gdbarch-components.py +++ b/gdb/gdbarch-components.py @@ -868,6 +868,21 @@ for instance). invalid=True, ) +Function( + comment=""" +Return the address at which the value being returned from +the current function will be stored. This routine is only +called if the current function uses the the "struct return +convention". + +May return 0 when unable to determine that address.""", + type="CORE_ADDR", + name="get_return_buf_addr", + params=[("struct type *", "val_type"), ("frame_info_ptr", "cur_frame")], + predefault="default_get_return_buf_addr", + invalid=False, +) + Method( comment=""" Return true if the return value of function is stored in the first hidden diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h index 840de585869..66e2e761385 100644 --- a/gdb/gdbarch-gen.h +++ b/gdb/gdbarch-gen.h @@ -441,6 +441,17 @@ typedef enum return_value_convention (gdbarch_return_value_ftype) (struct gdbarc extern enum return_value_convention gdbarch_return_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf); extern void set_gdbarch_return_value (struct gdbarch *gdbarch, gdbarch_return_value_ftype *return_value); +/* Return the address at which the value being returned from + the current function will be stored. This routine is only + called if the current function uses the the "struct return + convention". + + May return 0 when unable to determine that address. */ + +typedef CORE_ADDR (gdbarch_get_return_buf_addr_ftype) (struct type *val_type, frame_info_ptr cur_frame); +extern CORE_ADDR gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type *val_type, frame_info_ptr cur_frame); +extern void set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, gdbarch_get_return_buf_addr_ftype *get_return_buf_addr); + /* Return true if the return value of function is stored in the first hidden parameter. In theory, this feature should be language-dependent, specified by language and its ABI, such as C++. Unfortunately, compiler may diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c index 559e92dee58..3bbb70e7be9 100644 --- a/gdb/gdbarch.c +++ b/gdb/gdbarch.c @@ -116,6 +116,7 @@ struct gdbarch gdbarch_address_to_pointer_ftype *address_to_pointer = nullptr; gdbarch_integer_to_address_ftype *integer_to_address = nullptr; gdbarch_return_value_ftype *return_value = nullptr; + gdbarch_get_return_buf_addr_ftype *get_return_buf_addr = nullptr; gdbarch_return_in_first_hidden_param_p_ftype *return_in_first_hidden_param_p = nullptr; gdbarch_skip_prologue_ftype *skip_prologue = nullptr; gdbarch_skip_main_prologue_ftype *skip_main_prologue = nullptr; @@ -313,6 +314,7 @@ gdbarch_alloc (const struct gdbarch_info *info, gdbarch->value_from_register = default_value_from_register; gdbarch->pointer_to_address = unsigned_pointer_to_address; gdbarch->address_to_pointer = unsigned_address_to_pointer; + gdbarch->get_return_buf_addr = default_get_return_buf_addr; gdbarch->return_in_first_hidden_param_p = default_return_in_first_hidden_param_p; gdbarch->breakpoint_from_pc = default_breakpoint_from_pc; gdbarch->sw_breakpoint_from_kind = NULL; @@ -465,6 +467,7 @@ verify_gdbarch (struct gdbarch *gdbarch) /* Skip verify of address_to_pointer, invalid_p == 0 */ /* Skip verify of integer_to_address, has predicate. */ /* Skip verify of return_value, has predicate. */ + /* Skip verify of get_return_buf_addr, invalid_p == 0 */ /* Skip verify of return_in_first_hidden_param_p, invalid_p == 0 */ if (gdbarch->skip_prologue == 0) log.puts ("\n\tskip_prologue"); @@ -877,6 +880,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file) gdb_printf (file, "gdbarch_dump: return_value = <%s>\n", host_address_to_string (gdbarch->return_value)); + gdb_printf (file, + "gdbarch_dump: get_return_buf_addr = <%s>\n", + host_address_to_string (gdbarch->get_return_buf_addr)); gdb_printf (file, "gdbarch_dump: return_in_first_hidden_param_p = <%s>\n", host_address_to_string (gdbarch->return_in_first_hidden_param_p)); @@ -2684,6 +2690,23 @@ set_gdbarch_return_value (struct gdbarch *gdbarch, gdbarch->return_value = return_value; } +CORE_ADDR +gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type *val_type, frame_info_ptr cur_frame) +{ + gdb_assert (gdbarch != NULL); + gdb_assert (gdbarch->get_return_buf_addr != NULL); + if (gdbarch_debug >= 2) + gdb_printf (gdb_stdlog, "gdbarch_get_return_buf_addr called\n"); + return gdbarch->get_return_buf_addr (val_type, cur_frame); +} + +void +set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, + gdbarch_get_return_buf_addr_ftype get_return_buf_addr) +{ + gdbarch->get_return_buf_addr = get_return_buf_addr; +} + int gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch, struct type *type) { diff --git a/gdb/infcmd.c b/gdb/infcmd.c index d729732c81c..2e7cca1a1aa 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -55,6 +55,7 @@ #include "gdbsupport/gdb_optional.h" #include "source.h" #include "cli/cli-style.h" +#include "dwarf2/loc.h" /* Local functions: */ @@ -1598,6 +1599,12 @@ struct finish_command_fsm : public thread_fsm return value. */ struct return_value_info return_value_info {}; + /* If the current function uses the "struct return convention", + this holds the address at which the value being returned will + be stored, or zero if that address could not be determined or + the "struct return convention" is not being used. */ + CORE_ADDR return_buf; + explicit finish_command_fsm (struct interp *cmd_interp) : thread_fsm (cmd_interp) { @@ -1636,7 +1643,13 @@ finish_command_fsm::should_stop (struct thread_info *tp) struct value *func; func = read_var_value (function, NULL, get_current_frame ()); - rv->value = get_return_value (function, func); + + if (return_buf != 0) + /* Retrieve return value from the buffer where it was saved. */ + rv->value = value_at (rv->type, return_buf); + else + rv->value = get_return_value (function, func); + if (rv->value != NULL) rv->value_history_index = record_latest_value (rv->value); } @@ -1852,8 +1865,28 @@ finish_command (const char *arg, int from_tty) } /* Find the function we will return from. */ - - sm->function = find_pc_function (get_frame_pc (get_selected_frame (NULL))); + frame_info_ptr callee_frame = get_selected_frame (NULL); + sm->function = find_pc_function (get_frame_pc (callee_frame)); + + /* Determine the return convention. If it is RETURN_VALUE_STRUCT_CONVENTION, + attempt to determine the address of the return buffer. */ + enum return_value_convention return_value; + struct gdbarch *gdbarch = get_frame_arch (callee_frame); + + struct type * val_type + = check_typedef (sm->function->type()->target_type ()); + + return_value = gdbarch_return_value (gdbarch, + read_var_value (sm->function, NULL, + callee_frame), + val_type, NULL, NULL, NULL); + + if (return_value == RETURN_VALUE_STRUCT_CONVENTION + && val_type->code() != TYPE_CODE_VOID) + sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type, + callee_frame); + else + sm->return_buf = 0; /* Print info on the selected frame, including level number but not source. */ @@ -1871,7 +1904,7 @@ finish_command (const char *arg, int from_tty) gdb_printf (_("Run till exit from ")); } - print_stack_frame (get_selected_frame (NULL), 1, LOCATION, 0); + print_stack_frame (callee_frame, 1, LOCATION, 0); } frame.reinflate (); diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c index 14effb93210..f720e87e63b 100644 --- a/gdb/ppc-sysv-tdep.c +++ b/gdb/ppc-sysv-tdep.c @@ -28,6 +28,7 @@ #include "objfiles.h" #include "infcall.h" #include "dwarf2.h" +#include "dwarf2/loc.h" #include "target-float.h" #include <algorithm> @@ -2156,3 +2157,43 @@ ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, return RETURN_VALUE_STRUCT_CONVENTION; } +CORE_ADDR ppc64_sysv_get_return_buf_addr (struct type *val_type, + frame_info_ptr cur_frame) +{ + /* The PowerPC ABI specifies aggregates that are not returned by value + are returned in a storage buffer provided by the caller. The + address of the storage buffer is provided as a hidden first input + arguement in register r3. The PowerPC ABI does not guarantee that + register r3 will not be changed while executing the function. Hence, it + cannot be assumed that r3 will still contain the address of the storage + buffer when execution reaches the end of the function. + + This function attempts to determine the value of r3 on entry to the + function using the DW_OP_entry_value DWARF entries. This requires + compiling the user program with -fvar-tracking to resolve the + DW_TAG_call_sites in the binary file. */ + + union call_site_parameter_u kind_u; + enum call_site_parameter_kind kind; + CORE_ADDR return_val = 0; + + kind_u.dwarf_reg = 3; /* First passed arg/return value is in r3. */ + kind = CALL_SITE_PARAMETER_DWARF_REG; + + /* val_type is the type of the return value. Need the pointer type + to the return value. */ + val_type = lookup_pointer_type (val_type); + + try + { + return_val = value_as_address (value_of_dwarf_reg_entry (val_type, + cur_frame, + kind, kind_u)); + } + catch (const gdb_exception_error &e) + { + warning ("Cannot determine the function return value.\n" + "Try compiling with -fvar-tracking."); + } + return return_val; +} diff --git a/gdb/ppc-tdep.h b/gdb/ppc-tdep.h index 34b250849b9..cbccd87820d 100644 --- a/gdb/ppc-tdep.h +++ b/gdb/ppc-tdep.h @@ -175,6 +175,8 @@ extern void ppc_collect_vsxregset (const struct regset *regset, const struct regcache *regcache, int regnum, void *vsxregs, size_t len); +extern CORE_ADDR ppc64_sysv_get_return_buf_addr (type*, frame_info_ptr); + /* Private data that this module attaches to struct gdbarch. */ /* ELF ABI version used by the inferior. */ diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c index 8b6d666bbe7..637cbb41651 100644 --- a/gdb/rs6000-tdep.c +++ b/gdb/rs6000-tdep.c @@ -8234,7 +8234,11 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) set_gdbarch_ps_regnum (gdbarch, tdep->ppc_ps_regnum); if (wordsize == 8) - set_gdbarch_return_value (gdbarch, ppc64_sysv_abi_return_value); + { + set_gdbarch_return_value (gdbarch, ppc64_sysv_abi_return_value); + set_gdbarch_get_return_buf_addr (gdbarch, + ppc64_sysv_get_return_buf_addr); + } else set_gdbarch_return_value (gdbarch, ppc_sysv_abi_return_value); diff --git a/gdb/testsuite/gdb.cp/non-trivial-retval.exp b/gdb/testsuite/gdb.cp/non-trivial-retval.exp index 21c390bc937..93a3a6832f7 100644 --- a/gdb/testsuite/gdb.cp/non-trivial-retval.exp +++ b/gdb/testsuite/gdb.cp/non-trivial-retval.exp @@ -15,11 +15,18 @@ # This file is part of the gdb testsuite +set additional_flags "" + if {[skip_cplus_tests]} { continue } standard_testfile .cc -if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} { +if {[have_fvar_tracking]} { + set additional_flags "additional_flags= -fvar-tracking" +} + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile [list debug c++ $additional_flags]]} { + return -1 } diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index bfa9fec628e..d3b77b4869b 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -8582,6 +8582,14 @@ gdb_caching_proc have_fuse_ld_gold { return [gdb_simple_compile $me $src executable $flags] } +# Return 1 if compiler supports fvar-tracking, otherwise return 0. +gdb_caching_proc have_fvar_tracking { + set me "have_fvar_tracking" + set flags "additional_flags=-fvar-tracking" + set src { int main() { return 0; } } + return [gdb_simple_compile $me $src executable $flags] +} + # Return 1 if linker supports -Ttext-segment, otherwise return 0. gdb_caching_proc linker_supports_Ttext_segment_flag { set me "linker_supports_Ttext_segment_flag" -- 2.37.2 ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/2 ver 2] PowerPC, fix support for printing the function return value for non-trivial values. 2022-10-18 18:55 ` [PATCH 2/2 ver 2] " Carl Love @ 2022-10-31 16:07 ` Carl Love 2022-11-07 14:56 ` Bruno Larsen 2022-11-07 20:04 ` [PATCH 2/2 ver 3] " Carl Love 2 siblings, 0 replies; 44+ messages in thread From: Carl Love @ 2022-10-31 16:07 UTC (permalink / raw) To: Ulrich Weigand, gdb-patches; +Cc: will_schmidt, blarsen, Kevin Buettner Kevin, Bruno: I haven't seen any comments on the updated patch. Just wanted to check and see if the updated patches look OK to you. Thanks. Carl On Tue, 2022-10-18 at 11:55 -0700, Carl Love wrote: > GDB maintainers: > > Version 2, updated comments per Kevin's comments. Changed > "frame_info > *" to frame_info_ptr per Bruno's comments. Discussed supressing the > new warning after printing it with Kevin. In the end we decided to > not > suppress the warnings. Suppression can be added later if deemed > necessary. The white space introduced by the gdb/gdbarch- > components.py > needs to be addressed by a future patch to fix the script and the > generated files. > > On PowerPC, a non-trivial return value for a function cannot be > reliably determined with the current PowerPC ABI. The issue is the > address of the return buffer for a non-trivial value is passed to the > function in register r3. However, at the end of the function the ABI > does not guarantee r3 will not be changed. Hence it cannot be > reliably > used to get the address of the return buffer. > > This patch adds a new GDB ABI to allow GDB to obtain the input value > of > r3 when the function returns. This is done by using the DW_OP_entry > value for the DWARF entries to obtain the value of r3 on entry to the > function. The ABI allows GDB to get the correct address for the > return > buffer on exit from the function. > > This patch fixes the five test failures in gdb.cp/non-trivial- > retval.exp. > > The patch has been re-tested on PowerPC and X86-64 with no regression > failures. > > Please let me know if this version of the patch is acceptable for GDB > mainline. Thanks. > > Carl Love > > > ----------------------------------- > PowerPC, fix support for printing the function return value for non- > trivial values. > > Currently, a non-trivial return value from a function cannot > currently be > reliably determined on PowerPC. This is due to the fact that the > PowerPC > ABI uses register r3 to store the address of the buffer containing > the > non-trivial return value when the function is called. The PowerPC > ABI > does not guarantee the value in register r3 is not modified in the > function. Thus the value in r3 cannot be reliably used to obtain the > return addreses on exit from the function. > > This patch adds a new gdbarch method to allow PowerPC to access the > value > of r3 on entry to a function. On PowerPC, the new gdbarch method > attempts > to use the DW_OP_entry_value for the DWARF entries, when exiting the > function, to determine the value of r3 on entry to the > function. This > requires the use of the -fvar-tracking compiler option to compile the > user application thus generating the DW_OP_entry_value in the > binary. The > DW_OP_entry_value entries in the binary file allows GDB to resolve > the > DW_TAG_call_site entries. This new gdbarch method is used to get the > return buffer address, in the case of a function returning a > nontrivial > data type, on exit from the function. The GDB function should_stop > checks > to see if RETURN_BUF is non-zero. By default, RETURN_BUF will be set > to > zero by the new gdbarch method call for all architectures except > PowerPC. > The get_return_value function will be used to obtain the return value > on > all other architectures as is currently being done if RETURN_BUF is > zero. > On PowerPC, the new gdbarch method will return a nonzero address in > RETURN_BUF if the value can be determined. The value_at function > uses the > return buffer address to get the return value. > > This patch fixes five testcase failures in gdb.cp/non-trivial- > retval.exp. > The correct function return values are now reported. > > Note this patch is dependent on patch: "PowerPC, function > ppc64_sysv_abi_return_value add missing return value convention". > > This patch has been tested on Power 10 and x86-64 with no > regressions. > > Signed-off-by: Carl Love <cel@us.ibm.com> > --- > gdb/arch-utils.c | 6 +++ > gdb/arch-utils.h | 4 ++ > gdb/dwarf2/loc.c | 10 +---- > gdb/dwarf2/loc.h | 11 ++++++ > gdb/gdbarch-components.py | 15 ++++++++ > gdb/gdbarch-gen.h | 11 ++++++ > gdb/gdbarch.c | 23 ++++++++++++ > gdb/infcmd.c | 41 > +++++++++++++++++++-- > gdb/ppc-sysv-tdep.c | 41 > +++++++++++++++++++++ > gdb/ppc-tdep.h | 2 + > gdb/rs6000-tdep.c | 6 ++- > gdb/testsuite/gdb.cp/non-trivial-retval.exp | 9 ++++- > gdb/testsuite/lib/gdb.exp | 8 ++++ > 13 files changed, 173 insertions(+), 14 deletions(-) > > diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c > index 6a72b3f5efd..1358987c02f 100644 > --- a/gdb/arch-utils.c > +++ b/gdb/arch-utils.c > @@ -1092,6 +1092,12 @@ default_read_core_file_mappings > { > } > > +CORE_ADDR > +default_get_return_buf_addr (struct type *val_type, frame_info_ptr > cur_frame) > +{ > + return 0; > +} > + > /* Non-zero if we want to trace architecture code. */ > > #ifndef GDBARCH_DEBUG > diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h > index f6229f434fd..46018a6fbbb 100644 > --- a/gdb/arch-utils.h > +++ b/gdb/arch-utils.h > @@ -300,4 +300,8 @@ extern void default_read_core_file_mappings > struct bfd *cbfd, > read_core_file_mappings_pre_loop_ftype pre_loop_cb, > read_core_file_mappings_loop_ftype loop_cb); > + > +/* Default implementation of gdbarch default_get_return_buf_addr > method. */ > +extern CORE_ADDR default_get_return_buf_addr (struct type > *val_typegdbarch, > + frame_info_ptr > cur_frame); > #endif /* ARCH_UTILS_H */ > diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c > index 791648d6e7e..b88226db610 100644 > --- a/gdb/dwarf2/loc.c > +++ b/gdb/dwarf2/loc.c > @@ -1325,14 +1325,8 @@ static const struct lval_funcs > entry_data_value_funcs = > entry_data_value_free_closure > }; > > -/* Read parameter of TYPE at (callee) FRAME's function entry. KIND > and KIND_U > - are used to match DW_AT_location at the caller's > - DW_TAG_call_site_parameter. > - > - Function always returns non-NULL value. It throws > NO_ENTRY_VALUE_ERROR if it > - cannot resolve the parameter for any reason. */ > - > -static struct value * > +/* See dwarf2/loc.h. */ > +struct value * > value_of_dwarf_reg_entry (struct type *type, frame_info_ptr frame, > enum call_site_parameter_kind kind, > union call_site_parameter_u kind_u) > diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h > index d6709f2e342..9156e1ee533 100644 > --- a/gdb/dwarf2/loc.h > +++ b/gdb/dwarf2/loc.h > @@ -296,4 +296,15 @@ extern struct value *indirect_synthetic_pointer > dwarf2_per_objfile *per_objfile, frame_info_ptr frame, > struct type *type, bool resolve_abstract_p = false); > > +/* Read parameter of TYPE at (callee) FRAME's function entry. KIND > and KIND_U > + are used to match DW_AT_location at the caller's > + DW_TAG_call_site_parameter. > + > + Function always returns non-NULL value. It throws > NO_ENTRY_VALUE_ERROR if > + it cannot resolve the parameter for any reason. */ > + > +extern struct value *value_of_dwarf_reg_entry (struct type *type, > + struct frame_info_ptr > frame, > + enum > call_site_parameter_kind kind, > + union > call_site_parameter_u kind_u); > #endif /* DWARF2LOC_H */ > diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py > index 46e7565f293..325801256a2 100644 > --- a/gdb/gdbarch-components.py > +++ b/gdb/gdbarch-components.py > @@ -868,6 +868,21 @@ for instance). > invalid=True, > ) > > +Function( > + comment=""" > +Return the address at which the value being returned from > +the current function will be stored. This routine is only > +called if the current function uses the the "struct return > +convention". > + > +May return 0 when unable to determine that address.""", > + type="CORE_ADDR", > + name="get_return_buf_addr", > + params=[("struct type *", "val_type"), ("frame_info_ptr", > "cur_frame")], > + predefault="default_get_return_buf_addr", > + invalid=False, > +) > + > Method( > comment=""" > Return true if the return value of function is stored in the first > hidden > diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h > index 840de585869..66e2e761385 100644 > --- a/gdb/gdbarch-gen.h > +++ b/gdb/gdbarch-gen.h > @@ -441,6 +441,17 @@ typedef enum return_value_convention > (gdbarch_return_value_ftype) (struct gdbarc > extern enum return_value_convention gdbarch_return_value (struct > gdbarch *gdbarch, struct value *function, struct type *valtype, > struct regcache *regcache, gdb_byte *readbuf, const gdb_byte > *writebuf); > extern void set_gdbarch_return_value (struct gdbarch *gdbarch, > gdbarch_return_value_ftype *return_value); > > +/* Return the address at which the value being returned from > + the current function will be stored. This routine is only > + called if the current function uses the the "struct return > + convention". > + > + May return 0 when unable to determine that address. */ > + > +typedef CORE_ADDR (gdbarch_get_return_buf_addr_ftype) (struct type > *val_type, frame_info_ptr cur_frame); > +extern CORE_ADDR gdbarch_get_return_buf_addr (struct gdbarch > *gdbarch, struct type *val_type, frame_info_ptr cur_frame); > +extern void set_gdbarch_get_return_buf_addr (struct gdbarch > *gdbarch, gdbarch_get_return_buf_addr_ftype *get_return_buf_addr); > + > /* Return true if the return value of function is stored in the > first hidden > parameter. In theory, this feature should be language-dependent, > specified > by language and its ABI, such as C++. Unfortunately, compiler > may > diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c > index 559e92dee58..3bbb70e7be9 100644 > --- a/gdb/gdbarch.c > +++ b/gdb/gdbarch.c > @@ -116,6 +116,7 @@ struct gdbarch > gdbarch_address_to_pointer_ftype *address_to_pointer = nullptr; > gdbarch_integer_to_address_ftype *integer_to_address = nullptr; > gdbarch_return_value_ftype *return_value = nullptr; > + gdbarch_get_return_buf_addr_ftype *get_return_buf_addr = nullptr; > gdbarch_return_in_first_hidden_param_p_ftype > *return_in_first_hidden_param_p = nullptr; > gdbarch_skip_prologue_ftype *skip_prologue = nullptr; > gdbarch_skip_main_prologue_ftype *skip_main_prologue = nullptr; > @@ -313,6 +314,7 @@ gdbarch_alloc (const struct gdbarch_info *info, > gdbarch->value_from_register = default_value_from_register; > gdbarch->pointer_to_address = unsigned_pointer_to_address; > gdbarch->address_to_pointer = unsigned_address_to_pointer; > + gdbarch->get_return_buf_addr = default_get_return_buf_addr; > gdbarch->return_in_first_hidden_param_p = > default_return_in_first_hidden_param_p; > gdbarch->breakpoint_from_pc = default_breakpoint_from_pc; > gdbarch->sw_breakpoint_from_kind = NULL; > @@ -465,6 +467,7 @@ verify_gdbarch (struct gdbarch *gdbarch) > /* Skip verify of address_to_pointer, invalid_p == 0 */ > /* Skip verify of integer_to_address, has predicate. */ > /* Skip verify of return_value, has predicate. */ > + /* Skip verify of get_return_buf_addr, invalid_p == 0 */ > /* Skip verify of return_in_first_hidden_param_p, invalid_p == 0 > */ > if (gdbarch->skip_prologue == 0) > log.puts ("\n\tskip_prologue"); > @@ -877,6 +880,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct > ui_file *file) > gdb_printf (file, > "gdbarch_dump: return_value = <%s>\n", > host_address_to_string (gdbarch- > >return_value)); > + gdb_printf (file, > + "gdbarch_dump: get_return_buf_addr = <%s>\n", > + host_address_to_string (gdbarch- > >get_return_buf_addr)); > gdb_printf (file, > "gdbarch_dump: return_in_first_hidden_param_p > = <%s>\n", > host_address_to_string (gdbarch- > >return_in_first_hidden_param_p)); > @@ -2684,6 +2690,23 @@ set_gdbarch_return_value (struct gdbarch > *gdbarch, > gdbarch->return_value = return_value; > } > > +CORE_ADDR > +gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type > *val_type, frame_info_ptr cur_frame) > +{ > + gdb_assert (gdbarch != NULL); > + gdb_assert (gdbarch->get_return_buf_addr != NULL); > + if (gdbarch_debug >= 2) > + gdb_printf (gdb_stdlog, "gdbarch_get_return_buf_addr called\n"); > + return gdbarch->get_return_buf_addr (val_type, cur_frame); > +} > + > +void > +set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, > + gdbarch_get_return_buf_addr_ftype > get_return_buf_addr) > +{ > + gdbarch->get_return_buf_addr = get_return_buf_addr; > +} > + > int > gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch, > struct type *type) > { > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index d729732c81c..2e7cca1a1aa 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -55,6 +55,7 @@ > #include "gdbsupport/gdb_optional.h" > #include "source.h" > #include "cli/cli-style.h" > +#include "dwarf2/loc.h" > > /* Local functions: */ > > @@ -1598,6 +1599,12 @@ struct finish_command_fsm : public thread_fsm > return value. */ > struct return_value_info return_value_info {}; > > + /* If the current function uses the "struct return convention", > + this holds the address at which the value being returned will > + be stored, or zero if that address could not be determined or > + the "struct return convention" is not being used. */ > + CORE_ADDR return_buf; > + > explicit finish_command_fsm (struct interp *cmd_interp) > : thread_fsm (cmd_interp) > { > @@ -1636,7 +1643,13 @@ finish_command_fsm::should_stop (struct > thread_info *tp) > struct value *func; > > func = read_var_value (function, NULL, get_current_frame ()); > - rv->value = get_return_value (function, func); > + > + if (return_buf != 0) > + /* Retrieve return value from the buffer where it was > saved. */ > + rv->value = value_at (rv->type, return_buf); > + else > + rv->value = get_return_value (function, func); > + > if (rv->value != NULL) > rv->value_history_index = record_latest_value (rv->value); > } > @@ -1852,8 +1865,28 @@ finish_command (const char *arg, int from_tty) > } > > /* Find the function we will return from. */ > - > - sm->function = find_pc_function (get_frame_pc (get_selected_frame > (NULL))); > + frame_info_ptr callee_frame = get_selected_frame (NULL); > + sm->function = find_pc_function (get_frame_pc (callee_frame)); > + > + /* Determine the return convention. If it is > RETURN_VALUE_STRUCT_CONVENTION, > + attempt to determine the address of the return buffer. */ > + enum return_value_convention return_value; > + struct gdbarch *gdbarch = get_frame_arch (callee_frame); > + > + struct type * val_type > + = check_typedef (sm->function->type()->target_type ()); > + > + return_value = gdbarch_return_value (gdbarch, > + read_var_value (sm->function, > NULL, > + callee_frame), > + val_type, NULL, NULL, NULL); > + > + if (return_value == RETURN_VALUE_STRUCT_CONVENTION > + && val_type->code() != TYPE_CODE_VOID) > + sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type, > + callee_frame); > + else > + sm->return_buf = 0; > > /* Print info on the selected frame, including level number but > not > source. */ > @@ -1871,7 +1904,7 @@ finish_command (const char *arg, int from_tty) > gdb_printf (_("Run till exit from ")); > } > > - print_stack_frame (get_selected_frame (NULL), 1, LOCATION, 0); > + print_stack_frame (callee_frame, 1, LOCATION, 0); > } > frame.reinflate (); > > diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c > index 14effb93210..f720e87e63b 100644 > --- a/gdb/ppc-sysv-tdep.c > +++ b/gdb/ppc-sysv-tdep.c > @@ -28,6 +28,7 @@ > #include "objfiles.h" > #include "infcall.h" > #include "dwarf2.h" > +#include "dwarf2/loc.h" > #include "target-float.h" > #include <algorithm> > > @@ -2156,3 +2157,43 @@ ppc64_sysv_abi_return_value (struct gdbarch > *gdbarch, struct value *function, > return RETURN_VALUE_STRUCT_CONVENTION; > } > > +CORE_ADDR ppc64_sysv_get_return_buf_addr (struct type *val_type, > + frame_info_ptr cur_frame) > +{ > + /* The PowerPC ABI specifies aggregates that are not returned by > value > + are returned in a storage buffer provided by the caller. The > + address of the storage buffer is provided as a hidden first > input > + arguement in register r3. The PowerPC ABI does not guarantee > that > + register r3 will not be changed while executing the > function. Hence, it > + cannot be assumed that r3 will still contain the address of the > storage > + buffer when execution reaches the end of the function. > + > + This function attempts to determine the value of r3 on entry to > the > + function using the DW_OP_entry_value DWARF entries. This > requires > + compiling the user program with -fvar-tracking to resolve the > + DW_TAG_call_sites in the binary file. */ > + > + union call_site_parameter_u kind_u; > + enum call_site_parameter_kind kind; > + CORE_ADDR return_val = 0; > + > + kind_u.dwarf_reg = 3; /* First passed arg/return value is in > r3. */ > + kind = CALL_SITE_PARAMETER_DWARF_REG; > + > + /* val_type is the type of the return value. Need the pointer > type > + to the return value. */ > + val_type = lookup_pointer_type (val_type); > + > + try > + { > + return_val = value_as_address (value_of_dwarf_reg_entry > (val_type, > + cur_fram > e, > + kind, > kind_u)); > + } > + catch (const gdb_exception_error &e) > + { > + warning ("Cannot determine the function return value.\n" > + "Try compiling with -fvar-tracking."); > + } > + return return_val; > +} > diff --git a/gdb/ppc-tdep.h b/gdb/ppc-tdep.h > index 34b250849b9..cbccd87820d 100644 > --- a/gdb/ppc-tdep.h > +++ b/gdb/ppc-tdep.h > @@ -175,6 +175,8 @@ extern void ppc_collect_vsxregset (const struct > regset *regset, > const struct regcache *regcache, > int regnum, void *vsxregs, size_t > len); > > +extern CORE_ADDR ppc64_sysv_get_return_buf_addr (type*, > frame_info_ptr); > + > /* Private data that this module attaches to struct gdbarch. */ > > /* ELF ABI version used by the inferior. */ > diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c > index 8b6d666bbe7..637cbb41651 100644 > --- a/gdb/rs6000-tdep.c > +++ b/gdb/rs6000-tdep.c > @@ -8234,7 +8234,11 @@ rs6000_gdbarch_init (struct gdbarch_info info, > struct gdbarch_list *arches) > set_gdbarch_ps_regnum (gdbarch, tdep->ppc_ps_regnum); > > if (wordsize == 8) > - set_gdbarch_return_value (gdbarch, ppc64_sysv_abi_return_value); > + { > + set_gdbarch_return_value (gdbarch, > ppc64_sysv_abi_return_value); > + set_gdbarch_get_return_buf_addr (gdbarch, > + ppc64_sysv_get_return_buf_addr); > + } > else > set_gdbarch_return_value (gdbarch, ppc_sysv_abi_return_value); > > diff --git a/gdb/testsuite/gdb.cp/non-trivial-retval.exp > b/gdb/testsuite/gdb.cp/non-trivial-retval.exp > index 21c390bc937..93a3a6832f7 100644 > --- a/gdb/testsuite/gdb.cp/non-trivial-retval.exp > +++ b/gdb/testsuite/gdb.cp/non-trivial-retval.exp > @@ -15,11 +15,18 @@ > > # This file is part of the gdb testsuite > > +set additional_flags "" > + > if {[skip_cplus_tests]} { continue } > > standard_testfile .cc > > -if {[prepare_for_testing "failed to prepare" $testfile $srcfile > {debug c++}]} { > +if {[have_fvar_tracking]} { > + set additional_flags "additional_flags= -fvar-tracking" > +} > + > +if {[prepare_for_testing "failed to prepare" $testfile $srcfile > [list debug c++ $additional_flags]]} { > + > return -1 > } > > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp > index bfa9fec628e..d3b77b4869b 100644 > --- a/gdb/testsuite/lib/gdb.exp > +++ b/gdb/testsuite/lib/gdb.exp > @@ -8582,6 +8582,14 @@ gdb_caching_proc have_fuse_ld_gold { > return [gdb_simple_compile $me $src executable $flags] > } > > +# Return 1 if compiler supports fvar-tracking, otherwise return 0. > +gdb_caching_proc have_fvar_tracking { > + set me "have_fvar_tracking" > + set flags "additional_flags=-fvar-tracking" > + set src { int main() { return 0; } } > + return [gdb_simple_compile $me $src executable $flags] > +} > + > # Return 1 if linker supports -Ttext-segment, otherwise return 0. > gdb_caching_proc linker_supports_Ttext_segment_flag { > set me "linker_supports_Ttext_segment_flag" ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/2 ver 2] PowerPC, fix support for printing the function return value for non-trivial values. 2022-10-18 18:55 ` [PATCH 2/2 ver 2] " Carl Love 2022-10-31 16:07 ` Carl Love @ 2022-11-07 14:56 ` Bruno Larsen 2022-11-07 19:53 ` Carl Love 2022-11-07 20:04 ` [PATCH 2/2 ver 3] " Carl Love 2 siblings, 1 reply; 44+ messages in thread From: Bruno Larsen @ 2022-11-07 14:56 UTC (permalink / raw) To: Carl Love, Ulrich Weigand, gdb-patches; +Cc: will_schmidt, Kevin Buettner Cheers, Bruno On 18/10/2022 20:55, Carl Love wrote: > GDB maintainers: > > Version 2, updated comments per Kevin's comments. Changed "frame_info > *" to frame_info_ptr per Bruno's comments. Discussed supressing the > new warning after printing it with Kevin. In the end we decided to not > suppress the warnings. Suppression can be added later if deemed > necessary. The white space introduced by the gdb/gdbarch-components.py > needs to be addressed by a future patch to fix the script and the > generated files. > > On PowerPC, a non-trivial return value for a function cannot be > reliably determined with the current PowerPC ABI. The issue is the > address of the return buffer for a non-trivial value is passed to the > function in register r3. However, at the end of the function the ABI > does not guarantee r3 will not be changed. Hence it cannot be reliably > used to get the address of the return buffer. > > This patch adds a new GDB ABI to allow GDB to obtain the input value of > r3 when the function returns. This is done by using the DW_OP_entry > value for the DWARF entries to obtain the value of r3 on entry to the > function. The ABI allows GDB to get the correct address for the return > buffer on exit from the function. > > This patch fixes the five test failures in gdb.cp/non-trivial- > retval.exp. > > The patch has been re-tested on PowerPC and X86-64 with no regression > failures. > > Please let me know if this version of the patch is acceptable for GDB > mainline. Thanks. > > Carl Love This whole area of code is pure sorcery for me, but I do see that the patch fails to apply because of conflicts on gdbarch.c, due to Tom Tromey's "Inline initialization of gdbarch members" patch, and some style nits inlined. Sorry for taking so long to reply to this that it is not applying again, but in the future I wouldn't wait for my review if someone approves the patch. > > ----------------------------------- > PowerPC, fix support for printing the function return value for non-trivial values. > > Currently, a non-trivial return value from a function cannot currently be > reliably determined on PowerPC. This is due to the fact that the PowerPC > ABI uses register r3 to store the address of the buffer containing the > non-trivial return value when the function is called. The PowerPC ABI > does not guarantee the value in register r3 is not modified in the > function. Thus the value in r3 cannot be reliably used to obtain the > return addreses on exit from the function. > > This patch adds a new gdbarch method to allow PowerPC to access the value > of r3 on entry to a function. On PowerPC, the new gdbarch method attempts > to use the DW_OP_entry_value for the DWARF entries, when exiting the > function, to determine the value of r3 on entry to the function. This > requires the use of the -fvar-tracking compiler option to compile the > user application thus generating the DW_OP_entry_value in the binary. The > DW_OP_entry_value entries in the binary file allows GDB to resolve the > DW_TAG_call_site entries. This new gdbarch method is used to get the > return buffer address, in the case of a function returning a nontrivial > data type, on exit from the function. The GDB function should_stop checks > to see if RETURN_BUF is non-zero. By default, RETURN_BUF will be set to > zero by the new gdbarch method call for all architectures except PowerPC. > The get_return_value function will be used to obtain the return value on > all other architectures as is currently being done if RETURN_BUF is zero. > On PowerPC, the new gdbarch method will return a nonzero address in > RETURN_BUF if the value can be determined. The value_at function uses the > return buffer address to get the return value. > > This patch fixes five testcase failures in gdb.cp/non-trivial-retval.exp. > The correct function return values are now reported. > > Note this patch is dependent on patch: "PowerPC, function > ppc64_sysv_abi_return_value add missing return value convention". > > This patch has been tested on Power 10 and x86-64 with no regressions. > > Signed-off-by: Carl Love <cel@us.ibm.com> > --- > gdb/arch-utils.c | 6 +++ > gdb/arch-utils.h | 4 ++ > gdb/dwarf2/loc.c | 10 +---- > gdb/dwarf2/loc.h | 11 ++++++ > gdb/gdbarch-components.py | 15 ++++++++ > gdb/gdbarch-gen.h | 11 ++++++ > gdb/gdbarch.c | 23 ++++++++++++ > gdb/infcmd.c | 41 +++++++++++++++++++-- > gdb/ppc-sysv-tdep.c | 41 +++++++++++++++++++++ > gdb/ppc-tdep.h | 2 + > gdb/rs6000-tdep.c | 6 ++- > gdb/testsuite/gdb.cp/non-trivial-retval.exp | 9 ++++- > gdb/testsuite/lib/gdb.exp | 8 ++++ > 13 files changed, 173 insertions(+), 14 deletions(-) > > diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c > index 6a72b3f5efd..1358987c02f 100644 > --- a/gdb/arch-utils.c > +++ b/gdb/arch-utils.c > @@ -1092,6 +1092,12 @@ default_read_core_file_mappings > { > } > > +CORE_ADDR > +default_get_return_buf_addr (struct type *val_type, frame_info_ptr cur_frame) > +{ > + return 0; > +} > + > /* Non-zero if we want to trace architecture code. */ > > #ifndef GDBARCH_DEBUG > diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h > index f6229f434fd..46018a6fbbb 100644 > --- a/gdb/arch-utils.h > +++ b/gdb/arch-utils.h > @@ -300,4 +300,8 @@ extern void default_read_core_file_mappings > struct bfd *cbfd, > read_core_file_mappings_pre_loop_ftype pre_loop_cb, > read_core_file_mappings_loop_ftype loop_cb); > + > +/* Default implementation of gdbarch default_get_return_buf_addr method. */ > +extern CORE_ADDR default_get_return_buf_addr (struct type *val_typegdbarch, > + frame_info_ptr cur_frame); > #endif /* ARCH_UTILS_H */ > diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c > index 791648d6e7e..b88226db610 100644 > --- a/gdb/dwarf2/loc.c > +++ b/gdb/dwarf2/loc.c > @@ -1325,14 +1325,8 @@ static const struct lval_funcs entry_data_value_funcs = > entry_data_value_free_closure > }; > > -/* Read parameter of TYPE at (callee) FRAME's function entry. KIND and KIND_U > - are used to match DW_AT_location at the caller's > - DW_TAG_call_site_parameter. > - > - Function always returns non-NULL value. It throws NO_ENTRY_VALUE_ERROR if it > - cannot resolve the parameter for any reason. */ > - > -static struct value * > +/* See dwarf2/loc.h. */ > +struct value * > value_of_dwarf_reg_entry (struct type *type, frame_info_ptr frame, > enum call_site_parameter_kind kind, > union call_site_parameter_u kind_u) > diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h > index d6709f2e342..9156e1ee533 100644 > --- a/gdb/dwarf2/loc.h > +++ b/gdb/dwarf2/loc.h > @@ -296,4 +296,15 @@ extern struct value *indirect_synthetic_pointer > dwarf2_per_objfile *per_objfile, frame_info_ptr frame, > struct type *type, bool resolve_abstract_p = false); > > +/* Read parameter of TYPE at (callee) FRAME's function entry. KIND and KIND_U > + are used to match DW_AT_location at the caller's > + DW_TAG_call_site_parameter. > + > + Function always returns non-NULL value. It throws NO_ENTRY_VALUE_ERROR if > + it cannot resolve the parameter for any reason. */ > + > +extern struct value *value_of_dwarf_reg_entry (struct type *type, > + struct frame_info_ptr frame, > + enum call_site_parameter_kind kind, > + union call_site_parameter_u kind_u); > #endif /* DWARF2LOC_H */ > diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py > index 46e7565f293..325801256a2 100644 > --- a/gdb/gdbarch-components.py > +++ b/gdb/gdbarch-components.py > @@ -868,6 +868,21 @@ for instance). > invalid=True, > ) > > +Function( > + comment=""" > +Return the address at which the value being returned from > +the current function will be stored. This routine is only > +called if the current function uses the the "struct return > +convention". > + > +May return 0 when unable to determine that address.""", > + type="CORE_ADDR", > + name="get_return_buf_addr", > + params=[("struct type *", "val_type"), ("frame_info_ptr", "cur_frame")], > + predefault="default_get_return_buf_addr", > + invalid=False, > +) > + > Method( > comment=""" > Return true if the return value of function is stored in the first hidden > diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h > index 840de585869..66e2e761385 100644 > --- a/gdb/gdbarch-gen.h > +++ b/gdb/gdbarch-gen.h > @@ -441,6 +441,17 @@ typedef enum return_value_convention (gdbarch_return_value_ftype) (struct gdbarc > extern enum return_value_convention gdbarch_return_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf); > extern void set_gdbarch_return_value (struct gdbarch *gdbarch, gdbarch_return_value_ftype *return_value); > > +/* Return the address at which the value being returned from > + the current function will be stored. This routine is only > + called if the current function uses the the "struct return > + convention". > + > + May return 0 when unable to determine that address. */ > + > +typedef CORE_ADDR (gdbarch_get_return_buf_addr_ftype) (struct type *val_type, frame_info_ptr cur_frame); > +extern CORE_ADDR gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type *val_type, frame_info_ptr cur_frame); > +extern void set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, gdbarch_get_return_buf_addr_ftype *get_return_buf_addr); > + > /* Return true if the return value of function is stored in the first hidden > parameter. In theory, this feature should be language-dependent, specified > by language and its ABI, such as C++. Unfortunately, compiler may > diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c > index 559e92dee58..3bbb70e7be9 100644 > --- a/gdb/gdbarch.c > +++ b/gdb/gdbarch.c > @@ -116,6 +116,7 @@ struct gdbarch > gdbarch_address_to_pointer_ftype *address_to_pointer = nullptr; > gdbarch_integer_to_address_ftype *integer_to_address = nullptr; > gdbarch_return_value_ftype *return_value = nullptr; > + gdbarch_get_return_buf_addr_ftype *get_return_buf_addr = nullptr; > gdbarch_return_in_first_hidden_param_p_ftype *return_in_first_hidden_param_p = nullptr; > gdbarch_skip_prologue_ftype *skip_prologue = nullptr; > gdbarch_skip_main_prologue_ftype *skip_main_prologue = nullptr; > @@ -313,6 +314,7 @@ gdbarch_alloc (const struct gdbarch_info *info, > gdbarch->value_from_register = default_value_from_register; > gdbarch->pointer_to_address = unsigned_pointer_to_address; > gdbarch->address_to_pointer = unsigned_address_to_pointer; > + gdbarch->get_return_buf_addr = default_get_return_buf_addr; > gdbarch->return_in_first_hidden_param_p = default_return_in_first_hidden_param_p; > gdbarch->breakpoint_from_pc = default_breakpoint_from_pc; > gdbarch->sw_breakpoint_from_kind = NULL; > @@ -465,6 +467,7 @@ verify_gdbarch (struct gdbarch *gdbarch) > /* Skip verify of address_to_pointer, invalid_p == 0 */ > /* Skip verify of integer_to_address, has predicate. */ > /* Skip verify of return_value, has predicate. */ > + /* Skip verify of get_return_buf_addr, invalid_p == 0 */ > /* Skip verify of return_in_first_hidden_param_p, invalid_p == 0 */ > if (gdbarch->skip_prologue == 0) > log.puts ("\n\tskip_prologue"); > @@ -877,6 +880,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file) > gdb_printf (file, > "gdbarch_dump: return_value = <%s>\n", > host_address_to_string (gdbarch->return_value)); > + gdb_printf (file, > + "gdbarch_dump: get_return_buf_addr = <%s>\n", > + host_address_to_string (gdbarch->get_return_buf_addr)); > gdb_printf (file, > "gdbarch_dump: return_in_first_hidden_param_p = <%s>\n", > host_address_to_string (gdbarch->return_in_first_hidden_param_p)); > @@ -2684,6 +2690,23 @@ set_gdbarch_return_value (struct gdbarch *gdbarch, > gdbarch->return_value = return_value; > } > > +CORE_ADDR > +gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type *val_type, frame_info_ptr cur_frame) > +{ > + gdb_assert (gdbarch != NULL); > + gdb_assert (gdbarch->get_return_buf_addr != NULL); > + if (gdbarch_debug >= 2) > + gdb_printf (gdb_stdlog, "gdbarch_get_return_buf_addr called\n"); > + return gdbarch->get_return_buf_addr (val_type, cur_frame); > +} > + > +void > +set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, > + gdbarch_get_return_buf_addr_ftype get_return_buf_addr) > +{ > + gdbarch->get_return_buf_addr = get_return_buf_addr; > +} > + > int > gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch, struct type *type) > { > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index d729732c81c..2e7cca1a1aa 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -55,6 +55,7 @@ > #include "gdbsupport/gdb_optional.h" > #include "source.h" > #include "cli/cli-style.h" > +#include "dwarf2/loc.h" > > /* Local functions: */ > > @@ -1598,6 +1599,12 @@ struct finish_command_fsm : public thread_fsm > return value. */ > struct return_value_info return_value_info {}; > > + /* If the current function uses the "struct return convention", > + this holds the address at which the value being returned will > + be stored, or zero if that address could not be determined or > + the "struct return convention" is not being used. */ > + CORE_ADDR return_buf; > + > explicit finish_command_fsm (struct interp *cmd_interp) > : thread_fsm (cmd_interp) > { > @@ -1636,7 +1643,13 @@ finish_command_fsm::should_stop (struct thread_info *tp) > struct value *func; > > func = read_var_value (function, NULL, get_current_frame ()); > - rv->value = get_return_value (function, func); > + > + if (return_buf != 0) > + /* Retrieve return value from the buffer where it was saved. */ > + rv->value = value_at (rv->type, return_buf); > + else > + rv->value = get_return_value (function, func); > + > if (rv->value != NULL) > rv->value_history_index = record_latest_value (rv->value); > } > @@ -1852,8 +1865,28 @@ finish_command (const char *arg, int from_tty) > } > > /* Find the function we will return from. */ > - > - sm->function = find_pc_function (get_frame_pc (get_selected_frame (NULL))); > + frame_info_ptr callee_frame = get_selected_frame (NULL); > + sm->function = find_pc_function (get_frame_pc (callee_frame)); > + > + /* Determine the return convention. If it is RETURN_VALUE_STRUCT_CONVENTION, > + attempt to determine the address of the return buffer. */ > + enum return_value_convention return_value; > + struct gdbarch *gdbarch = get_frame_arch (callee_frame); > + > + struct type * val_type > + = check_typedef (sm->function->type()->target_type ()); missing space before (. > + > + return_value = gdbarch_return_value (gdbarch, > + read_var_value (sm->function, NULL, > + callee_frame), > + val_type, NULL, NULL, NULL); > + > + if (return_value == RETURN_VALUE_STRUCT_CONVENTION > + && val_type->code() != TYPE_CODE_VOID) same here. > + sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type, > + callee_frame); > + else > + sm->return_buf = 0; > > /* Print info on the selected frame, including level number but not > source. */ > @@ -1871,7 +1904,7 @@ finish_command (const char *arg, int from_tty) > gdb_printf (_("Run till exit from ")); > } > > - print_stack_frame (get_selected_frame (NULL), 1, LOCATION, 0); > + print_stack_frame (callee_frame, 1, LOCATION, 0); > } > frame.reinflate (); > > diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c > index 14effb93210..f720e87e63b 100644 > --- a/gdb/ppc-sysv-tdep.c > +++ b/gdb/ppc-sysv-tdep.c > @@ -28,6 +28,7 @@ > #include "objfiles.h" > #include "infcall.h" > #include "dwarf2.h" > +#include "dwarf2/loc.h" > #include "target-float.h" > #include <algorithm> > > @@ -2156,3 +2157,43 @@ ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, > return RETURN_VALUE_STRUCT_CONVENTION; > } > > +CORE_ADDR ppc64_sysv_get_return_buf_addr (struct type *val_type, > + frame_info_ptr cur_frame) > +{ > + /* The PowerPC ABI specifies aggregates that are not returned by value > + are returned in a storage buffer provided by the caller. The > + address of the storage buffer is provided as a hidden first input > + arguement in register r3. The PowerPC ABI does not guarantee that > + register r3 will not be changed while executing the function. Hence, it > + cannot be assumed that r3 will still contain the address of the storage > + buffer when execution reaches the end of the function. > + > + This function attempts to determine the value of r3 on entry to the > + function using the DW_OP_entry_value DWARF entries. This requires > + compiling the user program with -fvar-tracking to resolve the > + DW_TAG_call_sites in the binary file. */ > + > + union call_site_parameter_u kind_u; > + enum call_site_parameter_kind kind; > + CORE_ADDR return_val = 0; > + > + kind_u.dwarf_reg = 3; /* First passed arg/return value is in r3. */ > + kind = CALL_SITE_PARAMETER_DWARF_REG; > + > + /* val_type is the type of the return value. Need the pointer type > + to the return value. */ > + val_type = lookup_pointer_type (val_type); > + > + try > + { > + return_val = value_as_address (value_of_dwarf_reg_entry (val_type, > + cur_frame, > + kind, kind_u)); > + } > + catch (const gdb_exception_error &e) > + { > + warning ("Cannot determine the function return value.\n" > + "Try compiling with -fvar-tracking."); > + } > + return return_val; > +} > diff --git a/gdb/ppc-tdep.h b/gdb/ppc-tdep.h > index 34b250849b9..cbccd87820d 100644 > --- a/gdb/ppc-tdep.h > +++ b/gdb/ppc-tdep.h > @@ -175,6 +175,8 @@ extern void ppc_collect_vsxregset (const struct regset *regset, > const struct regcache *regcache, > int regnum, void *vsxregs, size_t len); > > +extern CORE_ADDR ppc64_sysv_get_return_buf_addr (type*, frame_info_ptr); > + > /* Private data that this module attaches to struct gdbarch. */ > > /* ELF ABI version used by the inferior. */ > diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c > index 8b6d666bbe7..637cbb41651 100644 > --- a/gdb/rs6000-tdep.c > +++ b/gdb/rs6000-tdep.c > @@ -8234,7 +8234,11 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) > set_gdbarch_ps_regnum (gdbarch, tdep->ppc_ps_regnum); > > if (wordsize == 8) > - set_gdbarch_return_value (gdbarch, ppc64_sysv_abi_return_value); > + { > + set_gdbarch_return_value (gdbarch, ppc64_sysv_abi_return_value); > + set_gdbarch_get_return_buf_addr (gdbarch, > + ppc64_sysv_get_return_buf_addr); > + } > else > set_gdbarch_return_value (gdbarch, ppc_sysv_abi_return_value); > > diff --git a/gdb/testsuite/gdb.cp/non-trivial-retval.exp b/gdb/testsuite/gdb.cp/non-trivial-retval.exp > index 21c390bc937..93a3a6832f7 100644 > --- a/gdb/testsuite/gdb.cp/non-trivial-retval.exp > +++ b/gdb/testsuite/gdb.cp/non-trivial-retval.exp > @@ -15,11 +15,18 @@ > > # This file is part of the gdb testsuite > > +set additional_flags "" > + > if {[skip_cplus_tests]} { continue } > > standard_testfile .cc > > -if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} { > +if {[have_fvar_tracking]} { > + set additional_flags "additional_flags= -fvar-tracking" > +} > + > +if {[prepare_for_testing "failed to prepare" $testfile $srcfile [list debug c++ $additional_flags]]} { > + > return -1 > } > > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp > index bfa9fec628e..d3b77b4869b 100644 > --- a/gdb/testsuite/lib/gdb.exp > +++ b/gdb/testsuite/lib/gdb.exp > @@ -8582,6 +8582,14 @@ gdb_caching_proc have_fuse_ld_gold { > return [gdb_simple_compile $me $src executable $flags] > } > > +# Return 1 if compiler supports fvar-tracking, otherwise return 0. > +gdb_caching_proc have_fvar_tracking { > + set me "have_fvar_tracking" > + set flags "additional_flags=-fvar-tracking" > + set src { int main() { return 0; } } > + return [gdb_simple_compile $me $src executable $flags] > +} > + > # Return 1 if linker supports -Ttext-segment, otherwise return 0. > gdb_caching_proc linker_supports_Ttext_segment_flag { > set me "linker_supports_Ttext_segment_flag" ^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH 2/2 ver 2] PowerPC, fix support for printing the function return value for non-trivial values. 2022-11-07 14:56 ` Bruno Larsen @ 2022-11-07 19:53 ` Carl Love 0 siblings, 0 replies; 44+ messages in thread From: Carl Love @ 2022-11-07 19:53 UTC (permalink / raw) To: Bruno Larsen, Ulrich Weigand, gdb-patches; +Cc: will_schmidt, Kevin Buettner Bruno: On Mon, 2022-11-07 at 15:56 +0100, Bruno Larsen wrote: > This whole area of code is pure sorcery for me, but I do see that > the > patch fails to apply because of conflicts on gdbarch.c, due to Tom > Tromey's "Inline initialization of gdbarch members" patch, and some > style nits inlined. > > Sorry for taking so long to reply to this that it is not applying > again, > but in the future I wouldn't wait for my review if someone approves > the > patch. Thanks for the reply. I was hoping that Kevin would also let me know if all of his concerns had been addressed. I did fix up the two formatting issues you mentioned, rebased and retested the patch on the latest gdb source code tree. I will post version 3 of the patch and ping version 2 of patch 1/2 and see if we can get a maintainer to review the patches. Thanks for all the help and input. Carl ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/2 ver 3] PowerPC, fix support for printing the function return value for non-trivial values. 2022-10-18 18:55 ` [PATCH 2/2 ver 2] " Carl Love 2022-10-31 16:07 ` Carl Love 2022-11-07 14:56 ` Bruno Larsen @ 2022-11-07 20:04 ` Carl Love 2022-11-14 16:47 ` Ulrich Weigand 2 siblings, 1 reply; 44+ messages in thread From: Carl Love @ 2022-11-07 20:04 UTC (permalink / raw) To: Ulrich Weigand, gdb-patches; +Cc: will_schmidt, blarsen, Kevin Buettner, cel GDB maintainers: Version 3, fixed two formatting issues noted by Bruno. Patch was rebased on the latest gdb source tree and retested with no regression failures. Version 2, updated comments per Kevin's comments. Changed "frame_info *" to frame_info_ptr per Bruno's comments. Discussed supressing the new warning after printing it with Kevin. In the end we decided to not suppress the warnings. Suppression can be added later if deemed necessary. The white space introduced by the gdb/gdbarch- components.py needs to be addressed by a future patch to fix the script and the generated files. On PowerPC, a non-trivial return value for a function cannot be reliably determined with the current PowerPC ABI. The issue is the address of the return buffer for a non-trivial value is passed to the function in register r3. However, at the end of the function the ABI does not guarantee r3 will not be changed. Hence it cannot be reliably used to get the address of the return buffer. This patch adds a new GDB ABI to allow GDB to obtain the input value of r3 when the function returns. This is done by using the DW_OP_entry value for the DWARF entries to obtain the value of r3 on entry to the function. The ABI allows GDB to get the correct address for the return buffer on exit from the function. This patch fixes the five test failures in gdb.cp/non-trivial- retval.exp. The patch has been re-tested on PowerPC and X86-64 with no regression failures. Please let me know if this version of the patch is acceptable for GDB mainline. Thanks. Carl Love ----------------------------------- PowerPC, fix support for printing the function return value for non-trivial values. Currently, a non-trivial return value from a function cannot currently be reliably determined on PowerPC. This is due to the fact that the PowerPC ABI uses register r3 to store the address of the buffer containing the non-trivial return value when the function is called. The PowerPC ABI does not guarantee the value in register r3 is not modified in the function. Thus the value in r3 cannot be reliably used to obtain the return addreses on exit from the function. This patch adds a new gdbarch method to allow PowerPC to access the value of r3 on entry to a function. On PowerPC, the new gdbarch method attempts to use the DW_OP_entry_value for the DWARF entries, when exiting the function, to determine the value of r3 on entry to the function. This requires the use of the -fvar-tracking compiler option to compile the user application thus generating the DW_OP_entry_value in the binary. The DW_OP_entry_value entries in the binary file allows GDB to resolve the DW_TAG_call_site entries. This new gdbarch method is used to get the return buffer address, in the case of a function returning a nontrivial data type, on exit from the function. The GDB function should_stop checks to see if RETURN_BUF is non-zero. By default, RETURN_BUF will be set to zero by the new gdbarch method call for all architectures except PowerPC. The get_return_value function will be used to obtain the return value on all other architectures as is currently being done if RETURN_BUF is zero. On PowerPC, the new gdbarch method will return a nonzero address in RETURN_BUF if the value can be determined. The value_at function uses the return buffer address to get the return value. This patch fixes five testcase failures in gdb.cp/non-trivial-retval.exp. The correct function return values are now reported. Note this patch is dependent on patch: "PowerPC, function ppc64_sysv_abi_return_value add missing return value convention". This patch has been tested on Power 10 and x86-64 with no regressions. --- gdb/arch-utils.c | 6 +++ gdb/arch-utils.h | 4 ++ gdb/dwarf2/loc.c | 10 +---- gdb/dwarf2/loc.h | 11 ++++++ gdb/gdbarch-components.py | 15 ++++++++ gdb/gdbarch-gen.h | 11 ++++++ gdb/gdbarch.c | 22 +++++++++++ gdb/infcmd.c | 41 +++++++++++++++++++-- gdb/ppc-sysv-tdep.c | 41 +++++++++++++++++++++ gdb/ppc-tdep.h | 2 + gdb/rs6000-tdep.c | 6 ++- gdb/testsuite/gdb.cp/non-trivial-retval.exp | 9 ++++- gdb/testsuite/lib/gdb.exp | 8 ++++ 13 files changed, 172 insertions(+), 14 deletions(-) diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c index 5218bfc05e1..7b84daf046e 100644 --- a/gdb/arch-utils.c +++ b/gdb/arch-utils.c @@ -1084,6 +1084,12 @@ default_read_core_file_mappings { } +CORE_ADDR +default_get_return_buf_addr (struct type *val_type, frame_info_ptr cur_frame) +{ + return 0; +} + /* Non-zero if we want to trace architecture code. */ #ifndef GDBARCH_DEBUG diff --git a/gdb/arch-utils.h b/gdb/arch-utils.h index f6229f434fd..46018a6fbbb 100644 --- a/gdb/arch-utils.h +++ b/gdb/arch-utils.h @@ -300,4 +300,8 @@ extern void default_read_core_file_mappings struct bfd *cbfd, read_core_file_mappings_pre_loop_ftype pre_loop_cb, read_core_file_mappings_loop_ftype loop_cb); + +/* Default implementation of gdbarch default_get_return_buf_addr method. */ +extern CORE_ADDR default_get_return_buf_addr (struct type *val_typegdbarch, + frame_info_ptr cur_frame); #endif /* ARCH_UTILS_H */ diff --git a/gdb/dwarf2/loc.c b/gdb/dwarf2/loc.c index c42359ab96e..8355aa44333 100644 --- a/gdb/dwarf2/loc.c +++ b/gdb/dwarf2/loc.c @@ -1325,14 +1325,8 @@ static const struct lval_funcs entry_data_value_funcs = entry_data_value_free_closure }; -/* Read parameter of TYPE at (callee) FRAME's function entry. KIND and KIND_U - are used to match DW_AT_location at the caller's - DW_TAG_call_site_parameter. - - Function always returns non-NULL value. It throws NO_ENTRY_VALUE_ERROR if it - cannot resolve the parameter for any reason. */ - -static struct value * +/* See dwarf2/loc.h. */ +struct value * value_of_dwarf_reg_entry (struct type *type, frame_info_ptr frame, enum call_site_parameter_kind kind, union call_site_parameter_u kind_u) diff --git a/gdb/dwarf2/loc.h b/gdb/dwarf2/loc.h index d6709f2e342..9156e1ee533 100644 --- a/gdb/dwarf2/loc.h +++ b/gdb/dwarf2/loc.h @@ -296,4 +296,15 @@ extern struct value *indirect_synthetic_pointer dwarf2_per_objfile *per_objfile, frame_info_ptr frame, struct type *type, bool resolve_abstract_p = false); +/* Read parameter of TYPE at (callee) FRAME's function entry. KIND and KIND_U + are used to match DW_AT_location at the caller's + DW_TAG_call_site_parameter. + + Function always returns non-NULL value. It throws NO_ENTRY_VALUE_ERROR if + it cannot resolve the parameter for any reason. */ + +extern struct value *value_of_dwarf_reg_entry (struct type *type, + struct frame_info_ptr frame, + enum call_site_parameter_kind kind, + union call_site_parameter_u kind_u); #endif /* DWARF2LOC_H */ diff --git a/gdb/gdbarch-components.py b/gdb/gdbarch-components.py index b2c7b784761..9b688998a7b 100644 --- a/gdb/gdbarch-components.py +++ b/gdb/gdbarch-components.py @@ -868,6 +868,21 @@ for instance). invalid=True, ) +Function( + comment=""" +Return the address at which the value being returned from +the current function will be stored. This routine is only +called if the current function uses the the "struct return +convention". + +May return 0 when unable to determine that address.""", + type="CORE_ADDR", + name="get_return_buf_addr", + params=[("struct type *", "val_type"), ("frame_info_ptr", "cur_frame")], + predefault="default_get_return_buf_addr", + invalid=False, +) + Method( comment=""" Return true if the return value of function is stored in the first hidden diff --git a/gdb/gdbarch-gen.h b/gdb/gdbarch-gen.h index e0d7a08ff6a..a663316df16 100644 --- a/gdb/gdbarch-gen.h +++ b/gdb/gdbarch-gen.h @@ -441,6 +441,17 @@ typedef enum return_value_convention (gdbarch_return_value_ftype) (struct gdbarc extern enum return_value_convention gdbarch_return_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf); extern void set_gdbarch_return_value (struct gdbarch *gdbarch, gdbarch_return_value_ftype *return_value); +/* Return the address at which the value being returned from + the current function will be stored. This routine is only + called if the current function uses the the "struct return + convention". + + May return 0 when unable to determine that address. */ + +typedef CORE_ADDR (gdbarch_get_return_buf_addr_ftype) (struct type *val_type, frame_info_ptr cur_frame); +extern CORE_ADDR gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type *val_type, frame_info_ptr cur_frame); +extern void set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, gdbarch_get_return_buf_addr_ftype *get_return_buf_addr); + /* Return true if the return value of function is stored in the first hidden parameter. In theory, this feature should be language-dependent, specified by language and its ABI, such as C++. Unfortunately, compiler may diff --git a/gdb/gdbarch.c b/gdb/gdbarch.c index 9d929da6da9..3227e945880 100644 --- a/gdb/gdbarch.c +++ b/gdb/gdbarch.c @@ -116,6 +116,7 @@ struct gdbarch gdbarch_address_to_pointer_ftype *address_to_pointer = unsigned_address_to_pointer; gdbarch_integer_to_address_ftype *integer_to_address = nullptr; gdbarch_return_value_ftype *return_value = nullptr; + gdbarch_get_return_buf_addr_ftype *get_return_buf_addr = default_get_return_buf_addr; gdbarch_return_in_first_hidden_param_p_ftype *return_in_first_hidden_param_p = default_return_in_first_hidden_param_p; gdbarch_skip_prologue_ftype *skip_prologue = 0; gdbarch_skip_main_prologue_ftype *skip_main_prologue = nullptr; @@ -369,6 +370,7 @@ verify_gdbarch (struct gdbarch *gdbarch) /* Skip verify of address_to_pointer, invalid_p == 0 */ /* Skip verify of integer_to_address, has predicate. */ /* Skip verify of return_value, has predicate. */ + /* Skip verify of get_return_buf_addr, invalid_p == 0 */ /* Skip verify of return_in_first_hidden_param_p, invalid_p == 0 */ if (gdbarch->skip_prologue == 0) log.puts ("\n\tskip_prologue"); @@ -780,6 +782,9 @@ gdbarch_dump (struct gdbarch *gdbarch, struct ui_file *file) gdb_printf (file, "gdbarch_dump: return_value = <%s>\n", host_address_to_string (gdbarch->return_value)); + gdb_printf (file, + "gdbarch_dump: get_return_buf_addr = <%s>\n", + host_address_to_string (gdbarch->get_return_buf_addr)); gdb_printf (file, "gdbarch_dump: return_in_first_hidden_param_p = <%s>\n", host_address_to_string (gdbarch->return_in_first_hidden_param_p)); @@ -2587,6 +2592,23 @@ set_gdbarch_return_value (struct gdbarch *gdbarch, gdbarch->return_value = return_value; } +CORE_ADDR +gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, struct type *val_type, frame_info_ptr cur_frame) +{ + gdb_assert (gdbarch != NULL); + gdb_assert (gdbarch->get_return_buf_addr != NULL); + if (gdbarch_debug >= 2) + gdb_printf (gdb_stdlog, "gdbarch_get_return_buf_addr called\n"); + return gdbarch->get_return_buf_addr (val_type, cur_frame); +} + +void +set_gdbarch_get_return_buf_addr (struct gdbarch *gdbarch, + gdbarch_get_return_buf_addr_ftype get_return_buf_addr) +{ + gdbarch->get_return_buf_addr = get_return_buf_addr; +} + int gdbarch_return_in_first_hidden_param_p (struct gdbarch *gdbarch, struct type *type) { diff --git a/gdb/infcmd.c b/gdb/infcmd.c index c03ca103c91..ef00504969f 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -55,6 +55,7 @@ #include "gdbsupport/gdb_optional.h" #include "source.h" #include "cli/cli-style.h" +#include "dwarf2/loc.h" /* Local functions: */ @@ -1598,6 +1599,12 @@ struct finish_command_fsm : public thread_fsm return value. */ struct return_value_info return_value_info {}; + /* If the current function uses the "struct return convention", + this holds the address at which the value being returned will + be stored, or zero if that address could not be determined or + the "struct return convention" is not being used. */ + CORE_ADDR return_buf; + explicit finish_command_fsm (struct interp *cmd_interp) : thread_fsm (cmd_interp) { @@ -1635,7 +1642,13 @@ finish_command_fsm::should_stop (struct thread_info *tp) struct value *func; func = read_var_value (function, NULL, get_current_frame ()); - rv->value = get_return_value (function, func); + + if (return_buf != 0) + /* Retrieve return value from the buffer where it was saved. */ + rv->value = value_at (rv->type, return_buf); + else + rv->value = get_return_value (function, func); + if (rv->value != NULL) rv->value_history_index = record_latest_value (rv->value); } @@ -1851,8 +1864,28 @@ finish_command (const char *arg, int from_tty) } /* Find the function we will return from. */ - - sm->function = find_pc_function (get_frame_pc (get_selected_frame (NULL))); + frame_info_ptr callee_frame = get_selected_frame (NULL); + sm->function = find_pc_function (get_frame_pc (callee_frame)); + + /* Determine the return convention. If it is RETURN_VALUE_STRUCT_CONVENTION, + attempt to determine the address of the return buffer. */ + enum return_value_convention return_value; + struct gdbarch *gdbarch = get_frame_arch (callee_frame); + + struct type * val_type + = check_typedef (sm->function->type ()->target_type ()); + + return_value = gdbarch_return_value (gdbarch, + read_var_value (sm->function, NULL, + callee_frame), + val_type, NULL, NULL, NULL); + + if (return_value == RETURN_VALUE_STRUCT_CONVENTION + && val_type->code () != TYPE_CODE_VOID) + sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type, + callee_frame); + else + sm->return_buf = 0; /* Print info on the selected frame, including level number but not source. */ @@ -1870,7 +1903,7 @@ finish_command (const char *arg, int from_tty) gdb_printf (_("Run till exit from ")); } - print_stack_frame (get_selected_frame (NULL), 1, LOCATION, 0); + print_stack_frame (callee_frame, 1, LOCATION, 0); } frame.reinflate (); diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c index 450162dd46e..1cbaaf2a4e6 100644 --- a/gdb/ppc-sysv-tdep.c +++ b/gdb/ppc-sysv-tdep.c @@ -28,6 +28,7 @@ #include "objfiles.h" #include "infcall.h" #include "dwarf2.h" +#include "dwarf2/loc.h" #include "target-float.h" #include <algorithm> @@ -2156,3 +2157,43 @@ ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, return RETURN_VALUE_STRUCT_CONVENTION; } +CORE_ADDR ppc64_sysv_get_return_buf_addr (struct type *val_type, + frame_info_ptr cur_frame) +{ + /* The PowerPC ABI specifies aggregates that are not returned by value + are returned in a storage buffer provided by the caller. The + address of the storage buffer is provided as a hidden first input + arguement in register r3. The PowerPC ABI does not guarantee that + register r3 will not be changed while executing the function. Hence, it + cannot be assumed that r3 will still contain the address of the storage + buffer when execution reaches the end of the function. + + This function attempts to determine the value of r3 on entry to the + function using the DW_OP_entry_value DWARF entries. This requires + compiling the user program with -fvar-tracking to resolve the + DW_TAG_call_sites in the binary file. */ + + union call_site_parameter_u kind_u; + enum call_site_parameter_kind kind; + CORE_ADDR return_val = 0; + + kind_u.dwarf_reg = 3; /* First passed arg/return value is in r3. */ + kind = CALL_SITE_PARAMETER_DWARF_REG; + + /* val_type is the type of the return value. Need the pointer type + to the return value. */ + val_type = lookup_pointer_type (val_type); + + try + { + return_val = value_as_address (value_of_dwarf_reg_entry (val_type, + cur_frame, + kind, kind_u)); + } + catch (const gdb_exception_error &e) + { + warning ("Cannot determine the function return value.\n" + "Try compiling with -fvar-tracking."); + } + return return_val; +} diff --git a/gdb/ppc-tdep.h b/gdb/ppc-tdep.h index 34b250849b9..cbccd87820d 100644 --- a/gdb/ppc-tdep.h +++ b/gdb/ppc-tdep.h @@ -175,6 +175,8 @@ extern void ppc_collect_vsxregset (const struct regset *regset, const struct regcache *regcache, int regnum, void *vsxregs, size_t len); +extern CORE_ADDR ppc64_sysv_get_return_buf_addr (type*, frame_info_ptr); + /* Private data that this module attaches to struct gdbarch. */ /* ELF ABI version used by the inferior. */ diff --git a/gdb/rs6000-tdep.c b/gdb/rs6000-tdep.c index 866d43ded7a..cbd84514795 100644 --- a/gdb/rs6000-tdep.c +++ b/gdb/rs6000-tdep.c @@ -8243,7 +8243,11 @@ rs6000_gdbarch_init (struct gdbarch_info info, struct gdbarch_list *arches) set_gdbarch_ps_regnum (gdbarch, tdep->ppc_ps_regnum); if (wordsize == 8) - set_gdbarch_return_value (gdbarch, ppc64_sysv_abi_return_value); + { + set_gdbarch_return_value (gdbarch, ppc64_sysv_abi_return_value); + set_gdbarch_get_return_buf_addr (gdbarch, + ppc64_sysv_get_return_buf_addr); + } else set_gdbarch_return_value (gdbarch, ppc_sysv_abi_return_value); diff --git a/gdb/testsuite/gdb.cp/non-trivial-retval.exp b/gdb/testsuite/gdb.cp/non-trivial-retval.exp index 21c390bc937..93a3a6832f7 100644 --- a/gdb/testsuite/gdb.cp/non-trivial-retval.exp +++ b/gdb/testsuite/gdb.cp/non-trivial-retval.exp @@ -15,11 +15,18 @@ # This file is part of the gdb testsuite +set additional_flags "" + if {[skip_cplus_tests]} { continue } standard_testfile .cc -if {[prepare_for_testing "failed to prepare" $testfile $srcfile {debug c++}]} { +if {[have_fvar_tracking]} { + set additional_flags "additional_flags= -fvar-tracking" +} + +if {[prepare_for_testing "failed to prepare" $testfile $srcfile [list debug c++ $additional_flags]]} { + return -1 } diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp index e2cda30b95a..ed70c08d1f2 100644 --- a/gdb/testsuite/lib/gdb.exp +++ b/gdb/testsuite/lib/gdb.exp @@ -8796,6 +8796,14 @@ gdb_caching_proc have_fuse_ld_gold { return [gdb_simple_compile $me $src executable $flags] } +# Return 1 if compiler supports fvar-tracking, otherwise return 0. +gdb_caching_proc have_fvar_tracking { + set me "have_fvar_tracking" + set flags "additional_flags=-fvar-tracking" + set src { int main() { return 0; } } + return [gdb_simple_compile $me $src executable $flags] +} + # Return 1 if linker supports -Ttext-segment, otherwise return 0. gdb_caching_proc linker_supports_Ttext_segment_flag { set me "linker_supports_Ttext_segment_flag" -- 2.37.2 ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/2 ver 3] PowerPC, fix support for printing the function return value for non-trivial values. 2022-11-07 20:04 ` [PATCH 2/2 ver 3] " Carl Love @ 2022-11-14 16:47 ` Ulrich Weigand 2022-11-15 7:15 ` Tom de Vries 0 siblings, 1 reply; 44+ messages in thread From: Ulrich Weigand @ 2022-11-14 16:47 UTC (permalink / raw) To: gdb-patches, cel; +Cc: kevinb, will_schmidt, blarsen Carl Love <cel@us.ibm.com> wrote: >PowerPC, fix support for printing the function return value for non-trivial values. > >Currently, a non-trivial return value from a function cannot currently be >reliably determined on PowerPC. This is due to the fact that the PowerPC >ABI uses register r3 to store the address of the buffer containing the >non-trivial return value when the function is called. The PowerPC ABI >does not guarantee the value in register r3 is not modified in the >function. Thus the value in r3 cannot be reliably used to obtain the >return addreses on exit from the function. > >This patch adds a new gdbarch method to allow PowerPC to access the value >of r3 on entry to a function. On PowerPC, the new gdbarch method attempts >to use the DW_OP_entry_value for the DWARF entries, when exiting the >function, to determine the value of r3 on entry to the function. This >requires the use of the -fvar-tracking compiler option to compile the >user application thus generating the DW_OP_entry_value in the binary. The >DW_OP_entry_value entries in the binary file allows GDB to resolve the >DW_TAG_call_site entries. This new gdbarch method is used to get the >return buffer address, in the case of a function returning a nontrivial >data type, on exit from the function. The GDB function should_stop checks >to see if RETURN_BUF is non-zero. By default, RETURN_BUF will be set to >zero by the new gdbarch method call for all architectures except PowerPC. >The get_return_value function will be used to obtain the return value on >all other architectures as is currently being done if RETURN_BUF is zero. >On PowerPC, the new gdbarch method will return a nonzero address in >RETURN_BUF if the value can be determined. The value_at function uses the >return buffer address to get the return value. > >This patch fixes five testcase failures in gdb.cp/non-trivial-retval.exp. >The correct function return values are now reported. > >Note this patch is dependent on patch: "PowerPC, function >ppc64_sysv_abi_return_value add missing return value convention". > >This patch has been tested on Power 10 and x86-64 with no regressions. I believe all of Kevin's and Bruno's comments have been addressed in this version. I've reviewed it myself as well, and it looks good to me. This is OK. Thanks, Ulrich ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/2 ver 3] PowerPC, fix support for printing the function return value for non-trivial values. 2022-11-14 16:47 ` Ulrich Weigand @ 2022-11-15 7:15 ` Tom de Vries 2022-11-15 10:16 ` Ulrich Weigand 0 siblings, 1 reply; 44+ messages in thread From: Tom de Vries @ 2022-11-15 7:15 UTC (permalink / raw) To: Ulrich Weigand, gdb-patches, cel; +Cc: kevinb, will_schmidt, blarsen On 11/14/22 17:47, Ulrich Weigand via Gdb-patches wrote: > Carl Love <cel@us.ibm.com> wrote: > >> PowerPC, fix support for printing the function return value for non-trivial values. >> >> Currently, a non-trivial return value from a function cannot currently be >> reliably determined on PowerPC. This is due to the fact that the PowerPC >> ABI uses register r3 to store the address of the buffer containing the >> non-trivial return value when the function is called. The PowerPC ABI >> does not guarantee the value in register r3 is not modified in the >> function. Thus the value in r3 cannot be reliably used to obtain the >> return addreses on exit from the function. >> >> This patch adds a new gdbarch method to allow PowerPC to access the value >> of r3 on entry to a function. On PowerPC, the new gdbarch method attempts >> to use the DW_OP_entry_value for the DWARF entries, when exiting the >> function, to determine the value of r3 on entry to the function. This >> requires the use of the -fvar-tracking compiler option to compile the >> user application thus generating the DW_OP_entry_value in the binary. The >> DW_OP_entry_value entries in the binary file allows GDB to resolve the >> DW_TAG_call_site entries. This new gdbarch method is used to get the >> return buffer address, in the case of a function returning a nontrivial >> data type, on exit from the function. The GDB function should_stop checks >> to see if RETURN_BUF is non-zero. By default, RETURN_BUF will be set to >> zero by the new gdbarch method call for all architectures except PowerPC. >> The get_return_value function will be used to obtain the return value on >> all other architectures as is currently being done if RETURN_BUF is zero. >> On PowerPC, the new gdbarch method will return a nonzero address in >> RETURN_BUF if the value can be determined. The value_at function uses the >> return buffer address to get the return value. >> >> This patch fixes five testcase failures in gdb.cp/non-trivial-retval.exp. >> The correct function return values are now reported. >> >> Note this patch is dependent on patch: "PowerPC, function >> ppc64_sysv_abi_return_value add missing return value convention". >> >> This patch has been tested on Power 10 and x86-64 with no regressions. > > I believe all of Kevin's and Bruno's comments have been addressed > in this version. I've reviewed it myself as well, and it looks > good to me. > > This is OK. > On x86_64-linux, I run into a segfault: ... PASS: gdb.asm/asm-source.exp: info source asmsrc1.s ERROR: GDB process no longer exists UNRESOLVED: gdb.asm/asm-source.exp: finish from foo3 ... Reproduced on command line: ... $ gdb -q -batch -x outputs/gdb.asm/asm-source/gdb.in.1 ... The problem seems to be that: ... Thread 1 "gdb" received signal SIGSEGV, Segmentation fault. 0x000000000043de7a in symbol::type (this=0x0) at /home/vries/gdb_versions/devel/src/gdb/symtab.h:1287 1287 return m_type; ... because: ... (gdb) up #1 0x0000000000852d94 in finish_command (arg=0x0, from_tty=0) at /home/vries/gdb_versions/devel/src/gdb/infcmd.c:1887 1887 = check_typedef (sm->function->type ()->target_type ()); (gdb) p sm->function $1 = (symbol *) 0x0 ... Thanks, - Tom ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/2 ver 3] PowerPC, fix support for printing the function return value for non-trivial values. 2022-11-15 7:15 ` Tom de Vries @ 2022-11-15 10:16 ` Ulrich Weigand 2022-11-15 16:04 ` Carl Love 2022-11-15 17:24 ` Carl Love 0 siblings, 2 replies; 44+ messages in thread From: Ulrich Weigand @ 2022-11-15 10:16 UTC (permalink / raw) To: gdb-patches, tdevries, cel; +Cc: kevinb, will_schmidt, blarsen Tom de Vries <tdevries@suse.de> wrote: >On 11/14/22 17:47, Ulrich Weigand via Gdb-patches wrote: > >I believe all of Kevin's and Bruno's comments have been addressed > >in this version. I've reviewed it myself as well, and it looks > >good to me. > > >This is OK. >On x86_64-linux, I run into a segfault: >[...] >(gdb) up >#1 0x0000000000852d94 in finish_command (arg=0x0, from_tty=0) > at /home/vries/gdb_versions/devel/src/gdb/infcmd.c:1887 >1887 = check_typedef (sm->function->type ()->target_type ()); >(gdb) p sm->function >$1 = (symbol *) 0x0 Ah, I missed that possibility, sorry. Carl, if sm->function is NULL, then the whole check needs to be skipped, and sm->return_buf should be set to 0. Can you come up with a fix along those lines (or else, just revert the patch for now)? Thanks! Bye, Ulrich ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/2 ver 3] PowerPC, fix support for printing the function return value for non-trivial values. 2022-11-15 10:16 ` Ulrich Weigand @ 2022-11-15 16:04 ` Carl Love 2022-11-15 16:55 ` Simon Marchi 2022-11-15 17:24 ` Carl Love 1 sibling, 1 reply; 44+ messages in thread From: Carl Love @ 2022-11-15 16:04 UTC (permalink / raw) To: Ulrich Weigand, gdb-patches, tdevries; +Cc: kevinb, will_schmidt, blarsen On Tue, 2022-11-15 at 10:16 +0000, Ulrich Weigand wrote: > Tom de Vries <tdevries@suse.de> wrote: > > On 11/14/22 17:47, Ulrich Weigand via Gdb-patches wrote: > > > I believe all of Kevin's and Bruno's comments have been addressed > > > in this version. I've reviewed it myself as well, and it looks > > > good to me. > > > This is OK. > > On x86_64-linux, I run into a segfault: > > [...] > > (gdb) up > > #1 0x0000000000852d94 in finish_command (arg=0x0, from_tty=0) > > at /home/vries/gdb_versions/devel/src/gdb/infcmd.c:1887 > > 1887 = check_typedef (sm->function->type ()->target_type > > ()); > > (gdb) p sm->function > > $1 = (symbol *) 0x0 > > Ah, I missed that possibility, sorry. > > Carl, if sm->function is NULL, then the whole check needs > to be skipped, and sm->return_buf should be set to 0. > > Can you come up with a fix along those lines (or else, > just revert the patch for now)? Thanks! > Yes, I will take a look and see about fixing or reverting it ASAP. Carl ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/2 ver 3] PowerPC, fix support for printing the function return value for non-trivial values. 2022-11-15 16:04 ` Carl Love @ 2022-11-15 16:55 ` Simon Marchi 2022-11-15 23:46 ` Carl Love 0 siblings, 1 reply; 44+ messages in thread From: Simon Marchi @ 2022-11-15 16:55 UTC (permalink / raw) To: Carl Love, Ulrich Weigand, gdb-patches, tdevries Cc: kevinb, will_schmidt, blarsen On 11/15/22 11:04, Carl Love via Gdb-patches wrote: > On Tue, 2022-11-15 at 10:16 +0000, Ulrich Weigand wrote: >> Tom de Vries <tdevries@suse.de> wrote: >>> On 11/14/22 17:47, Ulrich Weigand via Gdb-patches wrote: >>>> I believe all of Kevin's and Bruno's comments have been addressed >>>> in this version. I've reviewed it myself as well, and it looks >>>> good to me. >>>> This is OK. >>> On x86_64-linux, I run into a segfault: >>> [...] >>> (gdb) up >>> #1 0x0000000000852d94 in finish_command (arg=0x0, from_tty=0) >>> at /home/vries/gdb_versions/devel/src/gdb/infcmd.c:1887 >>> 1887 = check_typedef (sm->function->type ()->target_type >>> ()); >>> (gdb) p sm->function >>> $1 = (symbol *) 0x0 >> >> Ah, I missed that possibility, sorry. >> >> Carl, if sm->function is NULL, then the whole check needs >> to be skipped, and sm->return_buf should be set to 0. >> >> Can you come up with a fix along those lines (or else, >> just revert the patch for now)? Thanks! >> > Yes, I will take a look and see about fixing or reverting it ASAP. > > Carl > Just so you can test properly, I also see these failures that are likely due to the same bug: UNRESOLVED: gdb.base/foll-vfork.exp: exec: vfork parent follow, finish after tcatch vfork: finish UNRESOLVED: gdb.base/foll-vfork.exp: exec: vfork child follow, finish after tcatch vfork: finish UNRESOLVED: gdb.base/foll-vfork.exp: exit: vfork parent follow, finish after tcatch vfork: finish UNRESOLVED: gdb.base/foll-vfork.exp: exit: vfork child follow, finish after tcatch vfork: finish UNRESOLVED: gdb.base/sigaltstack.exp: finish to throw INNER UNRESOLVED: gdb.base/sigstep.exp: finish from handleri: leave signal trampoline Simon ^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH 2/2 ver 3] PowerPC, fix support for printing the function return value for non-trivial values. 2022-11-15 16:55 ` Simon Marchi @ 2022-11-15 23:46 ` Carl Love 0 siblings, 0 replies; 44+ messages in thread From: Carl Love @ 2022-11-15 23:46 UTC (permalink / raw) To: Simon Marchi, Ulrich Weigand, gdb-patches, tdevries Cc: kevinb, will_schmidt, blarsen Ulrich, Simon, Tom, GDB maintainers: > > Just so you can test properly, I also see these failures that are > likely > due to the same bug: > > UNRESOLVED: gdb.base/foll-vfork.exp: exec: vfork parent follow, > finish after tcatch vfork: finish > UNRESOLVED: gdb.base/foll-vfork.exp: exec: vfork child follow, finish > after tcatch vfork: finish > UNRESOLVED: gdb.base/foll-vfork.exp: exit: vfork parent follow, > finish after tcatch vfork: finish > UNRESOLVED: gdb.base/foll-vfork.exp: exit: vfork child follow, finish > after tcatch vfork: finish > UNRESOLVED: gdb.base/sigaltstack.exp: finish to throw INNER > UNRESOLVED: gdb.base/sigstep.exp: finish from handleri: leave signal > trampoline I have dug into why I didn't catch the issue earlier. Looking carefully at the regression test output, the foll-vfork.exp, sigaltstack.exp, sigstep.exp and asm-source.exp tests are all failing similarly. In the GDB regression test output I see: Running /home/carll/GDB/build-non-trivial/gdb/testsuite/../../../binutils-gdb-n\ on-trivial/gdb/testsuite/gdb.base/foll-vfork.exp ... ERROR: GDB process no longer exists ERROR: GDB process no longer exists ERROR: GDB process no longer exists ERROR: GDB process no longer exists Running .../binutils-gdb-non-trivial/gdb/testsuite/gdb.asm/asm-source.exp ... ERROR: GDB process no longer exists Running ../binutils-gdb-non-trivial/gdb/testsuite/gdb.base/sigaltstack.exp ... ERROR: GDB process no longer exists ERROR: Couldn't send finish to GDB. ERROR: can not find channel named "exp17" Running .../binutils-gdb-non-trivial/gdb/testsuite/gdb.base/sigstep.exp ... ERROR: GDB process no longer exists The failures then end up being recorded in the summary as "# of unresolved testcases" not as "# of unexpected failures". I can see with the current mainline code base there are 8 unresolved testcases. I updated the fix that I sent to Tom earlier per the comments from Ulrich. With that fix applied, the number of unresolved test cases drops to 1. If I run the regression tests with the tree backed up the the commit just before my commits from Monday, I see one unresolved test case. So, clearly my commit added 7 unresolved test cases, the six that Simon mentioned above and the one from Tom. The original unresolved test case is: Running .../binutils-gdb-non-trivial/gdb/testsuite/gdb.base/gdb-sigterm.exp ... FAIL: gdb.base/gdb-sigterm.exp: pass=0: expect eof (GDB internal error) ERROR: Could not resync from internal error (recursive internal problem) I always check that "# of unexpected failures" did not change with my patch applied. I didn't notice that the number of unresolved testcases changed and hence missed the breakage my patch caused. My bad. I will be sure to watch the other stats in the gdb summary a lot closer in the future. Thanks for pointing out the failures and the for help resolving the issue. I will post my updated fix for Tom to test/review. I think we have this all figured out and fixed. Sorry for breaking things. Carl Love ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/2 ver 3] PowerPC, fix support for printing the function return value for non-trivial values. 2022-11-15 10:16 ` Ulrich Weigand 2022-11-15 16:04 ` Carl Love @ 2022-11-15 17:24 ` Carl Love 2022-11-15 18:05 ` Ulrich Weigand 1 sibling, 1 reply; 44+ messages in thread From: Carl Love @ 2022-11-15 17:24 UTC (permalink / raw) To: Ulrich Weigand, gdb-patches, tdevries; +Cc: kevinb, will_schmidt, blarsen, cel Tom, Ulrich: On Tue, 2022-11-15 at 10:16 +0000, Ulrich Weigand wrote: > Tom de Vries <tdevries@suse.de> wrote: > > On 11/14/22 17:47, Ulrich Weigand via Gdb-patches wrote: > > > I believe all of Kevin's and Bruno's comments have been addressed > > > in this version. I've reviewed it myself as well, and it looks > > > good to me. > > > This is OK. > > On x86_64-linux, I run into a segfault: > > [...] > > (gdb) up > > #1 0x0000000000852d94 in finish_command (arg=0x0, from_tty=0) > > at /home/vries/gdb_versions/devel/src/gdb/infcmd.c:1887 > > 1887 = check_typedef (sm->function->type ()->target_type > > ()); > > (gdb) p sm->function > > $1 = (symbol *) 0x0 > > Ah, I missed that possibility, sorry. > > Carl, if sm->function is NULL, then the whole check needs > to be skipped, and sm->return_buf should be set to 0. > > Can you come up with a fix along those lines (or else, > just revert the patch for now)? Thanks! > I tested the last mainline code with the command on an X86-64 box. make check RUNTESTFLAGS='GDB=/home/carll/bin/gdb gdb.asm/asm-source.exp ' Which generated an error. I then ran the command, from Tom's email: gdb -q -batch -x outputs/gdb.asm/asm-source/gdb.in.1 from directory gdb/testsuite which generated a segmentation fault as expected. Once I applied the following patch, recompiled and retested, the make check seemed to run fine and the gdb -q batch test ran without generating a segmentation fault. The patch appears to fix the issue. Not sure why this didn't show up in my original X86 testing? I will go back and look some more to see if I can track down why I didn't see the issue. Sorry for the bug. Tom, can you take a look at the patch and try it out to make sure it fixes the issues on your system. Thanks. Carl Love ---------------------------------------------------------------- Bug fix in commit for printing the function return value for non-trivial values The recent commit: commit a0eda3df5b750ae32576a9be092b361281a41787 Author: Carl Love <cel@us.ibm.com> Date: Mon Nov 14 16:22:37 2022 -0500 PowerPC, fix support for printing the function return value for non-trivial values. Is generating a segmentation fault on x86_64-linux. segfault: ... PASS: gdb.asm/asm-source.exp: info source asmsrc1.s ERROR: GDB process no longer exists UNRESOLVED: gdb.asm/asm-source.exp: finish from foo3 ... Reproduced on command line: ... $ gdb -q -batch -x outputs/gdb.asm/asm-source/gdb.in.1 ... The problem seems to be that: ... Thread 1 "gdb" received signal SIGSEGV, Segmentation fault. 0x000000000043de7a in symbol::type (this=0x0) at .../gdb_versions/devel/src/gdb/symtab.h:1287 1287 return m_type; ... because: ... (gdb) up #1 0x0000000000852d94 in finish_command (arg=0x0, from_tty=0) at .../gdb_versions/devel/src/gdb/infcmd.c:1887 1887 = check_typedef (sm->function->type ()->target_type ()); (gdb) p sm->function $1 = (symbol *) 0x0 The code is not checking if sm->function is NULL. If sm->function is NULL the check for the return buffer should be skipped. --- gdb/infcmd.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/gdb/infcmd.c b/gdb/infcmd.c index b71dc10370b..2876f6d9c9d 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -1880,23 +1880,28 @@ finish_command (const char *arg, int from_tty) /* Determine the return convention. If it is RETURN_VALUE_STRUCT_CONVENTION, attempt to determine the address of the return buffer. */ - enum return_value_convention return_value; - struct gdbarch *gdbarch = get_frame_arch (callee_frame); + if (sm->function != NULL) + { + enum return_value_convention return_value; + struct gdbarch *gdbarch = get_frame_arch (callee_frame); - struct type * val_type - = check_typedef (sm->function->type ()->target_type ()); + struct type * val_type + = check_typedef (sm->function->type ()->target_type ()); - return_value = gdbarch_return_value (gdbarch, - read_var_value (sm->function, NULL, - callee_frame), - val_type, NULL, NULL, NULL); + return_value = gdbarch_return_value (gdbarch, + read_var_value (sm->function, NULL, + callee_frame), + val_type, NULL, NULL, NULL); - if (return_value == RETURN_VALUE_STRUCT_CONVENTION - && val_type->code () != TYPE_CODE_VOID) - sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type, - callee_frame); + if (return_value == RETURN_VALUE_STRUCT_CONVENTION + && val_type->code () != TYPE_CODE_VOID) + sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type, + callee_frame); + else + sm->return_buf = 0; + } else - sm->return_buf = 0; + sm->return_buf = 0; /* Return buffer address is not available. */ /* Print info on the selected frame, including level number but not source. */ -- 2.31.1 ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/2 ver 3] PowerPC, fix support for printing the function return value for non-trivial values. 2022-11-15 17:24 ` Carl Love @ 2022-11-15 18:05 ` Ulrich Weigand 2022-11-16 1:01 ` Carl Love 0 siblings, 1 reply; 44+ messages in thread From: Ulrich Weigand @ 2022-11-15 18:05 UTC (permalink / raw) To: gdb-patches, tdevries, cel; +Cc: kevinb, will_schmidt, blarsen Carl Love <cel@us.ibm.com> wrote: >+ else >+ sm->return_buf = 0; >+ } > else >- sm->return_buf = 0; >+ sm->return_buf = 0; /* Return buffer address is not available. */ Just as a minor nit, it might be cleaner to initialize sm->return_buf to 0 just once up-front. Patch is OK otherwise. Thanks, Ulrich ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/2 ver 3] PowerPC, fix support for printing the function return value for non-trivial values. 2022-11-15 18:05 ` Ulrich Weigand @ 2022-11-16 1:01 ` Carl Love 2022-11-16 9:52 ` Ulrich Weigand ` (2 more replies) 0 siblings, 3 replies; 44+ messages in thread From: Carl Love @ 2022-11-16 1:01 UTC (permalink / raw) To: Ulrich Weigand, gdb-patches, tdevries, Simon Marchi Cc: kevinb, will_schmidt, blarsen, cel Ulrich, Tom, Simon: On Tue, 2022-11-15 at 18:05 +0000, Ulrich Weigand wrote: > Carl Love <cel@us.ibm.com> wrote: > > > + else > > + sm->return_buf = 0; > > + } > > else > > - sm->return_buf = 0; > > + sm->return_buf = 0; /* Return buffer address is not > > available. */ > > Just as a minor nit, it might be cleaner to initialize > sm->return_buf to 0 just once up-front. > > Patch is OK otherwise. > > > Thanks, > Ulrich Yes, I agree, it would be best to just set sm->return_buf = 0 initially. Then if the return buffer is available update it as needed. That makes things a lot cleaner. The updated patch is below. Note, it has been tested on PowerPC and X86_64. It does not introduce any additional regression errors. It does fix the seven unresolved testcases that the initial patch introduced and were missed by my testing of the initial patch. Tom, if you can verify this fix works on your system it would be greatly appreciated. Thanks. Carl Love --------------------------------------------------- Bug fix in commit for printing the function return value for non-trivial values The recent commit: commit a0eda3df5b750ae32576a9be092b361281a41787 Author: Carl Love <cel@us.ibm.com> Date: Mon Nov 14 16:22:37 2022 -0500 PowerPC, fix support for printing the function return value for non-trivial values. Is generating a segmentation fault on x86_64-linux. segfault: ... PASS: gdb.asm/asm-source.exp: info source asmsrc1.s ERROR: GDB process no longer exists UNRESOLVED: gdb.asm/asm-source.exp: finish from foo3 ... Reproduced on command line: ... $ gdb -q -batch -x outputs/gdb.asm/asm-source/gdb.in.1 ... The problem seems to be that: ... Thread 1 "gdb" received signal SIGSEGV, Segmentation fault. 0x000000000043de7a in symbol::type (this=0x0) at .../gdb_versions/devel/src/gdb/symtab.h:1287 1287 return m_type; ... because: ... (gdb) up #1 0x0000000000852d94 in finish_command (arg=0x0, from_tty=0) at .../gdb_versions/devel/src/gdb/infcmd.c:1887 1887 = check_typedef (sm->function->type ()->target_type ()); (gdb) p sm->function $1 = (symbol *) 0x0 The code is not checking if sm->function is NULL. If sm->function is NULL the check for the return buffer should be skipped. --- gdb/infcmd.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/gdb/infcmd.c b/gdb/infcmd.c index b71dc10370b..a72df2d6a01 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -1877,26 +1877,28 @@ finish_command (const char *arg, int from_tty) /* Find the function we will return from. */ frame_info_ptr callee_frame = get_selected_frame (NULL); sm->function = find_pc_function (get_frame_pc (callee_frame)); + sm->return_buf = 0; /* Initialize buffer address is not available. */ /* Determine the return convention. If it is RETURN_VALUE_STRUCT_CONVENTION, attempt to determine the address of the return buffer. */ - enum return_value_convention return_value; - struct gdbarch *gdbarch = get_frame_arch (callee_frame); + if (sm->function != NULL) + { + enum return_value_convention return_value; + struct gdbarch *gdbarch = get_frame_arch (callee_frame); - struct type * val_type - = check_typedef (sm->function->type ()->target_type ()); + struct type * val_type + = check_typedef (sm->function->type ()->target_type ()); - return_value = gdbarch_return_value (gdbarch, - read_var_value (sm->function, NULL, - callee_frame), - val_type, NULL, NULL, NULL); + return_value = gdbarch_return_value (gdbarch, + read_var_value (sm->function, NULL, + callee_frame), + val_type, NULL, NULL, NULL); - if (return_value == RETURN_VALUE_STRUCT_CONVENTION - && val_type->code () != TYPE_CODE_VOID) - sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type, - callee_frame); - else - sm->return_buf = 0; + if (return_value == RETURN_VALUE_STRUCT_CONVENTION + && val_type->code () != TYPE_CODE_VOID) + sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type, + callee_frame); + } /* Print info on the selected frame, including level number but not source. */ -- 2.31.1 ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/2 ver 3] PowerPC, fix support for printing the function return value for non-trivial values. 2022-11-16 1:01 ` Carl Love @ 2022-11-16 9:52 ` Ulrich Weigand 2022-11-16 10:12 ` Tom de Vries 2022-11-16 10:20 ` Lancelot SIX 2 siblings, 0 replies; 44+ messages in thread From: Ulrich Weigand @ 2022-11-16 9:52 UTC (permalink / raw) To: gdb-patches, simark, tdevries, cel; +Cc: kevinb, will_schmidt, blarsen Carl Love <cel@us.ibm.com> wrote: >Bug fix in commit for printing the function return value for non-trivial values This is OK. Thanks, Ulrich ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/2 ver 3] PowerPC, fix support for printing the function return value for non-trivial values. 2022-11-16 1:01 ` Carl Love 2022-11-16 9:52 ` Ulrich Weigand @ 2022-11-16 10:12 ` Tom de Vries 2022-11-16 10:20 ` Lancelot SIX 2 siblings, 0 replies; 44+ messages in thread From: Tom de Vries @ 2022-11-16 10:12 UTC (permalink / raw) To: Carl Love, Ulrich Weigand, gdb-patches, Simon Marchi Cc: kevinb, will_schmidt, blarsen On 11/16/22 02:01, Carl Love wrote: > Tom, if you can verify this fix works on your system it would be > greatly appreciated. Thanks. > Hi Carl, it works, all fails are gone, I got my usual result again on openSUSE Leap 15.4 x86_64: ... === gdb Summary === # of unexpected core files 1 # of expected passes 117502 # of expected failures 150 # of known failures 89 # of untested testcases 26 # of unsupported tests 107 ... Thanks, - Tom ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 2/2 ver 3] PowerPC, fix support for printing the function return value for non-trivial values. 2022-11-16 1:01 ` Carl Love 2022-11-16 9:52 ` Ulrich Weigand 2022-11-16 10:12 ` Tom de Vries @ 2022-11-16 10:20 ` Lancelot SIX 2022-11-16 15:56 ` Carl Love 2022-11-16 20:55 ` [PATCH] Change NULL to nullptr in gdb/infcmd.c and gdb/infrun.c Carl Love 2 siblings, 2 replies; 44+ messages in thread From: Lancelot SIX @ 2022-11-16 10:20 UTC (permalink / raw) To: Carl Love Cc: Ulrich Weigand, gdb-patches, tdevries, Simon Marchi, kevinb, will_schmidt, blarsen Hi Carl, Thanks for the fix. > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index b71dc10370b..a72df2d6a01 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -1877,26 +1877,28 @@ finish_command (const char *arg, int from_tty) > /* Find the function we will return from. */ > frame_info_ptr callee_frame = get_selected_frame (NULL); > sm->function = find_pc_function (get_frame_pc (callee_frame)); > + sm->return_buf = 0; /* Initialize buffer address is not available. */ > > /* Determine the return convention. If it is RETURN_VALUE_STRUCT_CONVENTION, > attempt to determine the address of the return buffer. */ > - enum return_value_convention return_value; > - struct gdbarch *gdbarch = get_frame_arch (callee_frame); > + if (sm->function != NULL) I know that Ulrich has already approved the patch, but to follow the coding standard the NULL should be replaced by nullptr. If this is not too much trouble and is OK with maintainers, I think it would be nice to do this minor tweak. > + { > + enum return_value_convention return_value; > + struct gdbarch *gdbarch = get_frame_arch (callee_frame); > > - struct type * val_type > - = check_typedef (sm->function->type ()->target_type ()); > + struct type * val_type > + = check_typedef (sm->function->type ()->target_type ()); > > - return_value = gdbarch_return_value (gdbarch, > - read_var_value (sm->function, NULL, > - callee_frame), > - val_type, NULL, NULL, NULL); > + return_value = gdbarch_return_value (gdbarch, > + read_var_value (sm->function, NULL, > + callee_frame), > + val_type, NULL, NULL, NULL); While at it, those NULL could probably be updated I guess. Best, Lancelot. > > - if (return_value == RETURN_VALUE_STRUCT_CONVENTION > - && val_type->code () != TYPE_CODE_VOID) > - sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type, > - callee_frame); > - else > - sm->return_buf = 0; > + if (return_value == RETURN_VALUE_STRUCT_CONVENTION > + && val_type->code () != TYPE_CODE_VOID) > + sm->return_buf = gdbarch_get_return_buf_addr (gdbarch, val_type, > + callee_frame); > + } > > /* Print info on the selected frame, including level number but not > source. */ > -- > 2.31.1 > > ^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH 2/2 ver 3] PowerPC, fix support for printing the function return value for non-trivial values. 2022-11-16 10:20 ` Lancelot SIX @ 2022-11-16 15:56 ` Carl Love 2022-11-16 20:55 ` [PATCH] Change NULL to nullptr in gdb/infcmd.c and gdb/infrun.c Carl Love 1 sibling, 0 replies; 44+ messages in thread From: Carl Love @ 2022-11-16 15:56 UTC (permalink / raw) To: Lancelot SIX, cel Cc: Ulrich Weigand, gdb-patches, tdevries, Simon Marchi, kevinb, will_schmidt, blarsen On Wed, 2022-11-16 at 10:20 +0000, Lancelot SIX wrote: > Hi Carl, > <snip> > > s RETURN_VALUE_STRUCT_CONVENTION, > > attempt to determine the address of the return buffer. */ > > - enum return_value_convention return_value; > > - struct gdbarch *gdbarch = get_frame_arch (callee_frame); > > + if (sm->function != NULL) > > I know that Ulrich has already approved the patch, but to follow the > coding standard the NULL should be replaced by nullptr. If this is > not > too much trouble and is OK with maintainers, I think it would be nice > to > do this minor tweak. > > > + { > > + enum return_value_convention return_value; > > + struct gdbarch *gdbarch = get_frame_arch (callee_frame); > > > > - struct type * val_type > > - = check_typedef (sm->function->type ()->target_type ()); > > + struct type * val_type > > + = check_typedef (sm->function->type ()->target_type ()); > > > > - return_value = gdbarch_return_value (gdbarch, > > - read_var_value (sm->function, > > NULL, > > - callee_frame), > > - val_type, NULL, NULL, NULL); > > + return_value = gdbarch_return_value (gdbarch, > > + read_var_value (sm- > > >function, NULL, > > + callee_frame > > ), > > + val_type, NULL, NULL, NULL); > > While at it, those NULL could probably be updated I guess. I think at this point it is probably best to do a new clean up patch particularly since the other NULLs are not part of the fix patch. I will go ahead and commit the fix and then put together a clean up patch to address all the NULLs. Thanks for letting me know about it. Carl Love ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH] Change NULL to nullptr in gdb/infcmd.c and gdb/infrun.c 2022-11-16 10:20 ` Lancelot SIX 2022-11-16 15:56 ` Carl Love @ 2022-11-16 20:55 ` Carl Love 2022-11-16 21:15 ` Simon Marchi 1 sibling, 1 reply; 44+ messages in thread From: Carl Love @ 2022-11-16 20:55 UTC (permalink / raw) To: Lancelot SIX, cel Cc: Ulrich Weigand, gdb-patches, tdevries, Simon Marchi, kevinb, will_schmidt, blarsen Lancelot, GDB maintainers: The GDB coding style guide specifies that nullptr should be used instead of NULL as noted by Lancelot for the recent patch "PowerPC, fix support for printing the function return value for non-trivial values.". This patch changes all of the various NULL statements to nullptr statements in files gdb/infcmd.c and gdb/infrun.c per the coding style guide. The patch does not make any functional changes to the code. The patch has been tested on both X86_64 and PowerPC to ensure there were no new unexpected error, new core files generated, new unresolved tests etc. Please let me know if this patch is acceptable. Thanks. Carl Love ------------------------ Change NULL to nullptr in gdb/infcmd.c and gdb/infrun.c The GDB coding standard specifies that nullptr should be used instead of NULL. There are numerous uses of NULL and nullptr in files infcmd.c and infrun.c. This patch replaces the various uses of NULL with nullptr in the source files. The use of NULL in the comments was not changed. The patch does not introduce any functional changes. The patch has been tested on PowerPC and Intel X86_64 with no new unexpected test failures, unresolved tests, new core files etc. --- gdb/infcmd.c | 122 +++++++++++++++++++------------------- gdb/infrun.c | 162 +++++++++++++++++++++++++-------------------------- 2 files changed, 142 insertions(+), 142 deletions(-) diff --git a/gdb/infcmd.c b/gdb/infcmd.c index a72df2d6a01..f7bce0d0399 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -208,10 +208,10 @@ strip_bg_char (const char *args, int *bg_char_p) { const char *p; - if (args == NULL || *args == '\0') + if (args == nullptr || *args == '\0') { *bg_char_p = 0; - return NULL; + return nullptr; } p = args + strlen (args); @@ -297,7 +297,7 @@ post_create_inferior (int from_tty) /* If the solist is global across processes, there's no need to refetch it here. */ if (!gdbarch_has_global_solist (target_gdbarch ())) - solib_add (NULL, 0, auto_solib_add); + solib_add (nullptr, 0, auto_solib_add); } } @@ -450,12 +450,12 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how) want them to go away (PR 2207). This is probably reasonable. */ /* If there were other args, beside '&', process them. */ - if (args != NULL) + if (args != nullptr) current_inferior ()->set_args (args); if (from_tty) { - uiout->field_string (NULL, "Starting program"); + uiout->field_string (nullptr, "Starting program"); uiout->text (": "); if (exec_file) uiout->field_string ("execfile", exec_file, @@ -472,7 +472,7 @@ run_command_1 (const char *args, int from_tty, enum run_how run_how) from_tty); /* to_create_inferior should push the target, so after this point we shouldn't refer to run_target again. */ - run_target = NULL; + run_target = nullptr; infrun_debug_show_threads ("immediately after create_process", current_inferior ()->non_exited_threads ()); @@ -630,7 +630,7 @@ continue_1 (int all_threads) scoped_disable_commit_resumed disable_commit_resumed ("continue all threads in non-stop"); - iterate_over_threads (proceed_thread_callback, NULL); + iterate_over_threads (proceed_thread_callback, nullptr); if (current_ui->prompt_state == PROMPT_BLOCKED) { @@ -675,27 +675,27 @@ continue_command (const char *args, int from_tty) gdb::unique_xmalloc_ptr<char> stripped = strip_bg_char (args, &async_exec); args = stripped.get (); - if (args != NULL) + if (args != nullptr) { if (startswith (args, "-a")) { all_threads_p = true; args += sizeof ("-a") - 1; if (*args == '\0') - args = NULL; + args = nullptr; } } if (!non_stop && all_threads_p) error (_("`-a' is meaningless in all-stop mode.")); - if (args != NULL && all_threads_p) + if (args != nullptr && all_threads_p) error (_("Can't resume all threads and specify " "proceed count simultaneously.")); /* If we have an argument left, set proceed count of breakpoint we stopped at. */ - if (args != NULL) + if (args != nullptr) { bpstat *bs = nullptr; int num, stat; @@ -712,7 +712,7 @@ continue_command (const char *args, int from_tty) get_last_target_status (&last_target, &last_ptid, nullptr); tp = find_thread_ptid (last_target, last_ptid); } - if (tp != NULL) + if (tp != nullptr) bs = tp->control.stop_bpstat; while ((stat = bpstat_num (&bs, &num)) != 0) @@ -964,7 +964,7 @@ prepare_one_step (thread_info *tp, struct step_command_fsm *sm) && inline_skipped_frames (tp)) { ptid_t resume_ptid; - const char *fn = NULL; + const char *fn = nullptr; symtab_and_line sal; struct symbol *sym; @@ -978,7 +978,7 @@ prepare_one_step (thread_info *tp, struct step_command_fsm *sm) sal = find_frame_sal (frame); sym = get_frame_function (frame); - if (sym != NULL) + if (sym != nullptr) fn = sym->print_name (); if (sal.line == 0 @@ -1094,7 +1094,7 @@ jump_command (const char *arg, int from_tty) /* See if we are trying to jump to another function. */ fn = get_frame_function (get_current_frame ()); sfn = find_pc_function (sal.pc); - if (fn != NULL && sfn != fn) + if (fn != nullptr && sfn != fn) { if (!query (_("Line %d is not in `%s'. Jump anyway? "), sal.line, fn->print_name ())) @@ -1104,7 +1104,7 @@ jump_command (const char *arg, int from_tty) } } - if (sfn != NULL) + if (sfn != nullptr) { struct obj_section *section; @@ -1245,7 +1245,7 @@ queue_signal_command (const char *signum_exp, int from_tty) ensure_valid_thread (); ensure_not_running (); - if (signum_exp == NULL) + if (signum_exp == nullptr) error_no_arg (_("signal number")); /* It would be even slicker to make signal names be valid expressions, @@ -1356,7 +1356,7 @@ until_next_command (int from_tty) { struct bound_minimal_symbol msymbol = lookup_minimal_symbol_by_pc (pc); - if (msymbol.minsym == NULL) + if (msymbol.minsym == nullptr) error (_("Execution is not within a known function.")); tp->control.step_range_start = msymbol.value_address (); @@ -1456,7 +1456,7 @@ advance_command (const char *arg, int from_tty) ensure_valid_thread (); ensure_not_running (); - if (arg == NULL) + if (arg == nullptr) error_no_arg (_("a location")); /* Find out whether we must run in the background. */ @@ -1498,17 +1498,17 @@ get_return_value (struct symbol *func_symbol, struct value *function) calls are made async, this will likely be made the norm. */ switch (gdbarch_return_value (gdbarch, function, value_type, - NULL, NULL, NULL)) + nullptr, nullptr, nullptr)) { case RETURN_VALUE_REGISTER_CONVENTION: case RETURN_VALUE_ABI_RETURNS_ADDRESS: case RETURN_VALUE_ABI_PRESERVES_ADDRESS: value = allocate_value (value_type); gdbarch_return_value (gdbarch, function, value_type, stop_regs, - value_contents_raw (value).data (), NULL); + value_contents_raw (value).data (), nullptr); break; case RETURN_VALUE_STRUCT_CONVENTION: - value = NULL; + value = nullptr; break; default: internal_error (_("bad switch")); @@ -1539,7 +1539,7 @@ struct return_value_info static void print_return_value_1 (struct ui_out *uiout, struct return_value_info *rv) { - if (rv->value != NULL) + if (rv->value != nullptr) { /* Print it. */ uiout->text ("Value returned is "); @@ -1578,7 +1578,7 @@ print_return_value_1 (struct ui_out *uiout, struct return_value_info *rv) void print_return_value (struct ui_out *uiout, struct return_value_info *rv) { - if (rv->type == NULL + if (rv->type == nullptr || check_typedef (rv->type)->code () == TYPE_CODE_VOID) return; @@ -1637,22 +1637,22 @@ finish_command_fsm::should_stop (struct thread_info *tp) { struct return_value_info *rv = &return_value_info; - if (function != NULL + if (function != nullptr && bpstat_find_breakpoint (tp->control.stop_bpstat, - breakpoint.get ()) != NULL) + breakpoint.get ()) != nullptr) { /* We're done. */ set_finished (); rv->type = function->type ()->target_type (); - if (rv->type == NULL) + if (rv->type == nullptr) internal_error (_("finish_command: function has no target type")); if (check_typedef (rv->type)->code () != TYPE_CODE_VOID) { struct value *func; - func = read_var_value (function, NULL, get_current_frame ()); + func = read_var_value (function, nullptr, get_current_frame ()); if (return_buf != 0) /* Retrieve return value from the buffer where it was saved. */ @@ -1660,7 +1660,7 @@ finish_command_fsm::should_stop (struct thread_info *tp) else rv->value = get_return_value (function, func); - if (rv->value != NULL) + if (rv->value != nullptr) rv->value_history_index = record_latest_value (rv->value); } } @@ -1717,7 +1717,7 @@ finish_backward (struct finish_command_fsm *sm) pc = get_frame_pc (get_current_frame ()); - if (find_pc_partial_function (pc, NULL, &func_addr, NULL) == 0) + if (find_pc_partial_function (pc, nullptr, &func_addr, nullptr) == 0) error (_("Cannot find bounds of current function")); sal = find_pc_line (func_addr, 0); @@ -1734,7 +1734,7 @@ finish_backward (struct finish_command_fsm *sm) if (sal.pc != pc) { - frame_info_ptr frame = get_selected_frame (NULL); + frame_info_ptr frame = get_selected_frame (nullptr); struct gdbarch *gdbarch = get_frame_arch (frame); /* Set a step-resume at the function's entry point. Once that's @@ -1775,7 +1775,7 @@ finish_forward (struct finish_command_fsm *sm, frame_info_ptr frame) bp_finish); /* set_momentary_breakpoint invalidates FRAME. */ - frame = NULL; + frame = nullptr; set_longjmp_breakpoint (tp, frame_id); @@ -1797,11 +1797,11 @@ skip_finish_frames (frame_info_ptr frame) start = frame; frame = skip_tailcall_frames (frame); - if (frame == NULL) + if (frame == nullptr) break; frame = skip_unwritable_frames (frame); - if (frame == NULL) + if (frame == nullptr) break; } while (start != frame); @@ -1867,7 +1867,7 @@ finish_command (const char *arg, int from_tty) if (from_tty) { gdb_printf (_("Run till exit from ")); - print_stack_frame (get_selected_frame (NULL), 1, LOCATION, 0); + print_stack_frame (get_selected_frame (nullptr), 1, LOCATION, 0); } proceed ((CORE_ADDR) -1, GDB_SIGNAL_DEFAULT); @@ -1875,13 +1875,13 @@ finish_command (const char *arg, int from_tty) } /* Find the function we will return from. */ - frame_info_ptr callee_frame = get_selected_frame (NULL); + frame_info_ptr callee_frame = get_selected_frame (nullptr); sm->function = find_pc_function (get_frame_pc (callee_frame)); sm->return_buf = 0; /* Initialize buffer address is not available. */ /* Determine the return convention. If it is RETURN_VALUE_STRUCT_CONVENTION, attempt to determine the address of the return buffer. */ - if (sm->function != NULL) + if (sm->function != nullptr) { enum return_value_convention return_value; struct gdbarch *gdbarch = get_frame_arch (callee_frame); @@ -1890,9 +1890,9 @@ finish_command (const char *arg, int from_tty) = check_typedef (sm->function->type ()->target_type ()); return_value = gdbarch_return_value (gdbarch, - read_var_value (sm->function, NULL, + read_var_value (sm->function, nullptr, callee_frame), - val_type, NULL, NULL, NULL); + val_type, nullptr, nullptr, nullptr); if (return_value == RETURN_VALUE_STRUCT_CONVENTION && val_type->code () != TYPE_CODE_VOID) @@ -1908,7 +1908,7 @@ finish_command (const char *arg, int from_tty) gdb_printf (_("Run back to call of ")); else { - if (sm->function != NULL && TYPE_NO_RETURN (sm->function->type ()) + if (sm->function != nullptr && TYPE_NO_RETURN (sm->function->type ()) && !query (_("warning: Function %s does not return normally.\n" "Try to finish anyway? "), sm->function->print_name ())) @@ -1926,7 +1926,7 @@ finish_command (const char *arg, int from_tty) { frame = skip_finish_frames (frame); - if (frame == NULL) + if (frame == nullptr) error (_("Cannot find the caller frame.")); finish_forward (sm, frame); @@ -2029,7 +2029,7 @@ environment_info (const char *var, int from_tty) { char **envp = current_inferior ()->environment.envp (); - for (int idx = 0; envp[idx] != NULL; ++idx) + for (int idx = 0; envp[idx] != nullptr; ++idx) { gdb_puts (envp[idx]); gdb_puts ("\n"); @@ -2141,7 +2141,7 @@ path_command (const char *dirname, int from_tty) mod_path (dirname, exec_path); current_inferior ()->environment.set (path_var_name, exec_path.c_str ()); if (from_tty) - path_info (NULL, from_tty); + path_info (nullptr, from_tty); } \f @@ -2288,7 +2288,7 @@ registers_info (const char *addr_exp, int fpregs) if (!target_has_registers ()) error (_("The program has no registers now.")); - frame = get_selected_frame (NULL); + frame = get_selected_frame (nullptr); gdbarch = get_frame_arch (frame); if (!addr_exp) @@ -2364,7 +2364,7 @@ registers_info (const char *addr_exp, int fpregs) break; } } - if (group != NULL) + if (group != nullptr) { int regnum; @@ -2430,7 +2430,7 @@ info_vector_command (const char *args, int from_tty) if (!target_has_registers ()) error (_("The program has no registers now.")); - print_vector_info (gdb_stdout, get_selected_frame (NULL), args); + print_vector_info (gdb_stdout, get_selected_frame (nullptr), args); } \f /* Kill the inferior process. Make us have no inferior. */ @@ -2498,7 +2498,7 @@ setup_inferior (int from_tty) /* If no exec file is yet known, try to determine it from the process itself. */ - if (get_exec_file (0) == NULL) + if (get_exec_file (0) == nullptr) exec_file_locate_attach (inferior_ptid.pid (), 1, from_tty); else { @@ -2644,7 +2644,7 @@ attach_command (const char *args, int from_tty) attach_target->attach (args, from_tty); /* to_attach should push the target, so after this point we shouldn't refer to attach_target again. */ - attach_target = NULL; + attach_target = nullptr; infrun_debug_show_threads ("immediately after attach", current_inferior ()->non_exited_threads ()); @@ -2826,7 +2826,7 @@ detach_command (const char *args, int from_tty) /* If the solist is global across inferiors, don't clear it when we detach from a single inferior. */ if (!gdbarch_has_global_solist (target_gdbarch ())) - no_shared_libraries (NULL, from_tty); + no_shared_libraries (nullptr, from_tty); if (deprecated_detach_hook) deprecated_detach_hook (); @@ -2852,7 +2852,7 @@ disconnect_command (const char *args, int from_tty) query_if_trace_running (from_tty); disconnect_tracing (); target_disconnect (args, from_tty); - no_shared_libraries (NULL, from_tty); + no_shared_libraries (nullptr, from_tty); init_thread_list (); if (deprecated_detach_hook) deprecated_detach_hook (); @@ -2921,7 +2921,7 @@ interrupt_command (const char *args, int from_tty) dont_repeat (); /* Not for the faint of heart. */ - if (args != NULL + if (args != nullptr && startswith (args, "-a")) all_threads = 1; @@ -2959,7 +2959,7 @@ info_float_command (const char *args, int from_tty) if (!target_has_registers ()) error (_("The program has no registers now.")); - frame = get_selected_frame (NULL); + frame = get_selected_frame (nullptr); gdbarch_print_float_info (get_frame_arch (frame), gdb_stdout, frame, args); } \f @@ -3085,7 +3085,7 @@ void _initialize_infcmd () { static struct cmd_list_element *info_proc_cmdlist; - struct cmd_list_element *c = NULL; + struct cmd_list_element *c = nullptr; const char *cmd_name; /* Add the filename of the terminal connected to inferior I/O. */ @@ -3100,8 +3100,8 @@ is restored."), show_inferior_tty_command, &setlist, &showlist); cmd_name = "inferior-tty"; - c = lookup_cmd (&cmd_name, setlist, "", NULL, -1, 1); - gdb_assert (c != NULL); + c = lookup_cmd (&cmd_name, setlist, "", nullptr, -1, 1); + gdb_assert (c != nullptr); add_alias_cmd ("tty", c, class_run, 0, &cmdlist); cmd_name = "args"; @@ -3113,8 +3113,8 @@ Follow this command with any number of args, to be passed to the program."), set_args_command, show_args_command, &setlist, &showlist); - c = lookup_cmd (&cmd_name, setlist, "", NULL, -1, 1); - gdb_assert (c != NULL); + c = lookup_cmd (&cmd_name, setlist, "", nullptr, -1, 1); + gdb_assert (c != nullptr); set_cmd_completer (c, filename_completer); cmd_name = "cwd"; @@ -3132,8 +3132,8 @@ working directory."), set_cwd_command, show_cwd_command, &setlist, &showlist); - c = lookup_cmd (&cmd_name, setlist, "", NULL, -1, 1); - gdb_assert (c != NULL); + c = lookup_cmd (&cmd_name, setlist, "", nullptr, -1, 1); + gdb_assert (c != nullptr); set_cmd_completer (c, filename_completer); c = add_cmd ("environment", no_class, environment_info, _("\ @@ -3404,8 +3404,8 @@ List all available info about the specified process."), add_setshow_boolean_cmd ("finish", class_support, &finish_print, _("\ Set whether `finish' prints the return value."), _("\ -Show whether `finish' prints the return value."), NULL, - NULL, +Show whether `finish' prints the return value."), nullptr, + nullptr, show_print_finish, &setprintlist, &showprintlist); } diff --git a/gdb/infrun.c b/gdb/infrun.c index 6da46b75ac7..debc6054624 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -388,7 +388,7 @@ static const char follow_fork_mode_parent[] = "parent"; static const char *const follow_fork_mode_kind_names[] = { follow_fork_mode_child, follow_fork_mode_parent, - NULL + nullptr }; static const char *follow_fork_mode_string = follow_fork_mode_parent; @@ -644,8 +644,8 @@ holding the child stopped. Try \"set detach-on-fork\" or \ if (has_vforked) { - gdb_assert (child_inf->vfork_parent == NULL); - gdb_assert (parent_inf->vfork_child == NULL); + gdb_assert (child_inf->vfork_parent == nullptr); + gdb_assert (parent_inf->vfork_child == nullptr); child_inf->vfork_parent = parent_inf; child_inf->pending_detach = 0; parent_inf->vfork_child = child_inf; @@ -700,12 +700,12 @@ follow_fork () followed fork child thread should have a copy of most of the parent thread structure's run control related fields, not just these. Initialized to avoid "may be used uninitialized" warnings from gcc. */ - struct breakpoint *step_resume_breakpoint = NULL; - struct breakpoint *exception_resume_breakpoint = NULL; + struct breakpoint *step_resume_breakpoint = nullptr; + struct breakpoint *exception_resume_breakpoint = nullptr; CORE_ADDR step_range_start = 0; CORE_ADDR step_range_end = 0; int current_line = 0; - symtab *current_symtab = NULL; + symtab *current_symtab = nullptr; struct frame_id step_frame_id = { 0 }; if (!non_stop) @@ -930,8 +930,8 @@ handle_vfork_child_exec_or_exit (int exec) /* This exec or exit marks the end of the shared memory region between the parent and the child. Break the bonds. */ inferior *vfork_parent = inf->vfork_parent; - inf->vfork_parent->vfork_child = NULL; - inf->vfork_parent = NULL; + inf->vfork_parent->vfork_child = nullptr; + inf->vfork_parent = nullptr; /* If the user wanted to detach from the parent, now is the time. */ @@ -964,8 +964,8 @@ handle_vfork_child_exec_or_exit (int exec) pspace = inf->pspace; aspace = inf->aspace; - inf->aspace = NULL; - inf->pspace = NULL; + inf->aspace = nullptr; + inf->pspace = nullptr; if (print_inferior_events) { @@ -1104,7 +1104,7 @@ static const char *const follow_exec_mode_names[] = { follow_exec_mode_new, follow_exec_mode_same, - NULL, + nullptr, }; static const char *follow_exec_mode_string = follow_exec_mode_same; @@ -1178,9 +1178,9 @@ follow_exec (ptid_t ptid, const char *exec_file_target) breakpoint or similar, it's gone now. We cannot truly step-to-next statement through an exec(). */ thread_info *th = inferior_thread (); - th->control.step_resume_breakpoint = NULL; - th->control.exception_resume_breakpoint = NULL; - th->control.single_step_breakpoints = NULL; + th->control.step_resume_breakpoint = nullptr; + th->control.exception_resume_breakpoint = nullptr; + th->control.single_step_breakpoints = nullptr; th->control.step_range_start = 0; th->control.step_range_end = 0; @@ -1203,13 +1203,13 @@ follow_exec (ptid_t ptid, const char *exec_file_target) breakpoint_init_inferior (inf_execd); gdb::unique_xmalloc_ptr<char> exec_file_host - = exec_file_find (exec_file_target, NULL); + = exec_file_find (exec_file_target, nullptr); /* If we were unable to map the executable target pathname onto a host pathname, tell the user that. Otherwise GDB's subsequent behavior is confusing. Maybe it would even be better to stop at this point so that the user can specify a file manually before continuing. */ - if (exec_file_host == NULL) + if (exec_file_host == nullptr) warning (_("Could not load symbols for executable %s.\n" "Do you need \"set sysroot\"?"), exec_file_target); @@ -1220,7 +1220,7 @@ follow_exec (ptid_t ptid, const char *exec_file_target) /* Also, loading a symbol file below may trigger symbol lookups, and we don't want those to be satisfied by the libraries of the previous incarnation of this process. */ - no_shared_libraries (NULL, 0); + no_shared_libraries (nullptr, 0); struct inferior *inf = current_inferior (); @@ -1380,7 +1380,7 @@ static void clear_step_over_info (void) { infrun_debug_printf ("clearing step over info"); - step_over_info.aspace = NULL; + step_over_info.aspace = nullptr; step_over_info.address = 0; step_over_info.nonsteppable_watchpoint_p = 0; step_over_info.thread = -1; @@ -1392,7 +1392,7 @@ int stepping_past_instruction_at (struct address_space *aspace, CORE_ADDR address) { - return (step_over_info.aspace != NULL + return (step_over_info.aspace != nullptr && breakpoint_address_match (aspace, address, step_over_info.aspace, step_over_info.address)); @@ -1420,7 +1420,7 @@ stepping_past_nonsteppable_watchpoint (void) static bool step_over_info_valid_p (void) { - return (step_over_info.aspace != NULL + return (step_over_info.aspace != nullptr || stepping_past_nonsteppable_watchpoint ()); } @@ -1516,7 +1516,7 @@ step_over_info_valid_p (void) static bool displaced_step_in_progress_thread (thread_info *thread) { - gdb_assert (thread != NULL); + gdb_assert (thread != nullptr); return thread->displaced_step_state.in_progress (); } @@ -2101,7 +2101,7 @@ static const char *const scheduler_enums[] = { schedlock_on, schedlock_step, schedlock_replay, - NULL + nullptr }; static const char *scheduler_mode = schedlock_replay; static void @@ -2195,7 +2195,7 @@ process_stratum_target * user_visible_resume_target (ptid_t resume_ptid) { return (resume_ptid == minus_one_ptid && sched_multi - ? NULL + ? nullptr : current_inferior ()->process_target ()); } @@ -2417,7 +2417,7 @@ resume_1 (enum gdb_signal sig) clear_step_over_info (); tp->control.trap_expected = 0; - if (tp->control.step_resume_breakpoint == NULL) + if (tp->control.step_resume_breakpoint == nullptr) { /* Set a "high-priority" step-resume, as we don't want user breakpoints at PC to trigger (again) when this @@ -2559,7 +2559,7 @@ resume_1 (enum gdb_signal sig) a step-resume breakpoint set on the earlier handler. We cannot set another step-resume breakpoint; just continue on until the original breakpoint is hit. */ - if (tp->control.step_resume_breakpoint == NULL) + if (tp->control.step_resume_breakpoint == nullptr) { insert_hp_step_resume_breakpoint_at_frame (get_current_frame ()); tp->step_after_step_resume_breakpoint = 1; @@ -2768,7 +2768,7 @@ clear_proceed_status_thread (struct thread_info *tp) tp->control.step_frame_id = null_frame_id; tp->control.step_stack_frame_id = null_frame_id; tp->control.step_over_calls = STEP_OVER_UNDEBUGGABLE; - tp->control.step_start_function = NULL; + tp->control.step_start_function = nullptr; tp->stop_requested = 0; tp->control.stop_step = 0; @@ -3667,12 +3667,12 @@ do_target_wait_1 (inferior *inf, ptid_t ptid, /* We have a specific thread to check. */ tp = find_thread_ptid (inf, ptid); - gdb_assert (tp != NULL); + gdb_assert (tp != nullptr); if (!tp->has_pending_waitstatus ()) - tp = NULL; + tp = nullptr; } - if (tp != NULL + if (tp != nullptr && (tp->stop_reason () == TARGET_STOPPED_BY_SW_BREAKPOINT || tp->stop_reason () == TARGET_STOPPED_BY_HW_BREAKPOINT)) { @@ -3713,7 +3713,7 @@ do_target_wait_1 (inferior *inf, ptid_t ptid, } } - if (tp != NULL) + if (tp != nullptr) { infrun_debug_printf ("Using pending wait status %s for %s.", tp->pending_waitstatus ().to_string ().c_str (), @@ -4261,7 +4261,7 @@ fetch_inferior_event () if (should_notify_stop) { /* We may not find an inferior if this was a process exit. */ - if (inf == NULL || inf->control.stop_soon == NO_STOP_QUIETLY) + if (inf == nullptr || inf->control.stop_soon == NO_STOP_QUIETLY) proceeded = normal_stop (); } @@ -4560,7 +4560,7 @@ static bool stepped_in_from (frame_info_ptr frame, struct frame_id step_frame_id) { for (frame = get_prev_frame (frame); - frame != NULL; + frame != nullptr; frame = get_prev_frame (frame)) { if (get_frame_id (frame) == step_frame_id) @@ -4587,9 +4587,9 @@ inline_frame_is_marked_for_skip (bool prev_frame, struct thread_info *tp) if (prev_frame) frame = get_prev_frame (frame); - for (; frame != NULL; frame = get_prev_frame (frame)) + for (; frame != nullptr; frame = get_prev_frame (frame)) { - const char *fn = NULL; + const char *fn = nullptr; symtab_and_line sal; struct symbol *sym; @@ -4601,7 +4601,7 @@ inline_frame_is_marked_for_skip (bool prev_frame, struct thread_info *tp) sal = find_frame_sal (frame); sym = get_frame_function (frame); - if (sym != NULL) + if (sym != nullptr) fn = sym->print_name (); if (sal.line != 0 @@ -4726,7 +4726,7 @@ get_inferior_stop_soon (execution_control_state *ecs) { struct inferior *inf = find_inferior_ptid (ecs->target, ecs->ptid); - gdb_assert (inf != NULL); + gdb_assert (inf != nullptr); return inf->control.stop_soon; } @@ -4764,7 +4764,7 @@ wait_one () for (inferior *inf : all_inferiors ()) { process_stratum_target *target = inf->process_target (); - if (target == NULL + if (target == nullptr || !target->is_async_p () || !target->threads_executing) continue; @@ -4795,7 +4795,7 @@ wait_one () for (inferior *inf : all_inferiors ()) { process_stratum_target *target = inf->process_target (); - if (target == NULL + if (target == nullptr || !target->is_async_p () || !target->threads_executing) continue; @@ -4811,12 +4811,12 @@ wait_one () /* No waitable targets left. All must be stopped. */ target_waitstatus ws; ws.set_no_resumed (); - return {NULL, minus_one_ptid, std::move (ws)}; + return {nullptr, minus_one_ptid, std::move (ws)}; } QUIT; - int numfds = interruptible_select (nfds, &readfds, 0, NULL, 0); + int numfds = interruptible_select (nfds, &readfds, 0, nullptr, 0); if (numfds < 0) { if (errno == EINTR) @@ -4987,7 +4987,7 @@ handle_one (const wait_one_event &event) else { thread_info *t = find_thread_ptid (event.target, event.ptid); - if (t == NULL) + if (t == nullptr) t = add_thread (event.target, event.ptid); t->stop_requested = 0; @@ -5393,7 +5393,7 @@ handle_inferior_event (struct execution_control_state *ecs) { ecs->event_thread = find_thread_ptid (ecs->target, ecs->ptid); /* If it's a new thread, add it to the thread database. */ - if (ecs->event_thread == NULL) + if (ecs->event_thread == nullptr) ecs->event_thread = add_thread (ecs->target, ecs->ptid); /* Disable range stepping. If the next step request could use a @@ -6064,8 +6064,8 @@ finish_step_over (struct execution_control_state *ecs) return 0; pending = iterate_over_threads (resumed_thread_with_pending_status, - NULL); - if (pending != NULL) + nullptr); + if (pending != nullptr) { struct thread_info *tp = ecs->event_thread; struct regcache *regcache; @@ -6542,7 +6542,7 @@ handle_signal_stop (struct execution_control_state *ecs) if (ecs->event_thread->prev_pc == ecs->event_thread->stop_pc () && ecs->event_thread->control.trap_expected - && ecs->event_thread->control.step_resume_breakpoint == NULL) + && ecs->event_thread->control.step_resume_breakpoint == nullptr) { /* We were just starting a new sequence, attempting to single-step off of a breakpoint and expecting a SIGTRAP. @@ -6574,7 +6574,7 @@ handle_signal_stop (struct execution_control_state *ecs) || ecs->event_thread->control.step_range_end == 1) && (get_stack_frame_id (frame) == ecs->event_thread->control.step_stack_frame_id) - && ecs->event_thread->control.step_resume_breakpoint == NULL) + && ecs->event_thread->control.step_resume_breakpoint == nullptr) { /* The inferior is about to take a signal that will take it out of the single step range. Set a breakpoint at the @@ -6719,7 +6719,7 @@ process_event_stop_test (struct execution_control_state *ecs) infrun_debug_printf ("BPSTAT_WHAT_CLEAR_LONGJMP_RESUME"); gdb_assert (ecs->event_thread->control.exception_resume_breakpoint - != NULL); + != nullptr); delete_exception_resume_breakpoint (ecs->event_thread); if (what.is_longjmp) @@ -6853,7 +6853,7 @@ process_event_stop_test (struct execution_control_state *ecs) struct breakpoint *sr_bp = ecs->event_thread->control.step_resume_breakpoint; - if (sr_bp != NULL + if (sr_bp != nullptr && sr_bp->loc->permanent && sr_bp->type == bp_hp_step_resume && sr_bp->loc->address == ecs->event_thread->prev_pc) @@ -7269,7 +7269,7 @@ process_event_stop_test (struct execution_control_state *ecs) the trampoline processing logic, however, there are some trampolines that have no names, so we should do trampoline handling first. */ if (ecs->event_thread->control.step_over_calls == STEP_OVER_UNDEBUGGABLE - && ecs->stop_func_name == NULL + && ecs->stop_func_name == nullptr && stop_pc_sal.line == 0) { infrun_debug_printf ("stepped into undebuggable function"); @@ -7771,7 +7771,7 @@ static bool currently_stepping (struct thread_info *tp) { return ((tp->control.step_range_end - && tp->control.step_resume_breakpoint == NULL) + && tp->control.step_resume_breakpoint == nullptr) || tp->control.trap_expected || tp->stepped_breakpoint || bpstat_should_step ()); @@ -7789,7 +7789,7 @@ handle_step_into_function (struct gdbarch *gdbarch, compunit_symtab *cust = find_pc_compunit_symtab (ecs->event_thread->stop_pc ()); - if (cust != NULL && cust->language () != language_asm) + if (cust != nullptr && cust->language () != language_asm) ecs->stop_func_start = gdbarch_skip_prologue_noexcept (gdbarch, ecs->stop_func_start); @@ -7868,7 +7868,7 @@ handle_step_into_function_backward (struct gdbarch *gdbarch, fill_in_stop_func (gdbarch, ecs); cust = find_pc_compunit_symtab (ecs->event_thread->stop_pc ()); - if (cust != NULL && cust->language () != language_asm) + if (cust != nullptr && cust->language () != language_asm) ecs->stop_func_start = gdbarch_skip_prologue_noexcept (gdbarch, ecs->stop_func_start); @@ -7904,7 +7904,7 @@ insert_step_resume_breakpoint_at_sal_1 (struct gdbarch *gdbarch, /* There should never be more than one step-resume or longjmp-resume breakpoint per thread, so we should never be setting a new step_resume_breakpoint when one is already active. */ - gdb_assert (inferior_thread ()->control.step_resume_breakpoint == NULL); + gdb_assert (inferior_thread ()->control.step_resume_breakpoint == nullptr); gdb_assert (sr_type == bp_step_resume || sr_type == bp_hp_step_resume); infrun_debug_printf ("inserting step-resume breakpoint at %s", @@ -7934,7 +7934,7 @@ insert_step_resume_breakpoint_at_sal (struct gdbarch *gdbarch, static void insert_hp_step_resume_breakpoint_at_frame (frame_info_ptr return_frame) { - gdb_assert (return_frame != NULL); + gdb_assert (return_frame != nullptr); struct gdbarch *gdbarch = get_frame_arch (return_frame); @@ -7992,7 +7992,7 @@ insert_longjmp_resume_breakpoint (struct gdbarch *gdbarch, CORE_ADDR pc) /* There should never be more than one longjmp-resume breakpoint per thread, so we should never be setting a new longjmp_resume_breakpoint when one is already active. */ - gdb_assert (inferior_thread ()->control.exception_resume_breakpoint == NULL); + gdb_assert (inferior_thread ()->control.exception_resume_breakpoint == nullptr); infrun_debug_printf ("inserting longjmp-resume breakpoint at %s", paddress (gdbarch, pc)); @@ -8036,7 +8036,7 @@ insert_exception_resume_breakpoint (struct thread_info *tp, bp_exception_resume).release (); /* set_momentary_breakpoint_at_pc invalidates FRAME. */ - frame = NULL; + frame = nullptr; bp->thread = tp->global_num; inferior_thread ()->control.exception_resume_breakpoint = bp; @@ -8241,7 +8241,7 @@ keep_going_pass_signal (struct execution_control_state *ecs) ecs->event_thread->global_num); } else if (remove_wps) - set_step_over_info (NULL, 0, remove_wps, -1); + set_step_over_info (nullptr, 0, remove_wps, -1); /* If we now need to do an in-line step-over, we need to stop all other threads. Note this must be done before @@ -8403,7 +8403,7 @@ print_signal_received_reason (struct ui_out *uiout, enum gdb_signal siggnal) uiout->field_string ("thread-id", print_thread_id (thr)); const char *name = thread_name (thr); - if (name != NULL) + if (name != nullptr) { uiout->text (" \""); uiout->field_string ("name", name); @@ -8501,7 +8501,7 @@ print_stop_location (const target_waitstatus &ws) LOCATION: Print only location SRC_AND_LOC: Print location and source line. */ if (do_frame_printing) - print_stack_frame (get_selected_frame (NULL), 0, source_flag, 1); + print_stack_frame (get_selected_frame (nullptr), 0, source_flag, 1); } /* See infrun.h. */ @@ -8605,7 +8605,7 @@ stop_context::changed () const return true; if (inf_num != current_inferior ()->num) return true; - if (thread != NULL && thread->state != THREAD_STOPPED) + if (thread != nullptr && thread->state != THREAD_STOPPED) return true; if (get_stop_id () != stop_id) return true; @@ -8782,7 +8782,7 @@ normal_stop (void) gdb::observers::normal_stop.notify (inferior_thread ()->control.stop_bpstat, stop_print_frame); else - gdb::observers::normal_stop.notify (NULL, stop_print_frame); + gdb::observers::normal_stop.notify (nullptr, stop_print_frame); annotate_stopped (); @@ -8912,7 +8912,7 @@ handle_command (const char *args, int from_tty) enum gdb_signal oursig; int allsigs; - if (args == NULL) + if (args == nullptr) { error_no_arg (_("signal to handle")); } @@ -9088,7 +9088,7 @@ handle_completer (struct cmd_list_element *ignore, "noignore", "noprint", "nopass", - NULL, + nullptr, }; signal_completer (ignore, tracker, text, word); @@ -9172,7 +9172,7 @@ siginfo_value_read (struct value *v) transferred = target_read (current_inferior ()->top_target (), TARGET_OBJECT_SIGNAL_INFO, - NULL, + nullptr, value_contents_all_raw (v).data (), value_offset (v), value_type (v)->length ()); @@ -9195,7 +9195,7 @@ siginfo_value_write (struct value *v, struct value *fromval) transferred = target_write (current_inferior ()->top_target (), TARGET_OBJECT_SIGNAL_INFO, - NULL, + nullptr, value_contents_all_raw (fromval).data (), value_offset (v), value_type (fromval)->length ()); @@ -9224,7 +9224,7 @@ siginfo_make_value (struct gdbarch *gdbarch, struct internalvar *var, { struct type *type = gdbarch_get_siginfo_type (gdbarch); - return allocate_computed_value (type, &siginfo_value_funcs, NULL); + return allocate_computed_value (type, &siginfo_value_funcs, nullptr); } return allocate_value (builtin_type (gdbarch)->builtin_void); @@ -9259,7 +9259,7 @@ class infcall_suspend_state siginfo_data.reset ((gdb_byte *) xmalloc (len)); if (target_read (current_inferior ()->top_target (), - TARGET_OBJECT_SIGNAL_INFO, NULL, + TARGET_OBJECT_SIGNAL_INFO, nullptr, siginfo_data.get (), 0, len) != len) { /* Errors ignored. */ @@ -9295,7 +9295,7 @@ class infcall_suspend_state /* Errors ignored. */ target_write (current_inferior ()->top_target (), - TARGET_OBJECT_SIGNAL_INFO, NULL, + TARGET_OBJECT_SIGNAL_INFO, nullptr, m_siginfo_data.get (), 0, type->length ()); } @@ -9399,8 +9399,8 @@ save_infcall_control_state () inf_status->thread_control = tp->control; inf_status->inferior_control = inf->control; - tp->control.step_resume_breakpoint = NULL; - tp->control.exception_resume_breakpoint = NULL; + tp->control.step_resume_breakpoint = nullptr; + tp->control.exception_resume_breakpoint = nullptr; /* Save original bpstat chain to INF_STATUS; replace it in TP with copy of chain. If caller's caller is walking the chain, they'll be happier if we @@ -9490,7 +9490,7 @@ static const char *exec_direction = exec_forward; static const char *const exec_direction_names[] = { exec_forward, exec_reverse, - NULL + nullptr }; static void @@ -9541,7 +9541,7 @@ show_schedule_multiple (struct ui_file *file, int from_tty, static const struct internalvar_funcs siginfo_funcs = { siginfo_make_value, - NULL, + nullptr, }; /* Callback for infrun's target events source. This is marked when a @@ -9637,7 +9637,7 @@ _initialize_infrun () /* Register extra event sources in the event loop. */ infrun_async_inferior_event_token - = create_async_event_handler (infrun_async_inferior_event_handler, NULL, + = create_async_event_handler (infrun_async_inferior_event_handler, nullptr, "infrun"); cmd_list_element *info_signals_cmd @@ -9683,7 +9683,7 @@ of the program stops."), &cmdlist); _("Set inferior debugging."), _("Show inferior debugging."), _("When non-zero, inferior specific debugging is enabled."), - NULL, show_debug_infrun, &setdebuglist, &showdebuglist); + nullptr, show_debug_infrun, &setdebuglist, &showdebuglist); add_setshow_boolean_cmd ("non-stop", no_class, &non_stop_1, _("\ @@ -9786,7 +9786,7 @@ A fork or vfork creates a new process. follow-fork-mode can be:\n\ child - the new process is debugged after a fork\n\ The unfollowed process will continue to run.\n\ By default, the debugger will follow the parent process."), - NULL, + nullptr, show_follow_fork_mode_string, &setlist, &showlist); @@ -9810,7 +9810,7 @@ the inferior. Restarting the inferior after the exec call restarts\n\ the executable the process was running after the exec call.\n\ \n\ By default, the debugger will use the same inferior."), - NULL, + nullptr, show_follow_exec_mode_string, &setlist, &showlist); @@ -9837,7 +9837,7 @@ threads of all processes. When off (which is the default), execution\n\ commands only resume the threads of the current process. The set of\n\ threads that are resumed is further refined by the scheduler-locking\n\ mode (see help set scheduler-locking)."), - NULL, + nullptr, show_schedule_multiple, &setlist, &showlist); @@ -9847,7 +9847,7 @@ Show mode of the step operation."), _("\ When set, doing a step over a function without debug line information\n\ will stop at the first instruction of that function. Otherwise, the\n\ function is skipped and the step command stops at a different source line."), - NULL, + nullptr, show_step_stop_if_no_debug, &setlist, &showlist); @@ -9861,7 +9861,7 @@ stepping to step over breakpoints, even if such is supported by the target\n\ architecture. If auto (which is the default), gdb will use displaced stepping\n\ if the target architecture supports it and non-stop mode is active, but will not\n\ use it in all-stop mode (see help set non-stop)."), - NULL, + nullptr, show_can_use_displaced_stepping, &setlist, &showlist); @@ -9879,7 +9879,7 @@ Options are 'forward' or 'reverse'."), Set whether gdb will detach the child of a fork."), _("\ Show whether gdb will detach the child of a fork."), _("\ Tells gdb whether to detach the child of a fork."), - NULL, NULL, &setlist, &showlist); + nullptr, nullptr, &setlist, &showlist); /* Set/show disable address space randomization mode. */ @@ -9910,7 +9910,7 @@ enabled by default on some platforms."), value with a void typed value, and when we get here, gdbarch isn't initialized yet. At this point, we're quite sure there isn't another convenience variable of the same name. */ - create_internalvar_type_lazy ("_siginfo", &siginfo_funcs, NULL); + create_internalvar_type_lazy ("_siginfo", &siginfo_funcs, nullptr); add_setshow_boolean_cmd ("observer", no_class, &observer_mode_1, _("\ -- 2.31.1 ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH] Change NULL to nullptr in gdb/infcmd.c and gdb/infrun.c 2022-11-16 20:55 ` [PATCH] Change NULL to nullptr in gdb/infcmd.c and gdb/infrun.c Carl Love @ 2022-11-16 21:15 ` Simon Marchi 0 siblings, 0 replies; 44+ messages in thread From: Simon Marchi @ 2022-11-16 21:15 UTC (permalink / raw) To: Carl Love, Lancelot SIX Cc: Ulrich Weigand, gdb-patches, tdevries, kevinb, will_schmidt, blarsen On 11/16/22 15:55, Carl Love wrote: > Lancelot, GDB maintainers: > > The GDB coding style guide specifies that nullptr should be used > instead of NULL as noted by Lancelot for the recent patch "PowerPC, fix > support for printing the function return value for non-trivial > values.". This patch changes all of the various NULL statements to > nullptr statements in files gdb/infcmd.c and gdb/infrun.c per the > coding style guide. > > The patch does not make any functional changes to the code. > > The patch has been tested on both X86_64 and PowerPC to ensure there > were no new unexpected error, new core files generated, new unresolved > tests etc. > > Please let me know if this patch is acceptable. Thanks. > > Carl Love > > > ------------------------ > Change NULL to nullptr in gdb/infcmd.c and gdb/infrun.c > > The GDB coding standard specifies that nullptr should be used instead of NULL. > There are numerous uses of NULL and nullptr in files infcmd.c and infrun.c. > This patch replaces the various uses of NULL with nullptr in the source files. > The use of NULL in the comments was not changed. > > The patch does not introduce any functional changes. > > The patch has been tested on PowerPC and Intel X86_64 with no new unexpected > test failures, unresolved tests, new core files etc. Thanks, this LGTM: Approved-By: Simon Marchi <simon.marchi@efficios.com> I appreciate this kind of cleanup that bring the code in line with what the current recommended practices are. I was thinking the other day if we could do a mass find and replace from NULL to nullptr. I am a just a bit afraid that there might be instances we don't want to change, in comments or literal strings, so we have to be a bit more careful. Simon ^ permalink raw reply [flat|nested] 44+ messages in thread
[parent not found: <5a34aaeab59f0099b915d1780c701284a6cf691e.camel@us.ibm.com>]
[parent not found: <8aa882863b2f4cef38c22386387c5705bf63c3d5.camel@de.ibm.com>]
* [PATCH 1/2] PowerPC, function ppc64_sysv_abi_return_value add missing return value convention [not found] ` <8aa882863b2f4cef38c22386387c5705bf63c3d5.camel@de.ibm.com> @ 2022-10-06 16:37 ` Carl Love 2022-10-08 4:20 ` Kevin Buettner 2022-10-18 18:55 ` [PATCH 1/2 ver 2] " Carl Love 0 siblings, 2 replies; 44+ messages in thread From: Carl Love @ 2022-10-06 16:37 UTC (permalink / raw) To: Ulrich Weigand, gdb-patches GDB maintainers: The following PowerPC specific patch fixes an issue of GDB reporting a bogus return value for functions that return a non-trivial value. The bogus return values result in five testcase failures for test gdb.cp/non-trivial-retval.exp. The issue is the function ppc64_sysv_abi_return_value does not return the correct value when the valtype->code() is TYPE_CODE_STRUCT and the language_pass_by_reference is not trivially_copyable. This patch adds the needed code to return RETURN_VALUE_STRUCT_CONVENTION in these cases. The testcase gdb.cp/non-trivial-retval.exp still fails as gdb now correctly reports "Cannot determine contents" instead of the expected values, which is correct in this case. The PowerPC ABI uses passes the return buffer address in r3. The value of r3 is valid on entry to the function but the PowerPC ABI does not guarantee it will not be changed in the function. Hence the contents of r3 is not reliable on exit from the function. This issue will be addressed by the next patch in this patch series. The patch has been tested on PowerPC and on Intel X86-64 with no regression failures. Please let me know if this patch is acceptable for the GDB mainline. Thanks. Carl Love ----------------------------------------- PowerPC, function ppc64_sysv_abi_return_value add missing return value convention This patch address five testcase failures in gdb.cp/non-trivial-retval.exp. The following commit resulted in the five testcases failures on PowerPC. The value returned by the function is being reported incorrectly. commit b1718fcdd1d2a5c514f8ee504ba07fb3f42b8608 Author: Andrew Burgess <aburgess@redhat.com> Date: Mon Dec 13 16:56:16 2021 +0000 gdb: on x86-64 non-trivial C++ objects are returned in memory Fixes PR gdb/28681. It was observed that after using the `finish` command an incorrect value was displayed in some cases. Specifically, this behaviour was observed on an x86-64 target. The function: enum return_value_convention ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf) should return RETURN_VALUE_STRUCT_CONVENTION if the valtype->code() is TYPE_CODE_STRUCT and if the language_pass_by_reference is not trivially_copyable. This patch adds the need code to return the value RETURN_VALUE_STRUCT_CONVENTION in the case of this case. With this patch, the five test cases still fail but with the message "Value returned has type: A. Cannot determine contents". The PowerPC ABI stores the address of the buffer containing the function return value in register r3 on entry to the function. However, the PowerPC ABI does not guarentee that r3 will not be modified in the function. So when the function returns, the return buffer address cannot be reliably obtained from register r3. Thus the message "Cannot determine contents" is appropriate in this case. --- gdb/ppc-sysv-tdep.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c index f57c261d9dc..14effb93210 100644 --- a/gdb/ppc-sysv-tdep.c +++ b/gdb/ppc-sysv-tdep.c @@ -2099,6 +2099,10 @@ ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, return RETURN_VALUE_REGISTER_CONVENTION; } + if (!language_pass_by_reference (valtype).trivially_copyable + && valtype->code () == TYPE_CODE_STRUCT) + return RETURN_VALUE_STRUCT_CONVENTION; + /* In the ELFv2 ABI, aggregate types of up to 16 bytes are returned in registers r3:r4. */ if (tdep->elf_abi == POWERPC_ELF_V2 -- 2.31.1 ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 1/2] PowerPC, function ppc64_sysv_abi_return_value add missing return value convention 2022-10-06 16:37 ` [PATCH 1/2] PowerPC, function ppc64_sysv_abi_return_value add missing return value convention Carl Love @ 2022-10-08 4:20 ` Kevin Buettner 2022-10-14 23:20 ` Carl Love 2022-10-18 18:55 ` [PATCH 1/2 ver 2] " Carl Love 1 sibling, 1 reply; 44+ messages in thread From: Kevin Buettner @ 2022-10-08 4:20 UTC (permalink / raw) To: Carl Love via Gdb-patches; +Cc: Carl Love, Ulrich Weigand Hi Carl, See my comments below... On Thu, 06 Oct 2022 09:37:10 -0700 Carl Love via Gdb-patches <gdb-patches@sourceware.org> wrote: > GDB maintainers: > > The following PowerPC specific patch fixes an issue of GDB reporting a > bogus return value for functions that return a non-trivial value. The > bogus return values result in five testcase failures for test > gdb.cp/non-trivial-retval.exp. The issue is the function > ppc64_sysv_abi_return_value does not return the correct value when the > valtype->code() is TYPE_CODE_STRUCT and the language_pass_by_reference > is not trivially_copyable. This patch adds the needed code to return > RETURN_VALUE_STRUCT_CONVENTION in these cases. > > The testcase gdb.cp/non-trivial-retval.exp still fails as gdb now > correctly reports "Cannot determine contents" instead of the expected > values, which is correct in this case. The PowerPC ABI uses passes the > return buffer address in r3. The value of r3 is valid on entry to the > function but the PowerPC ABI does not guarantee it will not be changed > in the function. Hence the contents of r3 is not reliable on exit from > the function. This issue will be addressed by the next patch in this > patch series. > > The patch has been tested on PowerPC and on Intel X86-64 with no > regression failures. > > Please let me know if this patch is acceptable for the GDB mainline. > Thanks. > > Carl Love > > ----------------------------------------- > PowerPC, function ppc64_sysv_abi_return_value add missing return value convention > > This patch address five testcase failures in gdb.cp/non-trivial-retval.exp. > The following commit resulted in the five testcases failures on PowerPC. The > value returned by the function is being reported incorrectly. > > commit b1718fcdd1d2a5c514f8ee504ba07fb3f42b8608 > Author: Andrew Burgess <aburgess@redhat.com> > Date: Mon Dec 13 16:56:16 2021 +0000 > > gdb: on x86-64 non-trivial C++ objects are returned in memory > > Fixes PR gdb/28681. It was observed that after using the `finish` > command an incorrect value was displayed in some cases. Specifically, > this behaviour was observed on an x86-64 target. > > The function: > > enum return_value_convention > ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, > struct type *valtype, struct regcache *regcache, > gdb_byte *readbuf, const gdb_byte *writebuf) > > should return RETURN_VALUE_STRUCT_CONVENTION if the valtype->code() is > TYPE_CODE_STRUCT and if the language_pass_by_reference is not > trivially_copyable. > > This patch adds the need code to return the value s/need/needed/ > RETURN_VALUE_STRUCT_CONVENTION in the case of this case. s/the case of // > > With this patch, the five test cases still fail but with the message "Value > returned has type: A. Cannot determine contents". The PowerPC ABI stores the > address of the buffer containing the function return value in register r3 on > entry to the function. However, the PowerPC ABI does not guarentee that r3 > will not be modified in the function. So when the function returns, the return > buffer address cannot be reliably obtained from register r3. Thus the message > "Cannot determine contents" is appropriate in this case. > --- > gdb/ppc-sysv-tdep.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c > index f57c261d9dc..14effb93210 100644 > --- a/gdb/ppc-sysv-tdep.c > +++ b/gdb/ppc-sysv-tdep.c > @@ -2099,6 +2099,10 @@ ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, > return RETURN_VALUE_REGISTER_CONVENTION; > } > > + if (!language_pass_by_reference (valtype).trivially_copyable > + && valtype->code () == TYPE_CODE_STRUCT) > + return RETURN_VALUE_STRUCT_CONVENTION; > + > /* In the ELFv2 ABI, aggregate types of up to 16 bytes are > returned in registers r3:r4. */ > if (tdep->elf_abi == POWERPC_ELF_V2 > -- > 2.31.1 This change looks good to me, but note the tweaks to the commit log remarks above. Kevin P.S. I'm still looking at part 2. ^ permalink raw reply [flat|nested] 44+ messages in thread
* RE: [PATCH 1/2] PowerPC, function ppc64_sysv_abi_return_value add missing return value convention 2022-10-08 4:20 ` Kevin Buettner @ 2022-10-14 23:20 ` Carl Love 0 siblings, 0 replies; 44+ messages in thread From: Carl Love @ 2022-10-14 23:20 UTC (permalink / raw) To: Kevin Buettner, gdb-patches; +Cc: Ulrich Weigand, cel On Fri, 2022-10-07 at 21:20 -0700, Kevin Buettner wrote: > Hi Carl, > > See my comments below... > > On Thu, 06 Oct 2022 09:37:10 -0700 > Carl Love via Gdb-patches <gdb-patches@sourceware.org> wrote: > <snip> > > should return RETURN_VALUE_STRUCT_CONVENTION if the valtype->code() > > is > > TYPE_CODE_STRUCT and if the language_pass_by_reference is not > > trivially_copyable. > > > > This patch adds the need code to return the value > > s/need/needed/ fixed > > > RETURN_VALUE_STRUCT_CONVENTION in the case of this case. > > s/the case of // fixed. ^ permalink raw reply [flat|nested] 44+ messages in thread
* [PATCH 1/2 ver 2] PowerPC, function ppc64_sysv_abi_return_value add missing return value convention 2022-10-06 16:37 ` [PATCH 1/2] PowerPC, function ppc64_sysv_abi_return_value add missing return value convention Carl Love 2022-10-08 4:20 ` Kevin Buettner @ 2022-10-18 18:55 ` Carl Love 2022-11-07 20:04 ` [PATCH 1/2 ver 3] " Carl Love 1 sibling, 1 reply; 44+ messages in thread From: Carl Love @ 2022-10-18 18:55 UTC (permalink / raw) To: Ulrich Weigand, gdb-patches GDB maintainers: Version 2, updated the commit log per feedback. The following PowerPC specific patch fixes an issue of GDB reporting a bogus return value for functions that return a non-trivial value. The bogus return values result in five testcase failures for test gdb.cp/non-trivial-retval.exp. The issue is the function ppc64_sysv_abi_return_value does not return the correct value when the valtype->code() is TYPE_CODE_STRUCT and the language_pass_by_reference is not trivially_copyable. This patch adds the needed code to return RETURN_VALUE_STRUCT_CONVENTION in these cases. The testcase gdb.cp/non-trivial-retval.exp still fails as gdb now correctly reports "Cannot determine contents" instead of the expected values, which is correct in this case. The PowerPC ABI uses passes the return buffer address in r3. The value of r3 is valid on entry to the function but the PowerPC ABI does not guarantee it will not be changed in the function. Hence the contents of r3 is not reliable on exit from the function. This issue will be addressed by the next patch in this patch series. The patch has been re-tested on PowerPC and on Intel X86-64 with no regression failures. Please let me know if this version of the patch is acceptable for the GDB mainline. Thanks. Carl Love ---------------------------- PowerPC, function ppc64_sysv_abi_return_value add missing return value convention This patch address five testcase failures in gdb.cp/non-trivial-retval.exp. The following commit resulted in the five testcases failures on PowerPC. The value returned by the function is being reported incorrectly. commit b1718fcdd1d2a5c514f8ee504ba07fb3f42b8608 Author: Andrew Burgess <aburgess@redhat.com> Date: Mon Dec 13 16:56:16 2021 +0000 gdb: on x86-64 non-trivial C++ objects are returned in memory Fixes PR gdb/28681. It was observed that after using the `finish` command an incorrect value was displayed in some cases. Specifically, this behaviour was observed on an x86-64 target. The function: enum return_value_convention ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf) should return RETURN_VALUE_STRUCT_CONVENTION if the valtype->code() is TYPE_CODE_STRUCT and if the language_pass_by_reference is not trivially_copyable. This patch adds the need code to return the value RETURN_VALUE_STRUCT_CONVENTION in the case of this case. With this patch, the five test cases still fail but with the message "Value returned has type: A. Cannot determine contents". The PowerPC ABI stores the address of the buffer containing the function return value in register r3 on entry to the function. However, the PowerPC ABI does not guarentee that r3 will not be modified in the function. So when the function returns, the return buffer address cannot be reliably obtained from register r3. Thus the message "Cannot determine contents" is appropriate in this case. --- gdb/ppc-sysv-tdep.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c index f57c261d9dc..14effb93210 100644 --- a/gdb/ppc-sysv-tdep.c +++ b/gdb/ppc-sysv-tdep.c @@ -2099,6 +2099,10 @@ ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, return RETURN_VALUE_REGISTER_CONVENTION; } + if (!language_pass_by_reference (valtype).trivially_copyable + && valtype->code () == TYPE_CODE_STRUCT) + return RETURN_VALUE_STRUCT_CONVENTION; + /* In the ELFv2 ABI, aggregate types of up to 16 bytes are returned in registers r3:r4. */ if (tdep->elf_abi == POWERPC_ELF_V2 -- 2.37.2 ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 1/2 ver 3] PowerPC, function ppc64_sysv_abi_return_value add missing return value convention 2022-10-18 18:55 ` [PATCH 1/2 ver 2] " Carl Love @ 2022-11-07 20:04 ` Carl Love 2022-11-14 16:45 ` Ulrich Weigand 0 siblings, 1 reply; 44+ messages in thread From: Carl Love @ 2022-11-07 20:04 UTC (permalink / raw) To: Ulrich Weigand, gdb-patches; +Cc: will_schmidt, blarsen, Kevin Buettner, cel Ping on this patch. I have not received any additional comments on the patch. I have rebased and retested both patches in the series onto the latest gdb source tree. This patch applied cleanly on the latest tree. GDB maintainers: Version 3, rebased patch on the latest tree. No additional changes made to the patch. Patch has been retested with no regressions. Version 2, updated the commit log per feedback. The following PowerPC specific patch fixes an issue of GDB reporting a bogus return value for functions that return a non-trivial value. The bogus return values result in five testcase failures for test gdb.cp/non-trivial-retval.exp. The issue is the function ppc64_sysv_abi_return_value does not return the correct value when the valtype->code() is TYPE_CODE_STRUCT and the language_pass_by_reference is not trivially_copyable. This patch adds the needed code to return RETURN_VALUE_STRUCT_CONVENTION in these cases. The testcase gdb.cp/non-trivial-retval.exp still fails as gdb now correctly reports "Cannot determine contents" instead of the expected values, which is correct in this case. The PowerPC ABI uses passes the return buffer address in r3. The value of r3 is valid on entry to the function but the PowerPC ABI does not guarantee it will not be changed in the function. Hence the contents of r3 is not reliable on exit from the function. This issue will be addressed by the next patch in this patch series. The patch has been re-tested on PowerPC and on Intel X86-64 with no regression failures. Please let me know if this version of the patch is acceptable for the GDB mainline. Thanks. Carl Love ------------------------------- PowerPC, function ppc64_sysv_abi_return_value add missing return value convention This patch address five testcase failures in gdb.cp/non-trivial-retval.exp. The following commit resulted in the five testcases failures on PowerPC. The value returned by the function is being reported incorrectly. commit b1718fcdd1d2a5c514f8ee504ba07fb3f42b8608 Author: Andrew Burgess <aburgess@redhat.com> Date: Mon Dec 13 16:56:16 2021 +0000 gdb: on x86-64 non-trivial C++ objects are returned in memory Fixes PR gdb/28681. It was observed that after using the `finish` command an incorrect value was displayed in some cases. Specifically, this behaviour was observed on an x86-64 target. The function: enum return_value_convention ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf) should return RETURN_VALUE_STRUCT_CONVENTION if the valtype->code() is TYPE_CODE_STRUCT and if the language_pass_by_reference is not trivially_copyable. This patch adds the need code to return the value RETURN_VALUE_STRUCT_CONVENTION in the case of this case. With this patch, the five test cases still fail but with the message "Value returned has type: A. Cannot determine contents". The PowerPC ABI stores the address of the buffer containing the function return value in register r3 on entry to the function. However, the PowerPC ABI does not guarentee that r3 will not be modified in the function. So when the function returns, the return buffer address cannot be reliably obtained from register r3. Thus the message "Cannot determine contents" is appropriate in this case. --- gdb/ppc-sysv-tdep.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gdb/ppc-sysv-tdep.c b/gdb/ppc-sysv-tdep.c index d7f05ddc6b4..450162dd46e 100644 --- a/gdb/ppc-sysv-tdep.c +++ b/gdb/ppc-sysv-tdep.c @@ -2099,6 +2099,10 @@ ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, return RETURN_VALUE_REGISTER_CONVENTION; } + if (!language_pass_by_reference (valtype).trivially_copyable + && valtype->code () == TYPE_CODE_STRUCT) + return RETURN_VALUE_STRUCT_CONVENTION; + /* In the ELFv2 ABI, aggregate types of up to 16 bytes are returned in registers r3:r4. */ if (tdep->elf_abi == POWERPC_ELF_V2 -- 2.37.2 ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 1/2 ver 3] PowerPC, function ppc64_sysv_abi_return_value add missing return value convention 2022-11-07 20:04 ` [PATCH 1/2 ver 3] " Carl Love @ 2022-11-14 16:45 ` Ulrich Weigand 2022-11-14 19:38 ` Carl Love 0 siblings, 1 reply; 44+ messages in thread From: Ulrich Weigand @ 2022-11-14 16:45 UTC (permalink / raw) To: gdb-patches, cel; +Cc: kevinb, will_schmidt, blarsen Carl Love <cel@us.ibm.com> wrote: >------------------------------- >PowerPC, function ppc64_sysv_abi_return_value add missing return value convention > >This patch address five testcase failures in gdb.cp/non-trivial-retval.exp. >The following commit resulted in the five testcases failures on PowerPC. The >value returned by the function is being reported incorrectly. > > commit b1718fcdd1d2a5c514f8ee504ba07fb3f42b8608 > Author: Andrew Burgess <aburgess@redhat.com> > Date: Mon Dec 13 16:56:16 2021 +0000 > > gdb: on x86-64 non-trivial C++ objects are returned in memory > > Fixes PR gdb/28681. It was observed that after using the `finish` > command an incorrect value was displayed in some cases. Specifically, > this behaviour was observed on an x86-64 target. > >The function: > > enum return_value_convention > ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value *function, > struct type *valtype, struct regcache *regcache, > gdb_byte *readbuf, const gdb_byte *writebuf) > >should return RETURN_VALUE_STRUCT_CONVENTION if the valtype->code() is >TYPE_CODE_STRUCT and if the language_pass_by_reference is not >trivially_copyable. > >This patch adds the need code to return the value >RETURN_VALUE_STRUCT_CONVENTION in the case of this case. This still has the typos Kevin pointed out. >With this patch, the five test cases still fail but with the message "Value >returned has type: A. Cannot determine contents". The PowerPC ABI stores the >address of the buffer containing the function return value in register r3 on >entry to the function. However, the PowerPC ABI does not guarentee that r3 >will not be modified in the function. So when the function returns, the return >buffer address cannot be reliably obtained from register r3. Thus the message >"Cannot determine contents" is appropriate in this case. OK with the above typos fixed. Bye, Ulrich ^ permalink raw reply [flat|nested] 44+ messages in thread
* Re: [PATCH 1/2 ver 3] PowerPC, function ppc64_sysv_abi_return_value add missing return value convention 2022-11-14 16:45 ` Ulrich Weigand @ 2022-11-14 19:38 ` Carl Love 0 siblings, 0 replies; 44+ messages in thread From: Carl Love @ 2022-11-14 19:38 UTC (permalink / raw) To: Ulrich Weigand, gdb-patches; +Cc: kevinb, will_schmidt, blarsen, cel On Mon, 2022-11-14 at 16:45 +0000, Ulrich Weigand wrote: > Carl Love <cel@us.ibm.com> wrote: > > > ------------------------------- > > PowerPC, function ppc64_sysv_abi_return_value add missing return > > value convention > > > > This patch address five testcase failures in gdb.cp/non-trivial- > > retval.exp. > > The following commit resulted in the five testcases failures on > > PowerPC. The > > value returned by the function is being reported incorrectly. > > > > commit b1718fcdd1d2a5c514f8ee504ba07fb3f42b8608 > > Author: Andrew Burgess <aburgess@redhat.com> > > Date: Mon Dec 13 16:56:16 2021 +0000 > > > > gdb: on x86-64 non-trivial C++ objects are returned in memory > > > > Fixes PR gdb/28681. It was observed that after using the > > `finish` > > command an incorrect value was displayed in some > > cases. Specifically, > > this behaviour was observed on an x86-64 target. > > > > The function: > > > > enum return_value_convention > > ppc64_sysv_abi_return_value (struct gdbarch *gdbarch, struct value > > *function, > > struct type *valtype, struct regcache > > *regcache, > > gdb_byte *readbuf, const gdb_byte > > *writebuf) > > > > should return RETURN_VALUE_STRUCT_CONVENTION if the valtype->code() > > is > > TYPE_CODE_STRUCT and if the language_pass_by_reference is not > > trivially_copyable. > > > > This patch adds the need code to return the value > > RETURN_VALUE_STRUCT_CONVENTION in the case of this case. > > This still has the typos Kevin pointed out. > > > With this patch, the five test cases still fail but with the > > message "Value > > returned has type: A. Cannot determine contents". The PowerPC ABI > > stores the > > address of the buffer containing the function return value in > > register r3 on > > entry to the function. However, the PowerPC ABI does not guarentee > > that r3 > > will not be modified in the function. So when the function > > returns, the return > > buffer address cannot be reliably obtained from register r3. Thus > > the message > > "Cannot determine contents" is appropriate in this case. > > OK with the above typos fixed. > Yes, I had fixed the typos once. But it seems I didn't get the patch updated on all of the systems where I was testing/developing the patch and inadvertently lost the updates to this patch. I have fixed the typos again and updated the patch on the various test/development systems. Sorry about that. Carl ^ permalink raw reply [flat|nested] 44+ messages in thread
end of thread, other threads:[~2022-11-16 21:15 UTC | newest] Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-11 22:26 [PATCH] PowerPC, add support for printing non-trivial C++ object for the finish command Carl Love 2022-03-13 5:28 ` Joel Brobecker 2022-03-14 10:43 ` Luis Machado 2022-03-14 13:40 ` Tom Tromey 2022-03-14 13:45 ` Luis Machado 2022-03-14 14:58 ` Ulrich Weigand [not found] ` <ce6c71356c4a58fcdfb655a6e50c3a24f812e66c.camel@us.ibm.com> [not found] ` <d9f17525c9a03c20b54015a6a71c36cae50fe4e3.camel@de.ibm.com> [not found] ` <6ca2276426343756e103995e07ff951d6e26837b.camel@us.ibm.com> [not found] ` <939797b94ab71f3f7356747d84a1515939cb3dcc.camel@de.ibm.com> [not found] ` <fccc34c438fda9a35b8a8565e0f5026237e7eab9.camel@us.ibm.com> [not found] ` <bb5b9b137fc11886964113d4b524526b3c733f4a.camel@us.ibm.com> [not found] ` <1edb818bd2873a3fa5278f28131089d228a0a4f6.camel@de.ibm.com> [not found] ` <7c884a865d06890cb325225c65d7a52fdfbd20d2.camel@us.ibm.com> [not found] ` <846ca96309d2732d3db0e4c323a81105c098fa5f.camel@de.ibm.com> [not found] ` <5a858dd7b957ecf45cf5b00ffc140a839c8ef023.camel@us.ibm.com> [not found] ` <b634fecae5e33a3d1a278191c37f306a3b8622f2.camel@de.ibm.com> [not found] ` <25f2380ced176f58a8e3ea9b70c7e7786988d650.camel@us.ibm.com> [not found] ` <2b0481466e9ecc33d52c74c3a3b4babb05435f47.camel@de.ibm.com> [not found] ` <df3b049416ff666e7bd3e3a91e4ea90d34256ea5.camel@us.ibm.com> [not found] ` <71370ce02bd57827d3b7958772b1594d3591bd16.camel@de.ibm.com> [not found] ` <ec9dafb1671699b03b28ee4be528711c6988eaa5.camel@us.ibm.com> [not found] ` <148d8d3efcc8d110119e566027bfd0c65dd02525.camel@de.ibm.com> [not found] ` <eef62b295e97fc464c22f9d748ff818860137de9.camel@us.ibm.com> [not found] ` <afd6fa576f479359618b1ee50b08be8932735da8.camel@de.ibm.com> [not found] ` <cb6b19e9d287d2bae4b72627791f2a00af062c48.camel@us.ibm.com> [not found] ` <ee7101f86b5c8581905c53347fa603dc23ddc2bd.camel@de.ibm.com> [not found] ` <8decd662134d57e8caf43960a1cdc47723e2bfe3.camel@us.ibm.com> [not found] ` <f7cad695cf64540bad8c95cf5fd31691711d0eeb.camel@de.ibm.com> [not found] ` <79d82ed277308ed5ce312bff398e770ab234390a.camel@us.ibm.com> [not found] ` <63f21a897f452d81a73fb386cb99110a359ef0b7.camel@de.ibm.com> [not found] ` <be178bc4f356d7f1937458290cb5883eeee9eee1.camel@us.ibm.com> [not found] ` <dfd935e9414d3dd2c27d1e877d3718ae7510aa07.camel@de.ibm.com> [not found] ` <97275f61ef101a12cde8e5a45008ed8e479424eb.camel@us.ibm.com> [not found] ` <b629440707165f46fb466e48b0c95de3bfa334d2.camel@de.ibm.com> [not found] ` <191f5826b228a7614c084c9704b086851d418c78.camel@us.ibm.com> [not found] ` <5405a79ecd6ed34646ad77eed0779063ee222d37.camel@de.ibm.com> 2022-10-06 16:36 ` [PATCH 0/2] PowerPC, fix support for printing the function return value for non-trivial values Carl Love 2022-10-18 18:55 ` [PATCH 0/2 version 2] " Carl Love 2022-10-06 16:37 ` [PATCH 2/2] " Carl Love 2022-10-08 4:36 ` Kevin Buettner 2022-10-12 17:01 ` Carl Love 2022-10-14 2:49 ` Kevin Buettner 2022-10-14 7:36 ` Bruno Larsen 2022-10-14 23:25 ` Carl Love 2022-10-14 23:23 ` Carl Love 2022-10-18 1:06 ` Kevin Buettner 2022-10-18 18:26 ` Carl Love 2022-10-18 18:55 ` [PATCH 2/2 ver 2] " Carl Love 2022-10-31 16:07 ` Carl Love 2022-11-07 14:56 ` Bruno Larsen 2022-11-07 19:53 ` Carl Love 2022-11-07 20:04 ` [PATCH 2/2 ver 3] " Carl Love 2022-11-14 16:47 ` Ulrich Weigand 2022-11-15 7:15 ` Tom de Vries 2022-11-15 10:16 ` Ulrich Weigand 2022-11-15 16:04 ` Carl Love 2022-11-15 16:55 ` Simon Marchi 2022-11-15 23:46 ` Carl Love 2022-11-15 17:24 ` Carl Love 2022-11-15 18:05 ` Ulrich Weigand 2022-11-16 1:01 ` Carl Love 2022-11-16 9:52 ` Ulrich Weigand 2022-11-16 10:12 ` Tom de Vries 2022-11-16 10:20 ` Lancelot SIX 2022-11-16 15:56 ` Carl Love 2022-11-16 20:55 ` [PATCH] Change NULL to nullptr in gdb/infcmd.c and gdb/infrun.c Carl Love 2022-11-16 21:15 ` Simon Marchi [not found] ` <5a34aaeab59f0099b915d1780c701284a6cf691e.camel@us.ibm.com> [not found] ` <8aa882863b2f4cef38c22386387c5705bf63c3d5.camel@de.ibm.com> 2022-10-06 16:37 ` [PATCH 1/2] PowerPC, function ppc64_sysv_abi_return_value add missing return value convention Carl Love 2022-10-08 4:20 ` Kevin Buettner 2022-10-14 23:20 ` Carl Love 2022-10-18 18:55 ` [PATCH 1/2 ver 2] " Carl Love 2022-11-07 20:04 ` [PATCH 1/2 ver 3] " Carl Love 2022-11-14 16:45 ` Ulrich Weigand 2022-11-14 19:38 ` Carl Love
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).