Re: hurd: PIE support

Simon Marchi
2017-12-27T02:48:54Z

On 2017-12-22 11:55 AM, Samuel Thibault wrote:
> Hello,
>
> PIE is being pushed more and more, so we have to support it in the Hurd
> port :)
>
> The simplest way to fix things is to provide gdb with the entry address
> through auxv. The attached patch implements this. Could you have a look
> soon?
>
> Samuel

Hi Samuel,

The patch looks good to me, although I can't really try it and confirm it
works.

Some coding style comments: unless the variable is logically a boolean, we
try to be explicit in our comparisons:

if (pointer != NULL)
if (integer != 0)

instead of

if (pointer)
if (integer)

So if you could fix that throughout the patch, it would be appreciated. We
would also need a ChangeLog entry. If you can add that, as well as a proper
title and commit message, and re-submit using git-send-email, it would be
appreciated. More comments below.

--- gdb-7.12.orig/gdb/gnu-nat.c
+++ gdb-7.12/gdb/gnu-nat.c
@@ -52,6 +52,8 @@ extern "C"
#include <setjmp.h>
#include <signal.h>
#include <sys/ptrace.h>
+#include <elf.h>
+#include <link.h>

#include "inferior.h"
#include "symtab.h"
@@ -2542,6 +2544,61 @@ gnu_xfer_memory (gdb_byte *readbuf, cons
}
}

+/* GNU does not have auxv, but we can at least fake the AT_ENTRY entry for PIE
+ binaries. */
+static enum target_xfer_status
+gnu_xfer_auxv (gdb_byte *readbuf, const gdb_byte *writebuf,
+ CORE_ADDR memaddr, ULONGEST len, ULONGEST *xfered_len)
+{
+ task_t task = (gnu_current_inf
+ ? (gnu_current_inf->task
+ ? gnu_current_inf->task->port : 0)
+ : 0);
+ process_t proc;
+ int res;
+ kern_return_t err;
+ vm_address_t entry;
+ ElfW(auxv_t) auxv[2];
+
+ if (task == MACH_PORT_NULL)
+ return TARGET_XFER_E_IO;
+ if (writebuf != NULL)
+ return TARGET_XFER_E_IO;
+
+ err = proc_task2proc (proc_server, task, &proc);
+ if (err)
+ return TARGET_XFER_E_IO;
+
+ /* Get entry from proc server. */
+ err = proc_get_entry (proc, &entry);
+ if (err)
+ return TARGET_XFER_E_IO;
+
+ /* Fake auxv entry. */
+ auxv[0].a_type = AT_ENTRY;
+ auxv[0].a_un.a_val = entry;
+ auxv[1].a_type = AT_NULL;
+ auxv[1].a_un.a_val = 0;
+
+ inf_debug (gnu_current_inf, "reading auxv %s[%s] --> %s",
+ paddress (target_gdbarch (), memaddr), pulongest (len),
+ host_address_to_string (readbuf));
+
+ if (memaddr == sizeof(auxv))
+ return TARGET_XFER_EOF;
+
+ if (memaddr > sizeof(auxv))
+ return TARGET_XFER_E_IO;

Can we do these two checks earlier, so we don't do unnecessary work in
those cases?

Also, please add a space after sizeof (as if it was a function call).

+ if (memaddr + len > sizeof(auxv))
+ len = sizeof(auxv) - memaddr;
+
+ memcpy (readbuf, (gdb_byte*) &auxv + memaddr, len);

Space before *.

+ *xfered_len = len;
+
+ return TARGET_XFER_OK;
+}
+
/* Target to_xfer_partial implementation. */

static enum target_xfer_status
@@ -2554,6 +2611,8 @@ gnu_xfer_partial (struct target_ops *ops
{
case TARGET_OBJECT_MEMORY:
return gnu_xfer_memory (readbuf, writebuf, offset, len, xfered_len);
+ case TARGET_OBJECT_AUXV:
+ return gnu_xfer_auxv (readbuf, writebuf, offset, len, xfered_len);
default:
return TARGET_XFER_E_IO;
}

Thanks,

Simon