Fix gas/22304 by forcing a 64-bit bfd. Suggest the same for other targets.

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

Fix gas/22304 by forcing a 64-bit bfd. Suggest the same for other targets.

Hans-Peter Nilsson
I had a look at the gas tests failing for cris*-* with a 32-bit-bfd
(non-bfd64-hosts) again.  The last time I did that was between some 20
years ago and the time 64-bit hosts became the norm.  I may be able to
fix them, if I rewrite the whole of the gas expression handler...

(
I started trying to make use of the out_insnp->expr.X_unsigned field
to help identify the 32-bit-long-wrapping-issue that is the root
cause, until I found that a target, through the gas expression parser
(calling expr, looking at the resulting struct expressionS) can't tell
the difference between:

 .set mforty2, -42
 add.b mforty2,r5

and:

 add.b 0xffffffd6,r5

and since my goal was to detect the last line as invalid assembly
(can't fit a 32-bit constant into a byte-size operand), I decided to
climb up from *that* rabbit-hole.
)

Alternatively, I can hide the issue by improving the test-suite
machinery: import the gcc effective-target machinery *and* find out a
way to identity whether or not --enable-64-bit-bfd is in effect (which
is *not* as easy as checking the elf-class of the built linker mind
you), *then* add some effective-target-xfail identifier, *just* to be
able to xfail or kfail the failing tests for non-bfd64 builds...

But, I'm not going to do either of that.  Instead I took the easy
route and just force cris*-* to a 64-bit bfd.  On "modern machines",
the only people that will notice a difference are those configuring
with effectively CC="gcc -m32".  (Regrettably, there's no critical
mass for ILP32 ABI's on "64-bit-systems", even though that generally
has been found to have a performance benefit in the cases when you
don't really need the 64-bit address-space.)  If there are any users
with 32-bit systems, the solution is on the good side as it fixes the
buggy behavior.  Well, unless you strongly dislike binutils carrying
around 32 extra bits for the correctness of semi-obscure assembler
expressions.  For everyone else, these tests will just keep passing.
They actually used to XPASS for bfd64 builds, before a recent edit,
which is the reason for this...journey; cf. bugzilla.

I'm also cleaning up some quoting on those tests left-over from the
recent edit.

Maybe it's a good idea to force bfd64 for all targets: remove the
bfd64 machinery equivalent to setting it to true, as you can get bad
code from the buggy expression handling.  Though, I guess you can
construct a just-as-failing 64-bit test for bfd64...

Here's a generic reproducer test-case taken from one of the failing
tests, that will fail for all 32-bit-bfd builds.  I'm not going to add
it as-is, as there's no easy way to kfail it, where it's known to fail
(see rant above: it you've found a target specifier, try again, this
time with --enable-64-bit-bfd on a "32-bit host").

diff --git a/gas/testsuite/gas/all/gas.exp b/gas/testsuite/gas/all/gas.exp
index 942b0b4..53a6888 100644
--- a/gas/testsuite/gas/all/gas.exp
+++ b/gas/testsuite/gas/all/gas.exp
@@ -489,3 +489,4 @@ run_dump_test "org-5"
 run_dump_test "org-6"
 
 run_dump_test "fill-1"
+run_dump_test "shexpr"
diff --git a/gas/testsuite/gas/all/shexpr.d b/gas/testsuite/gas/all/shexpr.d
new file mode 100644
index 0000000..fee036a
--- /dev/null
+++ b/gas/testsuite/gas/all/shexpr.d
@@ -0,0 +1,7 @@
+#objdump: -s -j .text
+
+.*:     file format .*
+
+Contents of section \.text:
+[ ]+0+[ ]+(286fff0b|0bff6f28)[ ]+.*
+
diff --git a/gas/testsuite/gas/all/shexpr.s b/gas/testsuite/gas/all/shexpr.s
new file mode 100644
index 0000000..8eaaa8f
--- /dev/null
+++ b/gas/testsuite/gas/all/shexpr.s
@@ -0,0 +1,2 @@
+ .text
+ .dc.l ((0x17<<23)+((0xfede4194/8192)<<4)+8)

Anyway, the following is what I committed.

bfd:
        PR gas/22304
        * config.bfd (cris-*-* | crisv32-*-*): Require a 64-bit BFD.

diff --git a/bfd/config.bfd b/bfd/config.bfd
index dc24aab..62f8ac3 100644
--- a/bfd/config.bfd
+++ b/bfd/config.bfd
@@ -500,6 +500,7 @@ case "${targ}" in
     targ_underscore=yes
     ;;
 
+#ifdef BFD64
   cris-*-* | crisv32-*-*)
     targ_defvec=cris_aout_vec
     targ_selvecs="cris_elf32_us_vec cris_elf32_vec ieee_vec"
@@ -507,7 +508,9 @@ case "${targ}" in
  *-*-linux*) ;;
  *) targ_underscore=yes ;;
     esac
+    want64=true
     ;;
+#endif
 
   crx-*-elf*)
     targ_defvec=crx_elf32_vec

brgds, H-P
Reply | Threaded
Open this post in threaded view
|

Re: Fix gas/22304 by forcing a 64-bit bfd. Suggest the same for other targets.

Hans-Peter Nilsson
> Date: Sun, 22 Oct 2017 13:23:07 +0200
> From: Hans-Peter Nilsson <[hidden email]>

> Maybe it's a good idea to force bfd64 for all targets: remove the
> bfd64 machinery equivalent to setting it to true, as you can get bad
> code from the buggy expression handling.  Though, I guess you can
> construct a just-as-failing 64-bit test for bfd64...
>
> Here's a generic reproducer test-case taken from one of the failing
> tests, that will fail for all 32-bit-bfd builds.

I opened PR gas/22335 for this.

brgds, H-P
Reply | Threaded
Open this post in threaded view
|

Re: Fix gas/22304 by forcing a 64-bit bfd. Suggest the same for other targets.

Joseph Myers
In reply to this post by Hans-Peter Nilsson
On Sun, 22 Oct 2017, Hans-Peter Nilsson wrote:

> Maybe it's a good idea to force bfd64 for all targets: remove the
> bfd64 machinery equivalent to setting it to true, as you can get bad
> code from the buggy expression handling.  Though, I guess you can
> construct a just-as-failing 64-bit test for bfd64...

That would seem consistent with GCC now always using 64-bit HOST_WIDE_INT,
rather than only requiring it for certain targets.

--
Joseph S. Myers
[hidden email]