From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19676 invoked by alias); 16 Oct 2011 19:22:28 -0000 Received: (qmail 19666 invoked by uid 22791); 16 Oct 2011 19:22:26 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_DN 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, 16 Oct 2011 19:22:04 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p9GJM3hF006529 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Sun, 16 Oct 2011 15:22:03 -0400 Received: from host1.jankratochvil.net (ovpn-116-16.ams2.redhat.com [10.36.116.16]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p9GJM0Gw005473 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Sun, 16 Oct 2011 15:22:02 -0400 Received: from host1.jankratochvil.net (localhost [127.0.0.1]) by host1.jankratochvil.net (8.14.4/8.14.4) with ESMTP id p9GJLxMR006491; Sun, 16 Oct 2011 21:21:59 +0200 Received: (from jkratoch@localhost) by host1.jankratochvil.net (8.14.4/8.14.4/Submit) id p9GJLvRV006485; Sun, 16 Oct 2011 21:21:57 +0200 Date: Sun, 16 Oct 2011 19:42:00 -0000 From: Jan Kratochvil To: Aleksandar Ristovski Cc: gdb-patches@sources.redhat.com Subject: Re: [patch] Relocate phdr in read_program_header Message-ID: <20111016192157.GA619@host1.jankratochvil.net> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) 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: 2011-10/txt/msg00447.txt.bz2 On Fri, 30 Sep 2011 21:31:33 +0200, Aleksandar Ristovski wrote: > * solib-svr4.c (read_program_header): If PT_PHDR is present, use it > to calculate correct address in case of PIE. FSF ChangeLog should describe the changes, not their reason. Therefore: * solib-svr4.c (read_program_header): New variable pt_phdr, initialize it from target PT_PHDR p_vaddr, relocate sect_addr by it. > --- gdb/solib-svr4.c 30 Aug 2011 02:48:05 -0000 1.154 > +++ gdb/solib-svr4.c 30 Sep 2011 17:47:13 -0000 > @@ -385,7 +385,7 @@ static gdb_byte * > read_program_header (int type, int *p_sect_size, int *p_arch_size) > { > enum bfd_endian byte_order = gdbarch_byte_order (target_gdbarch); > - CORE_ADDR at_phdr, at_phent, at_phnum; > + CORE_ADDR at_phdr, at_phent, at_phnum, pt_phdr; > int arch_size, sect_size; > CORE_ADDR sect_addr; > gdb_byte *buf; > @@ -408,6 +408,10 @@ read_program_header (int type, int *p_se > else > return 0; > > + pt_phdr = (CORE_ADDR)-1; /* Address of the first entry. If not PIE, > + this will be zero. > + For PIE, it will be unrelocated at_phdr. */ There is CORE_ADDR_MAX. But I would prefer new `int pt_phdr_p' as "predicate" boolean flag for valid pt_phdr value, these special values are fragile and one has to think whether they can happen. > + > /* Find the requested segment. */ > if (type == -1) > { > @@ -427,6 +431,11 @@ read_program_header (int type, int *p_se > return 0; > > if (extract_unsigned_integer ((gdb_byte *)phdr.p_type, > + 4, byte_order) == PT_PHDR) > + pt_phdr = extract_unsigned_integer ((gdb_byte *)phdr.p_vaddr, > + 4, byte_order); > + > + if (extract_unsigned_integer ((gdb_byte *)phdr.p_type, > 4, byte_order) == type) That p_type could be decoded only once. Also any casta are `(type) val' with a space accoridng to GCS (GNU Coding Standards). > break; > } > @@ -453,6 +462,11 @@ read_program_header (int type, int *p_se > return 0; > > if (extract_unsigned_integer ((gdb_byte *)phdr.p_type, > + 4, byte_order) == PT_PHDR) > + pt_phdr = extract_unsigned_integer ((gdb_byte *)phdr.p_vaddr, > + 8, byte_order); > + > + if (extract_unsigned_integer ((gdb_byte *)phdr.p_type, > 4, byte_order) == type) > break; > } > @@ -467,6 +481,16 @@ read_program_header (int type, int *p_se > 8, byte_order); > } > > + /* at_phdr is real address in memory. pt_phdr is what pheader says it is. > + Relocation offset is the difference between the two. */ > + if (pt_phdr != (CORE_ADDR)-1) > + { > + /* PT_PHDR is optional in the execution view, but we really need it > + for PIE to make this work in general. However, if not found, > + work with what you have. */ > + sect_addr = sect_addr + (at_phdr - pt_phdr); > + } > + > /* Read in requested program header. */ > buf = xmalloc (sect_size); > if (target_read_memory (sect_addr, buf, sect_size)) Just about the style, I do not see any problems of the patch, thanks. Wrote the testcase, it is probably GNU/Linux-specific. I would like to review yet the changes. Thanks, Jan 2011-10-16 Jan Kratochvil * gdb.base/attach-pie-noexec.c: New files. * gdb.base/attach-pie-noexec.exp: New files. --- /dev/null +++ b/gdb/testsuite/gdb.base/attach-pie-noexec.c @@ -0,0 +1,25 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2011 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 . */ + +#include + +int +main (void) +{ + sleep (600); + return 0; +} --- /dev/null +++ b/gdb/testsuite/gdb.base/attach-pie-noexec.exp @@ -0,0 +1,66 @@ +# Copyright (C) 2011 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 . + +# Manipulation with PID on target is not supported. +if [is_remote target] then { + return 0 +} + +set testfile attach-pie-noexec +set executable ${testfile} +set binfile ${objdir}/${subdir}/${executable} + +if { [prepare_for_testing ${testfile}.exp $executable "" [list debug "additional_flags=-fPIE -pie"]] } { + return -1 +} + +clean_restart $executable +set arch "" +set test "show architecture" +gdb_test_multiple $test $test { + -re "The target architecture is set automatically \\(currently (.*)\\)\r\n$gdb_prompt $" { + set arch $expect_out(1,string) + pass $test + } +} +if ![runto_main] { + return 0 +} +set test "sanity check info shared" +gdb_test_multiple "info shared" $test { + -re "From\[ \t\]+To\[ \t\]+Syms Read\[ \t\]+Shared Object Library\r\n0x.*\r\n$gdb_prompt $" { + pass $test + } + -re "No shared libraries loaded at this time\\.\r\n$gdb_prompt $" { + untested ${testfile}.exp + } +} +gdb_exit + +if {$arch == ""} { + untested ${testfile}.exp + return 0 +} + +set testpid [eval exec $binfile &] +exec sleep 2 + +gdb_start +file delete -- $binfile +gdb_test "attach $testpid" "Attaching to process $testpid\r\n.*: No such file or directory\\." "attach" +gdb_test "set architecture $arch" "The target architecture is assumed to be $arch" +gdb_test "info shared" "From\[ \t\]+To\[ \t\]+Syms Read\[ \t\]+Shared Object Library\r\n0x.*" + +eval exec kill -9 $testpid