add missing write barrier; export and reuse Region.rs allocation func…

…tion in compile.ml; add Region.init_region
dfinity · Aug 16, 2023 · 18f938c · 18f938c
1 parent ee79edf
commit 18f938c
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 17 deletions.
diff --git a/rts/motoko-rts/src/region.rs b/rts/motoko-rts/src/region.rs
@@ -370,28 +370,54 @@ fn write_magic() {
     write_u32(meta_data::offset::VERSION, meta_data::version::VERSION);
 }
 
+#[ic_mem_fn]
 unsafe fn alloc_region<M: Memory>(
     mem: &mut M,
-    id: u16,
+    id: u32,
     page_count: u32,
     vec_pages: Value,
 ) -> Value {
     let r_ptr = mem.alloc_words(size_of::<Region>());
-
     // NB. cannot use as_region() here as we didn't write the header yet
     let region = r_ptr.get_ptr() as *mut Region;
     (*region).header.tag = TAG_REGION;
     (*region).header.init_forward(r_ptr);
-    (*region).id = id;
+    debug_assert!(id <= meta_data::max::REGIONS as u32);
+    (*region).id = id as u16;
     // The padding must be initialized with zero because it is read by the compiler-generated code.
     (*region).zero_padding = 0;
+    debug_assert!(
+        page_count
+            <= (vec_pages.as_blob().len().as_u32() / meta_data::bytes_of::<u16>() as u32)
+                * meta_data::size::PAGES_IN_BLOCK
+    );
     (*region).page_count = page_count;
     init_with_barrier(mem, &mut (*region).vec_pages, vec_pages);
 
     allocation_barrier(r_ptr);
     r_ptr
 }
 
+#[ic_mem_fn]
+unsafe fn init_region<M: Memory>(
+    mem: &mut M,
+    r: Value,
+    id: u32,
+    page_count: u32,
+    vec_pages: Value,
+) {
+    let r = r.as_region();
+    debug_assert!(id <= meta_data::max::REGIONS as u32);
+    (*r).id = id as u16;
+    debug_assert!(
+        page_count
+            <= (vec_pages.as_blob().len().as_u32() / meta_data::bytes_of::<u16>() as u32)
+                * meta_data::size::PAGES_IN_BLOCK
+    );
+    (*r).page_count = page_count;
+    write_with_barrier(mem, &mut (*r).vec_pages, vec_pages);
+}
+
 #[ic_mem_fn]
 pub unsafe fn region_id<M: Memory>(_mem: &mut M, r: Value) -> u32 {
     let r = r.as_region();
@@ -430,7 +456,7 @@ pub unsafe fn region_new<M: Memory>(mem: &mut M) -> Value {
 
     let vec_pages = alloc_blob(mem, Bytes(0));
     allocation_barrier(vec_pages);
-    let r_ptr = alloc_region(mem, next_id, 0, vec_pages);
+    let r_ptr = alloc_region(mem, next_id as u32, 0, vec_pages);
 
     // Update Region table.
     {
@@ -479,7 +505,7 @@ pub unsafe fn region_recover<M: Memory>(mem: &mut M, rid: &RegionId) -> Value {
     assert_eq!(recovered_blocks, block_count);
     allocation_barrier(vec_pages);
 
-    let r_ptr = alloc_region(mem, rid.0, page_count as u32, vec_pages);
+    let r_ptr = alloc_region(mem, rid.0 as u32, page_count as u32, vec_pages);
     r_ptr
 }
 

diff --git a/src/codegen/compile.ml b/src/codegen/compile.ml
@@ -998,6 +998,8 @@ module RTS = struct
     E.add_func_import env "rts" "text_size" [I32Type] [I32Type];
     E.add_func_import env "rts" "text_to_buf" [I32Type; I32Type] [];
     E.add_func_import env "rts" "region_init" [I32Type] [];
+    E.add_func_import env "rts" "alloc_region" [I32Type; I32Type; I32Type] [I32Type];
+    E.add_func_import env "rts" "init_region" [I32Type; I32Type; I32Type; I32Type] [];
     E.add_func_import env "rts" "region_new" [] [I32Type];
     E.add_func_import env "rts" "region_id" [I32Type] [I32Type];
     E.add_func_import env "rts" "region_size" [I32Type] [I64Type];
@@ -3947,7 +3949,7 @@ module Region = struct
     Tagged.load_forwarding_pointer env ^^
     Tagged.load_field env field
 
-  let store_field env region_field =
+  let _store_field env region_field =
     let field = region_field env in
     assert (field = id_field env || field = page_count_field env || field = vec_pages_field env);
     let (set_value, get_value) = new_local env "value" in
@@ -3956,8 +3958,11 @@ module Region = struct
     get_value ^^
     Tagged.store_field env field
 
-  let alloc env get_id get_pagecount get_vec_pages =
-    Tagged.obj env Tagged.Region [ get_id; get_pagecount; get_vec_pages ]
+  let alloc_region env =
+    E.call_import env "rts" "alloc_region"
+
+  let init_region env =
+    E.call_import env "rts" "init_region"
 
   let sanity_check s env =
     if !Flags.sanity then
@@ -6855,17 +6860,19 @@ module MakeSerialization (Strm : Stream) = struct
           get_region_typ ^^
           compile_eq_const (Int32.neg (Option.get (to_idl_prim (Prim Region)))) ^^
           E.else_trap_with env "deserialize_go (Region): unexpected idl_typ" ^^
-          Region.alloc env (compile_unboxed_const 0l) (compile_unboxed_const 0l) (Blob.lit env "") ^^ set_region ^^
+          (* pre-allocate a region object, with dummy fields *)
+          compile_unboxed_const 0l ^^ (* id *)
+          compile_unboxed_const 0l ^^ (* pagecount *)
+          Blob.lit env "" ^^ (* vec_pages *)
+          Region.alloc_region env ^^
+          set_region ^^
           on_alloc get_region ^^
-          get_region ^^ ReadBuf.read_word32 env get_data_buf ^^ Region.store_field env Region.id_field ^^
-          get_region ^^ ReadBuf.read_word32 env get_data_buf ^^ Region.store_field env Region.page_count_field ^^
-          (*@Luc: vec_pages_field object field update! do we need a write_barrier or is the allocation barrier enough?
-            Note that Region.alloc above already containes an allocation_barrier, but we later update the vec_pages_field.
-           *)
-          get_region ^^ read_blob () ^^ Region.store_field env Region.vec_pages_field ^^
+          (* read and initialize the region's fields *)
           get_region ^^
-          Tagged.allocation_barrier env ^^
-          G.i Drop
+          ReadBuf.read_word32 env get_data_buf ^^ (* id *)
+          ReadBuf.read_word32 env get_data_buf ^^ (* pagecount *)
+          read_blob () ^^ (* vec_pages *)
+          Region.init_region env
         )
       | Array t ->
         let (set_len, get_len) = new_local env "len" in