On Tue, Sep 30, 2008 at 12:47 PM, Nick Clifton <[EMAIL PROTECTED]> wrote:
> Do you have an FSF binutils copyright assignment in place ?
No. As you pointed out though, the patch was small, more like a
targeted bug report really.
> Anyway I have checked over your patch - it is good although there are a few
> problems with it:
>
> * It calls bfd_coff_classify_symbol() which will issue an error
> message if it is asked to process a local symbol. This
> produces lots of new failure results in the gas testsuite.
>
> * Not all coff backends define the classify_symbol() function.
> Well OK, then is only one backend (64-bit RS6000) which does
> this, but you still need a test to make sure that the
> function exists before calling it.
>
> * Not actually your problem, but the tic4x port of gas does
> not set the BSF_GLOBAL flag or unset the BSF_LOCAL flag when
> it is converting a local symbol into a global one. (Ie this
> is a bug in the tic4x port).
>
> * Finally you have not included a ChangeLog entry with your patch.
>
> Anyway I have taken the liberty of addressing these issues and I am going to
> apply the attached enhanced version of your patch along with the changelog
> entries below.
I also found that my original patch screwed up gas as run by gcc. Your
version of the patch seems to not have this issue, though. I have
attached the patch I had been using instead. I tried to make it more
conservative in it's modifications. You decide if it's an improvement
or not. =)
> PS. Ideally a patch like this ought to include a new test in the binutils
> test suite. Maybe you would like to write one ?
I'm already happy that my compiles work now.
--- coffgen.c.orig 2008-09-30 13:52:37.485748195 +0200
+++ coffgen.c 2008-09-30 14:14:54.080753833 +0200
@@ -1133,6 +1133,28 @@
}
else
{
+ /* If the symbol class has been changed (eg objcopy/ld script/etc)
+ * we cannot retain the sclass from the original symbol. Only
+ * adjust symbol visibility for simply defined weak, local, and
+ * global symbols.
+ */
+ switch (c_symbol->native->u.syment.n_sclass) {
+#ifdef COFF_WITH_PE
+ case C_NT_WEAK:
+#endif
+ case C_WEAKEXT:
+ case C_EXT:
+ case C_STAT:
+ /* If it's defined and not common, change the visibility */
+ if (c_symbol->native->u.syment.n_scnum != 0 &&
+ symbol->flags & (BSF_WEAK|BSF_LOCAL|BSF_GLOBAL))
+ c_symbol->native->u.syment.n_sclass =
+ (symbol->flags & BSF_WEAK) && obj_pe (abfd) ? C_NT_WEAK :
+ (symbol->flags & BSF_WEAK) ? C_WEAKEXT :
+ (symbol->flags & BSF_LOCAL) ? C_STAT :
+ C_EXT;
+ }
+
if (!coff_write_native_symbol (abfd, c_symbol, &written,
&string_size, &debug_string_section,
&debug_string_size))
_______________________________________________
bug-binutils mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/bug-binutils
