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

[Simon Marchi <simark at simark dot ca>]

On 2020-02-13 1:14 p.m., 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.

Thanks, hopefully that way of finding the base address is sufficient for what
you need.  I like it, it's much more self-contained.

> ---
>  gdb/windows-tdep.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> 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;
> +    }
> +  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)

Use explicit comparison operators when comparing pointers and integers (except
those that are really meant to represent boolean values and haven't been
converted to `bool` yet).  So,

    if (symfile_objfile != nullptr && exec_base != 0)

> +    {
> +      CORE_ADDR vmaddr = pe_data (exec_bfd)->pe_opthdr.ImageBase;
> +      if (vmaddr != exec_base)
> +	objfile_rebase (symfile_objfile, exec_base - vmaddr);
> +    }
> +}
> +
>  /* To be called from the various GDB_OSABI_CYGWIN handlers for the
>     various Windows architectures and machine types.  */
>  
> @@ -830,6 +877,8 @@ windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>  
>    set_gdbarch_gdb_signal_to_target (gdbarch, windows_gdb_signal_to_target);
>  
> +  solib_target_so_ops.solib_create_inferior_hook
> +    = windows_solib_create_inferior_hook;

I don't think this bit is right.  Some other architecture/target could be using
solib_target_so_ops, and you force that solib_create_inferior_hook value for them
too.  I mean, it won't matter for you in practice when debugging a single process
on Windows, but still let's do it right.

Ideally, all this would be a C++ class hierarchy and you would extend the
solib_target_so_ops class to implement the solib_create_inferior_hook method.
But here, we have a structure with function pointers, so you need to copy
solib_target_so_ops into a windows-specific version, and assign the
solib_create_inferior_hook pointer in that one.

Check how mips_linux_init_abi does it, it's not very complicated.

Simon