[binutils-gdb] Fix "bins" simulation for v850e3v5

[binutils-gdb] Fix "bins" simulation for v850e3v5

[Jeff Law]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=49fffa58f7e6da777d10fe77663bc7c8f531fe7f

commit 49fffa58f7e6da777d10fe77663bc7c8f531fe7f
Author: Jeff Law <jeffreyalaw@gmail.com>
Date:   Wed Apr 6 11:06:53 2022 -0400

    Fix "bins" simulation for v850e3v5
    
    I've been carrying this for a few years.   One test in the GCC testsuite is
    failing due to a bug in the handling of the v850e3v5 instruction "bins".
    
    When the "bins" instruction specifies a 32bit bitfield size, the simulator
    exhibits undefined behavior by trying to shift a 32 bit quantity by 32 bits.
    In the case of a 32 bit shift, we know what the resultant mask should be.  So
    we can just set it.
    
    That seemed better than using 1UL for the constant (on a 32bit host unsigned
    long might still just be 32 bits) or needlessly forcing everything to
    long long types.
    
    Thankfully the case where this shows up is only bins <src>, 0, 32, <dest>
    which would normally be encoded as a simple move.
    
            * testsuite/v850/allinsns.exp: Add v850e3v5.
            * testsuite/v850/bins.cgs: New test.
            * v850/simops.c (v850_bins): Avoid undefined behavior on left shift.

Diff:
---
 sim/testsuite/v850/allinsns.exp |  2 +-
 sim/testsuite/v850/bins.cgs     | 12 ++++++++++++
 sim/v850/simops.c               |  9 ++++++++-
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/sim/testsuite/v850/allinsns.exp b/sim/testsuite/v850/allinsns.exp
index 4cc461c30fd..ee22a5d93b7 100644
--- a/sim/testsuite/v850/allinsns.exp
+++ b/sim/testsuite/v850/allinsns.exp
@@ -5,7 +5,7 @@ sim_init
 # All machines.
 # Should add more cpus if the testsuite adds coverage for their insns, but
 # at the core level, there's no deviation beyond these two.
-set all_machs "v850e v850"
+set all_machs "v850e3v5 v850e v850"
 
 # gas doesn't support any '=' option for v850.
 set cpu_option_sep ""
diff --git a/sim/testsuite/v850/bins.cgs b/sim/testsuite/v850/bins.cgs
new file mode 100644
index 00000000000..dedc5ceaafd
--- /dev/null
+++ b/sim/testsuite/v850/bins.cgs
@@ -0,0 +1,12 @@
+# v850 bins
+# mach: v850e3v5
+# as: -mv850e3v5
+
+	.include "testutils.inc"
+
+	seti	0x7fff, r10
+	seti	0x0, r11
+	bins	r10, 0, 32, r11
+	reg	r11, 0x7fff
+
+	pass
diff --git a/sim/v850/simops.c b/sim/v850/simops.c
index d2640577fc8..e9a5d489d88 100644
--- a/sim/v850/simops.c
+++ b/sim/v850/simops.c
@@ -3267,7 +3267,14 @@ v850_bins (SIM_DESC sd, unsigned int source, unsigned int lsb, unsigned int msb,
   pos = lsb;
   width = (msb - lsb) + 1;
 
-  mask = ~ (-(1 << width));
+  /* A width of 32 exhibits undefined behavior on the shift.  The easiest
+     way to make this code safe is to just avoid that case and set the mask
+     to the right value.  */
+  if (width >= 32)
+    mask = 0xffffffff;
+  else
+    mask = ~ (-(1 << width));
+
   source &= mask;
   mask <<= pos;
   result = (* dest) & ~ mask;