RE: bfd/peigen.c problems and fix

[Donn Terry <donnte at microsoft dot com>]

In the process of merging my patch with the new peigen.c (1.9)
I discovered a couple of things in that patch, clearly related
to this.

1) I believe that the DataSize entry for the ImportTable is
correct: it's the size of the Import Directory Table (the n*0x14 byte
header), not including the corresponding Import Lookup Table and
Import Address Table.  Thus a (smallish) value of the form n*0x14
appears correct (just not intuitive).

2) The for loop that dumps the ILT/IAT is checking for datasize, when
it just should be exiting when finding the terminating zero.  Delete
the test for datasize in the for statement itself (replacing it with
nothing) and
it should work just right (it does for me).  (That's why taking datasize
from the .idata section size worked better than taking it from the
DataDirectory, and why now, when taking it from the DataDirectory, it
truncates all .import sections.)

3) A silent change (maybe intended, maybe not) was made.  The size of the
*EXport* table is printed twice, as below.
  Number in:
        Export Address Table            000002a9
        [Name Pointer/Ordinal] Table    000002a9

Prior to this, it was printed without leading zeros (after all, it isn't an
address), with the EAT in hex and the NP/OT in decimal.  Since this is a
count (and the actual dump counts in decimal) decimal may be a better
choice,
but I don't much care.  (In principle the two numbers can be different.)

Donn

> -----Original Message-----
> From: Szabolcs Szakacsits [mailto:szaka@F-Secure.com]
> Sent: Tuesday, May 09, 2000 5:49 PM
> To: Donn Terry
> Cc: Alan Modra; binutils@sourceware.cygnus.com; Mumit Khan
> Subject: RE: bfd/peigen.c problems and fix
> 
> 
> 
> 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
> > > 
> > 
> 
>