compiling binutils with -DDEBUG

compiling binutils with -DDEBUG

[Alan Modra]

Attempting to building binutils with -DDEBUG (ie. gas know() assertion
does something) is interesting.  Lots more warnings, gas doesn't even
compile, and it's obvious some targets have never been built with
-DDEBUG turned on.  This fixes some of the problems.

bfd/
	* targets.h: Typo fix.
	* bfd-in2.h: Regenerate.
gas/
	* as.h (assert): Warning fix.
	* expr.c (expr): Correct assertion.
	* read.c (s_comm_internal): Remove assertion.
It's not true for .lcomm.
	* write.c (relax_segment): Enable vma assertion only for BFD_ASSEMBLER.
	(fixup_segment): Remove assertion.
Didn't seem worth including struct-symbol.h just for this.  I suspect
the assertion isn't true too, since a similar assertion in obj-coff.c
fails with X_op an O_constant.  Maybe this is a real bug.  Ian, you're
probably the only one who can answer mri mode queries off the top of
your head.  Care to comment?
	* config/tc-dlx.c (machine_ip): Remove untrue assertions.
	(md_apply_fix3): Likewise.
	* config/tc-i370.c (md_begin): Correct assertion.
	(i370_macro): Warning fix for assertion.
	* config/tc-m68k.c (md_assemble): Don't use sizeof host short.
A portability fix that I accidentally left out of the last commit.


Index: bfd/targets.c
===================================================================
RCS file: /cvs/src/src/bfd/targets.c,v
retrieving revision 1.122
diff -u -p -r1.122 targets.c
--- bfd/targets.c	31 Jan 2005 23:13:30 -0000	1.122
+++ bfd/targets.c	1 Mar 2005 01:18:36 -0000
@@ -209,7 +209,7 @@ DESCRIPTION
 .  unsigned short ar_max_namelen;
 .
 .  {* Entries for byte swapping for data. These are different from the
-.     other entry points, since they don't take a BFD asthe first argument.
+.     other entry points, since they don't take a BFD as the first argument.
 .     Certain other handlers could do the same.  *}
 .  bfd_uint64_t   (*bfd_getx64) (const void *);
 .  bfd_int64_t    (*bfd_getx_signed_64) (const void *);
Index: gas/as.h
===================================================================
RCS file: /cvs/src/src/gas/as.h,v
retrieving revision 1.43
diff -u -p -r1.43 as.h
--- gas/as.h	17 Feb 2005 13:46:00 -0000	1.43
+++ gas/as.h	1 Mar 2005 01:18:47 -0000
@@ -104,7 +104,8 @@ extern void *alloca ();
 #if __GNUC__ < 2 || (__GNUC__ == 2 && __GNUC_MINOR__ < 6)
 #define __PRETTY_FUNCTION__  ((char*)0)
 #endif
-#define assert(P) ((P) ? 0 : (as_assert (__FILE__, __LINE__, __PRETTY_FUNCTION__), 0))
+#define assert(P) \
+  ((void) ((P) ? 0 : (as_assert (__FILE__, __LINE__, __PRETTY_FUNCTION__), 0)))
 #undef abort
 #define abort()		as_abort (__FILE__, __LINE__, __PRETTY_FUNCTION__)
 
Index: gas/expr.c
===================================================================
RCS file: /cvs/src/src/gas/expr.c,v
retrieving revision 1.52
diff -u -p -r1.52 expr.c
--- gas/expr.c	11 Aug 2004 02:11:57 -0000	1.52
+++ gas/expr.c	1 Mar 2005 01:18:50 -0000
@@ -1716,7 +1716,7 @@ expr (int rankarg,		/* Larger # is highe
       know (op_right == O_illegal
 	    || op_rank[(int) op_right] <= op_rank[(int) op_left]);
       know ((int) op_left >= (int) O_multiply
-	    && (int) op_left <= (int) O_logical_or);
+	    && (int) op_left <= (int) O_index);
 
       /* input_line_pointer->after right-hand quantity.  */
       /* left-hand quantity in resultP.  */
Index: gas/read.c
===================================================================
RCS file: /cvs/src/src/gas/read.c,v
retrieving revision 1.93
diff -u -p -r1.93 read.c
--- gas/read.c	23 Feb 2005 02:44:18 -0000	1.93
+++ gas/read.c	1 Mar 2005 01:18:53 -0000
@@ -1472,7 +1472,6 @@ s_comm_internal (int param,
 #endif
     }
 
-  know (symbolP == NULL || symbolP->sy_frag == &zero_address_frag);
   demand_empty_rest_of_line ();
  out:
   if (flag_mri)
Index: gas/write.c
===================================================================
RCS file: /cvs/src/src/gas/write.c,v
retrieving revision 1.83
diff -u -p -r1.83 write.c
--- gas/write.c	1 Mar 2005 00:43:57 -0000	1.83
+++ gas/write.c	1 Mar 2005 01:18:54 -0000
@@ -2395,7 +2395,9 @@ relax_segment (struct frag *segment_frag
                          into the section.  Here it is assumed that the
                          section's VMA is zero, and can omit subtracting it
                          from the symbol's value to get the address offset.  */
-                      know (S_GET_SECTION (symbolP)->vma == 0);
+#ifdef BFD_ASSEMBLER
+                      know (S_GET_SEGMENT (symbolP)->vma == 0);
+#endif
 		      target += S_GET_VALUE (symbolP) * OCTETS_PER_BYTE;
 		    }
 
@@ -2583,7 +2585,6 @@ fixup_segment (fixS *fixP, segT this_seg
       if (fixP->fx_addsy != NULL
 	  && symbol_mri_common_p (fixP->fx_addsy))
 	{
-	  know (fixP->fx_addsy->sy_value.X_op == O_symbol);
 	  add_number += S_GET_VALUE (fixP->fx_addsy);
 	  fixP->fx_offset = add_number;
 	  fixP->fx_addsy
Index: gas/config/tc-dlx.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-dlx.c,v
retrieving revision 1.10
diff -u -p -r1.10 tc-dlx.c
--- gas/config/tc-dlx.c	31 Jan 2005 23:18:27 -0000	1.10
+++ gas/config/tc-dlx.c	1 Mar 2005 01:18:59 -0000
@@ -972,21 +972,18 @@ machine_ip (str)
 	  /* Type 'a' Register.  */
 	case 'a':
 	  /* A general register at bits 21-25, rs1.  */
-	  know (operand->X_op != O_register);
 	  reg_shift = 21;
 	  goto general_reg;
 
 	  /* Type 'b' Register.  */
 	case 'b':
 	  /* A general register at bits 16-20, rs2/rd.  */
-	  know (operand->X_op != O_register);
 	  reg_shift = 16;
 	  goto general_reg;
 
 	  /* Type 'c' Register.  */
 	case 'c':
 	  /* A general register at bits 11-15, rd.  */
-	  know (operand->X_op != O_register);
 	  reg_shift = 11;
 
 	general_reg:
@@ -1135,9 +1132,6 @@ md_apply_fix3 (fixP, valP, seg)
   long val = *valP;
   char *place = fixP->fx_where + fixP->fx_frag->fr_literal;
 
-  know (fixP->fx_size == 4);
-  know (fixP->fx_r_type < NO_RELOC);
-
   switch (fixP->fx_r_type)
     {
     case RELOC_DLX_LO16:
Index: gas/config/tc-i370.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-i370.c,v
retrieving revision 1.23
diff -u -p -r1.23 tc-i370.c
--- gas/config/tc-i370.c	23 Feb 2005 12:28:04 -0000	1.23
+++ gas/config/tc-i370.c	1 Mar 2005 01:18:59 -0000
@@ -588,7 +588,8 @@ md_begin ()
    op_end = i370_opcodes + i370_num_opcodes;
    for (op = i370_opcodes; op < op_end; op++)
      {
-       know ((op->opcode & op->mask) == op->opcode);
+       know ((op->opcode.i[0] & op->mask.i[0]) == op->opcode.i[0]
+	     && (op->opcode.i[1] & op->mask.i[1]) == op->opcode.i[1]);
 
        if ((op->flags & i370_cpu) != 0)
          {
@@ -2422,7 +2423,7 @@ i370_macro (str, macro)
       else
         {
           arg = strtol (format + 1, &send, 10);
-          know (send != format && arg >= 0 && arg < count);
+          know (send != format && arg >= 0 && (unsigned) arg < count);
           len += strlen (operands[arg]);
           format = send;
         }
Index: gas/config/tc-m68k.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-m68k.c,v
retrieving revision 1.61
diff -u -p -r1.61 tc-m68k.c
--- gas/config/tc-m68k.c	17 Feb 2005 13:46:03 -0000	1.61
+++ gas/config/tc-m68k.c	1 Mar 2005 01:19:03 -0000
@@ -4067,7 +4067,7 @@ md_assemble (char *str)
   shorts_this_frag = 0;
   if (n)
     {
-      toP = frag_more (n * sizeof (short));
+      toP = frag_more (n * 2);
       while (n--)
 	{
 	  md_number_to_chars (toP, (long) (*fromP), 2);

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: compiling binutils with -DDEBUG

[Ian Lance Taylor]

Alan Modra <amodra@bigpond.net.au> writes:

> 	(fixup_segment): Remove assertion.
> Didn't seem worth including struct-symbol.h just for this.  I suspect
> the assertion isn't true too, since a similar assertion in obj-coff.c
> fails with X_op an O_constant.  Maybe this is a real bug.  Ian, you're
> probably the only one who can answer mri mode queries off the top of
> your head.  Care to comment?

> @@ -2583,7 +2585,6 @@ fixup_segment (fixS *fixP, segT this_seg
>        if (fixP->fx_addsy != NULL
>  	  && symbol_mri_common_p (fixP->fx_addsy))
>  	{
> -	  know (fixP->fx_addsy->sy_value.X_op == O_symbol);
>  	  add_number += S_GET_VALUE (fixP->fx_addsy);
>  	  fixP->fx_offset = add_number;
>  	  fixP->fx_addsy

The assertion should be true.  This is a symbol defined in an MRI
common section.  Since we don't generate IEEE object files, we treat
an MRI common section as a single large common symbol.  Then we handle
a reloc against a symbol defined within an MRI common section as an
offset to that symbol.  The assertion is just checking that we really
do point to the magic symbol we are using for the MRI common section.

So, I don't think it's a bug, but since MRI mode is not very important
I don't think we need to keep the assertion if it's convenient to
remove it.

Ian

Re: compiling binutils with -DDEBUG

[Alan Modra]

On Mon, Feb 28, 2005 at 09:36:00PM -0500, Ian Lance Taylor wrote:
> Alan Modra <amodra@bigpond.net.au> writes:
> 
> > 	(fixup_segment): Remove assertion.
> > Didn't seem worth including struct-symbol.h just for this.  I suspect
> > the assertion isn't true too, since a similar assertion in obj-coff.c
> > fails with X_op an O_constant.  Maybe this is a real bug.  Ian, you're
> > probably the only one who can answer mri mode queries off the top of
> > your head.  Care to comment?
> 
> > @@ -2583,7 +2585,6 @@ fixup_segment (fixS *fixP, segT this_seg
> >        if (fixP->fx_addsy != NULL
> >  	  && symbol_mri_common_p (fixP->fx_addsy))
> >  	{
> > -	  know (fixP->fx_addsy->sy_value.X_op == O_symbol);
> >  	  add_number += S_GET_VALUE (fixP->fx_addsy);
> >  	  fixP->fx_offset = add_number;
> >  	  fixP->fx_addsy
> 
> The assertion should be true.  This is a symbol defined in an MRI
> common section.  Since we don't generate IEEE object files, we treat
> an MRI common section as a single large common symbol.  Then we handle
> a reloc against a symbol defined within an MRI common section as an
> offset to that symbol.  The assertion is just checking that we really
> do point to the magic symbol we are using for the MRI common section.

I did a little analysis.  Nowadays, the assertion is always false for
BFD_ASSEMBLER because symbols have been resolved (write.c:1857) by the
time this code is reached.  The following would probably fix the
assertion, but I don't think it's worth worrying about.

Index: write.c
===================================================================
RCS file: /cvs/src/src/gas/write.c,v
retrieving revision 1.84
diff -u -p -r1.84 write.c
--- write.c	1 Mar 2005 02:00:14 -0000	1.84
+++ write.c	1 Mar 2005 10:46:56 -0000
@@ -1854,7 +1854,8 @@ write_object_file (void)
       symbolS *symp;
 
       for (symp = symbol_rootP; symp; symp = symbol_next (symp))
-	resolve_symbol_value (symp);
+	if (!symbol_mri_common_p (symp))
+	  resolve_symbol_value (symp);
     }
   resolve_local_symbol_values ();
 
-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: compiling binutils with -DDEBUG

[Alan Modra]

On Tue, Mar 01, 2005 at 09:19:49PM +1030, Alan Modra wrote:
> I did a little analysis.  Nowadays, the assertion is always false for
> BFD_ASSEMBLER because symbols have been resolved (write.c:1857) by the
> time this code is reached.  The following would probably fix the
> assertion, but I don't think it's worth worrying about.

	* config/obj-coff.c (fixup_segment): Delete sy_mri_common assertion.

Index: gas/config/obj-coff.c
===================================================================
RCS file: /cvs/src/src/gas/config/obj-coff.c,v
retrieving revision 1.78
diff -u -p -r1.78 obj-coff.c
--- gas/config/obj-coff.c	22 Feb 2005 00:50:05 -0000	1.78
+++ gas/config/obj-coff.c	1 Mar 2005 10:51:57 -0000
@@ -4290,7 +4290,6 @@ fixup_segment (segP, this_segment_type)
       if (add_symbolP != NULL
 	  && add_symbolP->sy_mri_common)
 	{
-	  know (add_symbolP->sy_value.X_op == O_symbol);
 	  add_number += S_GET_VALUE (add_symbolP);
 	  fixP->fx_offset = add_number;
 	  add_symbolP = fixP->fx_addsy = add_symbolP->sy_value.X_add_symbol;

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: compiling binutils with -DDEBUG

[Ian Lance Taylor]

Alan Modra <amodra@bigpond.net.au> writes:

> On Mon, Feb 28, 2005 at 09:36:00PM -0500, Ian Lance Taylor wrote:
> > Alan Modra <amodra@bigpond.net.au> writes:
> > 
> > > 	(fixup_segment): Remove assertion.
> > > Didn't seem worth including struct-symbol.h just for this.  I suspect
> > > the assertion isn't true too, since a similar assertion in obj-coff.c
> > > fails with X_op an O_constant.  Maybe this is a real bug.  Ian, you're
> > > probably the only one who can answer mri mode queries off the top of
> > > your head.  Care to comment?
> > 
> > > @@ -2583,7 +2585,6 @@ fixup_segment (fixS *fixP, segT this_seg
> > >        if (fixP->fx_addsy != NULL
> > >  	  && symbol_mri_common_p (fixP->fx_addsy))
> > >  	{
> > > -	  know (fixP->fx_addsy->sy_value.X_op == O_symbol);
> > >  	  add_number += S_GET_VALUE (fixP->fx_addsy);
> > >  	  fixP->fx_offset = add_number;
> > >  	  fixP->fx_addsy
> > 
> > The assertion should be true.  This is a symbol defined in an MRI
> > common section.  Since we don't generate IEEE object files, we treat
> > an MRI common section as a single large common symbol.  Then we handle
> > a reloc against a symbol defined within an MRI common section as an
> > offset to that symbol.  The assertion is just checking that we really
> > do point to the magic symbol we are using for the MRI common section.
> 
> I did a little analysis.  Nowadays, the assertion is always false for
> BFD_ASSEMBLER because symbols have been resolved (write.c:1857) by the
> time this code is reached.  The following would probably fix the
> assertion, but I don't think it's worth worrying about.

There is code in resolve_symbol_value() to handle this case.  I wonder
why it isn't working?  If the assert in fixup_segment() fails, then
MRI common symbols will not be handled correctly.

Whether anybody actually cares is another issue.

Ian

Re: compiling binutils with -DDEBUG

[Alan Modra]

On Tue, Mar 01, 2005 at 09:24:54AM -0500, Ian Lance Taylor wrote:
> Alan Modra <amodra@bigpond.net.au> writes:
> 
> > On Mon, Feb 28, 2005 at 09:36:00PM -0500, Ian Lance Taylor wrote:
> > > Alan Modra <amodra@bigpond.net.au> writes:
> > > 
> > > > 	(fixup_segment): Remove assertion.
> > > > Didn't seem worth including struct-symbol.h just for this.  I suspect
> > > > the assertion isn't true too, since a similar assertion in obj-coff.c
> > > > fails with X_op an O_constant.  Maybe this is a real bug.  Ian, you're
> > > > probably the only one who can answer mri mode queries off the top of
> > > > your head.  Care to comment?
> > > 
> > > > @@ -2583,7 +2585,6 @@ fixup_segment (fixS *fixP, segT this_seg
> > > >        if (fixP->fx_addsy != NULL
> > > >  	  && symbol_mri_common_p (fixP->fx_addsy))
> > > >  	{
> > > > -	  know (fixP->fx_addsy->sy_value.X_op == O_symbol);
> > > >  	  add_number += S_GET_VALUE (fixP->fx_addsy);
> > > >  	  fixP->fx_offset = add_number;
> > > >  	  fixP->fx_addsy
> > > 
> > > The assertion should be true.  This is a symbol defined in an MRI
> > > common section.  Since we don't generate IEEE object files, we treat
> > > an MRI common section as a single large common symbol.  Then we handle
> > > a reloc against a symbol defined within an MRI common section as an
> > > offset to that symbol.  The assertion is just checking that we really
> > > do point to the magic symbol we are using for the MRI common section.
> > 
> > I did a little analysis.  Nowadays, the assertion is always false for
> > BFD_ASSEMBLER because symbols have been resolved (write.c:1857) by the
> > time this code is reached.  The following would probably fix the
> > assertion, but I don't think it's worth worrying about.
> 
> There is code in resolve_symbol_value() to handle this case.  I wonder
> why it isn't working?

The reason the assert fails is that finalize_syms is set by the time we
reach write.c:1857, so S_SET_VALUE is called for these symbols.  That
turns them into O_constant values.

>  If the assert in fixup_segment() fails, then
> MRI common symbols will not be handled correctly.

Are you sure?  I couldn't see why it mattered that the sym was resolved
at write.c:1857 versus being resolved at write.c:2588.  The mri common
testcase looks to produce good results.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: compiling binutils with -DDEBUG

[Ian Lance Taylor]

Alan Modra <amodra@bigpond.net.au> writes:

> >  If the assert in fixup_segment() fails, then
> > MRI common symbols will not be handled correctly.
> 
> Are you sure?  I couldn't see why it mattered that the sym was resolved
> at write.c:1857 versus being resolved at write.c:2588.  The mri common
> testcase looks to produce good results.

A relocation against a symbol in an MRI common section has to be
converted into a relocation against the MRI common symbol with the
appropriate addend.  That is what the code in fixup_segment is doing.

The MRI common test case isn't particularly convincing, since it
doesn't test the reloc information (presumably my fault).

Ian

Re: compiling binutils with -DDEBUG

[Alan Modra]

On Tue, Mar 01, 2005 at 08:40:37PM -0500, Ian Lance Taylor wrote:
> Alan Modra <amodra@bigpond.net.au> writes:
> 
> > >  If the assert in fixup_segment() fails, then
> > > MRI common symbols will not be handled correctly.
> > 
> > Are you sure?  I couldn't see why it mattered that the sym was resolved
> > at write.c:1857 versus being resolved at write.c:2588.  The mri common
> > testcase looks to produce good results.
> 
> A relocation against a symbol in an MRI common section has to be
> converted into a relocation against the MRI common symbol with the
> appropriate addend.  That is what the code in fixup_segment is doing.

Understood.  The relocs appear correct to me.  ie. resolve_symbol_value
is doing the right thing, I think.

dump.o:     file format elf32-m68k

RELOCATION RECORDS FOR [.data]:
OFFSET   TYPE              VALUE
00000000 R_68K_32          com1
00000004 R_68K_32          00com2+0x00000004

dump.o:     file format a.out-zero-big

RELOCATION RECORDS FOR [.data]:
OFFSET   TYPE              VALUE
00000000 32                com1
00000004 32                00com2

dump.o:     file format coff-m68k

RELOCATION RECORDS FOR [.data]:
OFFSET   TYPE              VALUE
00000000 32                com1
00000004 32                00com2

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: compiling binutils with -DDEBUG

[Ian Lance Taylor]

Alan Modra <amodra@bigpond.net.au> writes:

> Understood.  The relocs appear correct to me.  ie. resolve_symbol_value
> is doing the right thing, I think.

OK, good.  Thanks for checking.

Ian