changeset 32376:262768aefb8b

VM Fix loading and saving from bytecode script frame (bug #64705) Makes load() and save() not panic in bytecode script frames. * libinterp/corefcn/stack-frame.cc: Use varref() for scripts in insert_symbol that got proper support for scripts. * test/compile/: Tests updated.
author Petter T. <petter.vilhelm@gmail.com>
date Sat, 07 Oct 2023 00:50:18 +0200
parents 67945d0339cf
children 94ba5c27d895 dd9b6f2ae43b
files libinterp/corefcn/stack-frame.cc test/compile/bytecode.tst test/compile/bytecode_load_script_load_and_assert.m test/compile/bytecode_load_script_save.m test/compile/module.mk
diffstat 5 files changed, 98 insertions(+), 50 deletions(-) [+]
line wrap: on
line diff
--- a/libinterp/corefcn/stack-frame.cc	Thu Oct 05 23:57:25 2023 +0200
+++ b/libinterp/corefcn/stack-frame.cc	Sat Oct 07 00:50:18 2023 +0200
@@ -752,6 +752,8 @@
       {
         // Try to find the symbol in the root frame instead.
         auto sym_in_root = access_link ()->lookup_symbol (sym.name ());
+        if (!sym_in_root)
+          return {}; // Not found, return nil
         return access_link ()->varval (sym_in_root);
       }
     else if (!in_this_frame)
@@ -1165,15 +1167,22 @@
   symbol_record insert_symbol (const std::string& name)
   {
     bool is_script = m_lazy_data && m_lazy_data->m_is_script;
-    bool allready_added;
-
-    // If the symbols is already in the immediate scope, there is
-    // nothing more to do.
 
     symbol_scope scope = get_scope ();
 
     symbol_record sym = scope.lookup_symbol (name);
 
+    if (is_script)
+      {
+        // TODO: Instead of doing a call to varref to maybe insert a symbol,
+        //       common functionality between varref and insert_symbol should
+        //       be put in some function.
+        if (!sym)
+          sym = scope.find_symbol (name);
+        varref (sym, false);
+        return sym;
+      }
+
     if (!sym)
       {
         // If we have not created the extra slots, now is the time
@@ -1190,51 +1199,6 @@
           internal_resize (local_offset + 1);
 
         internal_set_extra_name (local_offset, sym.name ());
-
-        allready_added = false;
-      }
-    else
-      allready_added = true;
-
-    // If we are in a script, we might need to add the symbol to the eval frame too,
-    // and point a ref to the script's slot from the eval frame.
-    if (is_script)
-      {
-        CHECK_PANIC (m_access_link);
-
-        bool access_allready_added;
-        
-        symbol_record sym_access = m_access_link->lookup_symbol (name);
-        if (sym_access.is_valid ())
-          access_allready_added = true;
-        else
-          {
-            access_allready_added = false;
-            sym_access =  m_access_link->insert_symbol (name);
-          }
-
-        CHECK_PANIC ((allready_added && access_allready_added) || !allready_added);
-
-        if (!allready_added)
-          {
-            octave_value &ov_access = m_access_link->varref (sym_access.data_offset (), false);
-            octave_value &ov = varref (sym.data_offset (), false);
-
-            bool make_ref = true;
-
-            if (ov_access.is_ref ())
-              {
-                octave_value_ref *r = ov_access.ref_rep ();
-                if (r->is_local_ref ())
-                  make_ref = false;
-              }
-            
-            if (make_ref)
-              {
-                ov = ov_access;
-                ov_access = octave_value (new octave_value_ref_vmlocal {sym, this});
-              }
-          }
       }
 
     return sym;
--- a/test/compile/bytecode.tst	Thu Oct 05 23:57:25 2023 +0200
+++ b/test/compile/bytecode.tst	Sat Oct 07 00:50:18 2023 +0200
@@ -785,4 +785,44 @@
 %! %% Cleanup after the tests
 %! clear global bytecode_script_topscope_call_self
 %! clear global bytecode_script_topscope_place
-%! clear bytecode_script_topscope_cli_fn
\ No newline at end of file
+%! clear bytecode_script_topscope_cli_fn
+
+## Test save() and load() from scripts.
+%!test
+%! clear all
+%!
+%! for use_vm = [true, false] % Test with and without VM
+%!   __vm_enable__ (use_vm, "local");
+%!
+%!   % We need to pass the file name to the scripts with a global
+%!   global bytecode_load_script_file
+%!   bytecode_load_script_file = [tempname(), ".mat"];
+%!
+%!   eval ("function bytecode_load_script_save_fn\n  bytecode_load_script_save\n;  bytecode_load_script_load_and_assert;\n  end\n")
+%!
+%!   if use_vm
+%!     assert (__vm_compile__ ("bytecode_load_script_save"))
+%!     assert (__vm_compile__ ("bytecode_load_script_load_and_assert"))
+%!     assert (__vm_compile__ ("bytecode_load_script_save_fn"))
+%!   end
+%!
+%!   unwind_protect
+%!     evalin ("base", "bytecode_load_script_save"); % Saves some variables to a file with save ()
+%!     evalin ("base", "bytecode_load_script_load_and_assert"); % Loads them back and checks their values etc
+%!   unwind_protect_cleanup
+%!     unlink (bytecode_load_script_file);
+%!     evalin ("base", "clear local_aa local_bb local_cc")
+%!     evalin ("base", "clear global glb_aa glb_bb glb_cc glb_dd glb_ee")
+%!   end_unwind_protect
+%!   
+%!   bytecode_load_script_file = [tempname(), ".mat"];
+%!   %% Same test, but from a function
+%!   unwind_protect
+%!     bytecode_load_script_save_fn;
+%!   unwind_protect_cleanup
+%!     unlink (bytecode_load_script_file);
+%!   end_unwind_protect
+%!   
+%!   clear all
+%! end
+%!
\ No newline at end of file
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/compile/bytecode_load_script_load_and_assert.m	Sat Oct 07 00:50:18 2023 +0200
@@ -0,0 +1,26 @@
+local_cc = 33; %Defined before the load
+glb_ee = 123;
+
+assert (!isglobal ("glb_aa"))
+assert (!isglobal ("glb_bb"))
+assert (!isglobal ("glb_cc"))
+assert (!isglobal ("glb_dd"))
+assert (!isglobal ("glb_ee"))
+
+global bytecode_load_script_file
+load (bytecode_load_script_file)
+
+assert (isglobal ("glb_aa"))
+assert (isglobal ("glb_bb"))
+assert (isglobal ("glb_cc"))
+assert (isglobal ("glb_dd"))
+assert (isglobal ("glb_ee"))
+
+assert (local_aa == 1)
+assert (local_bb == 2)
+assert (local_cc == 3)
+assert (glb_aa == 1);
+assert (isempty (glb_bb));
+assert (isempty (glb_cc));
+assert (glb_dd == 4);
+assert (glb_ee == 5);
\ No newline at end of file
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/compile/bytecode_load_script_save.m	Sat Oct 07 00:50:18 2023 +0200
@@ -0,0 +1,16 @@
+global glb_aa = 1;
+global glb_bb
+eval ("global glb_cc;")
+eval ("global glb_dd = 4;")
+global glb_ee;
+glb_ee = 5;
+
+local_aa = 1;
+eval ("local_bb = 2;")
+local_cc = 3;
+
+global bytecode_load_script_file
+save (bytecode_load_script_file, "glb_aa", "glb_bb", "glb_cc", "glb_dd", "glb_ee", "local_aa", "local_bb", "local_cc")
+
+clear global glb_aa glb_bb glb_cc glb_dd glb_ee
+clear local_aa local_bb local_cc
\ No newline at end of file
--- a/test/compile/module.mk	Thu Oct 05 23:57:25 2023 +0200
+++ b/test/compile/module.mk	Sat Oct 07 00:50:18 2023 +0200
@@ -18,6 +18,8 @@
   %reldir%/bytecode_index_obj.m \
   %reldir%/bytecode_inputname.m \
   %reldir%/bytecode_leaks.m \
+  %reldir%/bytecode_load_script_save.m \
+  %reldir%/bytecode_load_script_load_and_assert.m \
   %reldir%/bytecode_matrix.m \
   %reldir%/bytecode_misc.m \
   %reldir%/bytecode_multi_assign.m \