diff --git a/CHANGES b/CHANGES index 2303cc84b8..33b4066ef2 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +7.1.0-dev.475 | 2024-11-08 15:35:00 +0100 + + * fixes for ZAM's propagation of control flow information for some degenerate constructs (Vern Paxson, Corelight) + 7.1.0-dev.473 | 2024-11-08 15:31:39 +0100 * fixed ZAM memory leak when looping over vectors of records (Vern Paxson, Corelight) diff --git a/VERSION b/VERSION index a4206b5ffa..341098f976 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -7.1.0-dev.473 +7.1.0-dev.475 diff --git a/src/script_opt/ZAM/AM-Opt.cc b/src/script_opt/ZAM/AM-Opt.cc index 953f8d81e4..b7d5fd897e 100644 --- a/src/script_opt/ZAM/AM-Opt.cc +++ b/src/script_opt/ZAM/AM-Opt.cc @@ -1020,8 +1020,11 @@ void ZAMCompiler::BackPropagateCFT(int inst_num, ControlFlowType cf_type) { if ( insts1[j]->live ) break; - // We should never wind up killing an instruction that has no predecessor. - ASSERT(j >= 0); + // Initializations of unused variables in functions with no arguments can + // come at the very beginning of the function, in which case there will + // be no predecessor to back-propagate to. + if ( j < 0 ) + return; // Make sure the CFT entry is created. AddCFT(insts1[j], cf_type); diff --git a/src/script_opt/ZAM/Stmt.cc b/src/script_opt/ZAM/Stmt.cc index c06dfd985a..65a9d94e13 100644 --- a/src/script_opt/ZAM/Stmt.cc +++ b/src/script_opt/ZAM/Stmt.cc @@ -154,7 +154,11 @@ const ZAMStmt ZAMCompiler::IfElse(const Expr* e, const Stmt* s1, const Stmt* s2) SetV(cond_stmt, GoToTargetBeyond(branch_after_s1), branch_v); SetGoTo(branch_after_s1, GoToTargetBeyond(s2_end)); - AddCFT(insts1[else_start], CFT_ELSE); + + if ( else_start < insts1.size() ) + // There was a non-empty else branch. + AddCFT(insts1[else_start], CFT_ELSE); + AddCFT(insts1.back(), CFT_BLOCK_END); return s2_end; @@ -848,6 +852,11 @@ const ZAMStmt ZAMCompiler::Loop(const Stmt* body) { auto head = StartingBlock(); auto b = CompileStmt(body); + if ( head.stmt_num == static_cast(insts1.size()) ) { + reporter->Error("infinite empty loop: %s", obj_desc(body).c_str()); + return head; + } + AddCFT(insts1[head.stmt_num], CFT_LOOP); AddCFT(insts1[b.stmt_num], CFT_LOOP_END); diff --git a/testing/btest/Baseline/opt.infinite-empty-loop/.stderr b/testing/btest/Baseline/opt.infinite-empty-loop/.stderr new file mode 100644 index 0000000000..7b0dbc9eb1 --- /dev/null +++ b/testing/btest/Baseline/opt.infinite-empty-loop/.stderr @@ -0,0 +1,3 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +error: infinite empty loop: ; <...>/infinite-empty-loop.zeek, line 13 +fatal error: Optimized script execution aborted due to errors diff --git a/testing/btest/Baseline/opt.null-inline/output b/testing/btest/Baseline/opt.null-inline/output new file mode 100644 index 0000000000..834e3e37ba --- /dev/null +++ b/testing/btest/Baseline/opt.null-inline/output @@ -0,0 +1,4 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +got through the conditional #1 +got through the conditional #2 +got through the conditional #3 diff --git a/testing/btest/opt/infinite-empty-loop.zeek b/testing/btest/opt/infinite-empty-loop.zeek new file mode 100644 index 0000000000..17f3e862a2 --- /dev/null +++ b/testing/btest/opt/infinite-empty-loop.zeek @@ -0,0 +1,14 @@ +# @TEST-DOC: Regression tests for past ZAM bugs handling empty infinite loops +# @TEST-REQUIRES: test "${ZEEK_USE_CPP}" != "1" +# +# @TEST-EXEC-FAIL: zeek -b -O ZAM %INPUT +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr + +event zeek_init() + { + # It used to be that ZAM would fault doing control-flow propagation + # when compiling empty infinite loops. Now it should generate a + # compile-time error. + while ( T ) + ; + } diff --git a/testing/btest/opt/null-inline.zeek b/testing/btest/opt/null-inline.zeek new file mode 100644 index 0000000000..a4d26f6575 --- /dev/null +++ b/testing/btest/opt/null-inline.zeek @@ -0,0 +1,36 @@ +# @TEST-DOC: Regression tests for past ZAM bugs inlining empty functions +# @TEST-REQUIRES: test "${ZEEK_USE_CPP}" != "1" +# +# @TEST-EXEC: zeek -b -O ZAM %INPUT >output +# @TEST-EXEC: btest-diff output + +function empty_func1() { } +function empty_func2() { local i: int; } +function empty_func3() { local i = 1; } + +# Use a global to avoid constant propagation optimizing out the conditional. +global bar = F; + +event zeek_init() + { + if ( bar ) + empty_func1(); + else + empty_func1(); + + print "got through the conditional #1"; + + if ( bar ) + empty_func2(); + else + empty_func2(); + + print "got through the conditional #2"; + + if ( bar ) + empty_func3(); + else + empty_func3(); + + print "got through the conditional #3"; + }