diff --git a/NEWS b/NEWS index 10860a324e..2105fc26b6 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 9b32b535c7..1fb6202b72 100644 --- a/src/Expr.cc +++ b/src/Expr.cc @@ -274,14 +274,7 @@ void Expr::AssignToIndex(ValPtr v1, ValPtr v2, ValPtr v3) const iterators_invalidated); if ( iterators_invalidated ) - { - ODesc d; - Describe(&d); - reporter->PushLocation(GetLocationInfo()); - reporter->Warning("possible loop/iterator invalidation caused by expression: %s", - d.Description()); - reporter->PopLocation(); - } + reporter->ExprRuntimeWarning(this, "possible loop/iterator invalidation"); if ( error_msg ) RuntimeErrorWithCallStack(error_msg); @@ -895,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(*); @@ -1454,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(); @@ -2974,14 +2971,7 @@ void IndexExpr::Add(Frame* f) v1->AsTableVal()->Assign(std::move(v2), nullptr, true, &iterators_invalidated); if ( iterators_invalidated ) - { - ODesc d; - Describe(&d); - reporter->PushLocation(GetLocationInfo()); - reporter->Warning("possible loop/iterator invalidation caused by expression: %s", - d.Description()); - reporter->PopLocation(); - } + reporter->ExprRuntimeWarning(this, "possible loop/iterator invalidation"); } void IndexExpr::Delete(Frame* f) @@ -3003,14 +2993,7 @@ void IndexExpr::Delete(Frame* f) v1->AsTableVal()->Remove(*v2, true, &iterators_invalidated); if ( iterators_invalidated ) - { - ODesc d; - Describe(&d); - reporter->PushLocation(GetLocationInfo()); - reporter->Warning("possible loop/iterator invalidation caused by expression: %s", - d.Description()); - reporter->PopLocation(); - } + reporter->ExprRuntimeWarning(this, "possible loop/iterator invalidation"); } ExprPtr IndexExpr::MakeLvalue() diff --git a/src/Reporter.cc b/src/Reporter.cc index 0ed84562e7..145bce4aea 100644 --- a/src/Reporter.cc +++ b/src/Reporter.cc @@ -174,6 +174,21 @@ void Reporter::ExprRuntimeError(const detail::Expr* expr, const char* fmt, ...) throw InterpreterException(); } +void Reporter::ExprRuntimeWarning(const detail::Expr* expr, const char* fmt, ...) + { + ODesc d; + expr->Describe(&d); + + PushLocation(expr->GetLocationInfo()); + va_list ap; + va_start(ap, fmt); + FILE* out = EmitToStderr(warnings_to_stderr) ? stderr : nullptr; + DoLog("expression warning", reporter_warning, out, nullptr, nullptr, true, true, + d.Description(), fmt, ap); + va_end(ap); + PopLocation(); + } + void Reporter::RuntimeError(const detail::Location* location, const char* fmt, ...) { ++errors; diff --git a/src/Reporter.h b/src/Reporter.h index c52d54f8b2..be5515ee11 100644 --- a/src/Reporter.h +++ b/src/Reporter.h @@ -112,6 +112,10 @@ public: [[noreturn]] void RuntimeError(const detail::Location* location, const char* fmt, ...) __attribute__((format(printf, 3, 4))); + // Report a rutnime warning in evaluating a Zeek script expression. + void ExprRuntimeWarning(const detail::Expr* expr, const char* fmt, ...) + __attribute__((format(printf, 3, 4))); + // Report a runtime error in executing a compiled script. This // function will not return but raise an InterpreterException. [[noreturn]] void CPPRuntimeError(const char* fmt, ...) __attribute__((format(printf, 2, 3))); 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/Baseline/language.table-set-iterator-invalidation/err b/testing/btest/Baseline/language.table-set-iterator-invalidation/err index e33c980fc3..238b43773d 100644 --- a/testing/btest/Baseline/language.table-set-iterator-invalidation/err +++ b/testing/btest/Baseline/language.table-set-iterator-invalidation/err @@ -1,5 +1,5 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -warning in <...>/table-set-iterator-invalidation.zeek, line 22: possible loop/iterator invalidation caused by expression: t[4] -warning in <...>/table-set-iterator-invalidation.zeek, line 31: possible loop/iterator invalidation caused by expression: t[4] -warning in <...>/table-set-iterator-invalidation.zeek, line 54: possible loop/iterator invalidation caused by expression: s[4] -warning in <...>/table-set-iterator-invalidation.zeek, line 63: possible loop/iterator invalidation caused by expression: s[4] +expression warning in <...>/table-set-iterator-invalidation.zeek, line 22: possible loop/iterator invalidation (t[4]) +expression warning in <...>/table-set-iterator-invalidation.zeek, line 31: possible loop/iterator invalidation (t[4]) +expression warning in <...>/table-set-iterator-invalidation.zeek, line 54: possible loop/iterator invalidation (s[4]) +expression warning in <...>/table-set-iterator-invalidation.zeek, line 63: possible loop/iterator invalidation (s[4]) 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|". # ------------------------------------------