Merge remote-tracking branch 'origin/topic/vern/script-opt.May24'

* origin/topic/vern/script-opt.May24:
  factored BTest constants into globals to prevent premature ZAM optimization when testing
  addressed minor issues flagged by Coverity

CHANGES

@@ -1,3 +1,9 @@
+7.0.0-dev.194 | 2024-04-29 09:08:20 -0700
+
+  * factored BTest constants into globals to prevent premature ZAM optimization when testing (Vern Paxson, Corelight)
+
+  * addressed minor issues flagged by Coverity (Vern Paxson, Corelight)
+
 7.0.0-dev.191 | 2024-04-29 09:06:24 -0700
 
   * Avoid calling typecasts in Val when we have direct access to the underlying value object (Tim Wojtulewicz, Corelight)

VERSION

@@ -1 +1 @@
-7.0.0-dev.191
+7.0.0-dev.194

src/script_opt/Expr.cc

@@ -229,7 +229,7 @@ ExprPtr Expr::ReduceToConditional(Reducer* c, StmtPtr& red_stmt) {
         // conditional.
         StmtPtr red_stmt2;
         new_me = new_me->ReduceToConditional(c, red_stmt2);
-        red_stmt = MergeStmts(red_stmt, red_stmt2);
+        red_stmt = MergeStmts(std::move(red_stmt), std::move(red_stmt2));
 
         return new_me;
     }
@@ -252,7 +252,7 @@ ExprPtr Expr::ReduceToConditional(Reducer* c, StmtPtr& red_stmt) {
 
             StmtPtr red_stmt2;
             auto res = Reduce(c, red_stmt2);
-            red_stmt = MergeStmts(red_stmt, red_stmt2);
+            red_stmt = MergeStmts(std::move(red_stmt), std::move(red_stmt2));
             return res;
         }
 
@@ -2950,7 +2950,7 @@ bool ScriptOptBuiltinExpr::IsPure() const { return arg1->IsPure() && (! arg2 ||
 
 ExprPtr ScriptOptBuiltinExpr::Duplicate() {
     auto new_me = make_intrusive<ScriptOptBuiltinExpr>(tag, arg1, arg2);
-    return with_location_of(new_me, this);
+    return with_location_of(std::move(new_me), this);
 }
 
 bool ScriptOptBuiltinExpr::IsReduced(Reducer* c) const {
@@ -2980,7 +2980,7 @@ ExprPtr ScriptOptBuiltinExpr::Reduce(Reducer* c, StmtPtr& red_stmt) {
         if ( arg2 ) {
             StmtPtr red_stmt2;
             arg2 = arg2->Reduce(c, red_stmt2);
-            red_stmt = MergeStmts(red_stmt, red_stmt2);
+            red_stmt = MergeStmts(std::move(red_stmt), std::move(red_stmt2));
         }
     }
 

src/script_opt/ZAM/BuiltIn.cc

@@ -290,7 +290,7 @@ MultiZBI::MultiZBI(std::string name, bool _ret_val_matters, BiFArgsInfo _args_in
     : ZAMBuiltIn(std::move(name), _ret_val_matters), args_info(std::move(_args_info)), type_arg(_type_arg) {}
 
 MultiZBI::MultiZBI(std::string name, BiFArgsInfo _args_info, BiFArgsInfo _assign_args_info, int _type_arg)
-    : MultiZBI(std::move(name), false, _args_info, _type_arg) {
+    : MultiZBI(std::move(name), false, std::move(_args_info), _type_arg) {
     assign_args_info = std::move(_assign_args_info);
     have_both = true;
 }

src/script_opt/ZAM/Ops.in

@@ -1233,7 +1233,7 @@ direct-unary-op Record-Constructor ConstructRecord
 macro ConstructRecordPost()
 	auto& r = frame[z.v1].record_val;
 	Unref(r);
-	r = new RecordVal(cast_intrusive<RecordType>(z.t), init_vals);
+	r = new RecordVal(cast_intrusive<RecordType>(z.t), std::move(init_vals));
 
 op Construct-Direct-Record
 type V

testing/btest/language/pattern.zeek

@@ -7,11 +7,37 @@ function test_case(msg: string, expect: bool)
         print fmt("%s (%s)", msg, expect ? "PASS" : "FAIL");
         }
 
+# We use globals rather than direct constants in the tests to prevent
+# ZAM from folding the expressions rather than evaluating them dynamically.
+
 global p1: pattern = /foo|bar/; 
 global p2: pattern = /oob/; 
 global p3: pattern = /^oob/; 
 global p4 = /foo/;
 
+global Foo = "Foo";
+global bAr = "bAr";
+global bArbEz = "bArbEz";
+global bArbez = "bArbez";
+global bEz = "bEz";
+global b_newline_r = "b\nr";
+global b_newline_rbez = "b\nrbez";
+global bar = "bar";
+global barbEz = "barbEz";
+global barbez = "barbez";
+global baroob = "baroob";
+global bez = "bez";
+global fOO = "fOO";
+global fOOab_newline_cdbAR = "fOOab\ncdbAR";
+global foo = "foo";
+global foobar = "foobar";
+global lazybarlazy = "lazybarlazy";
+global xBAry = "xBAry";
+global xFOoy = "xFOoy";
+global xFoObar = "xFoObar";
+global xfOObar = "xfOObar";
+global xoob = "xoob";
+
 event zeek_init()
 {
 	# Type inference tests
@@ -20,54 +46,54 @@ event zeek_init()
 
 	# Operator tests
 
-	test_case( "equality operator", "foo" == p1 );
-	test_case( "equality operator (order of operands)", p1 == "foo" );
+	test_case( "equality operator", foo == p1 );
+	test_case( "equality operator (order of operands)", p1 == foo );
 
-	test_case( "inequality operator", "foobar" != p1 );
-	test_case( "inequality operator (order of operands)", p1 != "foobar" );
+	test_case( "inequality operator", foobar != p1 );
+	test_case( "inequality operator (order of operands)", p1 != foobar );
 
-	test_case( "in operator", p1 in "foobar" );
-	test_case( "in operator", p2 in "foobar" );
-	test_case( "!in operator", p3 !in "foobar" );
+	test_case( "in operator", p1 in foobar );
+	test_case( "in operator", p2 in foobar );
+	test_case( "!in operator", p3 !in foobar );
 
-	test_case( "& operator", p1 & p2 in "baroob" );
-	test_case( "& operator", p2 & p1 in "baroob" );
+	test_case( "& operator", p1 & p2 in baroob );
+	test_case( "& operator", p2 & p1 in baroob );
 
-	test_case( "| operator", p1 | p2 in "lazybarlazy" );
-	test_case( "| operator", p3 | p4 in "xoob" );
+	test_case( "| operator", p1 | p2 in lazybarlazy );
+	test_case( "| operator", p3 | p4 in xoob );
 
-	test_case( "/i pattern modifier", /fOO/i in "xFoObar" );
-	test_case( "/i pattern modifier", /fOO/i == "Foo" );
+	test_case( "/i pattern modifier", /fOO/i in xFoObar );
+	test_case( "/i pattern modifier", /fOO/i == Foo );
 
-	test_case( "/i double-quote escape", /"fOO"/i in "xFoObar" );
-	test_case( "/i double-quote escape", /"fOO"/i in "xfOObar" );
+	test_case( "/i double-quote escape", /"fOO"/i in xFoObar );
+	test_case( "/i double-quote escape", /"fOO"/i in xfOObar );
 
-	test_case( "case-sensitive pattern", /fOO/ in "xFoObar" );
-	test_case( "case-sensitive pattern", /fOO/ == "Foo" );
-	test_case( "case-sensitive pattern", /fOO/ == "fOO" );
+	test_case( "case-sensitive pattern", /fOO/ in xFoObar );
+	test_case( "case-sensitive pattern", /fOO/ == Foo );
+	test_case( "case-sensitive pattern", /fOO/ == fOO );
 
-	test_case( "/i pattern disjunction", /bar/i | /bez/ == "bez" );
-	test_case( "/i pattern disjunction", /bar/i | /bez/ == "bEz" );
-	test_case( "/i pattern disjunction", /bar/i | /bez/ == "bar" );
-	test_case( "/i pattern disjunction", /bar/i | /bez/ == "bAr" );
+	test_case( "/i pattern disjunction", /bar/i | /bez/ == bez );
+	test_case( "/i pattern disjunction", /bar/i | /bez/ == bEz );
+	test_case( "/i pattern disjunction", /bar/i | /bez/ == bar );
+	test_case( "/i pattern disjunction", /bar/i | /bez/ == bAr );
 
-	test_case( "/i pattern concatenation", /bar/i & /bez/ == "barbez" );
-	test_case( "/i pattern concatenation", /bar/i & /bez/ == "barbEz" );
-	test_case( "/i pattern concatenation", /BAR/i & /bez/ == "barbEz" );
-	test_case( "/i pattern concatenation", /bar/i & /bez/ == "bArbez" );
-	test_case( "/i pattern concatenation", /BAR/i & /bez/ == "bArbez" );
-	test_case( "/i pattern concatenation", /bar/i & /bez/ == "bArbEz" );
+	test_case( "/i pattern concatenation", /bar/i & /bez/ == barbez );
+	test_case( "/i pattern concatenation", /bar/i & /bez/ == barbEz );
+	test_case( "/i pattern concatenation", /BAR/i & /bez/ == barbEz );
+	test_case( "/i pattern concatenation", /bar/i & /bez/ == bArbez );
+	test_case( "/i pattern concatenation", /BAR/i & /bez/ == bArbez );
+	test_case( "/i pattern concatenation", /bar/i & /bez/ == bArbEz );
 
-	test_case( "/i pattern character class", /ba[0a-c99S-Z0]/i & /bEz/ == "bArbEz" );
-	test_case( "/i pattern character class", /ba[0a-c99M-S0]/i & /bEz/ == "bArbEz" );
+	test_case( "/i pattern character class", /ba[0a-c99S-Z0]/i & /bEz/ == bArbEz );
+	test_case( "/i pattern character class", /ba[0a-c99M-S0]/i & /bEz/ == bArbEz );
 
-	test_case( "(?i:...) pattern construct", /foo|(?i:bar)/ in "xBAry" );
-	test_case( "(?i:...) pattern construct", /foo|(?i:bar)/ in "xFOoy" );
-	test_case( "(?i:...) pattern construct", /foo|(?i:bar)/ | /foo/i in "xFOoy" );
+	test_case( "(?i:...) pattern construct", /foo|(?i:bar)/ in xBAry );
+	test_case( "(?i:...) pattern construct", /foo|(?i:bar)/ in xFOoy );
+	test_case( "(?i:...) pattern construct", /foo|(?i:bar)/ | /foo/i in xFOoy );
 
-	test_case( "/s missing", /fOO.*bAR/ != "fOOab\ncdbAR");
-	test_case( "/s pattern modifier", /fOO.*bAR/s == "fOOab\ncdbAR");
-	test_case( "/s pattern disjunction", /b.r/s | /bez/ == "b\nr" );
-	test_case( "/s pattern concatenation", /b.r/s & /bez/ == "b\nrbez" );
+	test_case( "/s missing", /fOO.*bAR/ != fOOab_newline_cdbAR);
+	test_case( "/s pattern modifier", /fOO.*bAR/s == fOOab_newline_cdbAR);
+	test_case( "/s pattern disjunction", /b.r/s | /bez/ == b_newline_r );
+	test_case( "/s pattern concatenation", /b.r/s & /bez/ == b_newline_rbez );
 
 }