Memory Safety Issue when using patch or merge on state and assign the result back to state

@@ -51,6 +51,7 @@ macro_rules! test_cases {
                 out_json.reverse();
 
                 let mut results = Vec::new();
+                let mut state = Value::null();
                 for (id, mut json) in in_json.into_iter().enumerate() {
                     let uri = EventOriginUri{
                         host: "test".into(),
@@ -62,7 +63,6 @@ macro_rules! test_cases {
                     };
                     let context = EventContext::new(id as u64, Some(&uri));
                     let mut meta = Value::from(Object::default());
-                    let mut state = Value::null();
                     match script.run(&context, AggrType::Tick, &mut json, &mut state, &mut meta)? {
                         Return::Drop => (),
                         Return::EmitEvent{..} => results.push(json),
@@ -154,6 +154,7 @@ test_cases!(
     // TODO
     // const_in_const_lookup,
     // INSERT
+    merge_assign_target_state,
     expr_path,
     patch_default,
     patch_default_key,
@@ -186,7 +187,7 @@ test_cases!(
     heredoc_quoted_curly,
     string_interpolation_import,
     string_interpolation_prefix,
-    patch_in_place,
+    patch_assign_target,
     tuple_pattern,
     pattern_cmp,
     pass_args,

@@ -105,10 +105,10 @@ macro_rules! ignore_cases {
                 file.read_to_string(&mut err)?;
                 let _err = err.trim();
 
+                let mut state = Value::null();
                 if let Some(mut json) =  in_json.pop() {
                     let context = EventContext::new(0, None);
                     let mut meta = Value::object();
-                    let mut state = Value::null();
                     let s = script.run(&context, AggrType::Tick, &mut json, &mut state, &mut meta);
                     if let Err(e) = s {
                         let mut h = Dumb::new();
@@ -144,8 +144,8 @@ test_cases!(
     function_error_n,
     match_bad_guard_type,
     match_no_clause_hit,
-    merge_in_place_new_no_object,
-    merge_in_place_target_no_object,
+    merge_assign_target_new_no_object,
+    merge_assign_target_target_no_object,
     merge_new_no_object,
     merge_target_no_object,
     missing_local,

@@ -0,0 +1,2 @@
+{"foo": "bar"}
+{"snot": "badger", "foo": "grmpf"}
\ No newline at end of file

@@ -0,0 +1,2 @@
+{"foo":"bar"}
+{"foo":"grmpf","snot":"badger"}
\ No newline at end of file

@@ -0,0 +1,8 @@
+# initialize state to record, so we can do the merge
+let state = match state of
+	case null => {}
+	default => state
+end;
+
+let state = merge state of event end;
+emit state

@@ -248,8 +248,6 @@ impl<'script> BaseExpr for Expr<'script> {
             Expr::Emit(e) => e.mid(),
             Expr::Imut(e) => e.mid(),
             Expr::Match(e) => e.mid(),
-            Expr::MergeInPlace(e) => e.mid(),
-            Expr::PatchInPlace(e) => e.mid(),
             Expr::IfElse(e) => e.mid(),
         }
     }

@@ -18,14 +18,14 @@
 
 use crate::{
     ast::{
-        base_expr, eq::AstEq, query, upable::Upable, ArrayPattern, ArrayPredicatePattern,
-        AssignPattern, BinExpr, BinOpKind, Bytes, BytesPart, ClauseGroup, Comprehension,
-        ComprehensionCase, Costly, DefaultCase, EmitExpr, EventPath, Expr, ExprPath, Expression,
-        Field, FnDecl, FnDoc, Helper, Ident, IfElse, ImutExpr, ImutExprInt, Invocable, Invoke,
-        InvokeAggr, InvokeAggrFn, List, Literal, LocalPath, Match, Merge, MetadataPath, ModDoc,
-        NodeMetas, Patch, PatchOperation, Path, Pattern, PredicateClause, PredicatePattern, Record,
-        RecordPattern, Recur, ReservedPath, Script, Segment, StatePath, StrLitElement, StringLit,
-        TestExpr, TuplePattern, UnaryExpr, UnaryOpKind,
+        base_expr, query, upable::Upable, ArrayPattern, ArrayPredicatePattern, AssignPattern,
+        BinExpr, BinOpKind, Bytes, BytesPart, ClauseGroup, Comprehension, ComprehensionCase,
+        Costly, DefaultCase, EmitExpr, EventPath, Expr, ExprPath, Expression, Field, FnDecl, FnDoc,
+        Helper, Ident, IfElse, ImutExpr, ImutExprInt, Invocable, Invoke, InvokeAggr, InvokeAggrFn,
+        List, Literal, LocalPath, Match, Merge, MetadataPath, ModDoc, NodeMetas, Patch,
+        PatchOperation, Path, Pattern, PredicateClause, PredicatePattern, Record, RecordPattern,
+        Recur, ReservedPath, Script, Segment, StatePath, StrLitElement, StringLit, TestExpr,
+        TuplePattern, UnaryExpr, UnaryOpKind,
     },
     errors::{
         err_generic, error_generic, error_missing_effector, error_oops, Error, ErrorKind, Result,
@@ -646,28 +646,16 @@ impl<'script> Upable<'script> for ExprRaw<'script> {
                 let path = a.path.up(helper)?;
                 let mid = helper.add_meta(a.start, a.end);
                 match a.expr.up(helper)? {
-                    Expr::Imut(ImutExprInt::Merge(m)) => {
-                        if path.ast_eq(&m.target) {
-                            Expr::MergeInPlace(Box::new(*m))
-                        } else {
-                            Expr::Assign {
-                                mid,
-                                path,
-                                expr: Box::new(ImutExprInt::Merge(m).into()),
-                            }
-                        }
-                    }
-                    Expr::Imut(ImutExprInt::Patch(m)) => {
-                        if path.ast_eq(&m.target) {
-                            Expr::PatchInPlace(Box::new(*m))
-                        } else {
-                            Expr::Assign {
-                                mid,
-                                path,
-                                expr: Box::new(ImutExprInt::Patch(m).into()),
-                            }
-                        }
-                    }
+                    Expr::Imut(ImutExprInt::Merge(m)) => Expr::Assign {
+                        mid,
+                        path,
+                        expr: Box::new(ImutExprInt::Merge(m).into()),
+                    },
+                    Expr::Imut(ImutExprInt::Patch(m)) => Expr::Assign {
+                        mid,
+                        path,
+                        expr: Box::new(ImutExprInt::Patch(m).into()),
+                    },
                     expr => Expr::Assign {
                         mid,
                         path,

@@ -961,10 +961,6 @@ pub enum Expr<'script> {
     Match(Box<Match<'script, Self>>),
     /// IfElse style match expression
     IfElse(Box<IfElse<'script, Self>>),
-    /// In place patch expression
-    PatchInPlace(Box<Patch<'script>>),
-    /// In place merge expression
-    MergeInPlace(Box<Merge<'script>>),
     /// Assignment expression
     Assign {
         /// Id

@@ -159,8 +159,6 @@ impl<'script> Expr<'script> {
         match self {
             Expr::Match(e) => Expr::Match(Box::new(e.into_static())),
             Expr::IfElse(e) => Expr::IfElse(Box::new(e.into_static())),
-            Expr::PatchInPlace(e) => Expr::PatchInPlace(Box::new(e.into_static())),
-            Expr::MergeInPlace(e) => Expr::MergeInPlace(Box::new(e.into_static())),
             Expr::Assign { mid, path, expr } => Expr::Assign {
                 mid,
                 path: path.into_static(),

@@ -13,24 +13,23 @@
 // limitations under the License.
 
 use super::{
-    merge_values, patch_value, resolve, resolve_value, set_local_shadow, test_guard,
-    test_predicate_expr, Env, ExecOpts, LocalStack, NULL,
+    resolve, resolve_value, set_local_shadow, test_guard, test_predicate_expr, Env, ExecOpts,
+    LocalStack, NULL,
 };
 use crate::errors::{
     error_assign_array, error_assign_to_const, error_bad_key_err, error_invalid_assign_target,
-    error_need_obj, error_need_obj_err, error_no_clause_hit, Result,
+    error_need_obj_err, error_no_clause_hit, Result,
 };
 use crate::prelude::*;
 use crate::registry::RECUR_PTR;
 use crate::{
     ast::{
         BaseExpr, ClauseGroup, ClausePreCondition, Comprehension, DefaultCase, EmitExpr, EventPath,
-        Expr, IfElse, ImutExprInt, Match, Merge, Patch, Path, Segment,
+        Expr, IfElse, ImutExprInt, Match, Path, Segment,
     },
     errors::error_oops_err,
 };
 use crate::{stry, Value};
-use matches::matches;
 use std::mem;
 use std::{
     borrow::{Borrow, Cow},
@@ -219,104 +218,6 @@ impl<'script> Expr<'script> {
         }
     }
 
-    fn patch_in_place<'run, 'event>(
-        opts: ExecOpts,
-        env: &'run Env<'run, 'event>,
-        event: &'run Value<'event>,
-        state: &'run Value<'static>,
-        meta: &'run Value<'event>,
-        local: &'run LocalStack<'event>,
-        expr: &'run Patch<'event>,
-    ) -> Result<Cow<'run, Value<'event>>> {
-        // This function is called when we encounter code that consumes a value
-        // to patch it. So the following code:
-        // ```tremor
-        // let event = patch event of insert "key" => "value" end
-        // ```
-        // When executed on it's own would clone the event, add a key and
-        // overwrite original event.
-        //
-        // We optimise this as:
-        // ```
-        // patch_in_place event of insert "key" => "value" end
-        // ```
-        //
-        // This code is generated in impl Upable for ExprRaw where the following
-        // checks are performed:
-        //
-        // 1) the patch is on the RHS of an assignment
-        // 2) the path of the assigned value and the path of the patched
-        //    expression are identical.
-        //
-        // In turn this guarantees (at compile time):
-        //
-        // 1) The target (`expr`) is a path lookup
-        // 2) The target is not a known constant as otherwise the assignment
-        //    will complan
-        // 3) this leave the `expr` to be either a local, the event, the state,
-        //    metadata or a subkey thereof.
-        //
-        // And the following guarantees at run time:
-        //
-        // 1) the `expr` is an existing key of the mentioned categories,
-        //    otherwise `expr.target.run` will error.
-        // 2) `value` will never be owned (however the resolve function is
-        //    generic so it needs to return a Cow)
-
-        let value: Cow<'run, Value<'event>> =
-            stry!(expr.target.run(opts, env, event, state, meta, local));
-        debug_assert!(
-            !matches!(value, Cow::Owned(_)),
-            "We should never see a owned value here as patch_in_place is only ever called on existing data in event, state, meta or local"
-        );
-        let v: &Value<'event> = value.borrow();
-        // ALLOW: https://github.com/tremor-rs/tremor-runtime/issues/1032
-        #[allow(mutable_transmutes, clippy::transmute_ptr_to_ptr)]
-        // ALLOW: https://github.com/tremor-rs/tremor-runtime/issues/1032
-        let v: &mut Value<'event> = unsafe { mem::transmute(v) };
-        stry!(patch_value(opts, env, event, state, meta, local, v, expr));
-        Ok(value)
-    }
-
-    fn merge_in_place<'run, 'event>(
-        &'run self,
-        opts: ExecOpts,
-        env: &'run Env<'run, 'event>,
-        event: &'run mut Value<'event>,
-        state: &'run mut Value<'static>,
-        meta: &'run mut Value<'event>,
-        local: &'run mut LocalStack<'event>,
-        expr: &'run Merge<'event>,
-    ) -> Result<Cow<'run, Value<'event>>> {
-        // Please see the soundness reasoning in `patch_in_place` for details
-        // those functions perform the same function just with slighty different
-        // operations.
-        let value_cow: Cow<'run, Value<'event>> =
-            stry!(expr.target.run(opts, env, event, state, meta, local));
-        debug_assert!(
-            !matches!(value_cow, Cow::Owned(_)),
-            "We should never see a owned value here as merge_in_place is only ever called on existing data in event, state, meta or local"
-        );
-
-        if value_cow.is_object() {
-            let value: &Value<'event> = value_cow.borrow();
-            // ALLOW: https://github.com/tremor-rs/tremor-runtime/issues/1032
-            #[allow(mutable_transmutes, clippy::transmute_ptr_to_ptr)]
-            // ALLOW: https://github.com/tremor-rs/tremor-runtime/issues/1032
-            let value: &mut Value<'event> = unsafe { mem::transmute(value) };
-            let replacement = stry!(expr.expr.run(opts, env, event, state, meta, local,));
-
-            if replacement.is_object() {
-                stry!(merge_values(self, &expr.expr, value, &replacement));
-                Ok(value_cow)
-            } else {
-                error_need_obj(self, &expr.expr, replacement.value_type(), env.meta)
-            }
-        } else {
-            error_need_obj(self, &expr.target, value_cow.value_type(), env.meta)
-        }
-    }
-
     // TODO: Quite some overlap with `ImutExprInt::comprehension`
     fn comprehension<'run, 'event>(
         &'run self,
@@ -641,12 +542,6 @@ impl<'script> Expr<'script> {
             }
             Expr::Match(ref expr) => self.match_expr(opts, env, event, state, meta, local, expr),
             Expr::IfElse(ref expr) => self.if_expr(opts, env, event, state, meta, local, expr),
-            Expr::MergeInPlace(ref expr) => self
-                .merge_in_place(opts, env, event, state, meta, local, expr)
-                .map(Cont::Cont),
-            Expr::PatchInPlace(ref expr) => {
-                Self::patch_in_place(opts, env, event, state, meta, local, expr).map(Cont::Cont)
-            }
             Expr::Comprehension(ref expr) => {
                 self.comprehension(opts, env, event, state, meta, local, expr)
             }