[PATCH] Fix sleb128 encoding for difference expressions

[Julian Brown <julian at codesourcery dot com>]

Hi,

We encountered a situation where .sleb128 directives would behave in an
unexpected way. For a source file such as:

    .text
    .globl foo
foo:
.L1:
    .byte 0
    .byte 0
    .byte 0
.L2:

    .data
bar:
    .sleb128 .L1 - .L2
    .byte 42

assembled and objdump'd, we get:

Contents of section .data:
 0000 fdffffff ffffffff ff012a00 00000000  ..........*.....

the value of "-3" has been interpreted as a (64-bit) unsigned quantity
0xfffffffffffffffd, and henceforth encoded as a sleb128 value, whereas
what we wanted was simply the encoding "7d", i.e. -3.

The clause in read.c:emit_leb128_expr is a clue as to why this is
happening:

  else if (op == O_constant
	   && sign
	   && (exp->X_add_number < 0) != !exp->X_unsigned)
    {
      /* 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);
      op = O_big;
    }

so in this case we have exp->X_add_number less than zero, but
exp->X_unsigned is true: the operands (.L1, .L2) are unsigned, so the
result of the expression .L1 - .L2 is also unsigned. Hence,
convert_to_bignum converts "exp" to a bignum as an unsigned expression.

My proposed fix is a change to the ad-hoc type system for expressions.
The basic idea is simply to see if the result of a subtraction
operation will fall below zero, and if so force the "unsigned" flag for
the result value to be false (normally all of an expression's operands,
and its result, are unsigned). No other operators are altered.

This still works for expressions such as,

    .sleb128 .L2 - .L1 + (1 << 31)
(or .sleb128 .L2 - .L1 + (1 << 63))

where L2 is greater than L1, i.e. where the sign bit "looks like" it is
set, since the result of these will still be regarded as unsigned.
There may be other corner cases which behave unexpectedly, however.

OK to apply, or any comments? Maybe suggestions for other ways of
tackling the problem? (Tested with cross to mips-linux.)

Thanks,

Julian

ChangeLog

    gas/
    * expr.c (subtract_from_result): New.
    (expr): Use above.

    gas/testsuite/
    * gas/all/sleb128-2.s: New test.
    * gas/all/sleb128-3.s: Likewise.
    * gas/all/sleb128-4.s: Likewise.
    * gas/all/sleb128-2.d: New.
    * gas/all/sleb128-3.d: New.
    * gas/all/sleb123-4.d: New.
    * gas/all/gas.exp (sleb128-2, sleb128-3, sleb128-4): Run new tests.

Index: gas/testsuite/gas/all/sleb128-3.d
===================================================================
--- gas/testsuite/gas/all/sleb128-3.d	(revision 0)
+++ gas/testsuite/gas/all/sleb128-3.d	(revision 0)
@@ -0,0 +1,7 @@
+#objdump : -s -j .data
+#name : .sleb128 tests (3)
+
+.*: .*
+
+Contents of section \.data:
+ 0000 9c7f2a00 00000000 00000000 00000000  .*
Index: gas/testsuite/gas/all/gas.exp
===================================================================
--- gas/testsuite/gas/all/gas.exp	(revision 393919)
+++ gas/testsuite/gas/all/gas.exp	(working copy)
@@ -352,6 +352,9 @@ 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
 
 # .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-3.s
===================================================================
--- gas/testsuite/gas/all/sleb128-3.s	(revision 0)
+++ gas/testsuite/gas/all/sleb128-3.s	(revision 0)
@@ -0,0 +1,13 @@
+.text
+.globl foo
+foo:
+.L1:
+.byte 0
+.byte 0
+.byte 0
+.L2:
+
+.data
+bar:
+.sleb128 100 - 200
+.byte 42
Index: gas/testsuite/gas/all/sleb128-2.d
===================================================================
--- gas/testsuite/gas/all/sleb128-2.d	(revision 0)
+++ gas/testsuite/gas/all/sleb128-2.d	(revision 0)
@@ -0,0 +1,7 @@
+#objdump : -s -j .data
+#name : .sleb128 tests (2)
+
+.*: .*
+
+Contents of section \.data:
+ 0000 7d2a0000 00000000 00000000 00000000  .*
Index: gas/testsuite/gas/all/sleb128-4.d
===================================================================
--- gas/testsuite/gas/all/sleb128-4.d	(revision 0)
+++ gas/testsuite/gas/all/sleb128-4.d	(revision 0)
@@ -0,0 +1,7 @@
+#objdump : -s -j .data
+#name : .sleb128 tests (4)
+
+.*: .*
+
+Contents of section \.data:
+ 0000 83808080 082a0000 00000000 00000000  .*
Index: gas/testsuite/gas/all/sleb128-2.s
===================================================================
--- gas/testsuite/gas/all/sleb128-2.s	(revision 0)
+++ gas/testsuite/gas/all/sleb128-2.s	(revision 0)
@@ -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-4.s
===================================================================
--- gas/testsuite/gas/all/sleb128-4.s	(revision 0)
+++ gas/testsuite/gas/all/sleb128-4.s	(revision 0)
@@ -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/expr.c
===================================================================
--- gas/expr.c	(revision 393919)
+++ gas/expr.c	(working copy)
@@ -1717,6 +1717,17 @@ operatorf (int *num_chars)
   /* NOTREACHED  */
 }
 
+/* Subtract an amount from RESULTP's X_add_number field, making the result
+   signed if it falls below zero.  */
+
+static void
+subtract_from_result (expressionS *resultP, offsetT amount)
+{
+  if (amount > resultP->X_add_number)
+    resultP->X_unsigned = 0;
+  resultP->X_add_number -= amount;
+}
+
 /* Parse an expression.  */
 
 segT
@@ -1847,10 +1858,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 sym_neg_diff = S_GET_VALUE (right.X_add_symbol)
+				 - S_GET_VALUE (resultP->X_add_symbol);
+	  subtract_from_result (resultP, right.X_add_number);
+	  subtract_from_result (resultP, frag_off / OCTETS_PER_BYTE);
+	  subtract_from_result (resultP, sym_neg_diff);
 	  resultP->X_op = O_constant;
 	  resultP->X_add_symbol = 0;
 	}
@@ -1858,7 +1870,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);
 	}
       else if (op_left == O_add && resultP->X_op == O_constant
 	       && (md_register_arithmetic || right.X_op != O_register))
@@ -1907,7 +1919,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);
+	      break;
 	    case O_eq:
 	      resultP->X_add_number =
 		resultP->X_add_number == v ? ~ (offsetT) 0 : 0;
@@ -1954,7 +1968,7 @@ expr (int rankarg,		/* Larger # is highe
 	    resultP->X_add_number += right.X_add_number;
 	  else if (op_left == O_subtract)
 	    {
-	      resultP->X_add_number -= right.X_add_number;
+	      subtract_from_result (resultP, right.X_add_number);
 	      if (retval == rightseg
 		  && SEG_NORMAL (retval)
 		  && !S_FORCE_RELOC (resultP->X_add_symbol, 0)