[PING] Windows PECOFF weak symbol fixes for *-mingw32 and *-cygwin

[Aaron W. LaFramboise]

Thanks for taking the time to complete a detailed review.  I've
addressed your comments below, and attached a revised patch.

Nick Clifton wrote:

>    * The arm-wince-pe, arm-epoc-pe and mcore-pe targets do not build 
> after applying this patch.   This is because USE_UNIQUE is only defined 
> for the i386 port in obj-coff.h but the code in weak_uniqify() uses 
> any_external_name regardless of whether USE_UNIQUE is defined or not.

Since this code really should apply to all PE targets, I moved the
definiton so its defined for all targets.

>    * If this problem is fixed (by defining USE_UNIQUE for all TE_PE 
> targets) then there is still a problem in that new linker testsuite 
> failures are introduced for those targets.

I think these failures come from incorrect weak symbol relocations and
similar things in the target CPU code.  This has probably always been
broken, independent of my changes.  As I don't really know anything
about non-i386 targets, I think the best thing to do here is to XFAIL
these tests, since they really should work, and could be fixed pretty
easily.  I've filed a bug about this also, PR gas/517.

I've also changed the testsuites to report UNSUPPORTED rather than
simply return for targets that supposedly don't support weak symbols, as
quite a few of them probably should, and it might be helpful to
maintainers to have that reminder in the logs.

>    * The new code does not follow the GNU Coding conventions.  In 
> particular functions should be declared with their return type on one 
> line followed by the function name on the next line.

I fixed the return types.

>    * The patch to bfd/coff-i386.c creates a local variable called 
> 'howto' but does not use it.

I removed the nascent howto variable.

>    * The change to the documentation in gas/doc/as.texinfo removes the 
> fact that supporting non-alias weak symbols for PE is a GNU extension.

I beleive this is correct.

It is no longer an extension.  That is, when a gas input file specifies
an object file that is not an alias, the output object files are
conforming and compatible PECOFF files, where before this particular
patch, they were not.  In other words, with this patch, it is no longer
possible to cause gas to emit non-conforming weak symbols.

I decided that the small discussion I had previously included was now
irrelevent, partly because of this, and partly because the new syntax is
not as expressive as the old syntax, as it is not able to create
non-searched weak symbols.  I don't think this is a problem, since
bfd/ld doesn't support them, and Microsoft's assembler has actually
never supported them (only Intel's assembler does).  No existing product
documentation describes them.  In addition, theres some evidence that
they haven't been used in the Microsoft world in at least a decade.

(Theres also a third sort of PECOFF weak symbol that I do not beleive
any assemblers in existance support, so I don't think lack of a complete
implementation is evidence of a problem.)

With this patch, .weak should "just work" and do the right thing, and no
additional considerations should be necessary.

>    * The change to ld-undefined/weak-undef.exp ought to make use of the 
> newly defined is_pecoff_format proc.

I slightly reorganized the target tests in this file to fix this.


I tested this revised patch on all of native i686-pc-mingw32, cross
arm-wince-pe, cross arm-epoc-pe, and cross mcore-unknown-pe.  The latter
three have two new XFAILs, as expected, and no new FAILs.


Aaron W. LaFramboise

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: binutils-head-20041105-weak.patch
URL: <https://sourceware.org/pipermail/binutils/attachments/20041105/ab7d895e/attachment.ksh>