* [patch] Fix inferior execl of new PIE x86_64
@ 2010-09-30 21:58 Jan Kratochvil
2010-10-15 23:21 ` Pedro Alves
0 siblings, 1 reply; 3+ messages in thread
From: Jan Kratochvil @ 2010-09-30 21:58 UTC (permalink / raw)
To: gdb-patches
Hi,
gcc -o 1 1.c -Wall -g -fPIE -pie; gdb -nx ./1 -ex 'b main' -ex r -ex c
Starting program: .../gdb/1
Breakpoint 1, main (argc=1, argv=0x7fffffffdff8) at 1.c:7
7 setbuf (stdout, NULL);
Continuing.
re-exec
process 28056 is executing new program: .../gdb/1
Error in re-setting breakpoint 1: Cannot access memory at address 0x80c
exit
Program exited normally.
(gdb) _
while it should be:
re-exec
process 28095 is executing new program: .../gdb/1
Breakpoint 1, main (argc=2, argv=0x7fffffffe008) at 1.c:7
7 setbuf (stdout, NULL);
(gdb) _
The error comes from:
throw_error <- memory_error <- read_memory <- read_memory_unsigned_integer <-
amd64_analyze_prologue <- amd64_skip_prologue <- gdbarch_skip_prologue <-
skip_prologue_sal <- find_function_start_sal <- symbol_found <- decode_variable
<- decode_line_1 <- breakpoint_re_set_one <- catch_errors <- breakpoint_re_set
<- clear_symtab_users <- new_symfile_objfile <-
symbol_file_add_with_addrs_or_offsets <- symbol_file_add_from_bfd <-
symbol_file_add <- symbol_file_add_main_1 <- symbol_file_add_main <-
follow_exec
I understand this hack is not nice, the correct way would be to unify more
follow_exec with post_create_inferior. But I could break it for non-GNU/Linux
targets a lot.
No regressions on {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu.
Thanks,
Jan
for the demo above:
------------------------------------------------------------------------------
#include <unistd.h>
#include <assert.h>
#include <stdio.h>
int
main (int argc, char **argv)
{
setbuf (stdout, NULL);
if (argc == 1)
{
puts ("re-exec");
execl ("/proc/self/exe", argv[0], "foo", NULL);
assert (0);
}
puts ("exit");
return 0;
}
------------------------------------------------------------------------------
gdb/
2010-09-30 Jan Kratochvil <jan.kratochvil@redhat.com>
* infrun.c (follow_exec): Replace symbol_file_add_main by
symbol_file_add with SYMFILE_DEFER_BP_RESET, set_initial_language and
breakpoint_re_set.
* m32r-rom.c (m32r_load, m32r_upload_command): Use parameter 0 for
clear_symtab_users.
* objfiles.c (free_all_objfiles): Likewise.
* remote-m32r-sdi.c (m32r_load): Likewise.
* solib-som.c (som_solib_create_inferior_hook): Likewise.
* symfile.c (new_symfile_objfile): New comment for add_flags. Call
clear_symtab_users with ADD_FLAGS.
(reread_symbols): Use parameter 0 for clear_symtab_users.
(clear_symtab_users): New parameter add_flags. Do not call
breakpoint_re_set if SYMFILE_DEFER_BP_RESET.
(clear_symtab_users_cleanup): Use parameter 0 for clear_symtab_users.
* symtab.h (clear_symtab_users): New parameter add_flags.
gdb/testsuite/
2010-09-30 Jan Kratochvil <jan.kratochvil@redhat.com>
* gdb.base/pie-execl.exp: New file.
* gdb.base/pie-execl.c: New file.
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -835,8 +835,15 @@ follow_exec (ptid_t pid, char *execd_pathname)
/* That a.out is now the one to use. */
exec_file_attach (execd_pathname, 0);
- /* Load the main file's symbols. */
- symbol_file_add_main (execd_pathname, 0);
+ /* SYMFILE_DEFER_BP_RESET is used as the proper displacement for PIE
+ (Position Independent Executable) main symbol file will get applied by
+ solib_create_inferior_hook below. breakpoint_re_set would fail to insert
+ the breakpoints with the zero displacement. */
+
+ symbol_file_add (execd_pathname, SYMFILE_MAINLINE | SYMFILE_DEFER_BP_RESET,
+ NULL, 0);
+
+ set_initial_language ();
#ifdef SOLIB_CREATE_INFERIOR_HOOK
SOLIB_CREATE_INFERIOR_HOOK (PIDGET (inferior_ptid));
@@ -846,6 +853,8 @@ follow_exec (ptid_t pid, char *execd_pathname)
jit_inferior_created_hook ();
+ breakpoint_re_set ();
+
/* Reinsert all breakpoints. (Those which were symbolic have
been reset to the proper address in the new a.out, thanks
to symbol_file_command...) */
--- a/gdb/m32r-rom.c
+++ b/gdb/m32r-rom.c
@@ -188,7 +188,7 @@ m32r_load (char *filename, int from_tty)
the stack may not be valid, and things would get horribly
confused... */
- clear_symtab_users ();
+ clear_symtab_users (0);
}
static void
@@ -551,7 +551,7 @@ m32r_upload_command (char *args, int from_tty)
the stack may not be valid, and things would get horribly
confused... */
- clear_symtab_users ();
+ clear_symtab_users (0);
}
/* Provide a prototype to silence -Wmissing-prototypes. */
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -692,7 +692,7 @@ free_all_objfiles (void)
{
free_objfile (objfile);
}
- clear_symtab_users ();
+ clear_symtab_users (0);
}
\f
/* A helper function for objfile_relocate1 that relocates a single
--- a/gdb/remote-m32r-sdi.c
+++ b/gdb/remote-m32r-sdi.c
@@ -1377,7 +1377,7 @@ m32r_load (char *args, int from_tty)
might be to call normal_stop, except that the stack may not be valid,
and things would get horribly confused... */
- clear_symtab_users ();
+ clear_symtab_users (0);
if (!nostart)
{
--- a/gdb/solib-som.c
+++ b/gdb/solib-som.c
@@ -354,7 +354,7 @@ keep_going:
/* Make the breakpoint at "_start" a shared library event breakpoint. */
create_solib_event_breakpoint (target_gdbarch, anaddr);
- clear_symtab_users ();
+ clear_symtab_users (0);
}
static void
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1035,7 +1035,7 @@ syms_from_objfile (struct objfile *objfile,
/* Perform required actions after either reading in the initial
symbols for a new objfile, or mapping in the symbols from a reusable
- objfile. */
+ objfile. ADD_FLAGS is a bitmask of enum symfile_add_flags. */
void
new_symfile_objfile (struct objfile *objfile, int add_flags)
@@ -1048,7 +1048,7 @@ new_symfile_objfile (struct objfile *objfile, int add_flags)
/* OK, make it the "real" symbol file. */
symfile_objfile = objfile;
- clear_symtab_users ();
+ clear_symtab_users (add_flags);
}
else if ((add_flags & SYMFILE_DEFER_BP_RESET) == 0)
{
@@ -2527,7 +2527,7 @@ reread_symbols (void)
/* Notify objfiles that we've modified objfile sections. */
objfiles_changed ();
- clear_symtab_users ();
+ clear_symtab_users (0);
/* At least one objfile has changed, so we can consider that
the executable we're debugging has changed too. */
observer_notify_executable_changed ();
@@ -2745,10 +2745,10 @@ allocate_symtab (char *filename, struct objfile *objfile)
\f
/* Reset all data structures in gdb which may contain references to symbol
- table data. */
+ table data. ADD_FLAGS is a bitmask of enum symfile_add_flags. */
void
-clear_symtab_users (void)
+clear_symtab_users (int add_flags)
{
/* Someday, we should do better than this, by only blowing away
the things that really need to be blown. */
@@ -2758,7 +2758,8 @@ clear_symtab_users (void)
clear_current_source_symtab_and_line ();
clear_displays ();
- breakpoint_re_set ();
+ if ((add_flags & SYMFILE_DEFER_BP_RESET) == 0)
+ breakpoint_re_set ();
set_default_breakpoint (0, NULL, 0, 0, 0);
clear_pc_function_cache ();
observer_notify_new_objfile (NULL);
@@ -2777,7 +2778,7 @@ clear_symtab_users (void)
static void
clear_symtab_users_cleanup (void *ignore)
{
- clear_symtab_users ();
+ clear_symtab_users (0);
}
\f
/* OVERLAYS:
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -1186,7 +1186,7 @@ extern void skip_prologue_sal (struct symtab_and_line *);
/* symfile.c */
-extern void clear_symtab_users (void);
+extern void clear_symtab_users (int add_flags);
extern enum language deduce_language_from_filename (const char *);
--- /dev/null
+++ b/gdb/testsuite/gdb.base/pie-execl.c
@@ -0,0 +1,51 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2010 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 <stdio.h>
+#include <unistd.h>
+#include <assert.h>
+
+static void pie_execl_marker (void);
+
+int
+main (int argc, char **argv)
+{
+ setbuf (stdout, NULL);
+
+#if BIN == 1
+ if (argc == 2)
+ {
+ printf ("pie-execl: re-exec: %s\n", argv[1]);
+ execl (argv[1], argv[1], NULL);
+ assert (0);
+ }
+#endif
+
+ pie_execl_marker ();
+
+ return 0;
+}
+
+/* pie_execl_marker must be on a different address than in `pie-execl2.c'. */
+
+volatile int v;
+
+static void
+pie_execl_marker (void)
+{
+ v = 1;
+}
--- /dev/null
+++ b/gdb/testsuite/gdb.base/pie-execl.exp
@@ -0,0 +1,94 @@
+# Copyright 2010 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/>.
+
+# The problem was due to amd64_skip_prologue attempting to access inferior
+# memory before the PIE (Position Independent Executable) gets relocated.
+
+if { ![istarget *-linux*]} {
+ continue
+}
+
+set testfile "pie-execl"
+set srcfile ${testfile}.c
+set executable1 ${testfile}1
+set executable2 ${testfile}2
+set binfile1 ${objdir}/${subdir}/${executable1}
+set binfile2 ${objdir}/${subdir}/${executable2}
+
+# Use conditional compilation according to `BIN' as GDB remembers the source
+# file name of the breakpoint.
+
+set opts [list debug {additional_flags=-fPIE -pie}]
+if {[build_executable ${testfile}.exp $executable1 $srcfile [concat $opts {additional_flags=-DBIN=1}]] == ""
+ || [build_executable ${testfile}.exp $executable2 $srcfile [concat $opts {additional_flags=-DBIN=2}]] == ""} {
+ return -1
+}
+
+clean_restart ${executable1}
+
+gdb_test_no_output "set args ${binfile2}"
+
+if ![runto_main] {
+ return -1
+}
+
+# Do not stop on `main' after re-exec.
+delete_breakpoints
+
+gdb_breakpoint "pie_execl_marker"
+gdb_test "info breakpoints" ".*" ""
+
+set addr1 ""
+set test "pie_execl_marker address first"
+gdb_test_multiple "p/x &pie_execl_marker" $test {
+ -re " = (0x\[0-9a-f\]+)\r\n$gdb_prompt $" {
+ set addr1 $expect_out(1,string)
+ pass $test
+ }
+}
+verbose -log "addr1 is $addr1"
+
+set test "continue"
+gdb_test_multiple $test $test {
+ -re "Error in re-setting breakpoint" {
+ fail $test
+ }
+ -re "Cannot access memory" {
+ fail $test
+ }
+ -re "pie-execl: re-exec.*executing new program.*\r\nBreakpoint \[0-9\]+,\[^\r\n\]* pie_execl_marker .*\r\n$gdb_prompt $" {
+ pass $test
+ }
+}
+
+gdb_test "info breakpoints" ".*" ""
+
+set addr2 ""
+set test "pie_execl_marker address second"
+gdb_test_multiple "p/x &pie_execl_marker" $test {
+ -re " = (0x\[0-9a-f\]+)\r\n$gdb_prompt $" {
+ set addr2 $expect_out(1,string)
+ pass $test
+ }
+}
+verbose -log "addr2 is $addr2"
+
+# Ensure we cannot get a false PASS and the inferior has really changed.
+set test "pie_execl_marker address has changed"
+if [string equal $addr1 $addr2] {
+ fail $test
+} else {
+ pass $test
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch] Fix inferior execl of new PIE x86_64
2010-09-30 21:58 [patch] Fix inferior execl of new PIE x86_64 Jan Kratochvil
@ 2010-10-15 23:21 ` Pedro Alves
2010-10-17 17:55 ` Jan Kratochvil
0 siblings, 1 reply; 3+ messages in thread
From: Pedro Alves @ 2010-10-15 23:21 UTC (permalink / raw)
To: gdb-patches; +Cc: Jan Kratochvil
On Thursday 30 September 2010 19:52:57, Jan Kratochvil wrote:
> Hi,
>
> gcc -o 1 1.c -Wall -g -fPIE -pie; gdb -nx ./1 -ex 'b main' -ex r -ex c
>
> Starting program: .../gdb/1
> Breakpoint 1, main (argc=1, argv=0x7fffffffdff8) at 1.c:7
> 7 setbuf (stdout, NULL);
> Continuing.
> re-exec
> process 28056 is executing new program: .../gdb/1
> Error in re-setting breakpoint 1: Cannot access memory at address 0x80c
> exit
> Program exited normally.
> (gdb) _
>
> while it should be:
>
> re-exec
> process 28095 is executing new program: .../gdb/1
> Breakpoint 1, main (argc=2, argv=0x7fffffffe008) at 1.c:7
> 7 setbuf (stdout, NULL);
> (gdb) _
>
> The error comes from:
>
> throw_error <- memory_error <- read_memory <- read_memory_unsigned_integer <-
> amd64_analyze_prologue <- amd64_skip_prologue <- gdbarch_skip_prologue <-
> skip_prologue_sal <- find_function_start_sal <- symbol_found <- decode_variable
> <- decode_line_1 <- breakpoint_re_set_one <- catch_errors <- breakpoint_re_set
> <- clear_symtab_users <- new_symfile_objfile <-
> symbol_file_add_with_addrs_or_offsets <- symbol_file_add_from_bfd <-
> symbol_file_add <- symbol_file_add_main_1 <- symbol_file_add_main <-
> follow_exec
>
>
> I understand this hack is not nice, the correct way would be to unify more
> follow_exec with post_create_inferior. But I could break it for non-GNU/Linux
> targets a lot.
Hmm. I'm looking at clear_symtab_users, and noticing that varobj_invalidate
also appears to recreate varobj's, thus, it ends up using the unrelocated
symbols, and thus may manage to not trigger an exception and end up with
varobj's with the wrong values. (If an exception is triggered, say a memory
error, it appears that is caught and swallowed, and the varobjs ends
up with no value.)
We want to defer more than just breakpoint_re_set. Something like: "add
these symbols, but don't trust them for yet", which is a superset of
SYMFILE_DEFER_BP_RESET (or replacement? haven't checked all uses). With
such a flag, we'd make clear_symtab_users skip varobj_invalidate as
well (or some variant that makes its callees only clear symtab uses, and
not trust the current symbols, or some such).
Your patch looks like an improvement already, so it is okay as is with me.
Please make the test be skipped against remote targets though, since
there's no support for following execs in the remote protocol.
--
Pedro Alves
>
> No regressions on {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu.
>
>
> Thanks,
> Jan
>
>
> for the demo above:
> ------------------------------------------------------------------------------
> #include <unistd.h>
> #include <assert.h>
> #include <stdio.h>
> int
> main (int argc, char **argv)
> {
> setbuf (stdout, NULL);
> if (argc == 1)
> {
> puts ("re-exec");
> execl ("/proc/self/exe", argv[0], "foo", NULL);
> assert (0);
> }
> puts ("exit");
> return 0;
> }
> ------------------------------------------------------------------------------
>
>
> gdb/
> 2010-09-30 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * infrun.c (follow_exec): Replace symbol_file_add_main by
> symbol_file_add with SYMFILE_DEFER_BP_RESET, set_initial_language and
> breakpoint_re_set.
> * m32r-rom.c (m32r_load, m32r_upload_command): Use parameter 0 for
> clear_symtab_users.
> * objfiles.c (free_all_objfiles): Likewise.
> * remote-m32r-sdi.c (m32r_load): Likewise.
> * solib-som.c (som_solib_create_inferior_hook): Likewise.
> * symfile.c (new_symfile_objfile): New comment for add_flags. Call
> clear_symtab_users with ADD_FLAGS.
> (reread_symbols): Use parameter 0 for clear_symtab_users.
> (clear_symtab_users): New parameter add_flags. Do not call
> breakpoint_re_set if SYMFILE_DEFER_BP_RESET.
> (clear_symtab_users_cleanup): Use parameter 0 for clear_symtab_users.
> * symtab.h (clear_symtab_users): New parameter add_flags.
>
> gdb/testsuite/
> 2010-09-30 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * gdb.base/pie-execl.exp: New file.
> * gdb.base/pie-execl.c: New file.
>
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -835,8 +835,15 @@ follow_exec (ptid_t pid, char *execd_pathname)
> /* That a.out is now the one to use. */
> exec_file_attach (execd_pathname, 0);
>
> - /* Load the main file's symbols. */
> - symbol_file_add_main (execd_pathname, 0);
> + /* SYMFILE_DEFER_BP_RESET is used as the proper displacement for PIE
> + (Position Independent Executable) main symbol file will get applied by
> + solib_create_inferior_hook below. breakpoint_re_set would fail to insert
> + the breakpoints with the zero displacement. */
> +
> + symbol_file_add (execd_pathname, SYMFILE_MAINLINE | SYMFILE_DEFER_BP_RESET,
> + NULL, 0);
> +
> + set_initial_language ();
>
> #ifdef SOLIB_CREATE_INFERIOR_HOOK
> SOLIB_CREATE_INFERIOR_HOOK (PIDGET (inferior_ptid));
> @@ -846,6 +853,8 @@ follow_exec (ptid_t pid, char *execd_pathname)
>
> jit_inferior_created_hook ();
>
> + breakpoint_re_set ();
> +
> /* Reinsert all breakpoints. (Those which were symbolic have
> been reset to the proper address in the new a.out, thanks
> to symbol_file_command...) */
> --- a/gdb/m32r-rom.c
> +++ b/gdb/m32r-rom.c
> @@ -188,7 +188,7 @@ m32r_load (char *filename, int from_tty)
> the stack may not be valid, and things would get horribly
> confused... */
>
> - clear_symtab_users ();
> + clear_symtab_users (0);
> }
>
> static void
> @@ -551,7 +551,7 @@ m32r_upload_command (char *args, int from_tty)
> the stack may not be valid, and things would get horribly
> confused... */
>
> - clear_symtab_users ();
> + clear_symtab_users (0);
> }
>
> /* Provide a prototype to silence -Wmissing-prototypes. */
> --- a/gdb/objfiles.c
> +++ b/gdb/objfiles.c
> @@ -692,7 +692,7 @@ free_all_objfiles (void)
> {
> free_objfile (objfile);
> }
> - clear_symtab_users ();
> + clear_symtab_users (0);
> }
> \f
> /* A helper function for objfile_relocate1 that relocates a single
> --- a/gdb/remote-m32r-sdi.c
> +++ b/gdb/remote-m32r-sdi.c
> @@ -1377,7 +1377,7 @@ m32r_load (char *args, int from_tty)
> might be to call normal_stop, except that the stack may not be valid,
> and things would get horribly confused... */
>
> - clear_symtab_users ();
> + clear_symtab_users (0);
>
> if (!nostart)
> {
> --- a/gdb/solib-som.c
> +++ b/gdb/solib-som.c
> @@ -354,7 +354,7 @@ keep_going:
> /* Make the breakpoint at "_start" a shared library event breakpoint. */
> create_solib_event_breakpoint (target_gdbarch, anaddr);
>
> - clear_symtab_users ();
> + clear_symtab_users (0);
> }
>
> static void
> --- a/gdb/symfile.c
> +++ b/gdb/symfile.c
> @@ -1035,7 +1035,7 @@ syms_from_objfile (struct objfile *objfile,
>
> /* Perform required actions after either reading in the initial
> symbols for a new objfile, or mapping in the symbols from a reusable
> - objfile. */
> + objfile. ADD_FLAGS is a bitmask of enum symfile_add_flags. */
>
> void
> new_symfile_objfile (struct objfile *objfile, int add_flags)
> @@ -1048,7 +1048,7 @@ new_symfile_objfile (struct objfile *objfile, int add_flags)
> /* OK, make it the "real" symbol file. */
> symfile_objfile = objfile;
>
> - clear_symtab_users ();
> + clear_symtab_users (add_flags);
> }
> else if ((add_flags & SYMFILE_DEFER_BP_RESET) == 0)
> {
> @@ -2527,7 +2527,7 @@ reread_symbols (void)
> /* Notify objfiles that we've modified objfile sections. */
> objfiles_changed ();
>
> - clear_symtab_users ();
> + clear_symtab_users (0);
> /* At least one objfile has changed, so we can consider that
> the executable we're debugging has changed too. */
> observer_notify_executable_changed ();
> @@ -2745,10 +2745,10 @@ allocate_symtab (char *filename, struct objfile *objfile)
> \f
>
> /* Reset all data structures in gdb which may contain references to symbol
> - table data. */
> + table data. ADD_FLAGS is a bitmask of enum symfile_add_flags. */
>
> void
> -clear_symtab_users (void)
> +clear_symtab_users (int add_flags)
> {
> /* Someday, we should do better than this, by only blowing away
> the things that really need to be blown. */
> @@ -2758,7 +2758,8 @@ clear_symtab_users (void)
> clear_current_source_symtab_and_line ();
>
> clear_displays ();
> - breakpoint_re_set ();
> + if ((add_flags & SYMFILE_DEFER_BP_RESET) == 0)
> + breakpoint_re_set ();
> set_default_breakpoint (0, NULL, 0, 0, 0);
> clear_pc_function_cache ();
> observer_notify_new_objfile (NULL);
> @@ -2777,7 +2778,7 @@ clear_symtab_users (void)
> static void
> clear_symtab_users_cleanup (void *ignore)
> {
> - clear_symtab_users ();
> + clear_symtab_users (0);
> }
> \f
> /* OVERLAYS:
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -1186,7 +1186,7 @@ extern void skip_prologue_sal (struct symtab_and_line *);
>
> /* symfile.c */
>
> -extern void clear_symtab_users (void);
> +extern void clear_symtab_users (int add_flags);
>
> extern enum language deduce_language_from_filename (const char *);
>
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/pie-execl.c
> @@ -0,0 +1,51 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> + Copyright 2010 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 <stdio.h>
> +#include <unistd.h>
> +#include <assert.h>
> +
> +static void pie_execl_marker (void);
> +
> +int
> +main (int argc, char **argv)
> +{
> + setbuf (stdout, NULL);
> +
> +#if BIN == 1
> + if (argc == 2)
> + {
> + printf ("pie-execl: re-exec: %s\n", argv[1]);
> + execl (argv[1], argv[1], NULL);
> + assert (0);
> + }
> +#endif
> +
> + pie_execl_marker ();
> +
> + return 0;
> +}
> +
> +/* pie_execl_marker must be on a different address than in `pie-execl2.c'. */
> +
> +volatile int v;
> +
> +static void
> +pie_execl_marker (void)
> +{
> + v = 1;
> +}
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/pie-execl.exp
> @@ -0,0 +1,94 @@
> +# Copyright 2010 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/>.
> +
> +# The problem was due to amd64_skip_prologue attempting to access inferior
> +# memory before the PIE (Position Independent Executable) gets relocated.
> +
> +if { ![istarget *-linux*]} {
> + continue
> +}
> +
> +set testfile "pie-execl"
> +set srcfile ${testfile}.c
> +set executable1 ${testfile}1
> +set executable2 ${testfile}2
> +set binfile1 ${objdir}/${subdir}/${executable1}
> +set binfile2 ${objdir}/${subdir}/${executable2}
> +
> +# Use conditional compilation according to `BIN' as GDB remembers the source
> +# file name of the breakpoint.
> +
> +set opts [list debug {additional_flags=-fPIE -pie}]
> +if {[build_executable ${testfile}.exp $executable1 $srcfile [concat $opts {additional_flags=-DBIN=1}]] == ""
> + || [build_executable ${testfile}.exp $executable2 $srcfile [concat $opts {additional_flags=-DBIN=2}]] == ""} {
> + return -1
> +}
> +
> +clean_restart ${executable1}
> +
> +gdb_test_no_output "set args ${binfile2}"
> +
> +if ![runto_main] {
> + return -1
> +}
> +
> +# Do not stop on `main' after re-exec.
> +delete_breakpoints
> +
> +gdb_breakpoint "pie_execl_marker"
> +gdb_test "info breakpoints" ".*" ""
> +
> +set addr1 ""
> +set test "pie_execl_marker address first"
> +gdb_test_multiple "p/x &pie_execl_marker" $test {
> + -re " = (0x\[0-9a-f\]+)\r\n$gdb_prompt $" {
> + set addr1 $expect_out(1,string)
> + pass $test
> + }
> +}
> +verbose -log "addr1 is $addr1"
> +
> +set test "continue"
> +gdb_test_multiple $test $test {
> + -re "Error in re-setting breakpoint" {
> + fail $test
> + }
> + -re "Cannot access memory" {
> + fail $test
> + }
> + -re "pie-execl: re-exec.*executing new program.*\r\nBreakpoint \[0-9\]+,\[^\r\n\]* pie_execl_marker .*\r\n$gdb_prompt $" {
> + pass $test
> + }
> +}
> +
> +gdb_test "info breakpoints" ".*" ""
> +
> +set addr2 ""
> +set test "pie_execl_marker address second"
> +gdb_test_multiple "p/x &pie_execl_marker" $test {
> + -re " = (0x\[0-9a-f\]+)\r\n$gdb_prompt $" {
> + set addr2 $expect_out(1,string)
> + pass $test
> + }
> +}
> +verbose -log "addr2 is $addr2"
> +
> +# Ensure we cannot get a false PASS and the inferior has really changed.
> +set test "pie_execl_marker address has changed"
> +if [string equal $addr1 $addr2] {
> + fail $test
> +} else {
> + pass $test
> +}
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [patch] Fix inferior execl of new PIE x86_64
2010-10-15 23:21 ` Pedro Alves
@ 2010-10-17 17:55 ` Jan Kratochvil
0 siblings, 0 replies; 3+ messages in thread
From: Jan Kratochvil @ 2010-10-17 17:55 UTC (permalink / raw)
To: Pedro Alves; +Cc: gdb-patches
On Sat, 16 Oct 2010 01:20:45 +0200, Pedro Alves wrote:
> Hmm. I'm looking at clear_symtab_users, and noticing that varobj_invalidate
> also appears to recreate varobj's, thus, it ends up using the unrelocated
> symbols, and thus may manage to not trigger an exception and end up with
> varobj's with the wrong values. (If an exception is triggered, say a memory
> error, it appears that is caught and swallowed, and the varobjs ends
> up with no value.)
OK... I agree there is a problem.
time T+0: FE did -var-update, stopped.
time T+1: inferior did exec, GDB did follow-exec.
time T+2: FE did -var-update, stopped.
IMO the goal is to report at time T+2 the changes of types/values between T+0
and T+2. Even if we would call varobj_invalidate after relocations from
follow_exec still MI would report only changes between T+1 and T+2. Changes
between T+0 and T+1 would still get lost.
There is IMO already a problem in the defined API of varobj_invalidate. The
invalidation has been made more fine grained by (never checked-in):
[patch 8/8] Types GC [varobj]
http://sourceware.org/ml/gdb-patches/2009-05/msg00551.html
(Part of the types garbage collector as a preprequisite for VLA (Variable
Length Arrays, for var[variable] and/or Fortran.)
Therefore currently postponing that part of the problem till the resubmit of
GC/VLA.
> Your patch looks like an improvement already, so it is okay as is with me.
Checked-in.
> Please make the test be skipped against remote targets though, since
> there's no support for following execs in the remote protocol.
if [is_remote target] {
Thanks,
Jan
http://sourceware.org/ml/gdb-cvs/2010-10/msg00101.html
--- src/gdb/ChangeLog 2010/10/17 08:43:45 1.12265
+++ src/gdb/ChangeLog 2010/10/17 17:45:16 1.12266
@@ -1,5 +1,23 @@
2010-10-17 Jan Kratochvil <jan.kratochvil@redhat.com>
+ * infrun.c (follow_exec): Replace symbol_file_add_main by
+ symbol_file_add with SYMFILE_DEFER_BP_RESET, set_initial_language and
+ breakpoint_re_set.
+ * m32r-rom.c (m32r_load, m32r_upload_command): Use parameter 0 for
+ clear_symtab_users.
+ * objfiles.c (free_all_objfiles): Likewise.
+ * remote-m32r-sdi.c (m32r_load): Likewise.
+ * solib-som.c (som_solib_create_inferior_hook): Likewise.
+ * symfile.c (new_symfile_objfile): New comment for add_flags. Call
+ clear_symtab_users with ADD_FLAGS.
+ (reread_symbols): Use parameter 0 for clear_symtab_users.
+ (clear_symtab_users): New parameter add_flags. Do not call
+ breakpoint_re_set if SYMFILE_DEFER_BP_RESET.
+ (clear_symtab_users_cleanup): Use parameter 0 for clear_symtab_users.
+ * symtab.h (clear_symtab_users): New parameter add_flags.
+
+2010-10-17 Jan Kratochvil <jan.kratochvil@redhat.com>
+
Fix GCC false warning.
* varobj.c (value_get_print_value) <str_addr>: Initialize it.
--- src/gdb/testsuite/ChangeLog 2010/10/15 17:48:47 1.2481
+++ src/gdb/testsuite/ChangeLog 2010/10/17 17:45:17 1.2482
@@ -1,3 +1,8 @@
+2010-10-17 Jan Kratochvil <jan.kratochvil@redhat.com>
+
+ * gdb.base/pie-execl.exp: New file.
+ * gdb.base/pie-execl.c: New file.
+
2010-10-13 Doug Evans <dje@google.com>
Jan Kratochvil <jan.kratochvil@redhat.com>
--- src/gdb/infrun.c 2010/09/24 18:35:27 1.451
+++ src/gdb/infrun.c 2010/10/17 17:45:16 1.452
@@ -835,8 +835,15 @@
/* That a.out is now the one to use. */
exec_file_attach (execd_pathname, 0);
- /* Load the main file's symbols. */
- symbol_file_add_main (execd_pathname, 0);
+ /* SYMFILE_DEFER_BP_RESET is used as the proper displacement for PIE
+ (Position Independent Executable) main symbol file will get applied by
+ solib_create_inferior_hook below. breakpoint_re_set would fail to insert
+ the breakpoints with the zero displacement. */
+
+ symbol_file_add (execd_pathname, SYMFILE_MAINLINE | SYMFILE_DEFER_BP_RESET,
+ NULL, 0);
+
+ set_initial_language ();
#ifdef SOLIB_CREATE_INFERIOR_HOOK
SOLIB_CREATE_INFERIOR_HOOK (PIDGET (inferior_ptid));
@@ -846,6 +853,8 @@
jit_inferior_created_hook ();
+ breakpoint_re_set ();
+
/* Reinsert all breakpoints. (Those which were symbolic have
been reset to the proper address in the new a.out, thanks
to symbol_file_command...) */
--- src/gdb/m32r-rom.c 2010/09/14 08:01:12 1.41
+++ src/gdb/m32r-rom.c 2010/10/17 17:45:16 1.42
@@ -188,7 +188,7 @@
the stack may not be valid, and things would get horribly
confused... */
- clear_symtab_users ();
+ clear_symtab_users (0);
}
static void
@@ -551,7 +551,7 @@
the stack may not be valid, and things would get horribly
confused... */
- clear_symtab_users ();
+ clear_symtab_users (0);
}
/* Provide a prototype to silence -Wmissing-prototypes. */
--- src/gdb/objfiles.c 2010/09/22 20:00:53 1.122
+++ src/gdb/objfiles.c 2010/10/17 17:45:16 1.123
@@ -692,7 +692,7 @@
{
free_objfile (objfile);
}
- clear_symtab_users ();
+ clear_symtab_users (0);
}
\f
/* A helper function for objfile_relocate1 that relocates a single
--- src/gdb/remote-m32r-sdi.c 2010/09/14 08:01:12 1.53
+++ src/gdb/remote-m32r-sdi.c 2010/10/17 17:45:16 1.54
@@ -1377,7 +1377,7 @@
might be to call normal_stop, except that the stack may not be valid,
and things would get horribly confused... */
- clear_symtab_users ();
+ clear_symtab_users (0);
if (!nostart)
{
--- src/gdb/solib-som.c 2010/05/16 23:49:58 1.29
+++ src/gdb/solib-som.c 2010/10/17 17:45:16 1.30
@@ -354,7 +354,7 @@
/* Make the breakpoint at "_start" a shared library event breakpoint. */
create_solib_event_breakpoint (target_gdbarch, anaddr);
- clear_symtab_users ();
+ clear_symtab_users (0);
}
static void
--- src/gdb/symfile.c 2010/10/01 20:26:11 1.297
+++ src/gdb/symfile.c 2010/10/17 17:45:16 1.298
@@ -1038,7 +1038,7 @@
/* Perform required actions after either reading in the initial
symbols for a new objfile, or mapping in the symbols from a reusable
- objfile. */
+ objfile. ADD_FLAGS is a bitmask of enum symfile_add_flags. */
void
new_symfile_objfile (struct objfile *objfile, int add_flags)
@@ -1051,7 +1051,7 @@
/* OK, make it the "real" symbol file. */
symfile_objfile = objfile;
- clear_symtab_users ();
+ clear_symtab_users (add_flags);
}
else if ((add_flags & SYMFILE_DEFER_BP_RESET) == 0)
{
@@ -2530,7 +2530,7 @@
/* Notify objfiles that we've modified objfile sections. */
objfiles_changed ();
- clear_symtab_users ();
+ clear_symtab_users (0);
/* At least one objfile has changed, so we can consider that
the executable we're debugging has changed too. */
observer_notify_executable_changed ();
@@ -2748,10 +2748,10 @@
\f
/* Reset all data structures in gdb which may contain references to symbol
- table data. */
+ table data. ADD_FLAGS is a bitmask of enum symfile_add_flags. */
void
-clear_symtab_users (void)
+clear_symtab_users (int add_flags)
{
/* Someday, we should do better than this, by only blowing away
the things that really need to be blown. */
@@ -2761,7 +2761,8 @@
clear_current_source_symtab_and_line ();
clear_displays ();
- breakpoint_re_set ();
+ if ((add_flags & SYMFILE_DEFER_BP_RESET) == 0)
+ breakpoint_re_set ();
set_default_breakpoint (0, NULL, 0, 0, 0);
clear_pc_function_cache ();
observer_notify_new_objfile (NULL);
@@ -2780,7 +2781,7 @@
static void
clear_symtab_users_cleanup (void *ignore)
{
- clear_symtab_users ();
+ clear_symtab_users (0);
}
\f
/* OVERLAYS:
--- src/gdb/symtab.h 2010/09/08 17:17:42 1.163
+++ src/gdb/symtab.h 2010/10/17 17:45:16 1.164
@@ -1186,7 +1186,7 @@
/* symfile.c */
-extern void clear_symtab_users (void);
+extern void clear_symtab_users (int add_flags);
extern enum language deduce_language_from_filename (const char *);
--- src/gdb/testsuite/gdb.base/pie-execl.c
+++ src/gdb/testsuite/gdb.base/pie-execl.c 2010-10-17 17:45:47.138149000 +0000
@@ -0,0 +1,51 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+ Copyright 2010 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 <stdio.h>
+#include <unistd.h>
+#include <assert.h>
+
+static void pie_execl_marker (void);
+
+int
+main (int argc, char **argv)
+{
+ setbuf (stdout, NULL);
+
+#if BIN == 1
+ if (argc == 2)
+ {
+ printf ("pie-execl: re-exec: %s\n", argv[1]);
+ execl (argv[1], argv[1], NULL);
+ assert (0);
+ }
+#endif
+
+ pie_execl_marker ();
+
+ return 0;
+}
+
+/* pie_execl_marker must be on a different address than in `pie-execl2.c'. */
+
+volatile int v;
+
+static void
+pie_execl_marker (void)
+{
+ v = 1;
+}
--- src/gdb/testsuite/gdb.base/pie-execl.exp
+++ src/gdb/testsuite/gdb.base/pie-execl.exp 2010-10-17 17:45:47.456930000 +0000
@@ -0,0 +1,100 @@
+# Copyright 2010 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/>.
+
+# The problem was due to amd64_skip_prologue attempting to access inferior
+# memory before the PIE (Position Independent Executable) gets relocated.
+
+if ![istarget *-linux*] {
+ continue
+}
+
+# Remote protocol does not support follow-exec notifications.
+
+if [is_remote target] {
+ continue
+}
+
+set testfile "pie-execl"
+set srcfile ${testfile}.c
+set executable1 ${testfile}1
+set executable2 ${testfile}2
+set binfile1 ${objdir}/${subdir}/${executable1}
+set binfile2 ${objdir}/${subdir}/${executable2}
+
+# Use conditional compilation according to `BIN' as GDB remembers the source
+# file name of the breakpoint.
+
+set opts [list debug {additional_flags=-fPIE -pie}]
+if {[build_executable ${testfile}.exp $executable1 $srcfile [concat $opts {additional_flags=-DBIN=1}]] == ""
+ || [build_executable ${testfile}.exp $executable2 $srcfile [concat $opts {additional_flags=-DBIN=2}]] == ""} {
+ return -1
+}
+
+clean_restart ${executable1}
+
+gdb_test_no_output "set args ${binfile2}"
+
+if ![runto_main] {
+ return -1
+}
+
+# Do not stop on `main' after re-exec.
+delete_breakpoints
+
+gdb_breakpoint "pie_execl_marker"
+gdb_test "info breakpoints" ".*" ""
+
+set addr1 ""
+set test "pie_execl_marker address first"
+gdb_test_multiple "p/x &pie_execl_marker" $test {
+ -re " = (0x\[0-9a-f\]+)\r\n$gdb_prompt $" {
+ set addr1 $expect_out(1,string)
+ pass $test
+ }
+}
+verbose -log "addr1 is $addr1"
+
+set test "continue"
+gdb_test_multiple $test $test {
+ -re "Error in re-setting breakpoint" {
+ fail $test
+ }
+ -re "Cannot access memory" {
+ fail $test
+ }
+ -re "pie-execl: re-exec.*executing new program.*\r\nBreakpoint \[0-9\]+,\[^\r\n\]* pie_execl_marker .*\r\n$gdb_prompt $" {
+ pass $test
+ }
+}
+
+gdb_test "info breakpoints" ".*" ""
+
+set addr2 ""
+set test "pie_execl_marker address second"
+gdb_test_multiple "p/x &pie_execl_marker" $test {
+ -re " = (0x\[0-9a-f\]+)\r\n$gdb_prompt $" {
+ set addr2 $expect_out(1,string)
+ pass $test
+ }
+}
+verbose -log "addr2 is $addr2"
+
+# Ensure we cannot get a false PASS and the inferior has really changed.
+set test "pie_execl_marker address has changed"
+if [string equal $addr1 $addr2] {
+ fail $test
+} else {
+ pass $test
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-10-17 17:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-30 21:58 [patch] Fix inferior execl of new PIE x86_64 Jan Kratochvil
2010-10-15 23:21 ` Pedro Alves
2010-10-17 17:55 ` Jan Kratochvil
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox