public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [patch 6/6] PIE: Fix back re-run
@ 2010-03-29 16:19 Jan Kratochvil
  2010-06-09 15:10 ` ping: " Jan Kratochvil
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kratochvil @ 2010-03-29 16:19 UTC (permalink / raw)
  To: gdb-patches

Hi,

originally posted as:
	[patch] PIE: Fix back re-run [Re: [patch] svr4_exec_displacement success indicator]
	http://sourceware.org/ml/gdb-patches/2010-03/msg00868.html

Rediffed only (+some word change in the accompanying mail text below).

FYI on i686 Linux kernel (+x86_64 kernel for i686 programs)
"set disable-randomization on" acts the same as - apparently being applied -
- "set disable-randomization off" (the state before introducing that
"disable-randomization" GDB patch).  This is a Linux kernel bug; tracked at:
	https://bugzilla.redhat.com/show_bug.cgi?id=220892

------------------------------------------------------------------------------

currently:

$ echo 'main(){}'|gcc -o 1 -fPIE -pie -x c -; ./gdb -nx -ex 'set disable-randomization off' -ex 'b main' -ex r -ex c -ex r ./1
Breakpoint 1 at 0x6b0
Starting program: /home/jkratoch/redhat/gdb-clean/gdb/testsuite/1 
Breakpoint 1, 0x00007fbf73e8c6b0 in main ()
Continuing.
Program exited with code 0140.
Starting program: /home/jkratoch/redhat/gdb-clean/gdb/testsuite/1 
Error in re-setting breakpoint 1: Cannot access memory at address 0x7fbf73e8c6ac

It is since:
	[patch] svr4_exec_displacement success indicator [Re: PIE question]
	http://sourceware.org/ml/gdb-patches/2010-03/msg00336.html


On Mon, 08 Mar 2010 22:53:58 +0100, Jan Kratochvil wrote:
> Attached these changes:
> 
> * svr4_exec_displacement calling convention should have success indicator.
> 
> * Preserving now section_offsets if they are already set, inspired by
>   init_objfile_sect_indices.
> 
> I believe either of parts would be sufficient for this problem.

The part "Preserving now section_offsets if they are already set" has caused
the regression for PIE on native x86* GNU/Linux host.

As I believe for Daniel J.'s seen regression of `qOffsets' the checked-in part
"svr4_exec_displacement calling convention should have success indicator" is
sufficient - I would like to remove the "Preserving now section_offsets if they
are already set" part.


No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu for the whole
patch series together.


Thanks,
Jan


gdb/
2010-03-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix re-run of PIE executable.
	* solib-svr4.c (svr4_relocate_main_executable) <symfile_objfile>: Remove
	the part of pre-set SYMFILE_OBJFILE->SECTION_OFFSETS.

gdb/testsuite/
2010-03-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix re-run of PIE executable.
	* gdb.base/break-interp.exp (test_ld): Turn off "disable-randomization".
	Remove $displacement_main to match the solib-svr4.c change.  New "kill"
	and re-"run" of the inferior.

--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1960,17 +1960,10 @@ svr4_relocate_main_executable (void)
 {
   CORE_ADDR displacement;
 
-  if (symfile_objfile)
-    {
-      int i;
-
-      /* Remote target may have already set specific offsets by `qOffsets'
-	 which should be preferred.  */
-
-      for (i = 0; i < symfile_objfile->num_sections; i++)
-	if (ANOFFSET (symfile_objfile->section_offsets, i) != 0)
-	  return;
-    }
+  /* SYMFILE_OBJFILE->SECTION_OFFSETS may now contain displacement from the
+     previous run of the inferior.  Re-set it according to the current value,
+     if we can find it out.  But otherwise keep it as for remote target it may
+     have been pre-set by the `qOffsets' packet.  */
 
   if (! svr4_exec_displacement (&displacement))
     return;
--- a/gdb/testsuite/gdb.base/break-interp.exp
+++ b/gdb/testsuite/gdb.base/break-interp.exp
@@ -329,6 +329,9 @@ proc test_ld {file ifmain trynosym displacement} {
     # Print the "PIE (Position Independent Executable) displacement" message.
     gdb_test "set verbose on"
 
+    # A bit better test coverage.
+    gdb_test "set disable-randomization off"
+
     reach "dl_main" "run segv" $displacement
 
     gdb_test "bt" "#0 +\[^\r\n\]*\\mdl_main\\M.*" "dl bt"
@@ -339,7 +342,13 @@ proc test_ld {file ifmain trynosym displacement} {
 	reach "libfunc" continue "NONE"
 
 	gdb_test "bt" "#0 +\[^\r\n\]*\\mlibfunc\\M\[^\r\n\]*\r\n#1 +\[^\r\n\]*\\mmain\\M.*" "main bt"
+    }
 
+    # Try re-run if the new PIE displacement takes effect.
+    gdb_test "kill" "" "kill" {Kill the program being debugged\? \(y or n\) } "y"
+    reach "dl_main" "run  segv" $displacement
+
+    if $ifmain {
 	test_core $file $displacement
 
 	test_attach $file $displacement

^ permalink raw reply	[flat|nested] 6+ messages in thread

* ping: [patch 6/6] PIE: Fix back re-run
  2010-03-29 16:19 [patch 6/6] PIE: Fix back re-run Jan Kratochvil
@ 2010-06-09 15:10 ` Jan Kratochvil
  2010-07-01 19:10   ` Joel Brobecker
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kratochvil @ 2010-06-09 15:10 UTC (permalink / raw)
  To: gdb-patches

Hi,

originally posted as:
	[patch 6/6] PIE: Fix back re-run
	http://sourceware.org/ml/gdb-patches/2010-03/msg01005.html

Rediffed only.

------------------------------------------------------------------------------

originally posted as:
	[patch] PIE: Fix back re-run [Re: [patch] svr4_exec_displacement success indicator]
	http://sourceware.org/ml/gdb-patches/2010-03/msg00868.html

Rediffed only (+some word change in the accompanying mail text below).

FYI on i686 Linux kernel (+x86_64 kernel for i686 programs)
"set disable-randomization on" acts the same as - apparently being applied -
- "set disable-randomization off" (the state before introducing that
"disable-randomization" GDB patch).  This is a Linux kernel bug; tracked at:
	https://bugzilla.redhat.com/show_bug.cgi?id=220892

------------------------------------------------------------------------------

currently:

$ echo 'main(){}'|gcc -o 1 -fPIE -pie -x c -; ./gdb -nx -ex 'set disable-randomization off' -ex 'b main' -ex r -ex c -ex r ./1
Breakpoint 1 at 0x6b0
Starting program: /home/jkratoch/redhat/gdb-clean/gdb/testsuite/1 
Breakpoint 1, 0x00007fbf73e8c6b0 in main ()
Continuing.
Program exited with code 0140.
Starting program: /home/jkratoch/redhat/gdb-clean/gdb/testsuite/1 
Error in re-setting breakpoint 1: Cannot access memory at address 0x7fbf73e8c6ac

It is since:
	[patch] svr4_exec_displacement success indicator [Re: PIE question]
	http://sourceware.org/ml/gdb-patches/2010-03/msg00336.html


On Mon, 08 Mar 2010 22:53:58 +0100, Jan Kratochvil wrote:
> Attached these changes:
> 
> * svr4_exec_displacement calling convention should have success indicator.
> 
> * Preserving now section_offsets if they are already set, inspired by
>   init_objfile_sect_indices.
> 
> I believe either of parts would be sufficient for this problem.

The part "Preserving now section_offsets if they are already set" has caused
the regression for PIE on native x86* GNU/Linux host.

As I believe for Daniel J.'s seen regression of `qOffsets' the checked-in part
"svr4_exec_displacement calling convention should have success indicator" is
sufficient - I would like to remove the "Preserving now section_offsets if they
are already set" part.


No regressions on {x86_64,x86_64-m32,i686}-fedora12-linux-gnu for the whole
patch series together.


Thanks,
Jan


gdb/
2010-03-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix re-run of PIE executable.
	* solib-svr4.c (svr4_relocate_main_executable) <symfile_objfile>: Remove
	the part of pre-set SYMFILE_OBJFILE->SECTION_OFFSETS.

gdb/testsuite/
2010-03-29  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix re-run of PIE executable.
	* gdb.base/break-interp.exp (test_ld): Turn off "disable-randomization".
	Remove $displacement_main to match the solib-svr4.c change.  New "kill"
	and re-"run" of the inferior.

--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1985,17 +1985,10 @@ svr4_relocate_main_executable (void)
 {
   CORE_ADDR displacement;
 
-  if (symfile_objfile)
-    {
-      int i;
-
-      /* Remote target may have already set specific offsets by `qOffsets'
-	 which should be preferred.  */
-
-      for (i = 0; i < symfile_objfile->num_sections; i++)
-	if (ANOFFSET (symfile_objfile->section_offsets, i) != 0)
-	  return;
-    }
+  /* SYMFILE_OBJFILE->SECTION_OFFSETS may now contain displacement from the
+     previous run of the inferior.  Re-set it according to the current value,
+     if we can find it out.  But otherwise keep it as for remote target it may
+     have been pre-set by the `qOffsets' packet.  */
 
   if (! svr4_exec_displacement (&displacement))
     return;
--- a/gdb/testsuite/gdb.base/break-interp.exp
+++ b/gdb/testsuite/gdb.base/break-interp.exp
@@ -330,6 +330,9 @@ proc test_ld {file ifmain trynosym displacement} {
     # Print the "PIE (Position Independent Executable) displacement" message.
     gdb_test_no_output "set verbose on"
 
+    # A bit better test coverage.
+    gdb_test "set disable-randomization off"
+
     reach "dl_main" "run segv" $displacement
 
     gdb_test "bt" "#0 +\[^\r\n\]*\\mdl_main\\M.*" "dl bt"
@@ -340,7 +343,13 @@ proc test_ld {file ifmain trynosym displacement} {
 	reach "libfunc" continue "NONE"
 
 	gdb_test "bt" "#0 +\[^\r\n\]*\\mlibfunc\\M\[^\r\n\]*\r\n#1 +\[^\r\n\]*\\mmain\\M.*" "main bt"
+    }
 
+    # Try re-run if the new PIE displacement takes effect.
+    gdb_test "kill" "" "kill" {Kill the program being debugged\? \(y or n\) } "y"
+    reach "dl_main" "run  segv" $displacement
+
+    if $ifmain {
 	test_core $file $displacement
 
 	test_attach $file $displacement

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: ping: [patch 6/6] PIE: Fix back re-run
  2010-06-09 15:10 ` ping: " Jan Kratochvil
@ 2010-07-01 19:10   ` Joel Brobecker
  2010-07-04 10:19     ` Jan Kratochvil
  0 siblings, 1 reply; 6+ messages in thread
From: Joel Brobecker @ 2010-07-01 19:10 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

Sorry for the delay; it took me a long time to review everything that
you mentioned and try to get a more complete picture of what was going on.
I'll once again admit that it would be an understatement to say that
I have limited knowledge of this area.  But since holding up patches
for lack of review is worse IMO than allowing potentially buggy patches
in...

> gdb/
> 2010-03-29  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Fix re-run of PIE executable.
> 	* solib-svr4.c (svr4_relocate_main_executable) <symfile_objfile>: Remove
> 	the part of pre-set SYMFILE_OBJFILE->SECTION_OFFSETS.

Just one question, which could actually be dealt with as a followup
patch, if it's more convenient. And the usual nit-pick on comments...

> gdb/testsuite/
> 2010-03-29  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	Fix re-run of PIE executable.
> 	* gdb.base/break-interp.exp (test_ld): Turn off "disable-randomization".
> 	Remove $displacement_main to match the solib-svr4.c change.  New "kill"
> 	and re-"run" of the inferior.

OK, modulo a request about one of your comments.

BTW: I seem to be commenting a lot on your comments, but as said before,
you're one of the few who makes an active effort at writing them, and
I really think that this is extremely important and useful.  So a big
Thank You.

> +  /* SYMFILE_OBJFILE->SECTION_OFFSETS may now contain displacement from the
> +     previous run of the inferior.  Re-set it according to the current value,
> +     if we can find it out.  But otherwise keep it as for remote target it may
> +     have been pre-set by the `qOffsets' packet.  */

I was having a hard time trying to figure out what you were trying to
get at until I had a broader picture of the various scenarios that we
are trying to deal with.  I would like to rephrase your comment a little
differently:

  /* If we are re-running this executable, SYMFILE_OBJFILE->SECTION_OFFSETS
     probably contains the offsets computed using the PIE displacement
     from the previous run, which of course are irrelevant for this run.
     So we need to determine the new PIE displacement and recompute the
     section offsets accordingly, even if SYMFILE_OBJFILE->SECTION_OFFSETS
     already contains pre-computed offsets.

     If, for some reason we cannot compute the PIE displacement (why???),
     then we leave the section offsets untouched and use them as is for
     this run.  Either:
       
       - These section offsets were properly reset earlier, and thus
         already contain the correct values.  This can happen for instance
         when reconnecting via the remote protocol to a target that supports
         the `qOffsets' packet.

       - The section offsets were not reset earlier, and the best we can
         hope is that the old offsets are still applicable to the new run.
   */

But all this begs one important question: When does svr4_exec_displacement
return zero (fail)??? Based on the code, it can happen in two situations,
I think: (a) PIE is not in use; and (b) binary on disk is different from
program being executed. Case (b) is hopeless as I understand it? But (a)
shouldn't.

So, expanding the logic in the comment I am suggesting, I think we need
to account for the situation where svr4_exec_displacement fails because
of non-PIE executable.

Either: (a1) there was no PIE in sight during the previous run either
             => offsets should be already be correct;
        (a2) PIE was in use during the previous run => the offsets
             may be incorrect.  But it should be trivial to reset them.

So, I'm wondering whether it would make sense to amend your patch to
check for PIE after svr4_exec_displacement failed, and use a zero
displacement if PIE is not detected.

> +    # A bit better test coverage.
> +    gdb_test "set disable-randomization off"

I think that this does more than just "better coverage". It is a
critical part of the testcase that helps trying to obtain two runs
with different displacement addresses.  I think the comment should
make it clear, because otherwise people might think that this is not
necessarily an important part of the testcase.

Something like this might help making things clearer:

# We want to test the re-run of a PIE in the case where the executable
# is loaded with a different displacement, but disable-randomization
# prevents that from happening.  So turn it off.

> +    reach "dl_main" "run  segv" $displacement
                           ^^ detail: two spaces?

-- 
Joel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: ping: [patch 6/6] PIE: Fix back re-run
  2010-07-01 19:10   ` Joel Brobecker
@ 2010-07-04 10:19     ` Jan Kratochvil
  2010-07-05 17:48       ` Joel Brobecker
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Kratochvil @ 2010-07-04 10:19 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On Thu, 01 Jul 2010 21:10:41 +0200, Joel Brobecker wrote:
> BTW: I seem to be commenting a lot on your comments, but as said before,
> you're one of the few who makes an active effort at writing them, and
> I really think that this is extremely important and useful.  So a big
> Thank You.

I see it may be more simple to drop the comments before the final submit. :-)
Thanks for the reviewing patience, some problems of my comments I can
hopefully improve in the future.


> > +  /* SYMFILE_OBJFILE->SECTION_OFFSETS may now contain displacement from the
> > +     previous run of the inferior.  Re-set it according to the current value,
> > +     if we can find it out.  But otherwise keep it as for remote target it may
> > +     have been pre-set by the `qOffsets' packet.  */
> 
> I was having a hard time trying to figure out what you were trying to
> get at until I had a broader picture of the various scenarios that we
> are trying to deal with.  I would like to rephrase your comment a little
> differently:
> 
>   /* If we are re-running this executable, SYMFILE_OBJFILE->SECTION_OFFSETS
>      probably contains the offsets computed using the PIE displacement
>      from the previous run, which of course are irrelevant for this run.
>      So we need to determine the new PIE displacement and recompute the
>      section offsets accordingly, even if SYMFILE_OBJFILE->SECTION_OFFSETS
>      already contains pre-computed offsets.
> 
>      If, for some reason we cannot compute the PIE displacement (why???),
>      then we leave the section offsets untouched and use them as is for
>      this run.  Either:
>        
>        - These section offsets were properly reset earlier, and thus
>          already contain the correct values.  This can happen for instance
>          when reconnecting via the remote protocol to a target that supports
>          the `qOffsets' packet.
> 
>        - The section offsets were not reset earlier, and the best we can
>          hope is that the old offsets are still applicable to the new run.
>    */
> 
> But all this begs one important question: When does svr4_exec_displacement
> return zero (fail)??? Based on the code, it can happen in two situations,
> I think: (a) PIE is not in use; and (b) binary on disk is different from
> program being executed. Case (b) is hopeless as I understand it? But (a)
> shouldn't.

The (b) case is in use by Daniel Jacobowitz in:
	RFC: Verify AT_ENTRY before using it
	http://sourceware.org/ml/gdb-patches/2010-02/msg00612.html
	 - In one window, "gdbserver :PORT /path/to/this/loader /path/to/binary".
	 - In the other, "gdb /path/to/binary" and "target remote :PORT".

which is IMO invalid use of GDB it was the only working solution its time.
I broke it by the PIE patchset (fixed in that thread).  IMO the correct
solution is to have symbol file matching the target which was addressed by:
	[patch] Support gdb --args ld.so progname
	http://sourceware.org/ml/gdb-patches/2010-02/msg00647.html
	 - To be able to use "gdb /path/to/this/loader" on the client.
	 - To be pinged for gdb-7.3.

OTOH I understand GDB should not break backward compatibility even if/after
the latter patch gets accepted so we have to consider the (b) case as valid.


> So, expanding the logic in the comment I am suggesting, I think we need
> to account for the situation where svr4_exec_displacement fails because
> of non-PIE executable.
> 
> Either: (a1) there was no PIE in sight during the previous run either
>              => offsets should be already be correct;

Yes, if there is no PIE anywhere, the whole PIE patchset by me has no effect.


>         (a2) PIE was in use during the previous run => the offsets
>              may be incorrect.  But it should be trivial to reset them.

If PIE was in use during the previous run then svr4_exec_displacement should
not fail, it should succeed and report the current offset to reset to.
(If it fails it is an unexpected and not known so far PIE support bug.)

These offsets are bound to symfile_objfile so the offsets will be reset
/ different if PIE executable was run previously and non-PIE different
executable is loaded + executed now.


Therefore replaced there this part:
     If we cannot compute the PIE displacement, either:

       - The executable is not PIE.

       - SYMFILE_OBJFILE does not match the executable started in the target.
         This can happen for main executable symbols loaded at the host while
         `ld.so --ld-args main-executable' is loaded in the target.

     Then we leave the section offsets untouched and use them as is for
     this run.  Either:


> So, I'm wondering whether it would make sense to amend your patch to
> check for PIE after svr4_exec_displacement failed, and use a zero
> displacement if PIE is not detected.

It is based on
	PIE question
	http://sourceware.org/ml/gdb/2010-03/msg00040.html
	 - Remote target's qOffsets being reset.
and



> > +    # A bit better test coverage.
> > +    gdb_test "set disable-randomization off"
> 
> I think that this does more than just "better coverage". It is a
> critical part of the testcase that helps trying to obtain two runs
> with different displacement addresses.

True, without it FAILs only on testfile message check
	reach-dl_main: seen displacement message as NONZERO
while with it there is a real GDB failure:
	Cannot insert breakpoint -2.^M
	Error accessing memory address 0x7f9467321d00: Input/output error.^M
	(gdb) FAIL: gdb.base/break-interp.exp: LDprelinkNOdebugNO: reach-dl_main: reach


> > +    reach "dl_main" "run  segv" $displacement
>                            ^^ detail: two spaces?

It was a "hidden parameter" in some versions (not suitable for FSF GDB).  But
I can no longer find the code using it in repositories history.  Fixed.


Thanks,
Jan


gdb/
2010-07-04  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Joel Brobecker  <brobecker@adacore.com>

	Fix re-run of PIE executable.
	* solib-svr4.c (svr4_relocate_main_executable) <symfile_objfile>: Remove
	the part of pre-set SYMFILE_OBJFILE->SECTION_OFFSETS.

gdb/testsuite/
2010-07-04  Jan Kratochvil  <jan.kratochvil@redhat.com>
	    Joel Brobecker  <brobecker@adacore.com>

	Fix re-run of PIE executable.
	* gdb.base/break-interp.exp (test_ld): Turn off "disable-randomization".
	Remove $displacement_main to match the solib-svr4.c change.  New "kill"
	and re-"run" of the inferior.

--- a/gdb/solib-svr4.c
+++ b/gdb/solib-svr4.c
@@ -1989,17 +1989,32 @@ svr4_relocate_main_executable (void)
 {
   CORE_ADDR displacement;
 
-  if (symfile_objfile)
-    {
-      int i;
+  /* If we are re-running this executable, SYMFILE_OBJFILE->SECTION_OFFSETS
+     probably contains the offsets computed using the PIE displacement
+     from the previous run, which of course are irrelevant for this run.
+     So we need to determine the new PIE displacement and recompute the
+     section offsets accordingly, even if SYMFILE_OBJFILE->SECTION_OFFSETS
+     already contains pre-computed offsets.
 
-      /* Remote target may have already set specific offsets by `qOffsets'
-	 which should be preferred.  */
+     If we cannot compute the PIE displacement, either:
 
-      for (i = 0; i < symfile_objfile->num_sections; i++)
-	if (ANOFFSET (symfile_objfile->section_offsets, i) != 0)
-	  return;
-    }
+       - The executable is not PIE.
+
+       - SYMFILE_OBJFILE does not match the executable started in the target.
+	 This can happen for main executable symbols loaded at the host while
+	 `ld.so --ld-args main-executable' is loaded in the target.
+
+     Then we leave the section offsets untouched and use them as is for
+     this run.  Either:
+
+       - These section offsets were properly reset earlier, and thus
+	 already contain the correct values.  This can happen for instance
+	 when reconnecting via the remote protocol to a target that supports
+	 the `qOffsets' packet.
+
+       - The section offsets were not reset earlier, and the best we can
+	 hope is that the old offsets are still applicable to the new run.
+   */
 
   if (! svr4_exec_displacement (&displacement))
     return;
--- a/gdb/testsuite/gdb.base/break-interp.exp
+++ b/gdb/testsuite/gdb.base/break-interp.exp
@@ -337,6 +337,11 @@ proc test_ld {file ifmain trynosym displacement} {
     # Print the "PIE (Position Independent Executable) displacement" message.
     gdb_test_no_output "set verbose on"
 
+    # We want to test the re-run of a PIE in the case where the executable
+    # is loaded with a different displacement, but disable-randomization
+    # prevents that from happening.  So turn it off.
+    gdb_test "set disable-randomization off"
+
     reach "dl_main" "run segv" $displacement
 
     gdb_test "bt" "#0 +\[^\r\n\]*\\mdl_main\\M.*" "dl bt"
@@ -347,7 +352,13 @@ proc test_ld {file ifmain trynosym displacement} {
 	reach "libfunc" continue "NONE"
 
 	gdb_test "bt" "#0 +\[^\r\n\]*\\mlibfunc\\M\[^\r\n\]*\r\n#1 +\[^\r\n\]*\\mmain\\M.*" "main bt"
+    }
 
+    # Try re-run if the new PIE displacement takes effect.
+    gdb_test "kill" "" "kill" {Kill the program being debugged\? \(y or n\) } "y"
+    reach "dl_main" "run segv" $displacement
+
+    if $ifmain {
 	test_core $file $displacement
 
 	test_attach $file $displacement

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: ping: [patch 6/6] PIE: Fix back re-run
  2010-07-04 10:19     ` Jan Kratochvil
@ 2010-07-05 17:48       ` Joel Brobecker
  2010-07-05 18:16         ` Jan Kratochvil
  0 siblings, 1 reply; 6+ messages in thread
From: Joel Brobecker @ 2010-07-05 17:48 UTC (permalink / raw)
  To: Jan Kratochvil; +Cc: gdb-patches

> gdb/
> 2010-07-04  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 	    Joel Brobecker  <brobecker@adacore.com>
> 
> 	Fix re-run of PIE executable.
> 	* solib-svr4.c (svr4_relocate_main_executable) <symfile_objfile>: Remove
> 	the part of pre-set SYMFILE_OBJFILE->SECTION_OFFSETS.
> 
> gdb/testsuite/
> 2010-07-04  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 	    Joel Brobecker  <brobecker@adacore.com>
> 
> 	Fix re-run of PIE executable.
> 	* gdb.base/break-interp.exp (test_ld): Turn off "disable-randomization".
> 	Remove $displacement_main to match the solib-svr4.c change.  New "kill"
> 	and re-"run" of the inferior.

No comment, OK to checkin :).

Congrats on being able to teach me some about PIE and prelinking ;-).

-- 
Joel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: ping: [patch 6/6] PIE: Fix back re-run
  2010-07-05 17:48       ` Joel Brobecker
@ 2010-07-05 18:16         ` Jan Kratochvil
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Kratochvil @ 2010-07-05 18:16 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On Mon, 05 Jul 2010 19:48:20 +0200, Joel Brobecker wrote:
> Congrats on being able to teach me some about PIE and prelinking ;-).

Thanks, I even discovered that PIEs are intentionally skipped during
prelinking while responding to your review.  Not that it changes too much on
the code IIRC, anyway it is done.


Checked-in.  The whole series is now in.


Thanks,
Jan


http://sourceware.org/ml/gdb-cvs/2010-07/msg00029.html

--- src/gdb/ChangeLog	2010/07/05 18:00:39	1.11966
+++ src/gdb/ChangeLog	2010/07/05 18:04:32	1.11967
@@ -1,6 +1,13 @@
 2010-07-05  Jan Kratochvil  <jan.kratochvil@redhat.com>
 	    Joel Brobecker  <brobecker@adacore.com>
 
+	Fix re-run of PIE executable, PR shlibs/11776.
+	* solib-svr4.c (svr4_relocate_main_executable) <symfile_objfile>: Remove
+	the part of pre-set SYMFILE_OBJFILE->SECTION_OFFSETS.
+
+2010-07-05  Jan Kratochvil  <jan.kratochvil@redhat.com>
+	    Joel Brobecker  <brobecker@adacore.com>
+
 	* auxv.c (memory_xfer_auxv): Update attach comment.
 	* solib-svr4.c (svr4_special_symbol_handling): Remove the call to
 	svr4_relocate_main_executable.
--- src/gdb/solib-svr4.c	2010/07/05 18:00:39	1.136
+++ src/gdb/solib-svr4.c	2010/07/05 18:04:33	1.137
@@ -1989,17 +1989,32 @@
 {
   CORE_ADDR displacement;
 
-  if (symfile_objfile)
-    {
-      int i;
-
-      /* Remote target may have already set specific offsets by `qOffsets'
-	 which should be preferred.  */
-
-      for (i = 0; i < symfile_objfile->num_sections; i++)
-	if (ANOFFSET (symfile_objfile->section_offsets, i) != 0)
-	  return;
-    }
+  /* If we are re-running this executable, SYMFILE_OBJFILE->SECTION_OFFSETS
+     probably contains the offsets computed using the PIE displacement
+     from the previous run, which of course are irrelevant for this run.
+     So we need to determine the new PIE displacement and recompute the
+     section offsets accordingly, even if SYMFILE_OBJFILE->SECTION_OFFSETS
+     already contains pre-computed offsets.
+
+     If we cannot compute the PIE displacement, either:
+
+       - The executable is not PIE.
+
+       - SYMFILE_OBJFILE does not match the executable started in the target.
+	 This can happen for main executable symbols loaded at the host while
+	 `ld.so --ld-args main-executable' is loaded in the target.
+
+     Then we leave the section offsets untouched and use them as is for
+     this run.  Either:
+
+       - These section offsets were properly reset earlier, and thus
+	 already contain the correct values.  This can happen for instance
+	 when reconnecting via the remote protocol to a target that supports
+	 the `qOffsets' packet.
+
+       - The section offsets were not reset earlier, and the best we can
+	 hope is that the old offsets are still applicable to the new run.
+   */
 
   if (! svr4_exec_displacement (&displacement))
     return;
--- src/gdb/testsuite/ChangeLog	2010/07/05 18:02:56	1.2375
+++ src/gdb/testsuite/ChangeLog	2010/07/05 18:04:33	1.2376
@@ -1,6 +1,14 @@
 2010-07-05  Jan Kratochvil  <jan.kratochvil@redhat.com>
 	    Joel Brobecker  <brobecker@adacore.com>
 
+	Fix re-run of PIE executable, PR shlibs/11776.
+	* gdb.base/break-interp.exp (test_ld): Turn off "disable-randomization".
+	Remove $displacement_main to match the solib-svr4.c change.  New "kill"
+	and re-"run" of the inferior.
+
+2010-07-05  Jan Kratochvil  <jan.kratochvil@redhat.com>
+	    Joel Brobecker  <brobecker@adacore.com>
+
 	Cope with missing /usr/sbin/prelink.
 	* lib/prelink-support.exp (prelink_no):
 	<result == 1 && $output is "no such file or directory">: New.
--- src/gdb/testsuite/gdb.base/break-interp.exp	2010/07/05 18:01:53	1.17
+++ src/gdb/testsuite/gdb.base/break-interp.exp	2010/07/05 18:04:33	1.18
@@ -337,6 +337,11 @@
     # Print the "PIE (Position Independent Executable) displacement" message.
     gdb_test_no_output "set verbose on"
 
+    # We want to test the re-run of a PIE in the case where the executable
+    # is loaded with a different displacement, but disable-randomization
+    # prevents that from happening.  So turn it off.
+    gdb_test "set disable-randomization off"
+
     reach "dl_main" "run segv" $displacement
 
     gdb_test "bt" "#0 +\[^\r\n\]*\\mdl_main\\M.*" "dl bt"
@@ -347,7 +352,13 @@
 	reach "libfunc" continue "NONE"
 
 	gdb_test "bt" "#0 +\[^\r\n\]*\\mlibfunc\\M\[^\r\n\]*\r\n#1 +\[^\r\n\]*\\mmain\\M.*" "main bt"
+    }
 
+    # Try re-run if the new PIE displacement takes effect.
+    gdb_test "kill" "" "kill" {Kill the program being debugged\? \(y or n\) } "y"
+    reach "dl_main" "run segv" $displacement
+
+    if $ifmain {
 	test_core $file $displacement
 
 	test_attach $file $displacement

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-07-05 18:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-29 16:19 [patch 6/6] PIE: Fix back re-run Jan Kratochvil
2010-06-09 15:10 ` ping: " Jan Kratochvil
2010-07-01 19:10   ` Joel Brobecker
2010-07-04 10:19     ` Jan Kratochvil
2010-07-05 17:48       ` Joel Brobecker
2010-07-05 18:16         ` Jan Kratochvil

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).