This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] Fix sleb128 encoding for difference expressions
- From: Julian Brown <julian at codesourcery dot com>
- To: Alan Modra <amodra at gmail dot com>
- Cc: <binutils at sourceware dot org>, <clm at codesourcery dot com>
- Date: Thu, 11 Apr 2013 12:04:05 +0100
- Subject: Re: [PATCH] Fix sleb128 encoding for difference expressions
- References: <20121108130512 dot 4991d16a at octopus> <509BC97B02000078000A73D9 at nat28 dot tlf dot novell dot com> <20121109175729 dot 795ff7d8 at octopus> <50A0BAFC02000078000A7A1A at nat28 dot tlf dot novell dot com> <20121112121918 dot 76e18240 at octopus> <20121119153757 dot 64572565 at octopus> <20121127152921 dot 6264be7d at octopus> <20130405170002 dot 32274b7c at octopus> <20130408050413 dot GA4102 at bubble dot grove dot modra dot org> <20130408062955 dot GA28916 at bubble dot grove dot modra dot org> <20130408063944 dot GB28916 at bubble dot grove dot modra dot org>
On Mon, 8 Apr 2013 16:09:44 +0930
Alan Modra <amodra@gmail.com> wrote:
> On Mon, Apr 08, 2013 at 03:59:55PM +0930, Alan Modra wrote:
> > On Mon, Apr 08, 2013 at 02:34:13PM +0930, Alan Modra wrote:
> > > On Fri, Apr 05, 2013 at 05:00:02PM +0100, Julian Brown wrote:
> > > > > > On Mon, 12 Nov 2012 12:19:18 +0000
> > > > > > Julian Brown <julian@codesourcery.com> wrote:
> > > > > > > This is a more conservative version of the patch, which
> > > > > > > *only* affects assembly of .sleb128 directives.
> > >
> > > That one was
> > > http://sourceware.org/ml/binutils/2012-11/msg00141.html
> > >
> > > It still applies, but I see testsuite failures on x86_64-linux.
> > >
> > > First one is:
> > > regexp_diff match failure
> > > regexp "^ 0000 7d2a0000 00000000 00000000 00000000 .*$"
> > > line " 0000
> > > 7d2a }* "
> > > FAIL: .sleb128 tests (2)
> > >
> > > I'd suggest making the match
> > > 0+ 7d2a.*
> > > and similarly with other testcases.
> > >
> > > Patch is OK with those changes.
> >
> > Oh, and another thing I just noticed, you need to ensure these tests
> > are only run on ELF targets. Probably best to move them to
> > gas/testsuite/gas/elf/
>
> Oops, no, sorry. We do support .sleb128 on non-ELF. But AOUT for
> instance typically won't have .data starting at zero for your
> testcases.. So my suggestion to use 0+ wasn't quite correct.
>
> .* 7d2a.*
> instead.
Thanks, I've committed the attached version of the patch. Unfortunately
though, I forgot that one of the tests fails on non-BFD64 targets
(sleb128-6.[sd]) -- quite rightly, since the assembly file tries to
unconditionally manipulate 64-bit quantities. I had a hack in our local
sources' gas.exp:
# The following test requires a "BFD64" host, but there's no easy way of
# determining whether we have one of those here. Only run the test on MIPS
# for now.
if { [istarget "mips*-*-*"] } {
run_dump_test sleb128-6
}
I don't think that's suitable for upstream, and will try to figure out a
better solution (suggestions appreciated), but for now I've simply
omitted the whole test.
Cheers,
Julian
? gas/testsuite/gas/all/sleb128-6.d.copy
? gas/testsuite/gas/all/sleb128-6.s.copy
Index: gas/expr.c
===================================================================
RCS file: /cvs/src/src/gas/expr.c,v
retrieving revision 1.92
diff -u -p -r1.92 expr.c
--- gas/expr.c 8 Mar 2013 10:17:00 -0000 1.92
+++ gas/expr.c 11 Apr 2013 10:39:26 -0000
@@ -90,6 +90,7 @@ make_expr_symbol (expressionS *expressio
zero.X_op = O_constant;
zero.X_add_number = 0;
zero.X_unsigned = 0;
+ zero.X_extrabit = 0;
clean_up_expression (&zero);
expressionP = &zero;
}
@@ -161,6 +162,7 @@ expr_build_uconstant (offsetT value)
e.X_op = O_constant;
e.X_add_number = value;
e.X_unsigned = 1;
+ e.X_extrabit = 0;
return make_expr_symbol (&e);
}
@@ -732,6 +734,7 @@ operand (expressionS *expressionP, enum
something like ``.quad 0x80000000'' is not sign extended even
though it appears negative if valueT is 32 bits. */
expressionP->X_unsigned = 1;
+ expressionP->X_extrabit = 0;
/* Digits, assume it is a bignum. */
@@ -1026,6 +1029,8 @@ operand (expressionS *expressionP, enum
This is compatible with other people's
assemblers. Sigh. */
expressionP->X_unsigned = 0;
+ if (expressionP->X_add_number)
+ expressionP->X_extrabit ^= 1;
}
else if (c == '~' || c == '"')
expressionP->X_add_number = ~ expressionP->X_add_number;
@@ -1078,6 +1083,7 @@ operand (expressionS *expressionP, enum
expressionP->X_add_number = i >= expressionP->X_add_number;
expressionP->X_op = O_constant;
expressionP->X_unsigned = 1;
+ expressionP->X_extrabit = 0;
}
}
else if (expressionP->X_op != O_illegal
@@ -1717,6 +1723,42 @@ operatorf (int *num_chars)
/* NOTREACHED */
}
+/* Implement "word-size + 1 bit" addition for
+ {resultP->X_extrabit:resultP->X_add_number} + {rhs_highbit:amount}. This
+ is used so that the full range of unsigned word values and the full range of
+ signed word values can be represented in an O_constant expression, which is
+ useful e.g. for .sleb128 directives. */
+
+static void
+add_to_result (expressionS *resultP, offsetT amount, int rhs_highbit)
+{
+ valueT ures = resultP->X_add_number;
+ valueT uamount = amount;
+
+ resultP->X_add_number += amount;
+
+ resultP->X_extrabit ^= rhs_highbit;
+
+ if (ures + uamount < ures)
+ resultP->X_extrabit ^= 1;
+}
+
+/* Similarly, for subtraction. */
+
+static void
+subtract_from_result (expressionS *resultP, offsetT amount, int rhs_highbit)
+{
+ valueT ures = resultP->X_add_number;
+ valueT uamount = amount;
+
+ resultP->X_add_number -= amount;
+
+ resultP->X_extrabit ^= rhs_highbit;
+
+ if (ures < uamount)
+ resultP->X_extrabit ^= 1;
+}
+
/* Parse an expression. */
segT
@@ -1832,7 +1874,7 @@ expr (int rankarg, /* Larger # is highe
&& (md_register_arithmetic || resultP->X_op != O_register))
{
/* X + constant. */
- resultP->X_add_number += right.X_add_number;
+ add_to_result (resultP, right.X_add_number, right.X_extrabit);
}
/* This case comes up in PIC code. */
else if (op_left == O_subtract
@@ -1850,10 +1892,11 @@ expr (int rankarg, /* Larger # is highe
symbol_get_frag (right.X_add_symbol),
&frag_off))
{
- resultP->X_add_number -= right.X_add_number;
- resultP->X_add_number -= frag_off / OCTETS_PER_BYTE;
- resultP->X_add_number += (S_GET_VALUE (resultP->X_add_symbol)
- - S_GET_VALUE (right.X_add_symbol));
+ offsetT symval_diff = S_GET_VALUE (resultP->X_add_symbol)
+ - S_GET_VALUE (right.X_add_symbol);
+ subtract_from_result (resultP, right.X_add_number, right.X_extrabit);
+ subtract_from_result (resultP, frag_off / OCTETS_PER_BYTE, 0);
+ add_to_result (resultP, symval_diff, symval_diff < 0);
resultP->X_op = O_constant;
resultP->X_add_symbol = 0;
}
@@ -1861,7 +1904,7 @@ expr (int rankarg, /* Larger # is highe
&& (md_register_arithmetic || resultP->X_op != O_register))
{
/* X - constant. */
- resultP->X_add_number -= right.X_add_number;
+ subtract_from_result (resultP, right.X_add_number, right.X_extrabit);
}
else if (op_left == O_add && resultP->X_op == O_constant
&& (md_register_arithmetic || right.X_op != O_register))
@@ -1870,7 +1913,7 @@ expr (int rankarg, /* Larger # is highe
resultP->X_op = right.X_op;
resultP->X_add_symbol = right.X_add_symbol;
resultP->X_op_symbol = right.X_op_symbol;
- resultP->X_add_number += right.X_add_number;
+ add_to_result (resultP, right.X_add_number, right.X_extrabit);
retval = rightseg;
}
else if (resultP->X_op == O_constant && right.X_op == O_constant)
@@ -1910,7 +1953,9 @@ expr (int rankarg, /* Larger # is highe
/* Constant + constant (O_add) is handled by the
previous if statement for constant + X, so is omitted
here. */
- case O_subtract: resultP->X_add_number -= v; break;
+ case O_subtract:
+ subtract_from_result (resultP, v, 0);
+ break;
case O_eq:
resultP->X_add_number =
resultP->X_add_number == v ? ~ (offsetT) 0 : 0;
@@ -1954,10 +1999,11 @@ expr (int rankarg, /* Larger # is highe
resultP->X_op = op_left;
resultP->X_op_symbol = right.X_add_symbol;
if (op_left == O_add)
- resultP->X_add_number += right.X_add_number;
+ add_to_result (resultP, right.X_add_number, right.X_extrabit);
else if (op_left == O_subtract)
{
- resultP->X_add_number -= right.X_add_number;
+ subtract_from_result (resultP, right.X_add_number,
+ right.X_extrabit);
if (retval == rightseg
&& SEG_NORMAL (retval)
&& !S_FORCE_RELOC (resultP->X_add_symbol, 0)
@@ -1977,6 +2023,7 @@ expr (int rankarg, /* Larger # is highe
resultP->X_op = op_left;
resultP->X_add_number = 0;
resultP->X_unsigned = 1;
+ resultP->X_extrabit = 0;
}
if (retval != rightseg)
Index: gas/expr.h
===================================================================
RCS file: /cvs/src/src/gas/expr.h,v
retrieving revision 1.21
diff -u -p -r1.21 expr.h
--- gas/expr.h 22 Jun 2010 07:43:40 -0000 1.21
+++ gas/expr.h 11 Apr 2013 10:39:26 -0000
@@ -136,6 +136,11 @@ typedef struct expressionS {
when performing arithmetic on these values).
FIXME: This field is not set very reliably. */
unsigned int X_unsigned : 1;
+ /* This is used to implement "word size + 1 bit" arithmetic, so that e.g.
+ expressions used with .sleb128 directives can use the full range available
+ for an unsigned word, but can also properly represent all values of a
+ signed word. */
+ unsigned int X_extrabit : 1;
/* 7 additional bits can be defined if needed. */
Index: gas/read.c
===================================================================
RCS file: /cvs/src/src/gas/read.c,v
retrieving revision 1.183
diff -u -p -r1.183 read.c
--- gas/read.c 8 Mar 2013 10:17:00 -0000 1.183
+++ gas/read.c 11 Apr 2013 10:39:27 -0000
@@ -1306,10 +1306,10 @@ read_a_source_file (char *name)
}
/* Convert O_constant expression EXP into the equivalent O_big representation.
- Take the sign of the number from X_unsigned rather than X_add_number. */
+ Take the sign of the number from SIGN rather than X_add_number. */
static void
-convert_to_bignum (expressionS *exp)
+convert_to_bignum (expressionS *exp, int sign)
{
valueT value;
unsigned int i;
@@ -1322,8 +1322,8 @@ convert_to_bignum (expressionS *exp)
}
/* Add a sequence of sign bits if the top bit of X_add_number is not
the sign of the original value. */
- if ((exp->X_add_number < 0) != !exp->X_unsigned)
- generic_bignum[i++] = exp->X_unsigned ? 0 : LITTLENUM_MASK;
+ if ((exp->X_add_number < 0) == !sign)
+ generic_bignum[i++] = sign ? LITTLENUM_MASK : 0;
exp->X_op = O_big;
exp->X_add_number = i;
}
@@ -4250,7 +4250,7 @@ emit_expr (expressionS *exp, unsigned in
if (op == O_constant && nbytes > sizeof (valueT))
{
extra_digit = exp->X_unsigned ? 0 : -1;
- convert_to_bignum (exp);
+ convert_to_bignum (exp, !exp->X_unsigned);
op = O_big;
}
@@ -4544,6 +4544,7 @@ parse_bitfield_cons (exp, nbytes)
exp->X_add_number = value;
exp->X_op = O_constant;
exp->X_unsigned = 1;
+ exp->X_extrabit = 0;
}
}
@@ -5103,12 +5104,12 @@ emit_leb128_expr (expressionS *exp, int
}
else if (op == O_constant
&& sign
- && (exp->X_add_number < 0) != !exp->X_unsigned)
+ && (exp->X_add_number < 0) == !exp->X_extrabit)
{
/* We're outputting a signed leb128 and the sign of X_add_number
doesn't reflect the sign of the original value. Convert EXP
to a correctly-extended bignum instead. */
- convert_to_bignum (exp);
+ convert_to_bignum (exp, exp->X_extrabit);
op = O_big;
}
Index: gas/testsuite/gas/all/gas.exp
===================================================================
RCS file: /cvs/src/src/gas/testsuite/gas/all/gas.exp,v
retrieving revision 1.80
diff -u -p -r1.80 gas.exp
--- gas/testsuite/gas/all/gas.exp 17 Dec 2012 16:55:48 -0000 1.80
+++ gas/testsuite/gas/all/gas.exp 11 Apr 2013 10:39:27 -0000
@@ -369,6 +369,11 @@ if { ![istarget "bfin-*-*"] } then {
run_dump_test assign
}
run_dump_test sleb128
+run_dump_test sleb128-2
+run_dump_test sleb128-3
+run_dump_test sleb128-4
+run_dump_test sleb128-5
+run_dump_test sleb128-7
# .byte is 32 bits on tic4x, and .p2align isn't supported on tic54x
# .space is different on hppa*-hpux.
Index: gas/testsuite/gas/all/sleb128-2.d
===================================================================
RCS file: gas/testsuite/gas/all/sleb128-2.d
diff -N gas/testsuite/gas/all/sleb128-2.d
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gas/testsuite/gas/all/sleb128-2.d 11 Apr 2013 10:39:27 -0000
@@ -0,0 +1,7 @@
+#objdump : -s -j .data
+#name : .sleb128 tests (2)
+
+.*: .*
+
+Contents of section \.data:
+ .* 7d2a.*
Index: gas/testsuite/gas/all/sleb128-2.s
===================================================================
RCS file: gas/testsuite/gas/all/sleb128-2.s
diff -N gas/testsuite/gas/all/sleb128-2.s
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gas/testsuite/gas/all/sleb128-2.s 11 Apr 2013 10:39:27 -0000
@@ -0,0 +1,13 @@
+.text
+.globl foo
+foo:
+.L1:
+.byte 0
+.byte 0
+.byte 0
+.L2:
+
+.data
+bar:
+.sleb128 .L1 - .L2
+.byte 42
Index: gas/testsuite/gas/all/sleb128-3.d
===================================================================
RCS file: gas/testsuite/gas/all/sleb128-3.d
diff -N gas/testsuite/gas/all/sleb128-3.d
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gas/testsuite/gas/all/sleb128-3.d 11 Apr 2013 10:39:27 -0000
@@ -0,0 +1,7 @@
+#objdump : -s -j .data
+#name : .sleb128 tests (3)
+
+.*: .*
+
+Contents of section \.data:
+ .* 9c7f2a.*
Index: gas/testsuite/gas/all/sleb128-3.s
===================================================================
RCS file: gas/testsuite/gas/all/sleb128-3.s
diff -N gas/testsuite/gas/all/sleb128-3.s
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gas/testsuite/gas/all/sleb128-3.s 11 Apr 2013 10:39:27 -0000
@@ -0,0 +1,4 @@
+.data
+bar:
+.sleb128 100 - 200
+.byte 42
Index: gas/testsuite/gas/all/sleb128-4.d
===================================================================
RCS file: gas/testsuite/gas/all/sleb128-4.d
diff -N gas/testsuite/gas/all/sleb128-4.d
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gas/testsuite/gas/all/sleb128-4.d 11 Apr 2013 10:39:27 -0000
@@ -0,0 +1,7 @@
+#objdump : -s -j .data
+#name : .sleb128 tests (4)
+
+.*: .*
+
+Contents of section \.data:
+ .* 83808080 082a.*
Index: gas/testsuite/gas/all/sleb128-4.s
===================================================================
RCS file: gas/testsuite/gas/all/sleb128-4.s
diff -N gas/testsuite/gas/all/sleb128-4.s
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gas/testsuite/gas/all/sleb128-4.s 11 Apr 2013 10:39:27 -0000
@@ -0,0 +1,13 @@
+.text
+.globl foo
+foo:
+.L1:
+.byte 0
+.byte 0
+.byte 0
+.L2:
+
+.data
+bar:
+.sleb128 .L2 - .L1 + (1 << 31)
+.byte 42
Index: gas/testsuite/gas/all/sleb128-5.d
===================================================================
RCS file: gas/testsuite/gas/all/sleb128-5.d
diff -N gas/testsuite/gas/all/sleb128-5.d
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gas/testsuite/gas/all/sleb128-5.d 11 Apr 2013 10:39:27 -0000
@@ -0,0 +1,7 @@
+#objdump : -s -j .data
+#name : .sleb128 tests (5)
+
+.*: .*
+
+Contents of section \.data:
+ .* 012a.*
Index: gas/testsuite/gas/all/sleb128-5.s
===================================================================
RCS file: gas/testsuite/gas/all/sleb128-5.s
diff -N gas/testsuite/gas/all/sleb128-5.s
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gas/testsuite/gas/all/sleb128-5.s 11 Apr 2013 10:39:27 -0000
@@ -0,0 +1,13 @@
+.text
+.globl foo
+foo:
+.L1:
+.byte 0
+.byte 0
+.byte 0
+.L2:
+
+.data
+bar:
+.sleb128 .L1 - .L2 + 4
+.byte 42
Index: gas/testsuite/gas/all/sleb128-7.d
===================================================================
RCS file: gas/testsuite/gas/all/sleb128-7.d
diff -N gas/testsuite/gas/all/sleb128-7.d
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gas/testsuite/gas/all/sleb128-7.d 11 Apr 2013 10:39:27 -0000
@@ -0,0 +1,7 @@
+#objdump : -s -j .data
+#name : .sleb128 tests (7)
+
+.*: .*
+
+Contents of section \.data:
+ .* cb012ac5 012acb01 2ac5012a.*
Index: gas/testsuite/gas/all/sleb128-7.s
===================================================================
RCS file: gas/testsuite/gas/all/sleb128-7.s
diff -N gas/testsuite/gas/all/sleb128-7.s
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gas/testsuite/gas/all/sleb128-7.s 11 Apr 2013 10:39:27 -0000
@@ -0,0 +1,19 @@
+.text
+.globl foo
+foo:
+.L1:
+.byte 0
+.byte 0
+.byte 0
+.L2:
+
+.data
+bar:
+.sleb128 200+(.L2 - .L1)
+.byte 42
+.sleb128 200+(.L1 - .L2)
+.byte 42
+.sleb128 (.L2 - .L1)+200
+.byte 42
+.sleb128 (.L1 - .L2)+200
+.byte 42