Re: objcopy --globalize-symbols on MinGW [PATCH]

["Wesley W. Terpstra" <wesley at terpstra dot ca>]

On Fri, Sep 26, 2008 at 1:55 PM, Wesley W. Terpstra <wesley@terpstra.ca> wrote:
I've traced binutils 2.18.50-cvs20080109 (debian mingw package)
> and the line 1074 (sym->flags &= ~ (BSF_GLOBAL | BSF_WEAK); does get
> run but the result object still has BFS_GLOBAL set. So I suspect this
> is a binutils/bfd bug.

I've traced the bug down (it was a bug). The problem is that if a
symbol being written has an existing 'native' symbol (which is always
the case for objcopy) the coffgen.c ignores any BFS_* flags set by the
utility. This prevents both ld and objcopy (and probably any other
binutil which manipulates symbol visibility) from changing global
symbols to local, but also weakening symbols or globalizing them. If
the symbol was newly created, it has no native information and so the
sclass gets set correctly.

The attached patch compares the desired BFS_{GLOBAL,LOCAL,WEAK} state
against the classification of the native symbol. If these differ, it
overwrites the sclass in the same way a new symbol would have been
created.

Is this the right place to send a binutils patch? And could someone
familiar with the binutils coffgen please check it over?

--- binutils-2.18.50-20080109/bfd/coffgen.c.orig	2008-09-26 18:30:38.584084653 +0200
+++ binutils-2.18.50-20080109/bfd/coffgen.c	2008-09-26 18:30:40.084188976 +0200
@@ -1090,8 +1090,10 @@
   bfd_size_type string_size;
   asection *debug_string_section;
   bfd_size_type debug_string_size;
+  enum coff_symbol_classification class;
   unsigned int i;
   unsigned int limit = bfd_get_symcount (abfd);
+  unsigned char *n_sclass;
   bfd_vma written = 0;
   asymbol **p;
 
@@ -1138,6 +1140,27 @@
 	}
       else
 	{
+          /* If the symbol class has been changed (eg objcopy/ld script/etc)
+           * we cannot retain the existing sclass from the original symbol.
+           * Weak symbols only have one valid sclass, so just set it always.
+           * If it is not local class and should be, set it C_STAT.
+           * If it is global and not classified as global, or if it is
+           * weak (which is also classified as global), set it C_EXT.
+           */
+          class = bfd_coff_classify_symbol (abfd, &c_symbol->native->u.syment);
+          n_sclass = &c_symbol->native->u.syment.n_sclass;
+          if (symbol->flags & BSF_WEAK)
+            *n_sclass = obj_pe (abfd) ? C_NT_WEAK : C_WEAKEXT;
+          else if (symbol->flags & BSF_LOCAL && class != COFF_SYMBOL_LOCAL)
+            *n_sclass = C_STAT;
+          else if (symbol->flags & BSF_GLOBAL && 
+                   (class != COFF_SYMBOL_GLOBAL || 
+#ifdef COFF_WITH_PE
+                    *n_sclass == C_NT_WEAK ||
+#endif
+                    *n_sclass == C_WEAKEXT))
+            c_symbol->native->u.syment.n_sclass = C_EXT;
+
 	  if (!coff_write_native_symbol (abfd, c_symbol, &written,
 					 &string_size, &debug_string_section,
 					 &debug_string_size))