Re: PATCH: [Bug gas/4029] relax_segment can't stabilize .gcc_except_table

[Alan Modra <amodra at bigpond dot net dot au>]

On Sun, Mar 11, 2007 at 05:51:49PM -0700, H. J. Lu wrote:
> On Sun, Mar 11, 2007 at 01:37:04PM -0000, amodra at bigpond dot net dot au wrote:
> > 
> > Yes, we can modify the output, but we must do so in a way that does not confuse
> > consumers of .gcc_except_table.  I don't believe changing the .align is correct,
> > because data normally follows the .align (a number of .long's before the end
> > label).  Changing alignment in a way that cures the uleb128 problem necessarily
> > inserts zero bytes, which I think will confuse the unwinder.
> > 
> 
> I don't think increasing alignment should break any well formed
> assembly codes.

Yes, you a probably correct.  It does look like the type_info table is
stored in reverse order and accessed from the end label.

>  This patch breaks the infinite loop by increasing
> alignment before the left symbol in leb128.

I'm a little nervous about this patch.  I think you may be assuming
too much.  A leb128 might shrink in size for some reason, but not
follow this shrink with another growth.  You might even hit the wrong
align frag, and furthermore, increasing the alignment might not
change the amount of alignment padding.

This is what I've been playing with.  We look for pairs of leb128
and align frags, one of which grows when the other shrinks.  We only
do any adjustment of the align frag once we've finished relaxing every
other frag type, and the adjustment consists of adding a zero fill.
I'm not completely happy with this patch either, since requiring that
no frag other than leb128 and a following align change might be too
restrictive.  eg. an increase in size of the leb128 might cause
another leb128 to grow.  However, I think the likelihood of that
happenning is vanishingly small, and it's better to be safe than sorry
when producing assembler output that doesn't correspond to the input.

	PR 4029
	* write.c (relax_segment): Insert extra alignment padding
	to break infinite relax loop when given impossible
	gcc_except_table assembly.

Index: gas/write.c
===================================================================
RCS file: /cvs/src/src/gas/write.c,v
retrieving revision 1.110
diff -u -p -r1.110 write.c
--- gas/write.c	22 Feb 2007 05:54:51 -0000	1.110
+++ gas/write.c	12 Mar 2007 09:49:33 -0000
@@ -1960,13 +1960,38 @@ relax_segment (struct frag *segment_frag
   /* Do relax().  */
   {
     unsigned long max_iterations;
-    offsetT stretch;	/* May be any size, 0 or negative.  */
-    /* Cumulative number of addresses we have relaxed this pass.
-       We may have relaxed more than one address.  */
-    int stretched;	/* Have we stretched on this pass?  */
-    /* This is 'cuz stretch may be zero, when, in fact some piece of code
-       grew, and another shrank.  If a branch instruction doesn't fit anymore,
-       we could be scrod.  */
+
+    /* Cumulative address adjustment.  */
+    offsetT stretch;
+
+    /* Have we made any adjustment this pass?  We can't just test
+       stretch because one piece of code may have grown and another
+       shrank.  */
+    int stretched;
+
+    /* Most horrible, but gcc may give us some exception data that
+       is impossible to assemble, of the form
+
+       .align 4
+       .byte 0, 0
+       .uleb128 end - start
+       start:
+       .space 128*128 - 1
+       .align 4
+       end:
+
+       If the leb128 is two bytes in size, then end-start is 128*128,
+       which requires a three byte leb128.  If the leb128 is three
+       bytes in size, then end-start is 128*128-1, which requires a
+       two byte leb128.  We work around this dilemma by inserting
+       an extra 4 bytes of alignment just after the .align.  This
+       works because the data after the align is accessed relative to
+       the end label.
+
+       This counter is used in a tiny state machine to detect
+       whether a leb128 followed by an align is impossible to
+       relax.  */
+    int rs_leb128_fudge = 0;
 
     /* We want to prevent going into an infinite loop where one frag grows
        depending upon the location of a symbol which is in turn moved by
@@ -2089,6 +2114,41 @@ relax_segment (struct frag *segment_frag
 		    }
 
 		  growth = newoff - oldoff;
+
+		  /* If this align happens to follow a leb128 and
+		     we have determined that the leb128 is bouncing
+		     in size, then break the cycle by inserting an
+		     extra alignment.  */
+		  if (growth < 0
+		      && (rs_leb128_fudge & 16) != 0
+		      && (rs_leb128_fudge & 15) >= 2)
+		    {
+		      segment_info_type *seginfo = seg_info (segment);
+		      struct obstack *ob = &seginfo->frchainP->frch_obstack;
+		      struct frag *newf;
+
+		      newf = frag_alloc (ob);
+		      obstack_blank_fast (ob, 1);
+		      obstack_finish (ob);
+		      memcpy (newf, fragP, SIZEOF_STRUCT_FRAG);
+		      fragP->fr_next = newf;
+		      newf->fr_type = rs_fill;
+		      newf->fr_fix = 0;
+		      newf->fr_var = 1;
+		      newf->fr_literal[0] = 0;
+		      newf->fr_offset = 1 << newf->fr_offset;
+		      /* Include growth of new frag, because rs_fill
+			 frags don't normally grow.  */
+		      growth += newf->fr_offset;
+		      /* The new frag address is newoff.  Adjust this
+			 for the amount we'll add when we process the
+			 new frag.  */
+		      newf->fr_address = newoff - stretch - growth;
+		      newf->relax_marker ^= 1;
+#ifdef DEBUG
+		      as_warn (_("padding added"));
+#endif
+		    }
 		}
 		break;
 
@@ -2228,8 +2288,23 @@ relax_segment (struct frag *segment_frag
 	      {
 		stretch += growth;
 		stretched = 1;
+		if (fragP->fr_type == rs_leb128)
+		  rs_leb128_fudge += 16;
+		else if (fragP->fr_type == rs_align
+			 && (rs_leb128_fudge & 16) != 0
+			 && stretch == 0)
+		  rs_leb128_fudge += 16;
+		else
+		  rs_leb128_fudge = 0;
 	      }
 	  }
+
+	if (stretch == 0
+	    && (rs_leb128_fudge & 16) == 0
+	    && (rs_leb128_fudge & -16) != 0)
+	  rs_leb128_fudge += 1;
+	else
+	  rs_leb128_fudge = 0;
       }
     /* Until nothing further to relax.  */
     while (stretched && -- max_iterations);


-- 
Alan Modra
IBM OzLabs - Linux Technology Centre