Re: binutils-20000625 ld.exe --shared broken

Re: binutils-20000625 ld.exe --shared broken

[DJ Delorie]

[moved from cygwin to binutils - DJ]

No, I think that code is right.  Normally, when an archive is created,
the member BFDs are read from disk, and written to the archive's disk
file.  Thus, they must be in read mode to be read from the disk.  I
don't think this test is testing the writability of the archive
members, because it hasn't created them yet.  So, I think the right
thing to do is make sure that pe_dll_generate_implib switches all the
member bfds it creates from write mode (when it creates them) to read
mode (so it can store them in the archive).

> Date: Fri, 07 Jul 2000 02:12:23 -0400
> From: Charles Wilson <cwilson@ece.gatech.edu>
> X-Accept-Language: en
> CC: cygwin@sourceware.cygnus.com
> Content-Type: text/plain; charset=us-ascii
> 
> DJ Delorie wrote:
> > 
> > Try searching for BFD_IN_MEMORY.  It's notoriously buggy, and used to
> > build the import libraries.
> 
> I'm not sure, but I think I've *located* the problem, but not solved it:
> 
> Here's the stack trace:
> 
> #0  _bfd_write_archive_contents (arch=0xa17f970) at archive.c:1689
> #1  0xa04d830 in ?? ()
> #2  0x434c9d in bfd_close (abfd=0xa17f970) at opncls.c:413
> #3  0x42bce4 in pe_dll_generate_implib (def=0xa04c010,
> impfilename=0xa042190 "libz.dll.a") at pe-dll.c:1606
> #4  0x424f51 in gld_i386pe_finish () at ei386pe.c:1093
> #5  0x41f19c in ldemul_finish () at ldemul.c:91
> #6  0x4172f8 in lang_process () at ldlang.c:4122
> #7  0x41a334 in main (argc=32, argv=0xa041b90) at ./ldmain.c:346
> #8  0x61002385 in _size_of_stack_reserve__ ()
> #9  0x610027d5 in _size_of_stack_reserve__ ()
> #10 0x4771c7 in cygwin_crt0 (f=0x419ccc <main>) at
> /cygnus/netrel/src/cygwin-1.1.2/winsup/cygwin/libccrt0.cc:84
> 
> Basically, when pe_dll_generate_implib is ready to write the implib to
> disk, it calls bfd_close on the implib bfd pointer. This in turn calls 
> "BFD_SEND_FMT (abfd, _bfd_write_contents, (abfd)))" (e.g.
> _bfd_write_archive_contents). 
> 
> _bfd_write_archive_contents cycles through the bfd's of the members of
> the implib. One the very first one ('archive_head')
> 
> (bfd/archive.c)
>         1685      for (current = arch->archive_head; current; current =
> current->next)
>  	1686	    {
> 	1687	      if (bfd_write_p (current))
>  	1688		{
> -	1689		  bfd_set_error (bfd_error_invalid_operation);
> -	1690		  return false;
>  	1691            }
> 
> The clause in 1687 is triggered. Now, bfd_write_p(current) is #defined
> so that it expands to 
> 
> ((current)->direction == write_direction || (current)->direction ==
> both_direction))
> 
> Checking 'current->direction' in the watch window shows that it is,
> indeed, equal to 'write_direction'.
> 
> And so it dies.
> 
> However, it seems to me that the members of the output archive SHOULD be
> writeable. Basically, I can't tell if current->direction is set
> incorrectly, or if the test in line 1687 of archive.c is incorrect (or
> even necessary).
> 
> Any ideas?
> 
> --Chuck
> 

Re: binutils-20000625 ld.exe --shared broken

[Charles Wilson]

[Just a little context on this thread, for the binutils folks who didn't
catch its beginning on the cygwin mailing list]


> I reported this previously, but in a different thread 

[on the cygwin list]

> I figure this
> bug deserves its own thread. Somewhere between Mumit's custom 19990818
> version of binutils as distributed in

[the 'latest' cygwin distribution directory]

> and the more recent cygwin
> binutils (20000625) which is more-or-less direct from binutils-CVS, the
> --shared option broke. 
> 
> I'm getting the following error, using the ==stock== cygwin
> binutils-20000625 ld.exe:
> 
> ------
> ld --shared -Bdynamic -e __cygwin_dll_entry@12 -o libz.dll
> -L/usr/lib/gcc-lib/i686-pc-cygwin/2.95.2
> -L/usr/lib/gcc-lib/i686-pc-cygwin/2.95.2/../../../../i686-pc-cygwin/lib 
> --out-implib=libz.dll.a libz.def adler32.o compress.o crc32.o gzio.o
> uncompr.o deflate.o trees.o zutil.o inflate.o infblock.o inftrees.o
> infcodes.o infutil.o inffast.o -lgcc -lcygwin -luser32 -lkernel32
> -ladvapi32 -lshell32 -lgcc 
> 
> Creating library file: libz.dll.a
> bfd_close libz.dll.a: Invalid operation
> ------
> 
> This command works fine if I simply replace ld.exe with the one from the
> 19990818 cygwin tarball.
> 
> Can anybody recommend *where* in the code I should start looking for
> this problem? (Yeah, 'bfd_close' is pretty obvious, but I saw nothing
> enlightening there. The binutils code with its cross-platform-ness is
> really a tangled confusing mess...)

--------------

DJ responded:
> Try searching for BFD_IN_MEMORY.  It's notoriously buggy, and used to
> build the import libraries.

--------------

And that brings us up to the message that DJ crossposted here on
binutils.

--Chuck

Re: binutils-20000625 ld.exe --shared broken

[Nick Clifton]

Hi Guys,

: No, I think that code is right.  Normally, when an archive is created,
: the member BFDs are read from disk, and written to the archive's disk
: file.  Thus, they must be in read mode to be read from the disk.  I
: don't think this test is testing the writability of the archive
: members, because it hasn't created them yet.  So, I think the right
: thing to do is make sure that pe_dll_generate_implib switches all the
: member bfds it creates from write mode (when it creates them) to read
: mode (so it can store them in the archive).

I agree with DJ, although I must confess I had to read the explanation
and look through the code a couple of times before understanding what
is going on.

I belive that the [not writable] test in _bfd_write_archive_contents()
is there because up until this point none of the archive contents
should have been written out, so there should be no reason for the
archive memeber to have the writeable attribute.  (Adding a comment to
this effect in the code would be a good idea).

Cheers
	Nick

Re: binutils-20000625 ld.exe --shared broken

[DJ Delorie]

> I agree with DJ, although I must confess I had to read the explanation
> and look through the code a couple of times before understanding what
> is going on.

I wish I had understood it after just a *couple* of reads.  It took me
a lot longer than that to figure out why pe-dll was having such a hard
time of it, until Ian clued me in that BFD_IN_MEMORY was an old hack
and unlikely to still work right.

I'll work up a comment for that part of the code, though.

Re: binutils-20000625 ld.exe --shared broken

[Charles Wilson]

DJ Delorie wrote:
> 
> > I agree with DJ, although I must confess I had to read the explanation
> > and look through the code a couple of times before understanding what
> > is going on.
> 
> I wish I had understood it after just a *couple* of reads.  It took me
> a lot longer than that to figure out why pe-dll was having such a hard
> time of it, until Ian clued me in that BFD_IN_MEMORY was an old hack
> and unlikely to still work right.
> 
> I'll work up a comment for that part of the code, though.

Okay, further investigation shows:

'pe_dll_generate_implib' calls 'make_head', 'make_one' (many times), and
'make_tail' -- so each member bfd of the implib is processed by one of
these three routines. Each of these three routines call
'bfd_make_readable' on the bfd. However, that call fails.

So, 'bfd_make_readable' is broken.

(bfd/opncls.c: bfd_make_readable)
	613	  if (! BFD_SEND_FMT (abfd, _bfd_write_contents, (abfd)))
 	614	    return false;
 	615	

The clause expands to 'if (! coff_write_object_contents(abfd))' based on
all the #define's and stuff, so:

(bfd/coffcode.h: coff_write_object_contents)
-	3250	  if (bfd_seek (abfd, scn_base, SEEK_SET) != 0)
-	3251	    return false;

And this clause is tripped, so coff_write_... returns false, and
bfd_make_readable returns false, and the bfd is still set to direction =
write_direction, etc, etc.

Just before the return, the values of some of the variables are:

amount		169187232
lineno_base	169187232
reloc_base	198
scn_base	20
sym_base	198
reloc_size	30
lnno_size	168176232

And that's as far as I've been able to get.

--Chuck

Re: binutils-20000625 ld.exe --shared broken

[DJ Delorie]

> (bfd/coffcode.h: coff_write_object_contents)
> -	3250	  if (bfd_seek (abfd, scn_base, SEEK_SET) != 0)
> -	3251	    return false;

While you're in there, could you check one thing for me?  In bfd_seek,
is it actually using the BFD_IN_MEMORY code, but failing because we're
trying to seek past the end of the data block?  If so, it's probably
trying to "seek beyond EOF" to write out one of the structures, which
you can probably fix pretty quickly, by just growing the data block
(see the code to do that in bfd_write).  It probably should check for
the bfd being writable before allowing that, too.

Re: binutils-20000625 ld.exe --shared broken

[Charles Wilson]

DJ Delorie wrote:
> 
> > (bfd/coffcode.h: coff_write_object_contents)
> > -     3250      if (bfd_seek (abfd, scn_base, SEEK_SET) != 0)
> > -     3251        return false;
> 
> While you're in there, could you check one thing for me?  In bfd_seek,
> is it actually using the BFD_IN_MEMORY code, but failing because we're
> trying to seek past the end of the data block?  If so, it's probably
> trying to "seek beyond EOF" to write out one of the structures, which
> you can probably fix pretty quickly, by just growing the data block
> (see the code to do that in bfd_write).  It probably should check for
> the bfd being writable before allowing that, too.

Yup, that's what the problem was -- it was executing the BFD_IN_MEMORY
code, and the bim had no allocated memory at all. The following seems to
work for me, but I'm unsure of some of the finer details. 

1) Are the bfd_set_error's correct?

2) Is this *really* the right thing to do? 

Fri Jul 7 2000 23:52:00 Charles Wilson <cwilson@ece.gatech.edu>
 
       * bfd/libbfd.c: fix 'seek beyond EOF' error when
	 writing out a structure that is BFD_IN_MEMORY.

-----------------
--- binutils-20000625-orig/bfd/libbfd.c Sat Jun 24 21:52:33 2000
+++ binutils-20000625/bfd/libbfd.c      Sat Jul  8 00:01:47 2000
@@ -691,11 +691,31 @@
       
       if ((bfd_size_type) abfd->where > bim->size)
        {
-         abfd->where = bim->size;
-         bfd_set_error (bfd_error_file_truncated);
-         return -1;
-       }
-      
+         if ((abfd->direction == write_direction) || 
+             (abfd->direction == both_direction))
+           {
+             long newsize, oldsize = (bim->size + 127) & ~127;
+             bim->size = abfd->where;
+             /* Round up to cut down on memory fragmentation */
+             newsize = (bim->size + 127) & ~127;
+             if (newsize > oldsize)
+               {
+                 bim->buffer = bfd_realloc (bim->buffer, newsize);
+                 if (bim->buffer == 0)
+                   {
+                     bim->size = 0;
+                     bfd_set_error (bfd_error_no_memory);
+                     return -1;
+                   }
+               }
+           }
+         else
+           {
+             abfd->where = bim->size;
+             bfd_set_error (bfd_error_file_truncated);
+             return -1;
+           }   
+       }      
       return 0;
     }
 
-------------

begin 664 binutils.patch.gz
M'XL("`:H9CD``V)I;G5T:6QS+G!A=&-H`(U3RV[;,!`\2U\QI\*,S%J2\VAM
MR/"YQ_8#!#THAP!-!C0%(TW<;R\I2K(<.VAY69([LSN[7%)*47+9&BX.-(WM
M>DP?J-)\MRB;>B%X:<W7*OA5&/QH)=)[I,GJ(5TMEW#P,(JBZP"WN`+XACA>
MQ<GJ_LESMUO0Q^_)/$D0.;M,L-V&\&NPO,%L9@/E!_Z;Y>;UA1$4]DPWQV>F
M&38V_9YNG)>$"-Y"&N`"D)T!Z\[9!6,F9UHKC2YVM\T;+FP&W<JJ,*PF'JV9
M:;4$3=SQ%-)>713TTGRJFFM6&:XDL@Q'S0W+QRN"]W=/<.L6H53F>8(G/?AM
M)`DE=Y#LZ(J80XG:;6QEL[$T1$C2)X(O^&/M>F2>`=FT*V?`X@X_52MKM"\P
M"E5K4*NCA%6V9WNE7]'H8K=GTA2=W+O%2.T%_9<.UZL!OQDJ(*/;%QOT>LNV
M:9AV+V??1K-""%7U.;QK/N0F:T]SX2^H&6+B76/HC^V(UY/[SV9"JMRW@4S0
MDYGH[TZ32H:]MTP<V-5[?CJ?T;_T7,UH=$M3E]Z/Z>GR._4P6WNO%>%?MYQ@
%X00$``"3
`
end

Re: binutils-20000625 ld.exe --shared broken

[DJ Delorie]

> Fri Jul 7 2000 23:52:00 Charles Wilson <cwilson@ece.gatech.edu>
>  
>        * bfd/libbfd.c: fix 'seek beyond EOF' error when
> 	 writing out a structure that is BFD_IN_MEMORY.

Applied.  Thanks!