From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24579 invoked by alias); 17 Oct 2011 16:27:00 -0000 Received: (qmail 24518 invoked by uid 22791); 17 Oct 2011 16:26:58 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=AWL,BAYES_00,RCVD_NUMERIC_HELO,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_DN X-Spam-Check-By: sourceware.org Received: from lo.gmane.org (HELO lo.gmane.org) (80.91.229.12) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 17 Oct 2011 16:26:43 +0000 Received: from list by lo.gmane.org with local (Exim 4.69) (envelope-from ) id 1RFq1R-0003Pw-7I for gdb-patches@sources.redhat.com; Mon, 17 Oct 2011 18:26:41 +0200 Received: from 209.226.137.108 ([209.226.137.108]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Mon, 17 Oct 2011 18:26:41 +0200 Received: from aristovski by 209.226.137.108 with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Mon, 17 Oct 2011 18:26:41 +0200 To: gdb-patches@sources.redhat.com From: Aleksandar Ristovski Subject: Re: [patch] Relocate phdr in read_program_header Date: Mon, 17 Oct 2011 16:29:00 -0000 Message-ID: <4E9C5734.4000609@qnx.com> References: <20111016192157.GA619@host1.jankratochvil.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------050608040409000505070506" Cc: Jan Kratochvil User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:7.0) Gecko/20110923 Thunderbird/7.0 In-Reply-To: <20111016192157.GA619@host1.jankratochvil.net> 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/msg00469.txt.bz2 This is a multi-part message in MIME format. --------------050608040409000505070506 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 2283 On 11-10-16 03:21 PM, Jan Kratochvil wrote: > 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. Change log changed (see below). >> >> + 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. pt_phdr_p introduced. Also, pt_phdr initialized to 0 even though not strictly necessary, but gcc complained about possible uninitialized use. > > >> + >> /* 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). Cast fixed, p_type decoded only once. >> 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 was contemplating a testcase but I couldn't think of a trick to "make" gdb not find the executable... Thanks for the test. > > I would like to review yet the changes. Revised patch attached. Thanks, Aleksandar New change log: Aleksandar Ristovski * solib-svr4.c (read_program_header): New variable pt_phdr, initialize it from target PT_PHDR p_vaddr, relocate sect_addr by it. --------------050608040409000505070506 Content-Type: text/x-patch; name="PIE_read_program_header-201110170900.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="PIE_read_program_header-201110170900.patch" Content-length: 2678 Index: gdb/solib-svr4.c =================================================================== RCS file: /cvs/src/src/gdb/solib-svr4.c,v retrieving revision 1.157 diff -u -p -r1.157 solib-svr4.c --- gdb/solib-svr4.c 14 Oct 2011 07:58:58 -0000 1.157 +++ gdb/solib-svr4.c 17 Oct 2011 14:48:31 -0000 @@ -364,10 +364,11 @@ 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 = 0; int arch_size, sect_size; CORE_ADDR sect_addr; gdb_byte *buf; + int pt_phdr_p = 0; /* Get required auxv elements from target. */ if (target_auxv_search (¤t_target, AT_PHDR, &at_phdr) <= 0) @@ -401,12 +402,23 @@ read_program_header (int type, int *p_se /* Search for requested PHDR. */ for (i = 0; i < at_phnum; i++) { + int p_type; + if (target_read_memory (at_phdr + i * sizeof (phdr), (gdb_byte *)&phdr, sizeof (phdr))) return 0; - if (extract_unsigned_integer ((gdb_byte *)phdr.p_type, - 4, byte_order) == type) + p_type = extract_unsigned_integer ((gdb_byte *) phdr.p_type, + 4, byte_order); + + if (p_type == PT_PHDR) + { + pt_phdr_p = 1; + pt_phdr = extract_unsigned_integer ((gdb_byte *) phdr.p_vaddr, + 4, byte_order); + } + + if (p_type == type) break; } @@ -427,12 +439,23 @@ read_program_header (int type, int *p_se /* Search for requested PHDR. */ for (i = 0; i < at_phnum; i++) { + int p_type; + if (target_read_memory (at_phdr + i * sizeof (phdr), (gdb_byte *)&phdr, sizeof (phdr))) return 0; - if (extract_unsigned_integer ((gdb_byte *)phdr.p_type, - 4, byte_order) == type) + p_type = extract_unsigned_integer ((gdb_byte *) phdr.p_type, + 4, byte_order); + + if (p_type == PT_PHDR) + { + pt_phdr_p = 1; + pt_phdr = extract_unsigned_integer ((gdb_byte *) phdr.p_vaddr, + 8, byte_order); + } + + if (p_type == type) break; } @@ -446,6 +469,16 @@ read_program_header (int type, int *p_se 8, byte_order); } + /* PT_PHDR is optional, but we really need it + for PIE to make this work in general. */ + + if (pt_phdr_p) + { + /* at_phdr is real address in memory. pt_phdr is what pheader says it is. + Relocation offset is the difference between the two. */ + 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)) --------------050608040409000505070506--