RE: bfd/peigen.c problems and fix

[Szabolcs Szakacsits <szaka at F-Secure dot com>]

On Tue, 9 May 2000, Donn Terry wrote:

> Try kernel32.dll (for an import table that's not in .idata)
> and ntdll.dll or kernel32.dll (for import/export tables that are
> not in .idata and .edata).  This is true both on win2K and
> NT4 (can't say whether there are similar examples on win98).

Here is what your code gives:

[...]
There is an import table, but the section containing it could not be found
There is an export table, but the section containing it could not be found
[...]

The current objdump -p finds it but doesn't list all of the Member Names
in idata. Interestingly objdump from MinGW lists them but seg fault's on
other dll's and exe's. Below is a fix against the CVS version to list all
of them. Now the code doesn't rely on DataDirectory[].Size's since it's
bogus [maybe the infamous buggy MS calculator code is still floating
around at MS] and anyway the end of items are signed with 0's.

I can see problem if there isn't optional header. In this case only idata
should be examined but objdump always relied on optional header and I
don't know there was at all a tool to create such files [MSVC 1?].

Any other testcase?

	Szaka

--- peigen.c.orig	Wed May 10 01:14:28 2000
+++ peigen.c	Wed May 10 01:52:38 2000
@@ -1013,18 +1013,20 @@
 
   bfd_vma addr;
 
-  addr = extra->DataDirectory[1].VirtualAddress;
-  datasize = extra->DataDirectory[1].Size;
+  if ((addr = extra->DataDirectory[1].VirtualAddress) == 0 
+              && extra->DataDirectory[1].Size == 0)
+	return true;  /* no import table */
 
-  if (addr == 0 || datasize == 0)
+  addr += extra->ImageBase;
+
+  if (addr == 0)
     return true;
 
-  addr += extra->ImageBase;
 
   for (section = abfd->sections; section != NULL; section = section->next)
     {
       if (addr >= section->vma
-	  && addr < section->vma + bfd_section_size(abfd,section))
+	  && addr < section->vma + (datasize = bfd_section_size(abfd,section)))
 	break;
     }
 
@@ -1039,6 +1041,7 @@
 	   section->name, (unsigned long) addr);
 
   dataoff = addr - section->vma;
+  datasize -= dataoff;
 
 #ifdef POWERPC_LE_PE
   if (rel_section != 0 && bfd_section_size (abfd, rel_section) != 0)
@@ -1279,17 +1282,19 @@
 
   bfd_vma addr;
 
-  addr = extra->DataDirectory[0].VirtualAddress;
-  datasize = extra->DataDirectory[0].Size;
+  if ((addr = extra->DataDirectory[0].VirtualAddress) == 0 
+               && extra->DataDirectory[0].Size == 0)
+	return true;  /* no export table */
 
-  if (addr == 0 || datasize == 0)
+  addr += extra->ImageBase;
+
+  if (addr == 0)
     return true;
 
-  addr += extra->ImageBase;
   for (section = abfd->sections; section != NULL; section = section->next)
     {
       if (addr >= section->vma
-	  && addr < section->vma + bfd_section_size (abfd, section))
+	  && addr < section->vma + (datasize = bfd_section_size (abfd, section)))
 	break;
     }
 
@@ -1304,6 +1309,7 @@
 	   section->name, (unsigned long) addr);
 
   dataoff = addr - section->vma;
+  datasize -= dataoff;
 
   data = (bfd_byte *) bfd_malloc (datasize);
   if (data == NULL)

 
> I'm not arguing that there currently is a problem with ImageBase
> (since not all the patches that needed to be applied DID get applied,
> that happened; I'm still working on getting that fixed, but it's far
> more than just at technical problem).  However, the code that finds
> the import and export tables when they're not in .idata and .edata is
> still applicable.  Please restore it so that it can find them.  There
> will be patches forthcoming that fix all this (and more), if I can
> ever get past all the problems in my way.
> 
> Speaking only for myself.
> 
> Donn
> 
> > -----Original Message-----
> > From: Szabolcs Szakacsits [mailto:szaka@F-Secure.com]
> > Sent: Tuesday, May 09, 2000 9:01 AM
> > To: Donn Terry
> > Cc: 'Alan Modra'; binutils@sourceware.cygnus.com
> > Subject: RE: bfd/peigen.c problems and fix
> > 
> > 
> > 
> > On Mon, 8 May 2000, Donn Terry wrote:
> > 
> > > I'm still not to the point of being able to dig thru all
> > > of what's going on with this, but the patch made to peigen.c
> > > (1.9) doesn't seem right to me.
> > 
> > Maybe it doesn't *seem* right to you but now it *works* for 
> > me for every
> > dll's and exe's that I tested.
> >  
> > > Primarily, it removes a lot of code that doesn't have anything
> > > to do with the problem of Image Base.  In fact it removes code
> > > that allows it to dump DLLs that it otherwise could not dump.
> > 
> > The above statment correctly, 
> > In fact it removes code that *disallows* it to dump DLLs that 
> > it otherwise
> > *could* dump.
> > 
> > Before it was pretty annoying to watch the "There is an 
> > import table, but
> > the section containing it could not be found" messages, what 
> > I remember
> > you added but I don't have the picture what the reason was to 
> > break PEI so
> > badly.
> > 
> > > The change to insert or delete ImageBase (until the base problem
> > > gets fixed) is reasonable, but the other stuff should be left
> > > in place.
> > 
> > Yes, if you want a non-functioning objdump in the future as 
> > well. Please
> > send me testcases that don't work since I can't found [sure I don't
> > have sample from all of the PEI variants]. Thanks.
> > 
> > 	Szaka
> > 
>