From: Jan Beulich Date: Mon, 6 Jan 2025 15:01:07 +0000 (+0100) Subject: gas: special-case division / modulo by ±1 X-Git-Tag: binutils-2_44~253 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=30200464e9dd7903be8f186ea137b7982f812670;p=thirdparty%2Fbinutils-gdb.git 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 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