[PATCH] uniform behaviour of sleb128

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] uniform behaviour of sleb128

Tristan Gingold-2
Hello,

the current sequence of bytes for this sleb128 depends on whether BFD_ARCH_SIZE is 32 or 64:

   1               .data
   2 0000 FFFFFFFF .sleb128 281474976710655
   2      FFFF3F

or

   1               .data
   2 0000 7F       .sleb128 281474976710655

This is of course a real issue as gcc may generate this sleb128 number and gcc may hard
code the .debug_info section length.  In that case the object file is incorrect and
prevents debugging.

The patch fixes this issue by extended unsigned big number that would otherwise be reduced by sleb128.
(this is a complete rework of a previous patch).

No failure on i386-gnu-linux.

Ok to apply ?

Tristan.

diff --git a/gas/ChangeLog b/gas/ChangeLog
index 2d72b78..5f34823 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,11 @@
+2017-01-09  Tristan Gingold  <[hidden email]>
+
+ * read.c (emit_leb128_expr): Extended unsigned big number for
+ sleb128.
+ * testsuite/gas/all/gas.exp (test_cond): Add sleb128-8 test.
+ * testsuite/gas/all/sleb128.d: New test.
+ * testsuite/gas/all/sleb128.s: New test source.
+
 2017-01-09  Nick Clifton  <[hidden email]>
 
  * po/sv.po: New Swedish translation.
diff --git a/gas/read.c b/gas/read.c
index 5c0d320..3669b28 100644
--- a/gas/read.c
+++ b/gas/read.c
@@ -5344,13 +5344,21 @@ emit_leb128_expr (expressionS *exp, int sign)
   else if (op == O_big)
     {
       /* O_big is a different sort of constant.  */
-
+      int nbr_digits = exp->X_add_number;
       unsigned int size;
       char *p;
 
-      size = output_big_leb128 (NULL, generic_bignum, exp->X_add_number, sign);
+      /* If the leading littenum is 0xffff, prepend a 0 to avoid confusion with
+ a signed number.  Unary operators like - or ~ always extend the
+ bignum to its largest size.  */
+      if (exp->X_unsigned
+  && nbr_digits < SIZE_OF_LARGE_NUMBER
+  && generic_bignum[nbr_digits - 1] == LITTLENUM_MASK)
+ generic_bignum[nbr_digits++] = 0;
+
+      size = output_big_leb128 (NULL, generic_bignum, nbr_digits, sign);
       p = frag_more (size);
-      if (output_big_leb128 (p, generic_bignum, exp->X_add_number, sign) > size)
+      if (output_big_leb128 (p, generic_bignum, nbr_digits, sign) > size)
  abort ();
     }
   else
diff --git a/gas/testsuite/gas/all/gas.exp b/gas/testsuite/gas/all/gas.exp
index 8f97ed8..6b5aec0 100644
--- a/gas/testsuite/gas/all/gas.exp
+++ b/gas/testsuite/gas/all/gas.exp
@@ -393,6 +393,7 @@ run_dump_test sleb128-5
 if { ![istarget "tic4x*-*-*"] && ![istarget "tic54x*-*-*"] } {
     run_dump_test sleb128-7
 }
+run_dump_test sleb128-8
 
 # .byte is 32 bits on tic4x, and .p2align isn't supported on tic54x
 # .space is different on hppa*-hpux.
diff --git a/gas/testsuite/gas/all/sleb128-8.d b/gas/testsuite/gas/all/sleb128-8.d
new file mode 100644
index 0000000..793337c
--- /dev/null
+++ b/gas/testsuite/gas/all/sleb128-8.d
@@ -0,0 +1,7 @@
+#objdump : -s -j .data -j "\$DATA\$"
+#name : .sleb128 tests (8)
+
+.*: .*
+
+Contents of section (\.data|\$DATA\$):
+ 0000 ffffffff ffff3f .*
diff --git a/gas/testsuite/gas/all/sleb128-8.s b/gas/testsuite/gas/all/sleb128-8.s
new file mode 100644
index 0000000..ab3e785
--- /dev/null
+++ b/gas/testsuite/gas/all/sleb128-8.s
@@ -0,0 +1,2 @@
+ .data
+ .sleb128 281474976710655

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] uniform behaviour of sleb128

Nick Clifton
Hi Tristan,

> Ok to apply ?

Approved - please apply.

Cheers
  Nick

> + * read.c (emit_leb128_expr): Extended unsigned big number for
> + sleb128.
> + * testsuite/gas/all/gas.exp (test_cond): Add sleb128-8 test.
> + * testsuite/gas/all/sleb128.d: New test.
> + * testsuite/gas/all/sleb128.s: New test source.

(I assume that you know that changelog patches rarely apply cleanly...)

Cheers
  Nick

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] uniform behaviour of sleb128

Tristan Gingold-2

> On 09 Jan 2017, at 17:06, Nick Clifton <[hidden email]> wrote:
>
> Hi Tristan,
>
>> Ok to apply ?
>
> Approved - please apply.

Thanks, now committed (with the ChangeLog updated).

Tristan.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] uniform behaviour of sleb128

Nick Clifton
Hi Tristan,

> Thanks, now committed (with the ChangeLog updated).

Oops - there appear to be some regressions:

Checking Binutils in: arm-wince-pe ...     GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: bfin-elf ...         GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: mcore-pe ...         GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: mipsel-linux-gnu ... GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: s390-linux ...       GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: sh-pe ...            GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: tic54x-coff ...      GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: i686-pc-mingw32 ...  GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: mingw32-pe ...       GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: pdp11-dec-aout ...   GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: z8k-coff ...         GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: alpha-linuxecoff ... GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: i386-pc-go32 ...     GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: i686-pc-cygwin ...   GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: mipsisa32el-linux ...GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: powerpc-ibm-aix5.2.0 GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: rs6000-aix4.3.3 ...  GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: s390x-ibm-tpf ...    GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: alpha-unknown-osf4.0 GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: mips-sgi-irix6 ...   GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: mips64-linux ...     GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: sparc-aout ...       GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: tic30-unknown-aout . GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: tic4x-coff ...       GAS REGRESSION: .sleb128 tests (8)  
Checking Binutils in: x86_64-pc-cygwin ... GAS REGRESSION: .sleb128 tests (8)  

Could you have a look at them please ?

Cheers
  Nick

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] uniform behaviour of sleb128

Tristan Gingold-2

> On 10 Jan 2017, at 12:09, Nick Clifton <[hidden email]> wrote:
>
> Hi Tristan,
>
>> Thanks, now committed (with the ChangeLog updated).
>
> Oops - there appear to be some regressions:
>
> Checking Binutils in: arm-wince-pe ...     GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: bfin-elf ...         GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: mcore-pe ...         GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: mipsel-linux-gnu ... GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: s390-linux ...       GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: sh-pe ...            GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: tic54x-coff ...      GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: i686-pc-mingw32 ...  GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: mingw32-pe ...       GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: pdp11-dec-aout ...   GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: z8k-coff ...         GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: alpha-linuxecoff ... GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: i386-pc-go32 ...     GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: i686-pc-cygwin ...   GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: mipsisa32el-linux ...GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: powerpc-ibm-aix5.2.0 GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: rs6000-aix4.3.3 ...  GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: s390x-ibm-tpf ...    GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: alpha-unknown-osf4.0 GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: mips-sgi-irix6 ...   GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: mips64-linux ...     GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: sparc-aout ...       GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: tic30-unknown-aout . GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: tic4x-coff ...       GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: x86_64-pc-cygwin ... GAS REGRESSION: .sleb128 tests (8)  
>
> Could you have a look at them please ?

Sure.  I suppose this is an alignment issue, shouldn't be hard to adjust the testcase.
Thanks for the head up
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] uniform behaviour of sleb128

Tristan Gingold-2
In reply to this post by Nick Clifton

> On 10 Jan 2017, at 12:09, Nick Clifton <[hidden email]> wrote:
>
> Hi Tristan,
>
>> Thanks, now committed (with the ChangeLog updated).
>
> Oops - there appear to be some regressions:
>
> Checking Binutils in: arm-wince-pe ...     GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: bfin-elf ...         GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: mcore-pe ...         GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: mipsel-linux-gnu ... GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: s390-linux ...       GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: sh-pe ...            GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: tic54x-coff ...      GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: i686-pc-mingw32 ...  GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: mingw32-pe ...       GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: pdp11-dec-aout ...   GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: z8k-coff ...         GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: alpha-linuxecoff ... GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: i386-pc-go32 ...     GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: i686-pc-cygwin ...   GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: mipsisa32el-linux ...GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: powerpc-ibm-aix5.2.0 GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: rs6000-aix4.3.3 ...  GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: s390x-ibm-tpf ...    GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: alpha-unknown-osf4.0 GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: mips-sgi-irix6 ...   GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: mips64-linux ...     GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: sparc-aout ...       GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: tic30-unknown-aout . GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: tic4x-coff ...       GAS REGRESSION: .sleb128 tests (8)  
> Checking Binutils in: x86_64-pc-cygwin ... GAS REGRESSION: .sleb128 tests (8)  
>
> Could you have a look at them please ?

This patchlet fixes most regressions, but not on tic4x-coff.
I am investigating.

Tristan.

diff --git a/gas/testsuite/gas/all/sleb128-8.d b/gas/testsuite/gas/all/sleb128-8.d
index 793337c..6190f26 100644
--- a/gas/testsuite/gas/all/sleb128-8.d
+++ b/gas/testsuite/gas/all/sleb128-8.d
@@ -4,4 +4,4 @@
 .*: .*
 
 Contents of section (\.data|\$DATA\$):
- 0000 ffffffff ffff3f .*
+ 0000 ffffffff ffff3f.*

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] uniform behaviour of sleb128

Nick Clifton
Hi Tristan,

> This patchlet fixes most regressions, but not on tic4x-coff.

Patchlet approved...

Cheers
  Nick

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] uniform behaviour of sleb128

Tristan Gingold-2

> On 10 Jan 2017, at 13:31, Nick Clifton <[hidden email]> wrote:
>
> Hi Tristan,
>
>> This patchlet fixes most regressions, but not on tic4x-coff.
>
> Patchlet approved...

committed, with disabling the test on tic54x and tic4x (like was the slib128-7 test).

Tell me if any regression is still present as I haven't tested all the combinaisons.

Thanks!
Tristan.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] uniform behaviour of sleb128

Nick Clifton
Hi Tristan,

> committed, with disabling the test on tic54x and tic4x (like was the slib128-7 test).
>
> Tell me if any regression is still present as I haven't tested all the combinaisons.

Nope - I think that you got them all.

Cheers
  Nick