diff --git a/CHANGES b/CHANGES index 5f03f56ddc..2b5268262a 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,13 @@ +7.0.0-dev.265 | 2024-05-16 11:09:23 -0700 + + * Fix for ZAM inlining of nested function calls with the same parameter names (Vern Paxson, Corelight) + + * Fixed ZAM logic error in canonicalizing specialized min/max instructions (Vern Paxson, Corelight) + + * Fixed order-of-evaluation bug in ZAM Subnet-To-Addr instruction (Vern Paxson, Corelight) + + * "-a zam" BTest baseline update reflecting recent Spicy baseline change (Vern Paxson, Corelight) + 7.0.0-dev.260 | 2024-05-16 09:34:55 -0700 * CI: Disable spicy_head task for release branches (Tim Wojtulewicz, Corelight) diff --git a/VERSION b/VERSION index ca6dab8931..2e40534aa0 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -7.0.0-dev.260 +7.0.0-dev.265 diff --git a/src/script_opt/Expr.cc b/src/script_opt/Expr.cc index 6988f06354..ed70f1ffb0 100644 --- a/src/script_opt/Expr.cc +++ b/src/script_opt/Expr.cc @@ -2529,22 +2529,41 @@ ExprPtr InlineExpr::Duplicate() { bool InlineExpr::IsReduced(Reducer* c) const { return NonReduced(this); } ExprPtr InlineExpr::Reduce(Reducer* c, StmtPtr& red_stmt) { - // First, reduce each argument and assign it to a parameter. - // We do this one at a time because that will often allow the - // optimizer to collapse the final assignment. + // We have to be careful regarding the order in which we evaluate + // the various elements that go into inlining the call. First, the + // arguments need to be reduced in the current scope, not the block + // we'll create for the inlined code. Second, we need to generate the + // identifiers for the formal parameters *after* creating that inner + // block scope, so the variables are distinct to that context. Finally, + // when done we need to create the return variable within that scope + // (so it's unique to the inlined instance) even though we'll use it - + // and possibly other locals from the inlining, via optimization - in + // the outer scope. + + auto args_list = args->Exprs(); + std::vector red_args; // holds the arguments as singletons red_stmt = nullptr; - auto args_list = args->Exprs(); - + // Gather up the reduced arguments. loop_over_list(args_list, i) { StmtPtr arg_red_stmt; - auto red_i = args_list[i]->Reduce(c, arg_red_stmt); - auto assign_stmt = with_location_of(c->GenParam(params[i], red_i, param_is_modified[i]), this); - red_stmt = MergeStmts(red_stmt, arg_red_stmt, assign_stmt); + red_args.emplace_back(args_list[i]->Reduce(c, arg_red_stmt)); + red_stmt = MergeStmts(red_stmt, arg_red_stmt); } - auto ret_val = c->PushInlineBlock(type); + // Start the inline block, so the parameters we generate pick up + // its naming scope. + c->PushInlineBlock(); + + // Generate the parameters and assign them to the reduced arguments. + loop_over_list(args_list, j) { + auto assign_stmt = with_location_of(c->GenParam(params[j], red_args[j], param_is_modified[j]), this); + red_stmt = MergeStmts(red_stmt, assign_stmt); + } + + // Generate the return variable distinct to the inner block. + auto ret_val = c->GetRetVar(type); if ( ret_val ) ret_val->SetLocationInfo(GetLocationInfo()); diff --git a/src/script_opt/Reduce.cc b/src/script_opt/Reduce.cc index 14a9198f26..fbf5bc7eb1 100644 --- a/src/script_opt/Reduce.cc +++ b/src/script_opt/Reduce.cc @@ -359,11 +359,27 @@ NameExprPtr Reducer::GenInlineBlockName(const IDPtr& id) { return make_intrusive(GenLocal(id)); } -NameExprPtr Reducer::PushInlineBlock(TypePtr type) { +void Reducer::PushInlineBlock() { ++inline_block_level; - block_locals.emplace_back(std::unordered_map()); +} +void Reducer::PopInlineBlock() { + --inline_block_level; + + for ( auto& l : block_locals.back() ) { + auto key = l.first; + auto prev = l.second; + if ( prev ) + orig_to_new_locals[key] = prev; + else + orig_to_new_locals.erase(key); + } + + block_locals.pop_back(); +} + +NameExprPtr Reducer::GetRetVar(TypePtr type) { if ( ! type || type->Tag() == TYPE_VOID ) return nullptr; @@ -382,21 +398,6 @@ NameExprPtr Reducer::PushInlineBlock(TypePtr type) { return GenInlineBlockName(ret_id); } -void Reducer::PopInlineBlock() { - --inline_block_level; - - for ( auto& l : block_locals.back() ) { - auto key = l.first; - auto prev = l.second; - if ( prev ) - orig_to_new_locals[key] = prev; - else - orig_to_new_locals.erase(key); - } - - block_locals.pop_back(); -} - ExprPtr Reducer::NewVarUsage(IDPtr var, const Expr* orig) { auto var_usage = make_intrusive(var); BindExprToCurrStmt(var_usage); diff --git a/src/script_opt/Reduce.h b/src/script_opt/Reduce.h index a0c59c532a..b69e4e0229 100644 --- a/src/script_opt/Reduce.h +++ b/src/script_opt/Reduce.h @@ -57,11 +57,15 @@ public: int NumNewLocals() const { return new_locals.size(); } + // These should be used as a balanced pair to start and end a + // block being inlined. + void PushInlineBlock(); + void PopInlineBlock(); + // Returns the name of a temporary for holding the return // value of the block, or nil if the type indicates there's - // o return value. - NameExprPtr PushInlineBlock(TypePtr type); - void PopInlineBlock(); + // no return value. Call before popping the block. + NameExprPtr GetRetVar(TypePtr type); // Whether it's okay to split a statement into two copies for if-else // expansion. We only allow this to a particular depth because diff --git a/src/script_opt/ZAM/Expr.cc b/src/script_opt/ZAM/Expr.cc index facc6952ff..42bd16e6ff 100644 --- a/src/script_opt/ZAM/Expr.cc +++ b/src/script_opt/ZAM/Expr.cc @@ -242,7 +242,6 @@ const ZAMStmt ZAMCompiler::CompileZAMBuiltin(const NameExpr* lhs, const ScriptOp if ( op1->Tag() == EXPR_CONST ) { ASSERT(op2->Tag() != EXPR_CONST); std::swap(op1, op2); - is_min = ! is_min; } ZOp op; diff --git a/src/script_opt/ZAM/OPs/ZAM.op b/src/script_opt/ZAM/OPs/ZAM.op index 387bd6b382..2904afb4a5 100644 --- a/src/script_opt/ZAM/OPs/ZAM.op +++ b/src/script_opt/ZAM/OPs/ZAM.op @@ -2412,8 +2412,9 @@ eval auto handle = frame[z.v1].string_val; internal-op Subnet-To-Addr type VV -eval Unref(frame[z.v1].addr_val); - frame[z.v1] = ZVal(make_intrusive(frame[z.v2].subnet_val->Prefix())); +eval auto addr_v = make_intrusive(frame[z.v2].subnet_val->Prefix()); + Unref(frame[z.v1].addr_val); + frame[z.v1] = ZVal(addr_v); internal-op Sub-Bytes type VVVV diff --git a/testing/btest/Baseline.zam/spicy.replaces/conn.log b/testing/btest/Baseline.zam/spicy.replaces/conn.log new file mode 100644 index 0000000000..9a44cfd8c3 --- /dev/null +++ b/testing/btest/Baseline.zam/spicy.replaces/conn.log @@ -0,0 +1,11 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +#separator \x09 +#set_separator , +#empty_field (empty) +#unset_field - +#path conn +#open XXXX-XX-XX-XX-XX-XX +#fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto service duration orig_bytes resp_bytes conn_state local_orig local_resp missed_bytes history orig_pkts orig_ip_bytes resp_pkts resp_ip_bytes tunnel_parents +#types time string addr port addr port enum string interval count count string bool bool count string count count count count set[string] +XXXXXXXXXX.XXXXXX CHhAvVGS1DHFjwGM9 172.16.238.1 49656 172.16.238.131 80 tcp spicy_ssh 9.953807 2405 2887 SF T T 0 ShAdDaFf 40 4497 30 4455 - +#close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Baseline.zam/spicy.replaces/output b/testing/btest/Baseline.zam/spicy.replaces/output index 0a68502166..b8c41b3fa8 100644 --- a/testing/btest/Baseline.zam/spicy.replaces/output +++ b/testing/btest/Baseline.zam/spicy.replaces/output @@ -2,3 +2,6 @@ AllAnalyzers::ANALYZER_ANALYZER_SSH, 3 SSH banner, [orig_h=192.150.186.169, orig_p=49244/tcp, resp_h=131.159.14.23, resp_p=22/tcp], F, 1.99, OpenSSH_3.9p1 SSH banner, [orig_h=192.150.186.169, orig_p=49244/tcp, resp_h=131.159.14.23, resp_p=22/tcp], T, 2.0, OpenSSH_3.8.1p1 +AllAnalyzers::ANALYZER_ANALYZER_SSH, 6 +SSH banner, [orig_h=172.16.238.1, orig_p=49656/tcp, resp_h=172.16.238.131, resp_p=80/tcp], F, 2.0, OpenSSH_5.8p1 Debian-1ubuntu3 +SSH banner, [orig_h=172.16.238.1, orig_p=49656/tcp, resp_h=172.16.238.131, resp_p=80/tcp], T, 2.0, OpenSSH_5.2