[patchv2] compile: Fix GNU-IFUNC funcs called from injected code
Commit Message
Testcase cosmetic update.
gdb/ChangeLog
2015-02-24 Jan Kratochvil <jan.kratochvil@redhat.com>
* compile/compile-c-symbols.c (convert_one_symbol, convert_symbol_bmsym)
(gcc_symbol_address): Call gnu_ifunc_resolve_addr.
gdb/testsuite/ChangeLog
2015-02-24 Jan Kratochvil <jan.kratochvil@redhat.com>
* gdb.compile/compile-ifunc.c: New file.
* gdb.compile/compile-ifunc.exp: New file.
Comments
On 24/02/15 20:02, Jan Kratochvil wrote:
> Testcase cosmetic update.
>
> compile-gnu-ifunc2.patch
>
> gdb/ChangeLog
> 2015-02-24 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * compile/compile-c-symbols.c (convert_one_symbol, convert_symbol_bmsym)
> (gcc_symbol_address): Call gnu_ifunc_resolve_addr.
>
> gdb/testsuite/ChangeLog
> 2015-02-24 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * gdb.compile/compile-ifunc.c: New file.
> * gdb.compile/compile-ifunc.exp: New file.
Thanks. This looks great other than the following nit:
> case mst_text_gnu_ifunc:
> - type = objfile_type (objfile)->nodebug_text_gnu_ifunc_symbol;
> + // nodebug_text_gnu_ifunc_symbol would cause:
> + // function return type cannot be function
Not sure if we allow // comments yet.
> + if (TYPE_GNU_IFUNC (SYMBOL_TYPE (sym)))
> + result = gnu_ifunc_resolve_addr (target_gdbarch (), result);
Is this guaranteed to return a value to result (or an exception)?
Cheers
Phil
On Wed, 25 Feb 2015 09:20:28 +0100, Phil Muldoon wrote:
> Not sure if we allow // comments yet.
Given the codebase already contains them and additionally it should get
converted to C++ I assume they are OK.
> > + if (TYPE_GNU_IFUNC (SYMBOL_TYPE (sym)))
> > + result = gnu_ifunc_resolve_addr (target_gdbarch (), result);
>
> Is this guaranteed to return a value to result (or an exception)?
Yes.
Jan
> On Wed, 25 Feb 2015 09:20:28 +0100, Phil Muldoon wrote:
> > Not sure if we allow // comments yet.
>
> Given the codebase already contains them and additionally it should get
> converted to C++ I assume they are OK.
Let's please hold them off until the project is officially a C++
project. At the moment, we support the building of GDB using
a pure C compiler which may not understand those.
If other areas use them already, I consider that a bug.
On Thu, 26 Feb 2015 09:18:00 +0100, Joel Brobecker wrote:
> Let's please hold them off until the project is officially a C++
> project.
I will change that upon check-in.
> At the moment, we support the building of GDB using
> a pure C compiler which may not understand those.
+
> If other areas use them already, I consider that a bug.
Non-// compiler would not work anyway, last time such a patch check-in was:
commit bb2ec1b34e7cc8d8602512235016e74b800dac3b
Author: Tom Tromey <tromey@redhat.com>
Date: Wed May 14 14:35:45 2014 -0600
the "compile" command
Jan
> > If other areas use them already, I consider that a bug.
>
> Non-// compiler would not work anyway, last time such a patch check-in was:
> commit bb2ec1b34e7cc8d8602512235016e74b800dac3b
> Author: Tom Tromey <tromey@redhat.com>
> Date: Wed May 14 14:35:45 2014 -0600
> the "compile" command
As one of the authors of this patch, can you fix those, please?
(obvious fix, master and gdb-7.9-branch - remember that we need
a PR for fixes pushed to the branch)
I checked the sources, and outside of the checkin above, only
iq2000-tdep.c appears to have this issue at one location. I will
fix that one.
Thanks,
On 02/24/2015 08:02 PM, Jan Kratochvil wrote:
>
> gdb/ChangeLog
> 2015-02-24 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * compile/compile-c-symbols.c (convert_one_symbol, convert_symbol_bmsym)
> (gcc_symbol_address): Call gnu_ifunc_resolve_addr.
Looks good to me too. OK.
I think we could probably avoid the extra infcall done
within gnu_ifunc_resolve_addr if we're going to be running
the generated code anyway, but it's not worth the effort
at this point.
> +if { [prepare_for_testing ${testfile}.exp "$testfile-debug" $srcfile] } {
> + return -1
> +}
> +
> +if ![runto_main] {
> + return -1
> +}
> +
Could you move these inside the prefix too, so that if they
fail for some reason, we get distinct test messages from
the nodebug case?
> +with_test_prefix "debug" {
Thanks,
Pedro Alves
On Thu, 26 Feb 2015 16:49:48 +0100, Pedro Alves wrote:
> I think we could probably avoid the extra infcall done
> within gnu_ifunc_resolve_addr if we're going to be running
> the generated code anyway, but it's not worth the effort
> at this point.
Yes; it would also need hacking in the GCC part.
> Could you move these inside the prefix too, so that if they
> fail for some reason, we get distinct test messages from
> the nodebug case?
Done.
Checked in:
081a1c2cede38dfb837e3d89539416fd836be4fe
Thanks,
Jan
@@ -187,6 +187,8 @@ convert_one_symbol (struct compile_c_instance *context,
case LOC_BLOCK:
kind = GCC_C_SYMBOL_FUNCTION;
addr = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
+ if (is_global && TYPE_GNU_IFUNC (SYMBOL_TYPE (sym)))
+ addr = gnu_ifunc_resolve_addr (target_gdbarch (), addr);
break;
case LOC_CONST:
@@ -365,6 +367,8 @@ convert_symbol_bmsym (struct compile_c_instance *context,
gcc_decl decl;
CORE_ADDR addr;
+ addr = MSYMBOL_VALUE_ADDRESS (objfile, msym);
+
/* Conversion copied from write_exp_msymbol. */
switch (MSYMBOL_TYPE (msym))
{
@@ -376,8 +380,11 @@ convert_symbol_bmsym (struct compile_c_instance *context,
break;
case mst_text_gnu_ifunc:
- type = objfile_type (objfile)->nodebug_text_gnu_ifunc_symbol;
+ // nodebug_text_gnu_ifunc_symbol would cause:
+ // function return type cannot be function
+ type = objfile_type (objfile)->nodebug_text_symbol;
kind = GCC_C_SYMBOL_FUNCTION;
+ addr = gnu_ifunc_resolve_addr (target_gdbarch (), addr);
break;
case mst_data:
@@ -400,7 +407,6 @@ convert_symbol_bmsym (struct compile_c_instance *context,
}
sym_type = convert_type (context, type);
- addr = MSYMBOL_VALUE_ADDRESS (objfile, msym);
decl = C_CTX (context)->c_ops->build_decl (C_CTX (context),
MSYMBOL_NATURAL_NAME (msym),
kind, sym_type, NULL, addr,
@@ -497,6 +503,8 @@ gcc_symbol_address (void *datum, struct gcc_c_context *gcc_context,
"gcc_symbol_address \"%s\": full symbol\n",
identifier);
result = BLOCK_START (SYMBOL_BLOCK_VALUE (sym));
+ if (TYPE_GNU_IFUNC (SYMBOL_TYPE (sym)))
+ result = gnu_ifunc_resolve_addr (target_gdbarch (), result);
found = 1;
}
else
@@ -512,6 +520,8 @@ gcc_symbol_address (void *datum, struct gcc_c_context *gcc_context,
"symbol\n",
identifier);
result = BMSYMBOL_VALUE_ADDRESS (msym);
+ if (MSYMBOL_TYPE (msym.minsym) == mst_text_gnu_ifunc)
+ result = gnu_ifunc_resolve_addr (target_gdbarch (), result);
found = 1;
}
}
new file mode 100644
@@ -0,0 +1,46 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2015 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/>. */
+
+#include <stdlib.h>
+
+typedef int (*final_t) (int arg);
+
+int
+final (int arg)
+{
+ return arg + 1;
+}
+
+asm (".type gnu_ifunc, %gnu_indirect_function");
+
+final_t
+gnu_ifunc (void)
+{
+ return final;
+}
+
+extern int gnu_ifunc_alias (int arg) __attribute__ ((alias ("gnu_ifunc")));
+
+static int resultvar;
+
+int
+main (void)
+{
+ if (gnu_ifunc_alias (10) != 11)
+ abort ();
+ return resultvar;
+}
new file mode 100644
@@ -0,0 +1,54 @@
+# Copyright 2015 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/>.
+
+standard_testfile
+
+if { [prepare_for_testing ${testfile}.exp "$testfile-nodebug" $srcfile {}] } {
+ return -1
+}
+
+if ![runto_main] {
+ return -1
+}
+
+if {[skip_compile_feature_tests]} {
+ untested "compile command not supported (could not find libcc1 shared library?)"
+ return -1
+}
+
+with_test_prefix "nodebug" {
+
+ gdb_test_no_output "compile code resultvar = gnu_ifunc (10);"
+
+ gdb_test "p resultvar" " = 11"
+
+}
+
+if { [prepare_for_testing ${testfile}.exp "$testfile-debug" $srcfile] } {
+ return -1
+}
+
+if ![runto_main] {
+ return -1
+}
+
+with_test_prefix "debug" {
+
+ # gnu_ifunc (10): error: too many arguments to function 'gnu_ifunc'
+ gdb_test_no_output "compile code resultvar = gnu_ifunc_alias (10);"
+
+ gdb_test "p resultvar" " = 11"
+
+}