From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18096 invoked by alias); 29 Jun 2010 17:49:21 -0000 Received: (qmail 18085 invoked by uid 22791); 29 Jun 2010 17:49:20 -0000 X-SWARE-Spam-Status: No, hits=-1.4 required=5.0 tests=AWL,BAYES_05 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 29 Jun 2010 17:49:15 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 303A82BAB8A; Tue, 29 Jun 2010 13:42:21 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id j+JHkqFJ4oWb; Tue, 29 Jun 2010 13:42:21 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id E79D22BAB4E; Tue, 29 Jun 2010 13:42:20 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 7883CF5895; Tue, 29 Jun 2010 10:42:16 -0700 (PDT) Date: Tue, 29 Jun 2010 17:49:00 -0000 From: Joel Brobecker To: Jan Kratochvil Cc: gdb-patches@sourceware.org Subject: Re: ping: [patch 1/6] PIE: Attach binary even after re-prelinked underneath Message-ID: <20100629174216.GR2595@adacore.com> References: <20100329153031.GA31671@host0.dyn.jankratochvil.net> <20100609150753.GA7183@host0.dyn.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100609150753.GA7183@host0.dyn.jankratochvil.net> User-Agent: Mutt/1.5.20 (2009-06-14) 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-06/txt/msg00679.txt.bz2 > gdb/ > 2010-03-29 Jan Kratochvil > > Fix attaching to PIEs prelinked on the disk since their start. > * solib-svr4.c (svr4_exec_displacement): New variable arch_size. > Verify it against bfd_get_arch_size. Try to match arbitrary > displacement for the phdrs comparison. > > gdb/testsuite/ > 2010-03-29 Jan Kratochvil > > * gdb.base/break-interp.exp: Run $binpie with new value "ATTACH", new > code for it. New variable relink_args. > (prelinkYES): Call prelinkNO. > (test_attach): Accept new parameter relink_args. Re-prelink the binary > in such case. Move the core code to ... > (test_attach_gdb): ... a new function. Send GDB command "file". > Extend expected "Attaching to " string. OK with a few editorial changes: Instead of saying "*since* [process] started", can you use "*after* the process was started". That would make things a little clearer for me. I'll just highlight areas where I think the change should be made. > + /* We are dealing with three different addresses. EXEC_BFD > + represents current address in on-disk file. target memory content > + may be different from EXEC_BFD as the file may have been prelinked > + to a different address since the executable has been loaded. ^^^^^ after > + Moreover the address of placement in target memory can be > + different from what say the target memory program headers - this what the program headers in target memory say > + is the goal of PIE. > + Detected DISPLACEMENT covers both the offsets of PIE placement and > + possible new prelink since start of the program. Here relocate ^^^^^ performed after (?) > + /* DISPLACEMENT could be found easier by the difference of ^^^^^^ more easily > + ehdr2->e_entry but already read BUF does not contain ehdr. */ "already read BUF" is a bit terse and sounds like incomplete English to me (I am not a specialist, though). Is the "already read" part the important part? I think we need to explain why we use the more complicated route. For instance, we could say something like this: /* DISPLACEMENT could be found easier by the difference of ehdr2->e_entry. But we haven't read the ehdr yet, and we already have enough information to compute that displacement with what we've read. */ > + /* PT_GNU_STACK addresses are left as zero not being > + relocated by prelink, their displacing would create false > + verification failure. Feel free to test the unrelocated > + comparison for any segment type. */ Can you explain differently what you are try to say? > - set displacement "ZERO" > + # If the file has been randomly prelinked it must > + # be "NONZERO". We could see "ZERO" only if it was > + # unprelinked na it is now running at the same ^^ > + # ATTACH executables + libraries get modified since > + # they have been run. I'm having problems understanding this sentence. Do you mean perhaps ATTACH means that executables and libraries have been modified after they have been run. ? > + # they have been run. They cannot be used for > + # problem reproducibility after the testcase ends. I would personally add a conclusion to the last sentence, explaining that this is the reason why you are deleting all associated binary files. And I use "reused" instead of "used", to make it clearer that the binaries are saved in order to help reproduce issues found by this testcase. -- Joel