* PATCH: [Bug gas/4029] relax_segment can't stabilize .gcc_except_table
@ 2007-03-12 0:51 H. J. Lu
2007-03-12 10:39 ` Alan Modra
0 siblings, 1 reply; 3+ messages in thread
From: H. J. Lu @ 2007-03-12 0:51 UTC (permalink / raw)
To: binutils
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. This patch breaks the infinite loop by increasing
alignment before the left symbol in leb128.
H.J.
-----
gas/
2007-02-19 H.J. Lu <hongjiu.lu@intel.com>
PR gas/4029
* symbols.c (symbol_get_subtract_frags): New.
* symbols.h (symbol_get_subtract_frags): Likewise.
* write.c (relax_segment): Break infinite loop for rs_leb128
by increasing alignment.
gas/testsuite/
2007-02-19 Alan Modra <amodra@bigpond.net.au>
H.J. Lu <hongjiu.lu@intel.com>
PR gas/4029
* gas/all/gas.exp: Add relax.
* gas/all/relax.d: New file.
* gas/all/relax.s: Likewise.
--- gas/symbols.c.relax 2007-03-08 06:16:17.000000000 -0800
+++ gas/symbols.c 2007-03-11 16:59:39.000000000 -0700
@@ -3153,3 +3153,24 @@ symbol_relc_make_expr (expressionS * exp
}
#endif
+
+/* Get frags of left and right symbols in subtraction. */
+
+void
+symbol_get_subtract_frags (symbolS *sym, fragS **left, fragS **right)
+{
+ if (sym->sy_value.X_op == O_subtract)
+ {
+ if (left)
+ *left = symbol_get_frag (sym->sy_value.X_add_symbol);
+ if (right)
+ *right = symbol_get_frag (sym->sy_value.X_op_symbol);
+ }
+ else
+ {
+ if (left)
+ *left = NULL;
+ if (right)
+ *right = NULL;
+ }
+}
--- gas/symbols.h.relax 2007-02-05 17:27:16.000000000 -0800
+++ gas/symbols.h 2007-03-11 16:53:37.000000000 -0700
@@ -177,6 +177,7 @@ extern offsetT *symbol_X_add_number (sym
extern void symbol_set_value_now (symbolS *);
extern void symbol_set_frag (symbolS *, fragS *);
extern fragS *symbol_get_frag (symbolS *);
+extern void symbol_get_subtract_frags (symbolS *, fragS **, fragS **);
extern void symbol_mark_used (symbolS *);
extern void symbol_clear_used (symbolS *);
extern int symbol_used_p (symbolS *);
--- gas/testsuite/gas/all/gas.exp.relax 2007-02-06 07:53:27.000000000 -0800
+++ gas/testsuite/gas/all/gas.exp 2007-03-11 17:37:31.000000000 -0700
@@ -264,6 +264,7 @@ if { ([istarget "i*86-*-*pe*"] && ![ist
run_dump_test assign
run_dump_test sleb128
+run_dump_test relax
# .quad is 16 bytes on i960.
if { ![istarget "i960-*-*"] } {
--- gas/testsuite/gas/all/relax.d.relax 2007-03-11 17:27:00.000000000 -0700
+++ gas/testsuite/gas/all/relax.d 2007-03-11 17:31:12.000000000 -0700
@@ -0,0 +1,11 @@
+#objdump : -s -j .data -j "\$DATA\$"
+#name : relax .sleb128
+
+.*: .*
+
+Contents of section (\.data|\$DATA\$):
+ 0000 00008380 01070000 00000000 00000000 ................
+ 0010 00000000 00000000 00000000 00000000 ................
+#...
+ 4000 00000000 00000000 ........
+#pass
--- gas/testsuite/gas/all/relax.s.relax 2007-03-11 17:25:51.000000000 -0700
+++ gas/testsuite/gas/all/relax.s 2007-03-11 17:25:37.000000000 -0700
@@ -0,0 +1,9 @@
+ .data
+ .align 4
+ .byte 0, 0
+ .uleb128 end - start
+start:
+ .byte 7
+ .space 128*128 - 3 /* or -2 or -3 */
+ .align 4
+end:
--- gas/write.c.relax 2007-03-01 14:53:14.000000000 -0800
+++ gas/write.c 2007-03-11 17:43:43.000000000 -0700
@@ -1985,6 +1985,7 @@ relax_segment (struct frag *segment_frag
ret = 0;
do
{
+restart:
stretch = 0;
stretched = 0;
@@ -2209,6 +2210,36 @@ relax_segment (struct frag *segment_frag
size = sizeof_leb128 (value, fragP->fr_subtype);
growth = size - fragP->fr_offset;
fragP->fr_offset = size;
+ if (growth < 0)
+ {
+ struct frag *left;
+
+ /* We started it at size 1. If it shrinks now,
+ it means that we will go into an infinite
+ loop. We try to adjust aligment before
+ the left symbol. */
+ symbol_get_subtract_frags (fragP->fr_symbol,
+ &left, NULL);
+ if (left)
+ {
+ struct frag *alignP = NULL, *nextP;
+
+ for (nextP = fragP;
+ nextP && nextP != left;
+ nextP = nextP->fr_next)
+ if (nextP->fr_type == rs_align
+ && nextP->fr_offset != 0)
+ alignP = nextP;
+
+ if (alignP)
+ {
+ /* FIXME: Is it 100% safe to increase
+ alignment? */
+ alignP->fr_offset++;
+ goto restart;
+ }
+ }
+ }
}
break;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: PATCH: [Bug gas/4029] relax_segment can't stabilize .gcc_except_table
2007-03-12 0:51 PATCH: [Bug gas/4029] relax_segment can't stabilize .gcc_except_table H. J. Lu
@ 2007-03-12 10:39 ` Alan Modra
2007-03-14 11:04 ` Alan Modra
0 siblings, 1 reply; 3+ messages in thread
From: Alan Modra @ 2007-03-12 10:39 UTC (permalink / raw)
To: H. J. Lu; +Cc: binutils
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: PATCH: [Bug gas/4029] relax_segment can't stabilize .gcc_except_table
2007-03-12 10:39 ` Alan Modra
@ 2007-03-14 11:04 ` Alan Modra
0 siblings, 0 replies; 3+ messages in thread
From: Alan Modra @ 2007-03-14 11:04 UTC (permalink / raw)
To: binutils
This differs from the patch I posted previously in that the new
rs_fill padding uses the same fill pattern as the rs_align.
gas/
PR 4029
* write.c (relax_segment): Insert extra alignment padding
to break infinite relax loop when given impossible
gcc_except_table assembly.
gas/testsuite/
PR 4029
* gas/all/relax.s: New.
* gas/all/relax.d: New.
* gas/all/gas.exp: Run it.
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 14 Mar 2007 10:51:12 -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,49 @@ 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, fragP->fr_var);
+ obstack_finish (ob);
+ memcpy (newf, fragP, SIZEOF_STRUCT_FRAG);
+ memcpy (newf->fr_literal,
+ fragP->fr_literal + fragP->fr_fix,
+ fragP->fr_var);
+ newf->fr_type = rs_fill;
+ newf->fr_fix = 0;
+ newf->fr_offset = (((offsetT) 1 << fragP->fr_offset)
+ / fragP->fr_var);
+ if (newf->fr_offset * newf->fr_var
+ != (offsetT) 1 << fragP->fr_offset)
+ {
+ newf->fr_offset = (offsetT) 1 << fragP->fr_offset;
+ newf->fr_var = 1;
+ }
+ /* Include growth of new frag, because rs_fill
+ frags don't normally grow. */
+ growth += newf->fr_offset * newf->fr_var;
+ /* 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;
+ fragP->fr_next = newf;
+#ifdef DEBUG
+ as_warn (_("padding added"));
+#endif
+ }
}
break;
@@ -2228,8 +2296,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);
Index: gas/testsuite/gas/all/gas.exp
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/all/gas.exp,v
retrieving revision 1.44
diff -u -p -r1.44 gas.exp
--- gas/testsuite/gas/all/gas.exp 6 Feb 2007 15:13:26 -0000 1.44
+++ gas/testsuite/gas/all/gas.exp 14 Mar 2007 10:51:17 -0000
@@ -265,6 +265,11 @@ if { ([istarget "i*86-*-*pe*"] && ![ist
run_dump_test assign
run_dump_test sleb128
+# .byte is 32 bits on tic4x, and .p2align isn't supported on tic54x
+if { ![istarget "tic4x*-*-*"] && ![istarget "tic54x*-*-*"] } {
+ run_dump_test relax
+}
+
# .quad is 16 bytes on i960.
if { ![istarget "i960-*-*"] } {
run_dump_test quad
Index: gas/testsuite/gas/all/relax.d
===================================================================
RCS file: gas/testsuite/gas/all/relax.d
diff -N gas/testsuite/gas/all/relax.d
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gas/testsuite/gas/all/relax.d 14 Mar 2007 10:51:17 -0000
@@ -0,0 +1,13 @@
+#objdump : -s -j .data -j "\$DATA\$"
+#name : relax .uleb128
+
+.*: .*
+
+Contents of section .*
+ 0000 01020381 01000000 00000000 00000000.*
+#...
+ 0080 00000004 ffff0500 06078380 01000000.*
+#...
+ 4080 00000000 00000000 00000008 ffffffff.*
+ 4090 09090909 09090909 09090909 09090909.*
+#pass
Index: gas/testsuite/gas/all/relax.s
===================================================================
RCS file: gas/testsuite/gas/all/relax.s
diff -N gas/testsuite/gas/all/relax.s
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gas/testsuite/gas/all/relax.s 14 Mar 2007 10:51:17 -0000
@@ -0,0 +1,20 @@
+ .data
+ .byte 1, 2, 3
+ .uleb128 L2 - L1
+L1:
+ .space 128 - 2
+ .byte 4
+ .p2align 1, 0xff
+L2:
+ .byte 5
+
+ .p2align 2
+ .byte 6, 7
+ .uleb128 L4 - L3
+L3:
+ .space 128*128 - 2
+ .byte 8
+ .p2align 2, 0xff
+L4:
+ .byte 9
+ .p2align 4, 9
--
Alan Modra
IBM OzLabs - Linux Technology Centre
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2007-03-14 11:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-03-12 0:51 PATCH: [Bug gas/4029] relax_segment can't stabilize .gcc_except_table H. J. Lu
2007-03-12 10:39 ` Alan Modra
2007-03-14 11:04 ` Alan Modra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).