This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Relocate phdr in read_program_header
- From: Aleksandar Ristovski <aristovski at qnx dot com>
- To: gdb-patches at sources dot redhat dot com
- Cc: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Date: Mon, 17 Oct 2011 12:26:28 -0400
- Subject: Re: [patch] Relocate phdr in read_program_header
- References: <j655em$75r$1@dough.gmane.org> <20111016192157.GA619@host1.jankratochvil.net>
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:
<date> Aleksandar Ristovski <aristovski@qnx.com>
* solib-svr4.c (read_program_header): New variable pt_phdr,
initialize
it from target PT_PHDR p_vaddr, relocate sect_addr by it.
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))