This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Hi Terpstra,

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?

Yes it is.


And could someone
familiar with the binutils coffgen please check it over?

Do you have an FSF binutils copyright assignment in place ? For most patches we do require such an assignment, although this one is small enough that it can be considered to be "obvious".


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.

Cheers
  Nick

PS. Ideally a patch like this ought to include a new test in the binutils test suite. Maybe you would like to write one ?

PPS. Also it helps if you can file a bug report for a problem like this, so that we can have a place to collect all the emails on the subject and a number to associate with the problem.

bfd/ChangeLog
2008-09-30  Wesley W. Terpstra  <wesley@terpstra.ca>
	    Nick Clifton  <nickc@redhat.com>

	* coffgen.c (coff_write_symbols): Check to see if a symbol's flags
	do not match it class and if necessary update the class.
	(null_error_handler): New function.  Suppresses the generation of
	bfd error messages.
	* coff64-rs6000.c (bfd_xcoff_backend_data): Update comment.

gas/ChangeLog
2008-09-30  Wesley W. Terpstra  <wesley@terpstra.ca>
	    Nick Clifton  <nickc@redhat.com>

	* config/tc-tic4x.c (tic4x_globl): Call S_SET_EXTERNAL as well as
	S_SET_STORAGE_CLASS.


Index: bfd/coff64-rs6000.c
===================================================================
RCS file: /cvs/src/src/bfd/coff64-rs6000.c,v
retrieving revision 1.71
diff -c -3 -p -r1.71 coff64-rs6000.c
*** bfd/coff64-rs6000.c	18 Jul 2008 11:30:22 -0000	1.71
--- bfd/coff64-rs6000.c	30 Sep 2008 10:07:17 -0000
*************** static const struct xcoff_backend_data_r
*** 2580,2586 ****
        coff_print_aux,
        dummy_reloc16_extra_cases,
        dummy_reloc16_estimate,
!       NULL,			/* bfd_coff_sym_is_global */
        coff_compute_section_file_positions,
        NULL,			/* _bfd_coff_start_final_link */
        xcoff64_ppc_relocate_section,
--- 2580,2586 ----
        coff_print_aux,
        dummy_reloc16_extra_cases,
        dummy_reloc16_estimate,
!       NULL,			/* bfd_coff_symbol_classification */
        coff_compute_section_file_positions,
        NULL,			/* _bfd_coff_start_final_link */
        xcoff64_ppc_relocate_section,
Index: bfd/coffgen.c
===================================================================
RCS file: /cvs/src/src/bfd/coffgen.c,v
retrieving revision 1.67
diff -c -3 -p -r1.67 coffgen.c
*** bfd/coffgen.c	14 Aug 2008 02:38:22 -0000	1.67
--- bfd/coffgen.c	30 Sep 2008 10:07:17 -0000
*************** coff_write_native_symbol (bfd *abfd,
*** 1082,1087 ****
--- 1082,1092 ----
  			    debug_string_size_p);
  }
  
+ static void
+ null_error_handler (const char * fmt ATTRIBUTE_UNUSED, ...)
+ {
+ }
+ 
  /* Write out the COFF symbols.  */
  
  bfd_boolean
*************** coff_write_symbols (bfd *abfd)
*** 1138,1143 ****
--- 1143,1184 ----
  	}
        else
  	{
+ 	  if (coff_backend_info (abfd)->_bfd_coff_classify_symbol != NULL)
+ 	    {
+ 	      bfd_error_handler_type current_error_handler;
+ 	      enum coff_symbol_classification class;
+ 	      unsigned char *n_sclass;
+ 
+ 	      /* Suppress error reporting by bfd_coff_classify_symbol.
+ 		 Error messages can be generated when we are processing a local
+ 		 symbol which has no associated section and we do not have to
+ 		 worry about this, all we need to know is that it is local.  */
+ 	      current_error_handler = bfd_set_error_handler (null_error_handler);
+ 	      class = bfd_coff_classify_symbol (abfd, &c_symbol->native->u.syment);
+ 	      (void) bfd_set_error_handler (current_error_handler);
+ 	  
+ 	      n_sclass = &c_symbol->native->u.syment.n_sclass;
+ 
+ 	      /* 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.  */
+ 
+ 	      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))
Index: gas/config/tc-tic4x.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-tic4x.c,v
retrieving revision 1.26
diff -c -3 -p -r1.26 tc-tic4x.c
*** gas/config/tc-tic4x.c	14 Aug 2008 14:54:40 -0000	1.26
--- gas/config/tc-tic4x.c	30 Sep 2008 10:07:19 -0000
*************** tic4x_globl (int ignore ATTRIBUTE_UNUSED
*** 821,826 ****
--- 821,827 ----
        *input_line_pointer = c;
        SKIP_WHITESPACE ();
        S_SET_STORAGE_CLASS (symbolP, C_EXT);
+       S_SET_EXTERNAL (symbolP);
        if (c == ',')
  	{
  	  input_line_pointer++;

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]