From e48618e244ccb73d284516aa5fcebcdd4e010cfc Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Tue, 29 Nov 2022 15:58:36 +0100 Subject: [PATCH] Expr: Warn on count underflow for c -= 1 and c = c - 1 I've skipped treating overflows as warnings, as ++ wrapping around at 0 doesn't currently trigger a runtime error and might be expected to be quiet and silently wrap. Closes #2486 --- NEWS | 9 +++++ src/Expr.cc | 6 +++- .../Baseline/language.count-underflow/.stderr | 5 +++ .../Baseline/language.count-underflow/out | 8 +++++ .../btest/Baseline/language.sizeof/.stderr | 2 ++ testing/btest/language/count-underflow.zeek | 36 +++++++++++++++++++ testing/btest/language/sizeof.zeek | 3 +- 7 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 testing/btest/Baseline/language.count-underflow/.stderr create mode 100644 testing/btest/Baseline/language.count-underflow/out create mode 100644 testing/btest/Baseline/language.sizeof/.stderr create mode 100644 testing/btest/language/count-underflow.zeek diff --git a/NEWS b/NEWS index 623748c69c..bf94eddf65 100644 --- a/NEWS +++ b/NEWS @@ -146,6 +146,15 @@ Changed Functionality stopped. This fixes a few cases where we already had the logic to continue in place, but we still ended up considering them partial. +- Count underflows via ``--c`` or subtract from statements (``c = c - 1``) are + now consistently warned about. Previously, underflows through ``--c`` were + treated as runtime errors, while "subtract from" underflows were silently + accepted. The following (surprising behavior) now causes a warning, too: + + $ zeek -e 'print 1 - 2' + expression warning in , line 1: count underflow (1 - 2) + 18446744073709551615 + Deprecated Functionality ------------------------ diff --git a/src/Expr.cc b/src/Expr.cc index 237ca1faa6..b90c27344e 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -888,6 +888,10 @@ ValPtr BinaryExpr::Fold(Val* v1, Val* v2) const case EXPR_SUB: case EXPR_REMOVE_FROM: DO_FOLD(-); + // When subtracting and the result is larger than the left + // operand we mostly likely underflowed and log a warning. + if ( is_unsigned && u3 > u1 ) + reporter->ExprRuntimeWarning(this, "count underflow"); break; case EXPR_TIMES: DO_FOLD(*); @@ -1447,7 +1451,7 @@ ValPtr IncrExpr::DoSingleEval(Frame* f, Val* v) const --k; if ( k < 0 && v->GetType()->InternalType() == TYPE_INTERNAL_UNSIGNED ) - RuntimeError("count underflow"); + reporter->ExprRuntimeWarning(this, "count underflow"); } const auto& ret_type = IsVector(GetType()->Tag()) ? GetType()->Yield() : GetType(); diff --git a/testing/btest/Baseline/language.count-underflow/.stderr b/testing/btest/Baseline/language.count-underflow/.stderr new file mode 100644 index 0000000000..6f525550df --- /dev/null +++ b/testing/btest/Baseline/language.count-underflow/.stderr @@ -0,0 +1,5 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +expression warning in <...>/count-underflow.zeek, line 10: count underflow (--c1) +expression warning in <...>/count-underflow.zeek, line 14: count underflow (c2 - 1) +expression warning in <...>/count-underflow.zeek, line 18: count underflow (c3 - 1) +expression warning in <...>/count-underflow.zeek, line 22: count underflow (1 - 2) diff --git a/testing/btest/Baseline/language.count-underflow/out b/testing/btest/Baseline/language.count-underflow/out new file mode 100644 index 0000000000..731cabd888 --- /dev/null +++ b/testing/btest/Baseline/language.count-underflow/out @@ -0,0 +1,8 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +local c1 = 0; --c1; c1 == 18446744073709551615; T +local c2 = 0; c2 -= 1; c2 == 18446744073709551615; T +local c3 = 0; c3 = c3 - 1; c3 == 18446744073709551615; T +1 - 2, 18446744073709551615 +local c4 = count_max; ++c4; c4 == 0; T +local c5 = count_max; c5 += 1; c5 == 0; T +local c6 = count_max; c6 = c6 + 1; c6 == 0; T diff --git a/testing/btest/Baseline/language.sizeof/.stderr b/testing/btest/Baseline/language.sizeof/.stderr new file mode 100644 index 0000000000..837fbeb902 --- /dev/null +++ b/testing/btest/Baseline/language.sizeof/.stderr @@ -0,0 +1,2 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +expression warning in <...>/sizeof.zeek, line 73: count underflow (5 - 9) diff --git a/testing/btest/language/count-underflow.zeek b/testing/btest/language/count-underflow.zeek new file mode 100644 index 0000000000..b089899806 --- /dev/null +++ b/testing/btest/language/count-underflow.zeek @@ -0,0 +1,36 @@ +# @TEST-EXEC: zeek -b %INPUT >out +# @TEST-EXEC: btest-diff out +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr + +global count_max: count = 0xffffffffffffffff; + +event zeek_init() +{ + local c1 = 0; + --c1; + print fmt("local c1 = 0; --c1; c1 == %s; %s", c1, c1 == count_max); + + local c2 = 0; + c2 -= 1; + print fmt("local c2 = 0; c2 -= 1; c2 == %s; %s", c2, c2 == count_max); + + local c3 = 0; + c3 = c3 - 1; + print fmt("local c3 = 0; c3 = c3 - 1; c3 == %s; %s", c3, c3 == count_max); + + # This also triggers a warning now; + print "1 - 2", 1 - 2; + + # The following ones all overflow back to 0, but do not log a warning. + local c4 = count_max; + ++c4; + print fmt("local c4 = count_max; ++c4; c4 == %s; %s", c4, c4 == 0); + + local c5 = count_max; + c5 += 1; + print fmt("local c5 = count_max; c5 += 1; c5 == %s; %s", c5, c5 == 0); + + local c6 = count_max; + c6 = c6 + 1; + print fmt("local c6 = count_max; c6 = c6 + 1; c6 == %s; %s", c6, c6 == 0); +} diff --git a/testing/btest/language/sizeof.zeek b/testing/btest/language/sizeof.zeek index 9ca6e25b9c..5c0a043749 100644 --- a/testing/btest/language/sizeof.zeek +++ b/testing/btest/language/sizeof.zeek @@ -1,5 +1,6 @@ -# @TEST-EXEC: zeek -b %INPUT >output 2>&1 +# @TEST-EXEC: zeek -b %INPUT >output # @TEST-EXEC: btest-diff output +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr # Demo policy for the sizeof operator "|x|". # ------------------------------------------