Merge remote-tracking branch 'origin/topic/vern/zam-exception-leaks'

* origin/topic/vern/zam-exception-leaks:
  More robust memory management for ZAM execution - fixes #4052

CHANGES

@@ -1,3 +1,9 @@
+7.1.0-dev.618 | 2024-11-26 17:17:00 +0100
+
+  * GH-4052: More robust memory management for ZAM execution - fixes #4052 (Vern Paxson, Corelight)
+
+  * Update SQLite to 3.47.1 (Johanna Amann, Corelight)
+
 7.1.0-dev.614 | 2024-11-26 15:25:09 +0100
 
   * ZeekArgs: Deprecate val_list_to_args() (Arne Welzel, Corelight)

VERSION

@@ -1 +1 @@
-7.1.0-dev.614
+7.1.0-dev.618

src/script_opt/ZAM/ZBody.cc

@@ -317,18 +317,45 @@ std::shared_ptr<ProfVec> ZBody::BuildProfVec() const {
     return pv;
 }
 
-// Helper class for managing dynamic frames to ensure that their memory
+// Helper class for managing ZBody state to ensure that memory is recovered
-// is recovered if a ZBody is exited via an exception.
+// if a ZBody is exited via an exception.
-class ZBodyDynamicFrame {
+class ZBodyStateManager {
 public:
-    ZBodyDynamicFrame(int frame_size) { frame = frame_size > 0 ? new ZVal[frame_size] : nullptr; }
+    // If fixed_frame is nil then creates a dynamic frame.
+    ZBodyStateManager(ZVal* _fixed_frame, int frame_size, const std::vector<int>& _managed_slots)
+        : fixed_frame(_fixed_frame), managed_slots(_managed_slots) {
+        if ( fixed_frame )
+            frame = fixed_frame;
+        else {
+            frame = new ZVal[frame_size];
+            for ( auto s : managed_slots )
+                frame[s].ClearManagedVal();
+        }
+    }
 
-    ~ZBodyDynamicFrame() { delete[] frame; }
+    ~ZBodyStateManager() {
+        if ( fixed_frame ) {
+            // Recover memory and reset for use in next call.
+            for ( auto s : managed_slots ) {
+                ZVal::DeleteManagedType(frame[s]);
+                frame[s].ClearManagedVal();
+            }
+        }
+
+        else {
+            // Recover memory, no need to reset.
+            for ( auto s : managed_slots )
+                ZVal::DeleteManagedType(frame[s]);
+            delete[] frame;
+        }
+    }
 
     auto Frame() { return frame; }
 
 private:
+    ZVal* fixed_frame;
     ZVal* frame;
+    const std::vector<int>& managed_slots;
 };
 
 ValPtr ZBody::Exec(Frame* f, StmtFlowType& flow) {
@@ -366,7 +393,7 @@ ValPtr ZBody::Exec(Frame* f, StmtFlowType& flow) {
     }
 #endif
 
-    ZBodyDynamicFrame dynamic_frame(fixed_frame ? 0 : frame_size);
+    ZBodyStateManager state_mgr(fixed_frame, frame_size, managed_slots);
     std::unique_ptr<TableIterVec> local_table_iters;
     std::vector<StepIterInfo> step_iters(num_step_iters);
 
@@ -375,10 +402,7 @@ ValPtr ZBody::Exec(Frame* f, StmtFlowType& flow) {
     if ( fixed_frame )
         frame = fixed_frame;
     else {
-        frame = dynamic_frame.Frame();
+        frame = state_mgr.Frame();
-        // Clear slots for which we do explicit memory management.
-        for ( auto s : managed_slots )
-            frame[s].ClearManagedVal();
 
         if ( ! table_iters.empty() ) {
             local_table_iters = std::make_unique<TableIterVec>(table_iters.size());
@@ -433,31 +457,6 @@ ValPtr ZBody::Exec(Frame* f, StmtFlowType& flow) {
         ++pc;
     }
 
-    auto result = ret_type ? ret_u->ToVal(ret_type) : nullptr;
-
-    if ( fixed_frame ) {
-        // Make sure we don't have any dangling iterators.
-        for ( auto& ti : table_iters )
-            ti.Clear();
-
-        // Free slots for which we do explicit memory management,
-        // preparing them for reuse.
-        for ( auto& ms : managed_slots ) {
-            auto& v = frame[ms];
-            ZVal::DeleteManagedType(v);
-            v.ClearManagedVal();
-        }
-    }
-    else {
-        // Free those slots for which we do explicit memory management.
-        // No need to then clear them, as we're about to throw away
-        // the entire frame.
-        for ( auto& ms : managed_slots ) {
-            auto& v = frame[ms];
-            ZVal::DeleteManagedType(v);
-        }
-    }
-
 #ifdef ENABLE_ZAM_PROFILE
     if ( profiling_active ) {
         tot_CPU_time += util::curr_CPU_time() - start_CPU_time;
@@ -468,7 +467,7 @@ ValPtr ZBody::Exec(Frame* f, StmtFlowType& flow) {
     }
 #endif
 
-    return result;
+    return ret_type ? ret_u->ToVal(ret_type) : nullptr;
 }
 
 void ZBody::ReportExecutionProfile(ProfMap& pm) {

testing/btest/Baseline/core.expr-exception/reporter.log

@@ -7,13 +7,13 @@
 #open XXXX-XX-XX-XX-XX-XX
 #fields	ts	level	message	location
 #types	time	enum	string	string
-XXXXXXXXXX.XXXXXX	Reporter::ERROR	field value missing (c$ftp)	<...>/expr-exception.zeek, line 16
+XXXXXXXXXX.XXXXXX	Reporter::ERROR	field value missing (c$ftp)	<...>/expr-exception.zeek, line 10
-XXXXXXXXXX.XXXXXX	Reporter::ERROR	field value missing (c$ftp)	<...>/expr-exception.zeek, line 16
+XXXXXXXXXX.XXXXXX	Reporter::ERROR	field value missing (c$ftp)	<...>/expr-exception.zeek, line 10
-XXXXXXXXXX.XXXXXX	Reporter::ERROR	field value missing (c$ftp)	<...>/expr-exception.zeek, line 16
+XXXXXXXXXX.XXXXXX	Reporter::ERROR	field value missing (c$ftp)	<...>/expr-exception.zeek, line 10
-XXXXXXXXXX.XXXXXX	Reporter::ERROR	field value missing (c$ftp)	<...>/expr-exception.zeek, line 16
+XXXXXXXXXX.XXXXXX	Reporter::ERROR	field value missing (c$ftp)	<...>/expr-exception.zeek, line 10
-XXXXXXXXXX.XXXXXX	Reporter::ERROR	field value missing (c$ftp)	<...>/expr-exception.zeek, line 16
+XXXXXXXXXX.XXXXXX	Reporter::ERROR	field value missing (c$ftp)	<...>/expr-exception.zeek, line 10
-XXXXXXXXXX.XXXXXX	Reporter::ERROR	field value missing (c$ftp)	<...>/expr-exception.zeek, line 16
+XXXXXXXXXX.XXXXXX	Reporter::ERROR	field value missing (c$ftp)	<...>/expr-exception.zeek, line 10
-XXXXXXXXXX.XXXXXX	Reporter::ERROR	field value missing (c$ftp)	<...>/expr-exception.zeek, line 16
+XXXXXXXXXX.XXXXXX	Reporter::ERROR	field value missing (c$ftp)	<...>/expr-exception.zeek, line 10
-XXXXXXXXXX.XXXXXX	Reporter::ERROR	field value missing (c$ftp)	<...>/expr-exception.zeek, line 16
+XXXXXXXXXX.XXXXXX	Reporter::ERROR	field value missing (c$ftp)	<...>/expr-exception.zeek, line 10
-XXXXXXXXXX.XXXXXX	Reporter::ERROR	field value missing (c$ftp)	<...>/expr-exception.zeek, line 16
+XXXXXXXXXX.XXXXXX	Reporter::ERROR	field value missing (c$ftp)	<...>/expr-exception.zeek, line 10
 #close XXXX-XX-XX-XX-XX-XX

testing/btest/core/expr-exception.zeek

@@ -1,11 +1,5 @@
 # Expressions in an event handler that raise interpreter exceptions
 # shouldn't abort Zeek entirely, but just return from the function body.
-#
-# Skip this test when using ZAM. It generates a memory leak on CI under ASAN,
-# which causes a build failure.
-#
-# This should be removed when #4052 has been addressed.
-# @TEST-REQUIRES: test "${ZEEK_ZAM}" != "1"
 
 # @TEST-EXEC: zeek -b -r $TRACES/wikipedia.trace base/protocols/ftp base/protocols/http base/frameworks/reporter %INPUT >output
 # @TEST-EXEC: TEST_DIFF_CANONIFIER="$SCRIPTS/diff-remove-abspath | $SCRIPTS/diff-remove-timestamps" btest-diff reporter.log