[v4,3/3] Aarch64: Fix segfault when casting dummy calls
Commit Message
The following will segfault on aarch64 if foo is in another object,
was compiled as c++ and has no debug symbols:
(gdb) p (int)foo()
This is because to aarch64_push_dummy_call determines the return
type of the function and then does not check for null pointer.
A null pointer for the return type means the call has no debug
information. For the code to get here, then the call must have
been cast, otherwise we'd error out sooner. In the case of a
no-debug-info call cast, the return type is the type the user
had cast the call to, but we do not have that information
available here.
However, aarch64_push_dummy_call only requires the return type in
order to calculate lang_struct_return. This information is available
in the return_method enum. The fix is to simply use this instead.
Adds testcase to check calls across objects, with all combinations
of c, c++, debug and no debug.
gdb/ChangeLog:
2018-10-31 Alan Hayward <alan.hayward@arm.com>
PR gdb/22736:
* aarch64-tdep.c (aarch64_push_dummy_call): Remove
lang_struct_return code.
gdb/testsuite/ChangeLog:
2018-10-31 Alan Hayward <alan.hayward@arm.com>
PR gdb/22736:
* gdb.base/infcall-across-obj-lib.c: New test.
* gdb.base/infcall-across-obj-main.c: New test.
* gdb.base/infcall-across-obj.exp: New file.
---
gdb/aarch64-tdep.c | 32 +----
.../gdb.base/infcall-across-obj-lib.c | 22 +++
.../gdb.base/infcall-across-obj-main.c | 24 ++++
gdb/testsuite/gdb.base/infcall-across-obj.exp | 134 ++++++++++++++++++
4 files changed, 184 insertions(+), 28 deletions(-)
create mode 100644 gdb/testsuite/gdb.base/infcall-across-obj-lib.c
create mode 100644 gdb/testsuite/gdb.base/infcall-across-obj-main.c
create mode 100644 gdb/testsuite/gdb.base/infcall-across-obj.exp
Comments
On 10/31/2018 11:18 AM, Alan Hayward wrote:
> +++ b/gdb/testsuite/gdb.base/infcall-across-obj.exp
> @@ -0,0 +1,134 @@
> +# This testcase is part of GDB, the GNU debugger.
> +# Copyright 2018 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/>.
> +
> +# Test function calls on functions in another object.
> +# See gdb/22736
Add missing period.
This comment misses the most important aspect of the testcase/bugfix,
which is to call a C++ function with no debug info. Please extend the
comment to stress that.
The "in another object" thing is really a red herring; there's nothing
special about it, and there are other tests that cover that already.
Putting the called-function in a separate object is just so that the
main objfile is always compiled with debug info, so that GDB knows the
program is a C++ program.
(The only reason the infcall tests in gdb.base/nodebug.exp don't exercise
this is that that testcase is C. That's why I had suggested
gdb.cp/nodebug-infcall.exp previously. Anyway, naming not that important,
I guess.)
> +
> +if [target_info exists gdb,cannot_call_functions] {
> + unsupported "this target can not call functions"
> + continue
> +}
> +
> +# Only test c++ if we are able. Always use c
Double space after period. Add missing period in second
sentence.
> +if { [skip_cplus_tests] || [get_compiler_info "c++"] } {
> + set lang {c}
> +} else {
> + set lang {c c++}
> +}
> +
> +set main_basename infcall-across-obj-main
> +set lib_basename infcall-across-obj-lib
> +standard_testfile ${main_basename}.c ${lib_basename}.c
> +
> +set mainsrc "${srcdir}/${subdir}/${srcfile}"
> +set libsrc "${srcdir}/${subdir}/${srcfile2}"
> +
> +# Build both source files to objects using language LANG. Use SYMBOLS to build
> +# the with either debug symbols or without - but always build the main file
"build the with" is missing a word?
> +# with debug. Then make function calls across the files.
Double space after periods please, twice.
> +
> +proc build_and_run_test { lang symbols } {
> +
> + global main_basename lib_basename mainsrc libsrc binfile testfile
> + global gdb_prompt
> +
> + if { $symbols == "debug" } {
> + set debug_flags "debug"
> + } else {
> + set debug_flags ""
> + }
Mind space vs tabs above. Double check the rest of the patch too, please.
> +
> + # Setup directory.
> +
> + set dir "$lang-$symbols"
> + remote_exec build "rm -rf [standard_output_file ${dir}]"
> + remote_exec build "mkdir -p [standard_output_file ${dir}]"
> +
> + # Compile both files to objects, then link together.
> +
> + set main_flags "$lang debug"
> + set lib_flags "$lang $debug_flags"
> + set main_o [standard_output_file ${dir}/${main_basename}.o]
> + set lib_o [standard_output_file ${dir}/${lib_basename}.o]
> + set binfile [standard_output_file ${dir}/${testfile}]
> +
> + if { [gdb_compile $mainsrc $main_o object ${main_flags}] != "" } {
> + untested "failed to compile main file to object"
> + return -1
> + }
> +
> + if { [gdb_compile $libsrc $lib_o object ${lib_flags}] != "" } {
> + untested "failed to compile secondary file to object"
> + return -1
> + }
> +
> + if { [gdb_compile "$main_o $lib_o" ${binfile} executable ""] != "" } {
> + untested "failed to compile"
> + return -1
> + }
> +
> + # Startup and run to main.
> +
> + clean_restart $binfile
> +
> + if ![runto_main] then {
> + fail "can't run to main"
> + return
> + }
> +
> + # Function call with cast.
> +
> + set test "p (int)foo()"
> + gdb_test_multiple $test $test {
> + -re " = 1\r\n$gdb_prompt " {
> + pass $test
> + }
> + default {
> + fail $test
> + }
> + }
No need for the "default" case. gdb_test_multiple already
issues a FAIL in that case. And if you remove that, a
plain gdb_test instead should work:
gdb_test "p (int)foo()" " = 1"
Unless the missing $ anchor was on purpose?
> +
> + # Function call without cast. Will error if there are no debug symbols.
Double-space after periods.
> +
> + set test "p foo()"
> + gdb_test_multiple $test $test {
> + -re " = 1\r\n$gdb_prompt " {
> + if { $symbols == "debug" } {
> + pass $test
> + } else {
> + fail $test
> + }
You can write
gdb_assert { $symbols == "debug" }
instead of the if/else.
> + }
> + -re "has unknown return type; cast the call to its declared return type\r\n$gdb_prompt " {
> + if { $symbols != "debug" } {
> + pass $test
> + } else {
> + fail $test
> + }
Ditto.
> + }
> + default {
> + fail $test
> + }
Drop the default case.
> + }
> +
> +}
> +
> +foreach_with_prefix l $lang {
> + foreach_with_prefix s {debug nodebug} {
> + build_and_run_test $l $s
> + }
> +}
> +
>
OK with those fixed. Please post what you end up pushing.
Thanks,
Pedro Alves
@@ -1519,9 +1519,6 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
{
int argnum;
struct aarch64_call_info info;
- struct type *func_type;
- struct type *return_type;
- int lang_struct_return;
memset (&info, 0, sizeof (info));
@@ -1543,42 +1540,21 @@ aarch64_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
If the language code decides to pass in memory we want to move
the pointer inserted as the initial argument from the argument
list and into X8, the conventional AArch64 struct return pointer
- register.
-
- This is slightly awkward, ideally the flag "lang_struct_return"
- would be passed to the targets implementation of push_dummy_call.
- Rather that change the target interface we call the language code
- directly ourselves. */
-
- func_type = check_typedef (value_type (function));
-
- /* Dereference function pointer types. */
- if (TYPE_CODE (func_type) == TYPE_CODE_PTR)
- func_type = TYPE_TARGET_TYPE (func_type);
-
- gdb_assert (TYPE_CODE (func_type) == TYPE_CODE_FUNC
- || TYPE_CODE (func_type) == TYPE_CODE_METHOD);
-
- /* If language_pass_by_reference () returned true we will have been
- given an additional initial argument, a hidden pointer to the
- return slot in memory. */
- return_type = TYPE_TARGET_TYPE (func_type);
- lang_struct_return = language_pass_by_reference (return_type);
+ register. */
/* Set the return address. For the AArch64, the return breakpoint
is always at BP_ADDR. */
regcache_cooked_write_unsigned (regcache, AARCH64_LR_REGNUM, bp_addr);
- /* If we were given an initial argument for the return slot because
- lang_struct_return was true, lose it. */
- if (lang_struct_return)
+ /* If we were given an initial argument for the return slot, lose it. */
+ if (return_method == return_method_hidden_param)
{
args++;
nargs--;
}
/* The struct_return pointer occupies X8. */
- if (return_method == return_method_struct || lang_struct_return)
+ if (return_method != return_method_normal)
{
if (aarch64_debug)
{
new file mode 100644
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2018 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/>. */
+
+int
+foo (void)
+{
+ return 1;
+}
new file mode 100644
@@ -0,0 +1,24 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2018 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/>. */
+
+extern int foo (void);
+
+int
+main (void)
+{
+ return 0;
+}
new file mode 100644
@@ -0,0 +1,134 @@
+# This testcase is part of GDB, the GNU debugger.
+# Copyright 2018 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/>.
+
+# Test function calls on functions in another object.
+# See gdb/22736
+
+if [target_info exists gdb,cannot_call_functions] {
+ unsupported "this target can not call functions"
+ continue
+}
+
+# Only test c++ if we are able. Always use c
+if { [skip_cplus_tests] || [get_compiler_info "c++"] } {
+ set lang {c}
+} else {
+ set lang {c c++}
+}
+
+set main_basename infcall-across-obj-main
+set lib_basename infcall-across-obj-lib
+standard_testfile ${main_basename}.c ${lib_basename}.c
+
+set mainsrc "${srcdir}/${subdir}/${srcfile}"
+set libsrc "${srcdir}/${subdir}/${srcfile2}"
+
+# Build both source files to objects using language LANG. Use SYMBOLS to build
+# the with either debug symbols or without - but always build the main file
+# with debug. Then make function calls across the files.
+
+proc build_and_run_test { lang symbols } {
+
+ global main_basename lib_basename mainsrc libsrc binfile testfile
+ global gdb_prompt
+
+ if { $symbols == "debug" } {
+ set debug_flags "debug"
+ } else {
+ set debug_flags ""
+ }
+
+ # Setup directory.
+
+ set dir "$lang-$symbols"
+ remote_exec build "rm -rf [standard_output_file ${dir}]"
+ remote_exec build "mkdir -p [standard_output_file ${dir}]"
+
+ # Compile both files to objects, then link together.
+
+ set main_flags "$lang debug"
+ set lib_flags "$lang $debug_flags"
+ set main_o [standard_output_file ${dir}/${main_basename}.o]
+ set lib_o [standard_output_file ${dir}/${lib_basename}.o]
+ set binfile [standard_output_file ${dir}/${testfile}]
+
+ if { [gdb_compile $mainsrc $main_o object ${main_flags}] != "" } {
+ untested "failed to compile main file to object"
+ return -1
+ }
+
+ if { [gdb_compile $libsrc $lib_o object ${lib_flags}] != "" } {
+ untested "failed to compile secondary file to object"
+ return -1
+ }
+
+ if { [gdb_compile "$main_o $lib_o" ${binfile} executable ""] != "" } {
+ untested "failed to compile"
+ return -1
+ }
+
+ # Startup and run to main.
+
+ clean_restart $binfile
+
+ if ![runto_main] then {
+ fail "can't run to main"
+ return
+ }
+
+ # Function call with cast.
+
+ set test "p (int)foo()"
+ gdb_test_multiple $test $test {
+ -re " = 1\r\n$gdb_prompt " {
+ pass $test
+ }
+ default {
+ fail $test
+ }
+ }
+
+ # Function call without cast. Will error if there are no debug symbols.
+
+ set test "p foo()"
+ gdb_test_multiple $test $test {
+ -re " = 1\r\n$gdb_prompt " {
+ if { $symbols == "debug" } {
+ pass $test
+ } else {
+ fail $test
+ }
+ }
+ -re "has unknown return type; cast the call to its declared return type\r\n$gdb_prompt " {
+ if { $symbols != "debug" } {
+ pass $test
+ } else {
+ fail $test
+ }
+ }
+ default {
+ fail $test
+ }
+ }
+
+}
+
+foreach_with_prefix l $lang {
+ foreach_with_prefix s {debug nodebug} {
+ build_and_run_test $l $s
+ }
+}
+