[binutils-gdb] gas: special-case division / modulo by ±1
Jan Beulich
jbeulich@sourceware.org
Mon Jan 6 15:02:39 GMT 2025
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=30200464e9dd7903be8f186ea137b7982f812670
commit 30200464e9dd7903be8f186ea137b7982f812670
Author: Jan Beulich <jbeulich@suse.com>
Date: Mon Jan 6 16:01:07 2025 +0100
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:
---
gas/expr.c | 42 +++++++++++++++++++++++++++++++++++----
gas/symbols.c | 23 +++++++++++++++++++--
gas/testsuite/gas/all/gas.exp | 16 +++++++++++++++
gas/testsuite/gas/all/octa-div.d | 10 ++++++++++
gas/testsuite/gas/all/octa-div.s | 11 ++++++++++
gas/testsuite/gas/all/quad-div.d | 9 +++++++++
gas/testsuite/gas/all/quad-div.s | 32 +++++++++++++++++++++++++++++
gas/testsuite/gas/all/quad-div2.d | 16 +++++++++++++++
gas/testsuite/gas/all/quad-div2.s | 27 +++++++++++++++++++++++++
9 files changed, 180 insertions(+), 6 deletions(-)
diff --git a/gas/expr.c b/gas/expr.c
index 3004b958314..5e59b7c5fa1 100644
--- 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 50c8c9dcd8f..e6f3d85a75d 100644
--- 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 bac0aff0b6d..f8bab3c2bd1 100644
--- 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
index 00000000000..bd69a74d843
--- /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
index 00000000000..e80f562cf53
--- /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
index 00000000000..36d52d6f2b5
--- /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
index 00000000000..a5824d861e2
--- /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
index 00000000000..a70a42516d7
--- /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
index 00000000000..1a4d6519cd6
--- /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
More information about the Binutils-cvs
mailing list