[RFA:] Skip symbol after warning symbol in gas/write.c:write_object_file

[RFA:] Skip symbol after warning symbol in gas/write.c:write_object_file

[Hans-Peter Nilsson]

Finally, we get to the "real" warning bug.  As briefly mentioned
earlier, the warning construct caused the warned-about symbol to
disappear, instead causing undef'd symbol errors.  (Repeat by
e.g. building a cris-axis-aout toolchain and compile and link
"hello, world".)

Looking with objdump on the assembled file showed that the
symbol after the warning symbol had been pruned by
tc_frob_symbol.  Now you might think that fixing the bug should
be done in tc_frob_symbol, but IMO that'd be wrong; the symbol
following the warning symbol *must not* be subject to target
pruning.  And besides, it's neither defined, marked as a debug
symbol nor as used in a reloc -- not that any of those marks
would be really correct anyway.  So instead, I suggest we
recognize the warning symbol and skip over the next symbol, and
handle it gracefully if there is none; then it's just the users
fault and the input will be reflected in the output (as the same
brokenness).  Admittedly, the whole handling of symbol-must-
follow-warning-symbol is brittle in current code, but cleaning
that up is best done by removing it altogether: again, a.out
isn't something I consider worthwhile to providing polished
infrastructure for (only fixing bugs for it).  Perhaps a
test-case should be among the assembly tests, but that'd mean
checking for a.out object format being generated and grepping
out the specific stabs construct from objdump, when we can
instead check that it works as a whole when it reaches the
linker.  (Both with "whole" and "broken" warning construct.)
The test-cases assume that the consistent-prepend-of-"warning: "-
patch is applied.

I'm not sure whether the symbol following the warning stab
should be used other than picking the name, but I guess it
actually should, so there's an xfailed test-case for that (or
two if you count ELF which is xfailed for not supporting the
construct in its stabs support).

Ok to commit?

gas:
	* write.c (write_object_file): Recognize warning-symbol construct
	and skip object- and target- handling for the second symbol.

ld/testsuite:
	* ld-cris/globsymw2.s, ld-cris/globsymw3.s: New files.
	* ld-cris/warn3.d, ld-cris/warn4.d, ld-cris/warn5.d,
	ld-cris/warn6.d: New tests.

--- write.c.original	Thu Feb  3 18:56:02 2005
+++ write.c	Fri Feb  4 03:31:45 2005
@@ -1882,12 +1882,22 @@ write_object_file (void)
   if (symbol_rootP)
     {
       symbolS *symp;
+      bfd_boolean skip_next_symbol = FALSE;
 
       for (symp = symbol_rootP; symp; symp = symbol_next (symp))
 	{
 	  int punt = 0;
 	  const char *name;
 
+	  if (skip_next_symbol)
+	    {
+	      /* Don't do anything besides moving the value of the
+		 symbol from the GAS value-field to the BFD value-field.  */
+	      symbol_get_bfdsym (symp)->value = S_GET_VALUE (symp);
+	      skip_next_symbol = FALSE;
+	      continue;
+	    }
+
 	  if (symbol_mri_common_p (symp))
 	    {
 	      if (S_IS_EXTERNAL (symp))
@@ -1972,6 +1982,12 @@ write_object_file (void)
 	  /* Set the value into the BFD symbol.  Up til now the value
 	     has only been kept in the gas symbolS struct.  */
 	  symbol_get_bfdsym (symp)->value = S_GET_VALUE (symp);
+
+	  /* A warning construct is a warning symbol followed by the
+	     symbol warned about.  Don't let anything object-format or
+	     target-specific muck with it; it's ready for output.  */
+	  if (symbol_get_bfdsym (symp)->flags & BSF_WARNING)
+	    skip_next_symbol = TRUE;
 	}
     }
 

--- /dev/null	Tue Oct 29 15:57:07 2002
+++ globsymw2.s	Thu Feb  3 15:15:43 2005
@@ -0,0 +1,16 @@
+	.text
+	.stabn	162,0,0,0
+;# A bit like globsymw1.s but containing a valid, working, stabs
+;# symbol warning construct.
+	.stabs "isatty is not implemented and will always fail",30,0,0,0
+	.stabs "globsym1",1,0,0,0
+	.global globsym1
+	.type	globsym1, @function
+globsym1:
+	.stabd	46,0,0
+	.stabn	68,0,16,LM0-globsym1
+LM0:
+	.long 0
+	.size	globsym1, .-globsym1
+	.stabs	"",100,0,0,Letext0
+Letext0:
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ globsymw3.s	Fri Feb  4 04:03:26 2005
@@ -0,0 +1,17 @@
+	.text
+	.stabn	162,0,0,0
+;# A mix of globsymw1.s and globsym2.s: this one also missing
+;# the special warning symbol, but the "real" symbol should be
+;# in its place and work.  Unfortunately, binutils skips it
+;# regardless of type, so tests using this file should be xfailed.
+	.stabs "isatty is not implemented and will always fail",30,0,0,0
+	.global globsym1
+	.type	globsym1, @function
+globsym1:
+	.stabd	46,0,0
+	.stabn	68,0,16,LM0-globsym1
+LM0:
+	.long 0
+	.size	globsym1, .-globsym1
+	.stabs	"",100,0,0,Letext0
+Letext0:
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ warn5.d	Fri Feb  4 04:05:32 2005
@@ -0,0 +1,12 @@
+#source: start1.s
+#source: globsym1ref.s
+#source: globsymw3.s
+#target: cris-*-*elf* cris-*-*aout*
+#as: --em=crisaout
+#ld: -mcrisaout
+#warning: warning: isatty is not implemented and will always fail$
+#xfail: *-*-*
+#objdump: -p
+# See globsymw3.s for reason for xfail.
+.*:     file format a\.out-cris
+#pass
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ warn3.d	Fri Feb  4 18:02:05 2005
@@ -0,0 +1,10 @@
+#source: start1.s
+#source: globsym1ref.s
+#source: globsymw2.s
+#target: cris-*-*elf* cris-*-*aout*
+#as: --em=crisaout
+#ld: -mcrisaout
+#warning: warning: isatty is not implemented and will always fail$
+#objdump: -p
+.*:     file format a\.out-cris
+#pass
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ warn6.d	Fri Feb  4 18:04:13 2005
@@ -0,0 +1,12 @@
+#source: start1.s
+#source: globsym1ref.s
+#source: globsymw3.s
+#target: cris-*-*elf* cris-*-*aout*
+#as: --em=criself
+#ld: -mcriself
+#warning: warning: isatty is not implemented and will always fail$
+#xfail: *-*-*
+#objdump: -p
+# See globsymw3.s for reason for xfail.
+.*:     file format elf32.*-cris
+#pass
--- /dev/null	Tue Oct 29 15:57:07 2002
+++ warn4.d	Fri Feb  4 18:38:32 2005
@@ -0,0 +1,13 @@
+#source: start1.s
+#source: globsym1ref.s
+#source: globsymw2.s
+#target: cris-*-*elf* cris-*-*aout*
+#as: --em=criself
+#ld: -mcriself
+#warning: warning: isatty is not implemented and will always fail$
+#objdump: -p
+#xfail: *-*-*
+# The test is xfailed because ELF stabs doesn't handle the stabs
+# warning construct.
+.*:     file format elf32.*-cris
+#pass

brgds, H-P

Re: [RFA:] Skip symbol after warning symbol in gas/write.c:write_object_file

[Ian Lance Taylor]

Hans-Peter Nilsson <hans-peter.nilsson@axis.com> writes:

> I'm not sure whether the symbol following the warning stab
> should be used other than picking the name, but I guess it
> actually should, so there's an xfailed test-case for that (or
> two if you count ELF which is xfailed for not supporting the
> construct in its stabs support).

I don't think we should use the symbol following the a.out warning
stab for anything.  The warning stab should be followed by an
undefined symbol, which is what we should issue the warning about.
That undefined symbol should not be otherwise used for anything.  Note
that in typical uses, the symbol will be defined somewhere else in the
object file in any case.

Let me know if I am missing something.

> gas:
> 	* write.c (write_object_file): Recognize warning-symbol construct
> 	and skip object- and target- handling for the second symbol.
> 
> ld/testsuite:
> 	* ld-cris/globsymw2.s, ld-cris/globsymw3.s: New files.
> 	* ld-cris/warn3.d, ld-cris/warn4.d, ld-cris/warn5.d,
> 	ld-cris/warn6.d: New tests.

This is OK, if you adjust the testsuite according to the above.

Ian