From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9900 invoked by alias); 4 Jul 2010 10:19:30 -0000 Received: (qmail 9882 invoked by uid 22791); 4 Jul 2010 10:19:27 -0000 X-SWARE-Spam-Status: No, hits=-4.5 required=5.0 tests=AWL,BAYES_50,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,TW_BJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sun, 04 Jul 2010 10:19:21 +0000 Received: from int-mx05.intmail.prod.int.phx2.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.18]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o64AIw4x004264 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sun, 4 Jul 2010 06:18:58 -0400 Received: from host0.dyn.jankratochvil.net (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx05.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o64AItk9024033 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sun, 4 Jul 2010 06:18:57 -0400 Received: from host0.dyn.jankratochvil.net (localhost [127.0.0.1]) by host0.dyn.jankratochvil.net (8.14.4/8.14.4) with ESMTP id o64AIt79006073; Sun, 4 Jul 2010 12:18:55 +0200 Received: (from jkratoch@localhost) by host0.dyn.jankratochvil.net (8.14.4/8.14.4/Submit) id o64AItWT006070; Sun, 4 Jul 2010 12:18:55 +0200 Date: Sun, 04 Jul 2010 10:19:00 -0000 From: Jan Kratochvil To: Joel Brobecker Cc: gdb-patches@sourceware.org Subject: Re: ping: [patch 6/6] PIE: Fix back re-run Message-ID: <20100704101855.GF6875@host0.dyn.jankratochvil.net> References: <20100329161905.GE2940@host0.dyn.jankratochvil.net> <20100609151008.GF7183@host0.dyn.jankratochvil.net> <20100701191041.GX2595@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100701191041.GX2595@adacore.com> User-Agent: Mutt/1.5.20 (2009-12-10) X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-07/txt/msg00070.txt.bz2 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 Joel Brobecker Fix re-run of PIE executable. * solib-svr4.c (svr4_relocate_main_executable) : Remove the part of pre-set SYMFILE_OBJFILE->SECTION_OFFSETS. gdb/testsuite/ 2010-07-04 Jan Kratochvil Joel Brobecker 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