Re: Patches for Borland TDS debugging info

[Nick Clifton <nickc at redhat dot com>]

Hi Troy,

: The attached patch and additional files add support to BFD for
: recognising Borland TDS debugging information. I'll be sticking the
: copyright assignment and employer disclaimer for RMS in the mail
: tomorrow

Great - please let us know when these have been processed so that we
can go ahead with integrating this patch into the sources.

: (I have done these previously in early '93 for GDB and GAS,
: hence also for Binutils, but as I understand it I need to do another
: one now because of the new files).

Correct, although if you sign a 'present and future' copyright
assignment you should not need to do this again in the future.

: 1. I have added a new data element to the BFD structure - 
: "opened_for_symbols". TDS is a debugging only format which can
: either be in the executable, or in a separate file. It should *only*
: be recognised when a file is being opened to be read for debugging
: symbols. A reader is expected to set this element to "true"
: immediately after opening the file and before attempting to get BFD
: to recognise the type of file. 

So where (in your patch) is this ever set to true ?

: 2. In peicode.h, there was a check that assumed that the PE (COFF) header 
: in a Win32 executable was exactly 0x80 bytes from the DOS header (AKA the 
: start of the file). Unfortunately, this assumption is incorrect. To fix it 
: to do the "right" thing would have entailed huge changes due to a structure 
: that had been passed through multiple calls assuming this, and some of 
: these COFF generic calls would have had to know more about the PE format 
: than seemed appropriate, so I commented out this check altogether. This 
: shouldn't hurt anything since by the time we get to that check there have 
: been other checks which should only match a PE file anyway.

This is not a problem.  In fact in the current sources that code is
already commented out.

: 3. I have made no attempt to make TDS work in a heterogeneous remote 
: debugging environment.

This might actually be the biggest problem, since everything else in the
BFD has been designed to work in heterogenous environments.  I would
strongly urge you to consider abandoning the ALIGNED1 macro and
instead using external and internal versions of the structures in the
same way that the ELF headers do.


I have a question on the patch itself:

  diff -ru gdb-20000610/bfd/targets.c gdb-new/bfd/targets.c
  @@ -754,6 +756,9 @@
   #endif
   #if 0
   	&bfd_elf64_sparc_vec,
  +#endif
  +#if 0
  +	&borland_tds_vec,
   #endif

Why is this entry "#if 0"ed out ?  The sparc entry is suppressed
because it is not working yet, but I assume that (once your patch is
applied) that the borland tds support will actually work.

Oh - one other thing - please could you also add to your patch an
update of binutils/NEWS saying that support for the TDS files has been
added. 

Cheers
	Nick