[win32] wrong solib from/to addresses

[win32] wrong solib from/to addresses

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 1781 bytes --]

Hello,

We are working on porting our product to Vista, and we have noticed
an issue that this version of the MS OS makes more apparent: The from/to
addresses printed in the "info shared" listing are correct only when
the DLL was loaded at the prefered load address (which is in the
ImageBase field of the COFF/PE header).

I collegue of mine told me that, for security reasons, system DLLs
on Vista are now always rebased, and thus, the information printed
by info based is off by a certain offset.

The core of the attached patch is to implement the target_so_ops
method relocate_section_addresses. For that, I needed to compute
the offset between the load address and the image_base, and store
it for later use (during the call of our routine above). There
were two challenges:

  1. Compute this image base. Rather than dig into the COFF/PE
     data, I took a simpler route that I think has already been
     taken: Use the start address of the .text section. I think
     this is already used to do the symbol relocation.

  2. Make that information available: I found that the lm_info
     field was not allocated, so I had to add its initialization.

With all these changes, the address are correct again.

2007-06-12  Joel Brobecker  <brobecker@adacore.com>

        * win32-nat.c (struct lm_info): Add new field image_base.
        (solib_symbols_add): Compute the prefered load address
        and save it in the lm_info.
        (register_loaded_dll): Initialize new field image_base.
        (win32_relocate_section_addresses): Implement this routine.
        (win32_current_sos): Allocate and set lm_info data.

Tested on x86-windows, no regression. Dejagnu on Vista is not working
at all for me, so I ran the testsuite on XP instead.

OK to apply?

Thanks,
-- 
Joel

[-- Attachment #2: win32-solib-fsf.diff --]
[-- Type: text/plain, Size: 3057 bytes --]

Index: gdb/win32-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/win32-nat.c,v
retrieving revision 1.131
diff -u -p -r1.131 win32-nat.c
--- gdb/win32-nat.c	31 May 2007 17:32:21 -0000	1.131
+++ gdb/win32-nat.c	12 Jun 2007 20:17:19 -0000
@@ -541,6 +541,9 @@ struct safe_symbol_file_add_args
 struct lm_info
 {
   DWORD load_addr;
+
+  /* The ImageBase, aka the prefered load address.  */
+  CORE_ADDR image_base;
 };
 
 static struct so_list solib_start, *solib_end;
@@ -659,6 +662,7 @@ solib_symbols_add (struct so_list *so, C
   static struct objfile *result = NULL;
   char *name = so->so_name;
   bfd *abfd = NULL;
+  asection *text = NULL;
   char *p;
 
   /* The symbols in a dll are offset by 0x1000, which is the
@@ -701,10 +705,19 @@ solib_symbols_add (struct so_list *so, C
       do_cleanups (my_cleanups);
     }
 
+  if (abfd != NULL)
+    text = bfd_get_section_by_name (abfd, ".text");
+
+  /* Compute the ImageBase of our DLL. For that, we assume that
+     it is identical to the VMA of the ".text" section.  This is
+     an assumption that is being made in other places already,
+     so this should be ok.  */
+  if (text != NULL)
+    so->lm_info->image_base = bfd_section_vma (abfd, text);
+
   p = strchr (so->so_name, '\0') - (sizeof ("/cygwin1.dll") - 1);
   if (p >= so->so_name && strcasecmp (p, "/cygwin1.dll") == 0)
     {
-      asection *text = bfd_get_section_by_name (abfd, ".text");
       cygwin_load_start = bfd_section_vma (abfd, text);
       cygwin_load_end = cygwin_load_start + bfd_section_size (abfd, text);
     }
@@ -752,6 +765,7 @@ register_loaded_dll (const char *name, D
   so = XZALLOC (struct so_list);
   so->lm_info = (struct lm_info *) xmalloc (sizeof (struct lm_info));
   so->lm_info->load_addr = load_addr;
+  so->lm_info->image_base = 0; /* Will be filled in later.  */
   cygwin_conv_to_posix_path (buf, so->so_name);
   strcpy (so->so_original_name, so->so_name);
 
@@ -842,8 +856,18 @@ static void
 win32_relocate_section_addresses (struct so_list *so,
 				  struct section_table *sec)
 {
-  /* FIXME */
-  return;
+  const DWORD load_addr = so->lm_info->load_addr;
+  const CORE_ADDR image_base = so->lm_info->image_base;
+
+  /* If we couldn't determine the DLL prefered load address (image base),
+     then we can't adjust the section addresses.  Assume that the DLL was
+     loaded at the prefered load address, which means that the second
+     addresses do not need to be adjusted.  */
+  if (image_base == 0)
+    return;
+
+  sec->addr = sec->addr - image_base + load_addr;
+  sec->endaddr = sec->endaddr - image_base + load_addr;
 }
 
 static void
@@ -2231,6 +2255,8 @@ win32_current_sos (void)
       struct so_list *new = XZALLOC (struct so_list);
       strcpy (new->so_name, sop->so_name);
       strcpy (new->so_original_name, sop->so_original_name);
+      new->lm_info = xmalloc (sizeof (struct lm_info));
+      memcpy (new->lm_info, sop->lm_info, sizeof (struct lm_info));
       if (!start)
 	last = start = new;
       else

Re: [win32] wrong solib from/to addresses

[Pedro Alves]

Hi Joel,

Joel Brobecker wrote:
> We are working on porting our product to Vista, and we have noticed
> an issue that this version of the MS OS makes more apparent: The from/to
> addresses printed in the "info shared" listing are correct only when
> the DLL was loaded at the prefered load address (which is in the
> ImageBase field of the COFF/PE header).
> 
> I collegue of mine told me that, for security reasons, system DLLs
> on Vista are now always rebased, and thus, the information printed
> by info based is off by a certain offset.
> 
> The core of the attached patch is to implement the target_so_ops
> method relocate_section_addresses. For that, I needed to compute
> the offset between the load address and the image_base, and store
> it for later use (during the call of our routine above). There
> were two challenges:
> 
>   1. Compute this image base. Rather than dig into the COFF/PE
>      data, I took a simpler route that I think has already been
>      taken: Use the start address of the .text section. I think
>      this is already used to do the symbol relocation.
> 
>   2. Make that information available: I found that the lm_info
>      field was not allocated, so I had to add its initialization.
> 
> With all these changes, the address are correct again.
> 


I had a similar patch here, that I dumped in favor of a rewrite
of win32 solibs on top of Daniel's pending solib-target.c.  If
I remember correctly, if you are moving the relocation to
relocate_section_addresses, you should remove
get_relocated_section_addrs as it is doing the same work, albeit
bypassing gdb's solib mechanism.

In my version I didn't cache the image_base on lm_info, but computed
it inside relocate_section_addresses.

> Tested on x86-windows, no regression. Dejagnu on Vista is not working
> at all for me, so I ran the testsuite on XP instead.

Did you test this with a recent binutils/ld?  With the
binutils version distributed with Cygwin the dll tests fail,
because of the use of the .so/.sl extension instead of .dll.  Current
ld from cvs recognizes dlls without looking at the file extension.

Eg, does gdb.base/shreloc.exp pass for you?

Cheers,
Pedro Alves

Re: [win32] wrong solib from/to addresses

[Christopher Faylor]

On Wed, Jun 13, 2007 at 12:32:55AM +0100, Pedro Alves wrote:
> Hi Joel,
>
> Joel Brobecker wrote:
>> We are working on porting our product to Vista, and we have noticed
>> an issue that this version of the MS OS makes more apparent: The from/to
>> addresses printed in the "info shared" listing are correct only when
>> the DLL was loaded at the prefered load address (which is in the
>> ImageBase field of the COFF/PE header).
>> I collegue of mine told me that, for security reasons, system DLLs
>> on Vista are now always rebased, and thus, the information printed
>> by info based is off by a certain offset.
>> The core of the attached patch is to implement the target_so_ops
>> method relocate_section_addresses. For that, I needed to compute
>> the offset between the load address and the image_base, and store
>> it for later use (during the call of our routine above). There
>> were two challenges:
>>   1. Compute this image base. Rather than dig into the COFF/PE
>>      data, I took a simpler route that I think has already been
>>      taken: Use the start address of the .text section. I think
>>      this is already used to do the symbol relocation.
>>   2. Make that information available: I found that the lm_info
>>      field was not allocated, so I had to add its initialization.
>> With all these changes, the address are correct again.
>
>
> I had a similar patch here, that I dumped in favor of a rewrite
> of win32 solibs on top of Daniel's pending solib-target.c.

That sounds like the right way to go to me.  Can we hold of on this
until Daniel's changes are in?

cgf

Re: [win32] wrong solib from/to addresses

[Daniel Jacobowitz]

On Tue, Jun 12, 2007 at 10:21:50PM -0400, Christopher Faylor wrote:
> That sounds like the right way to go to me.  Can we hold of on this
> until Daniel's changes are in?

FYI, I was working on it again today and will again tomorrow.  Right
now I plan to require expat for it to work; is that a problem for
anyone?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [win32] wrong solib from/to addresses

[Christopher Faylor]

On Tue, Jun 12, 2007 at 11:09:35PM -0400, Daniel Jacobowitz wrote:
>On Tue, Jun 12, 2007 at 10:21:50PM -0400, Christopher Faylor wrote:
>> That sounds like the right way to go to me.  Can we hold of on this
>> until Daniel's changes are in?
>
>FYI, I was working on it again today and will again tomorrow.  Right
>now I plan to require expat for it to work; is that a problem for
>anyone?

I don't expat that it will be.

cgf

Re: [win32] wrong solib from/to addresses

[Joel Brobecker]

> > I had a similar patch here, that I dumped in favor of a rewrite
> > of win32 solibs on top of Daniel's pending solib-target.c.
> 
> That sounds like the right way to go to me.  Can we hold of on this
> until Daniel's changes are in?

As far as I'm concerned, I'm happy to withdraw this patch. I don't
think that the consequences of these wrong addresses are that bad,
so there is certainly no huge hurry.

At the AdaCore level, we have a tiny local change in the x86 unwinder
where we decided to take a different choice than Mark did, and that
choice involves a heuristic that needs to determine  whether we're
in a DLL or not. So this patch is a bit more urgent for us, but we
have it installed in our local tree, so we're good to go.

As far as the FSF tree is concerned, apart from the fact that
the addresses are wrong when the users asks for that information,
the only other visible symptom is a minor error in the backtrace.
For frames that involve a DLL symbol, the name of the DLL where
this symbol comes from can be either printed wrong, or not printed
at all. I had never noticed this until I hit this issue with our
local patch when testing on Vista.

So patch withdrawn for now, and the puck moves to Daniel and Pedro.

-- 
Joel

Re: [win32] wrong solib from/to addresses

[Joel Brobecker]

> FYI, I was working on it again today and will again tomorrow.  Right
> now I plan to require expat for it to work; is that a problem for
> anyone?

It is a bit for us, because we do not build with cygwin. But if that
makes it a lot easier for you, then I'm sure we'll be able to figure
things out on our side.

-- 
Joel

Re: [win32] wrong solib from/to addresses

[Daniel Jacobowitz]

On Wed, Jun 13, 2007 at 02:37:44AM -0700, Joel Brobecker wrote:
> > FYI, I was working on it again today and will again tomorrow.  Right
> > now I plan to require expat for it to work; is that a problem for
> > anyone?
> 
> It is a bit for us, because we do not build with cygwin. But if that
> makes it a lot easier for you, then I'm sure we'll be able to figure
> things out on our side.

Shouldn't be - I build expat without Cygwin all the time...

-- 
Daniel Jacobowitz
CodeSourcery