gas: special-case division / modulo by ±1

Dividing the largest possible negative value by -1 generally is UB, for
the result not being representable at least in commonly used binary
notation. This UB on x86, for example, is a Floating Point Exception on
Linux, i.e. resulting in an internal error (albeit only when
sizeof(valueT) == sizeof(void *); the library routine otherwise involved
apparently deals with the inputs quite okay).

Leave original values unaltered for division by 1; this may matter down
the road, in case we start including X_unsigned and X_extrabit in
arithmetic. For the same reason treat modulo by 1 the same as modulo by
-1.

The quad and octa tests have more relaxed expecations than intended, for
X_unsigned and X_extrabit not being taken into account [yet]. The upper
halves can wrongly end up as all ones (for .octa, when !BFD64, even the
upper three quarters). Yet it makes little sense to address this just
for div/mod by ±1. quad-div2 is yet more special, to cover for most
32-bit targets being unable to deal with forward-ref expressions in
.quad even when BFD64; even ones being able to (like x86) then still
don't get the values right.

diff --git a/gas/expr.c b/gas/expr.c
index 3004b958314eda5ae49a70f90f572f5eea3b7861..5e59b7c5fa16b0cd54815136c06687dbaf1d8011 100644 (file)
--- a/gas/expr.c
+++ b/gas/expr.c
@@ -2015,8 +2015,32 @@ expr (int rankarg,               /* Larger # is higher rank.  */
                 bits of the result.  */
              resultP->X_add_number *= (valueT) v;
              break;
-           case O_divide:              resultP->X_add_number /= v; break;
-           case O_modulus:             resultP->X_add_number %= v; break;
+
+           case O_divide:
+             if (v == 1)
+               break;
+             if (v == -1)
+               {
+                 /* Dividing the largest negative value representable in offsetT
+                    by -1 has a non-representable result in common binary
+                    notation.  Treat it as negation instead, carried out as an
+                    unsigned operation to avoid UB.  */
+                 resultP->X_add_number = - (valueT) resultP->X_add_number;
+               }
+             else
+               resultP->X_add_number /= v;
+             break;
+
+           case O_modulus:
+             /* See above for why in particular -1 needs special casing.
+                While the operation is UB in C, mathematically it has a well-
+                defined result.  */
+             if (v == 1 || v == -1)
+               resultP->X_add_number = 0;
+             else
+               resultP->X_add_number %= v;
+             break;
+
            case O_left_shift:
            case O_right_shift:
              /* We always use unsigned shifts.  According to the ISO
@@ -2372,12 +2396,22 @@ resolve_expression (expressionS *expressionP)
        case O_divide:
          if (right == 0)
            return 0;
-         left = (offsetT) left / (offsetT) right;
+         /* See expr() for reasons of the special casing.  */
+         if (right == 1)
+           break;
+         if ((offsetT) right == -1)
+           left = -left;
+         else
+           left = (offsetT) left / (offsetT) right;
          break;
        case O_modulus:
          if (right == 0)
            return 0;
-         left = (offsetT) left % (offsetT) right;
+         /* Again, see expr() for reasons of the special casing.  */
+         if (right == 1 || (offsetT) right == -1)
+           left = 0;
+         else
+           left = (offsetT) left % (offsetT) right;
          break;
        case O_left_shift:
          if (right >= sizeof (left) * CHAR_BIT)

diff --git a/gas/symbols.c b/gas/symbols.c
index 50c8c9dcd8fa7d0c1f9a0fe1aeedbc1222631778..e6f3d85a75d31adbdad8fbab0e4c3cf095eab172 100644 (file)
--- a/gas/symbols.c
+++ b/gas/symbols.c
@@ -1723,8 +1723,27 @@ resolve_symbol_value (symbolS *symp)
            {
            /* See expr() for reasons of the use of valueT casts here.  */
            case O_multiply:            left *= (valueT) right; break;
-           case O_divide:              left /= right; break;
-           case O_modulus:             left %= right; break;
+
+           /* See expr() for reasons of the special casing.  */
+           case O_divide:
+             if (right == 1)
+               break;
+             if (right == -1)
+               {
+                 left = -left;
+                 break;
+               }
+             left /= right;
+             break;
+
+           /* Again, see expr() for reasons of the special casing.  */
+           case O_modulus:
+             if (right == 1 || right == -1)
+               left = 0;
+             else
+               left %= right;
+             break;
+
            case O_left_shift:
              left = (valueT) left << (valueT) right; break;
            case O_right_shift:

diff --git a/gas/testsuite/gas/all/gas.exp b/gas/testsuite/gas/all/gas.exp
index bac0aff0b6d7dadd41e7f38032c33a7cac117897..f8bab3c2bd179081a8d26554a17cff3b325b405f 100644 (file)
--- a/gas/testsuite/gas/all/gas.exp
+++ b/gas/testsuite/gas/all/gas.exp
@@ -449,6 +449,22 @@ if { ![istarget "pdp11-*-*"] } {
     run_dump_test octa
 }
 
+# Some x86 flavors use '/' as a comment char, yet that can be suppressed.
+# Some other target use '/' or '%' as a comment char, without a way to
+# suppress it.
+if { [istarget "i?86-*-*"] || [istarget "x86_64-*-*"] } {
+    run_dump_test quad-div {{as --divide}}
+    run_dump_test quad-div2 {{as --divide}}
+    run_dump_test octa-div {{as --divide}}
+} elseif { ![istarget "mcore-*-*"]
+          && ![istarget "mmix-*-*"]
+          && ![istarget "pdp11-*-*"]
+          && ![istarget "pj*-*-*"] } {
+    run_dump_test quad-div
+    run_dump_test quad-div2
+    run_dump_test octa-div
+}
+
 # .set works differently on some targets.
 switch -glob $target_triplet {
     alpha*-*-* { }

diff --git a/gas/testsuite/gas/all/octa-div.d b/gas/testsuite/gas/all/octa-div.d
new file mode 100644 (file)
index 0000000..bd69a74
--- /dev/null
+++ b/gas/testsuite/gas/all/octa-div.d
@@ -0,0 +1,10 @@
+#objdump: -s -j .data
+#name: .octa with division
+
+.*: +file format .*
+
+Contents of section .data:
+ [^ ]* (00000080 ........ ........ ........|........ ........ ........ 80000000) .*
+ [^ ]* (00000000 ........ ........ ........|........ ........ ........ 00000000) .*
+ [^ ]* (00000000 00000080 ........ ........|........ ........ 80000000 00000000) .*
+ [^ ]* (00000000 00000000 ........ ........|........ ........ 00000000 00000000) .*

diff --git a/gas/testsuite/gas/all/octa-div.s b/gas/testsuite/gas/all/octa-div.s
new file mode 100644 (file)
index 0000000..e80f562
--- /dev/null
+++ b/gas/testsuite/gas/all/octa-div.s
@@ -0,0 +1,11 @@
+       .data
+       .octa -0x80000000 / -1
+       .octa -0x80000000 % -1
+       .if ((1 << 16) << 16) <> 0
+       .octa -0x8000000000000000 / -1
+       .octa -0x8000000000000000 % -1
+       .else
+       /* Not really useful fallback for non-BFD64 targets.  */
+       .octa 0x8000000000000000
+       .octa 0x0000000000000000
+       .endif

diff --git a/gas/testsuite/gas/all/quad-div.d b/gas/testsuite/gas/all/quad-div.d
new file mode 100644 (file)
index 0000000..36d52d6
--- /dev/null
+++ b/gas/testsuite/gas/all/quad-div.d
@@ -0,0 +1,9 @@
+#objdump : -s -j .data
+#name : .quad with division
+
+.*: .*
+
+Contents of section (\.data|\$DATA\$):
+ 0000 (........ 80000000 ........ 00000000|00000080 ........ 00000000 ........) .*
+ 00.. (80000000 00000000 00000000 00000000|00000000 00000080 00000000 00000000) .*
+#pass

diff --git a/gas/testsuite/gas/all/quad-div.s b/gas/testsuite/gas/all/quad-div.s
new file mode 100644 (file)
index 0000000..a5824d8
--- /dev/null
+++ b/gas/testsuite/gas/all/quad-div.s
@@ -0,0 +1,32 @@
+       .data
+
+       .eqv INT_MAX, 0x7fffffff
+       .eqv INT_MIN, -INT_MAX - 1
+
+       /* Note: Shifts are uniformly done as unsigned.  */
+       .rept (INT_MIN / -1) >> 31
+       .quad -0x80000000 / -1
+       .endr
+       .rept (INT_MIN % -1) + 1
+       .quad -0x80000000 % -1
+       .endr
+
+       .if ((1 << 16) << 16) <> 0
+
+       .eqv LONG_MAX, 0x7fffffffffffffff
+       .eqv LONG_MIN, -LONG_MAX - 1
+
+       /* Note: Shifts are uniformly done as unsigned.  */
+       .rept (LONG_MIN / -1) >> 63
+       .quad -0x8000000000000000 / -1
+       .endr
+       .rept (LONG_MIN % -1) + 1
+       .quad -0x8000000000000000 % -1
+       .endr
+
+       .else /* Not really useful fallback for non-BFD64 targets.  */
+
+       .quad 0x8000000000000000
+       .quad 0x0000000000000000
+
+       .endif

diff --git a/gas/testsuite/gas/all/quad-div2.d b/gas/testsuite/gas/all/quad-div2.d
new file mode 100644 (file)
index 0000000..a70a425
--- /dev/null
+++ b/gas/testsuite/gas/all/quad-div2.d
@@ -0,0 +1,16 @@
+#objdump : -s -j .data
+#name : .quad with division (fwdref)
+# bfin doesn't support 'symbol = expression'.
+# tic30 and tic4x have 4 octets per byte, tic54x has 2 octets per byte.
+# v850 re-purposes .offset.
+#notarget: bfin-*-* *c30-*-* *c4x-*-* *c54x-*-* v850*-*-*
+# linkrelax targets don't handle equivalence expressions well (nor any
+# other forward expression).  mep uses complex relocs.
+#xfail: crx-*-* h8300-*-* mn10200-*-* mn10300-*-* mep-*-*
+
+.*: .*
+
+Contents of section (\.data|\$DATA\$):
+ 0000 (00000000 80000000|80000000 00000000|00000080 00000000) 00000000 00000000 .*
+ 00.. (80000000 00000000 00000000 00000000|00000000 00000080 00000000 00000000) .*
+#pass

diff --git a/gas/testsuite/gas/all/quad-div2.s b/gas/testsuite/gas/all/quad-div2.s
new file mode 100644 (file)
index 0000000..1a4d651
--- /dev/null
+++ b/gas/testsuite/gas/all/quad-div2.s
@@ -0,0 +1,27 @@
+       .offset
+       .dc.a 0
+       vasz = .
+
+       .data
+
+       .if vasz >= 8
+
+       .quad INT_MIN / -1
+       .quad INT_MIN % -1
+       .quad LONG_MIN / -1
+       .quad LONG_MIN % -1
+
+       LONG_MIN = -0x8000000000000000
+
+       .else
+
+       /* Use .long to cover at least !BFD64 targets.  */
+       .long INT_MIN / -1, 0
+       .long INT_MIN % -1, 0
+       /* Not really useful fallback for less-than-64-bit-VMA targets.  */
+       .quad 0x8000000000000000
+       .quad 0x0000000000000000
+
+       .endif
+
+       INT_MIN = -0x80000000