From 46a6966b70be1321e7ef199679842d34b1c9ce2d Mon Sep 17 00:00:00 2001 From: Rangi <35663410+Rangi42@users.noreply.github.com> Date: Mon, 6 Jul 2026 15:08:44 -0400 Subject: [PATCH] Fix `$8000_0000 % -1` to warn with `-Wdiv` like `$8000_0000 / -1` does (#2012) --- src/asm/rpn.cpp | 7 +++---- src/link/patch.cpp | 5 ++++- src/opmath.cpp | 6 +++++- test/asm/overflow.err | 4 ++++ test/link/patch-diagnostics.asm | 1 + test/link/patch-diagnostics.out | 14 ++++++++------ 6 files changed, 25 insertions(+), 12 deletions(-) diff --git a/src/asm/rpn.cpp b/src/asm/rpn.cpp index c251e63e..d8c75dc5 100644 --- a/src/asm/rpn.cpp +++ b/src/asm/rpn.cpp @@ -375,8 +375,7 @@ void Expression::makeBinaryOp(RPNCommand op, Expression &&src1, Expression const case RPN_DIV: if (rval == 0) { fatal("Division by zero"); - } - if (lval == INT32_MIN && rval == -1) { + } else if (lval == INT32_MIN && rval == -1) { warning( WARNING_DIV, "Division of %" PRId32 " by -1 yields %" PRId32, @@ -391,8 +390,8 @@ void Expression::makeBinaryOp(RPNCommand op, Expression &&src1, Expression const case RPN_MOD: if (rval == 0) { fatal("Modulo by zero"); - } - if (lval == INT32_MIN && rval == -1) { + } else if (lval == INT32_MIN && rval == -1) { + warning(WARNING_DIV, "Modulo of %" PRId32 " by -1 yields 0", INT32_MIN); data = 0; } else { data = op_modulo(lval, rval); diff --git a/src/link/patch.cpp b/src/link/patch.cpp index 97ba3107..9fd66e32 100644 --- a/src/link/patch.cpp +++ b/src/link/patch.cpp @@ -143,8 +143,11 @@ static int32_t computeRPNExpr(Patch const &patch, std::vector const &fil firstErrorAt(patch, "Modulo by 0"); popRPN(patch); value = 0; + } else if (int32_t lval = popRPN(patch); lval == INT32_MIN && value == -1) { + diagnosticAt(patch, WARNING_DIV, "Modulo of %" PRId32 " by -1 yields 0", INT32_MIN); + value = 0; } else { - value = op_modulo(popRPN(patch), value); + value = op_modulo(lval, value); } break; case RPN_NEG: diff --git a/src/opmath.cpp b/src/opmath.cpp index 4df50a1f..44916a24 100644 --- a/src/opmath.cpp +++ b/src/opmath.cpp @@ -6,9 +6,11 @@ #include -#include "helpers.hpp" // clz, ctz +#include "helpers.hpp" // assume, clz, ctz int32_t op_divide(int32_t dividend, int32_t divisor) { + assume(divisor != 0); // Division by 0 is UB + assume(dividend != INT32_MIN || divisor != -1); // INT32_MIN / -1 is UB // Adjust division to floor toward negative infinity, // not truncate toward zero int32_t remainder = dividend % divisor; @@ -16,6 +18,8 @@ int32_t op_divide(int32_t dividend, int32_t divisor) { } int32_t op_modulo(int32_t dividend, int32_t divisor) { + assume(divisor != 0); // Modulo by 0 is UB + assume(dividend != INT32_MIN || divisor != -1); // INT32_MIN % -1 is UB // Adjust modulo to have the sign of the divisor, // not the sign of the dividend return static_cast( diff --git a/test/asm/overflow.err b/test/asm/overflow.err index 4546a7fc..86110ec4 100644 --- a/test/asm/overflow.err +++ b/test/asm/overflow.err @@ -2,6 +2,10 @@ warning: Division of -2147483648 by -1 yields -2147483648 [-Wdiv] at overflow.asm(23) warning: Division of -2147483648 by -1 yields -2147483648 [-Wdiv] at overflow.asm(24) +warning: Modulo of -2147483648 by -1 yields 0 [-Wdiv] + at overflow.asm(28) +warning: Modulo of -2147483648 by -1 yields 0 [-Wdiv] + at overflow.asm(29) warning: Integer constant is too large [-Wlarge-constant] at overflow.asm(44) warning: Graphics constant has too many digits; only first 8 pixels considered [-Wlarge-constant] diff --git a/test/link/patch-diagnostics.asm b/test/link/patch-diagnostics.asm index 21e1a0a8..6f400703 100644 --- a/test/link/patch-diagnostics.asm +++ b/test/link/patch-diagnostics.asm @@ -2,6 +2,7 @@ def fzero equs "startof(\"test\")" section "test", rom0 ld a, $8000_0000 / ({fzero} - 1) ld a, $8000_0000 / ({fzero} - 2) +ld a, $8000_0000 % ({fzero} - 1) ld a, 1 << ({fzero} - 1) ld a, 1 << ({fzero} + 32) ld a, ({fzero} - 1) >> 1 diff --git a/test/link/patch-diagnostics.out b/test/link/patch-diagnostics.out index 7183b51b..f0fc714a 100644 --- a/test/link/patch-diagnostics.out +++ b/test/link/patch-diagnostics.out @@ -1,16 +1,18 @@ warning: Shifting right by large amount 32 [-Wshift-amount] + at patch-diagnostics.asm(12) +warning: Shifting right by negative amount -1 [-Wshift-amount] at patch-diagnostics.asm(11) -warning: Shifting right by negative amount -1 [-Wshift-amount] - at patch-diagnostics.asm(10) warning: Shifting right by large amount 32 [-Wshift-amount] - at patch-diagnostics.asm(9) + at patch-diagnostics.asm(10) warning: Shifting right by negative amount -1 [-Wshift-amount] - at patch-diagnostics.asm(8) + at patch-diagnostics.asm(9) warning: Shifting right negative value -1 [-Wshift] - at patch-diagnostics.asm(7) + at patch-diagnostics.asm(8) warning: Shifting left by large amount 32 [-Wshift-amount] - at patch-diagnostics.asm(6) + at patch-diagnostics.asm(7) warning: Shifting left by negative amount -1 [-Wshift-amount] + at patch-diagnostics.asm(6) +warning: Modulo of -2147483648 by -1 yields 0 [-Wdiv] at patch-diagnostics.asm(5) warning: Value $40000000 is not 8-bit [-Wtruncation] at patch-diagnostics.asm(4)