Re: [PATCH v2] Rebase executable to match relocated base address

["Hannes Domani via gdb-patches" <gdb-patches@sourceware.org>]

 Am Freitag, 14. Februar 2020, 12:02:03 MEZ hat Luis Machado <luis.machado@linaro.org> Folgendes geschrieben:

> Hi,
>
> On 2/13/20 3:14 PM, Hannes Domani via gdb-patches wrote:
> > Windows executables linked with -dynamicbase get a new base address
> > when loaded, which makes debugging impossible if the executable isn't
> > also rebased in gdb.
> >
> > The new base address is read from the Process Environment Block.
> > ---
> > v2:
> > This version now no longer needs the fake auxv entry.
> > ---
> >  gdb/windows-tdep.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 49 insertions(+)
> >
>
> Thanks. This version looks better.
>
> > diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
> > index 6eef3fbd96..29c0a828a7 100644
> > --- a/gdb/windows-tdep.c
> > +++ b/gdb/windows-tdep.c
> > @@ -34,6 +34,9 @@
> >  #include "solib.h"
> >  #include "solib-target.h"
> >  #include "gdbcore.h"
> > +#include "coff/internal.h"
> > +#include "libcoff.h"
> > +#include "solist.h"
> >
> >  /* Windows signal numbers differ between MinGW flavors and between
> >      those and Cygwin.  The below enumeration was gleaned from the
> > @@ -812,6 +815,50 @@ windows_get_siginfo_type (struct gdbarch *gdbarch)
> >    return siginfo_type;
> >  }
> >
> > +/* Implement the "solib_create_inferior_hook" target_so_ops method.  */
> > +
> > +static void
> > +windows_solib_create_inferior_hook (int from_tty)
> > +{
> > +  CORE_ADDR exec_base = 0;
> > +
> > +  /* Find base address of main executable in
> > +    TIB->process_environment_block->image_base_address.  */ > +  struct gdbarch *gdbarch = target_gdbarch ();
> > +  enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> > +  int ptr_bytes;
> > +  int peb_offset;  /* Offset of process_environment_block in TIB.  */
> > +  int base_offset; /* Offset of image_base_address in PEB.  */
> > +  if (gdbarch_ptr_bit (gdbarch) == 32)
> > +    {
> > +      ptr_bytes = 4;
> > +      peb_offset = 48;
> > +      base_offset = 8;
> > +    }
> > +  else
> > +    {
> > +      ptr_bytes = 8;
> > +      peb_offset = 96;
> > +      base_offset = 16;
> > +    }
>
> How about stashing the above offsets in windows_gdbarch_data, and then
> using them here?

To be honest, that would seem a bit weird for me, since they are just these
simple numbers, and aren't used anywhere else.


> > +  CORE_ADDR tlb;
> > +  gdb_byte buf[8];
> > +  if (target_get_tib_address (inferior_ptid, &tlb)
> > +      && !target_read_memory (tlb + peb_offset, buf, ptr_bytes))
> > +    {
> > +      CORE_ADDR peb = extract_unsigned_integer (buf, ptr_bytes, byte_order);
> > +      if (!target_read_memory (peb + base_offset, buf, ptr_bytes))
> > +    exec_base = extract_unsigned_integer (buf, ptr_bytes, byte_order);
> > +    }
> > +
> > +  if (symfile_objfile && exec_base)
> > +    {
> > +      CORE_ADDR vmaddr = pe_data (exec_bfd)->pe_opthdr.ImageBase;
> > +      if (vmaddr != exec_base)
> > +    objfile_rebase (symfile_objfile, exec_base - vmaddr);
>
> > +    }
>
> I'd add a comment to the above conditional block on why we're doing this
> relocation now, if you think it is worth mentioning.
>
> It seems to me the behavior has changed now, hence why it seems
> worthwhile adding some information.

Something like?:
/* Rebase executable if the base address changed because of ASLR.  */


Regards
Hannes Domani