hurd: PIE support

Message ID 47cd545e-6931-cacf-5517-189f4dcaaea9@simark.ca
State New, archived
Headers

Commit Message

Simon Marchi Dec. 27, 2017, 2:48 a.m. UTC
  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.


+  *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
  

Comments

Samuel Thibault Dec. 27, 2017, 3:44 p.m. UTC | #1
Hello,

Simon Marchi, on mar. 26 déc. 2017 21:48:23 -0500, wrote:
> We would also need a ChangeLog entry.

Well, I had put a ChangeLog entry at the top of the posted file. Is
something else needed?  In the patch I have just sent to the list I have
directly put the gdb/ChangeLog update in addition to the commit message
log.  If that is what is wanted, then the gdb/CONTRIBUTE file should be
update, otherwise frustration happens on both sides.

Samuel
  
Simon Marchi Dec. 28, 2017, 2:05 a.m. UTC | #2
On 2017-12-27 10:44 AM, Samuel Thibault wrote:
> Hello,
> 
> Simon Marchi, on mar. 26 déc. 2017 21:48:23 -0500, wrote:
>> We would also need a ChangeLog entry.
> 
> Well, I had put a ChangeLog entry at the top of the posted file. Is
> something else needed?  In the patch I have just sent to the list I have
> directly put the gdb/ChangeLog update in addition to the commit message
> log.  If that is what is wanted, then the gdb/CONTRIBUTE file should be
> update, otherwise frustration happens on both sides.
> 
> Samuel
> 

Ah, sorry I missed it.  I ended up using git-apply to apply the patch (git-am
wouldn't work), which looses the information before the diff.

I'll reply to the other mail.

Simon
  

Patch

--- 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 *.