bugfix: fixes issue #4229 (#4230)

The regions PR added a new (extended) Candid type, IDL_EXT_REGION, that is only valid in extended Candid type tables.

This PR fixes the type table validation logic to properly accept/reject IDL_EXT_REGION occurrences accordingly.

(side hustle: don't delete Linux swap files on CI builds - seems to have been a source of non-(or-delayed)-termination in CI, leading to timeouts)

.github/workflows/test.yml

@@ -27,23 +27,23 @@ jobs:
         name: ic-hs-test
         authToken: '${{ secrets.CACHIX_AUTH_TOKEN }}'
 
-    #   # Free up disk space on Ubuntu
-    # - name: Free Disk Space (Ubuntu)
-    #   uses: insightsengineering/disk-space-reclaimer@v1
-    #   if: startsWith(matrix.os, 'ubuntu-')
-    #   with:
-    #     # this might remove tools that are actually needed,
-    #     # if set to "true" but frees about 6 GB
-    #     tool-cache: false
+      # Free up disk space on Ubuntu
+    - name: Free Disk Space (Ubuntu)
+      uses: insightsengineering/disk-space-reclaimer@v1
+      if: startsWith(matrix.os, 'ubuntu-')
+      with:
+        # this might remove tools that are actually needed,
+        # if set to "true" but frees about 6 GB
+        tool-cache: false
 
-    #     # all of these default to true, but feel free to set to
-    #     # "false" if necessary for your workflow
-    #     android: true
-    #     dotnet: true
-    #     haskell: true
-    #     large-packages: true
-    #     docker-images: true
-    #     swap-storage: true
+        # all of these default to true, but feel free to set to
+        # "false" if necessary for your workflow
+        android: true
+        dotnet: true
+        haskell: true
+        large-packages: true
+        docker-images: true
+        swap-storage: false
 
     # until https://github.com/cachix/cachix-action/issues/86 is fixed:
     - run: cachix watch-store ic-hs-test &

rts/motoko-rts/src/idl.rs

@@ -41,8 +41,11 @@ const IDL_CON_func: i32 = -22;
 const IDL_CON_service: i32 = -23;
 
 const IDL_REF_principal: i32 = -24;
+
+// Extended Candid only
 const IDL_EXT_region: i32 = -128;
 
+// Extended Candid only
 const IDL_CON_alias: i32 = 1;
 
 const IDL_PRIM_lowest: i32 = -17;
@@ -51,8 +54,9 @@ pub unsafe fn leb128_decode_ptr(buf: *mut Buf) -> (u32, *mut u8) {
     (leb128_decode(buf), (*buf).ptr)
 }
 
-unsafe fn is_primitive_type(ty: i32) -> bool {
-    ty < 0 && (ty >= IDL_PRIM_lowest || ty == IDL_REF_principal || ty == IDL_EXT_region)
+unsafe fn is_primitive_type(extended: bool, ty: i32) -> bool {
+    ty < 0
+        && (ty >= IDL_PRIM_lowest || ty == IDL_REF_principal || (extended && ty == IDL_EXT_region))
 }
 
 // TBR; based on Text.text_compare
@@ -74,14 +78,14 @@ unsafe fn utf8_cmp(len1: u32, p1: *mut u8, len2: u32, p2: *mut u8) -> i32 {
     }
 }
 
-unsafe fn check_typearg(ty: i32, n_types: u32) {
+unsafe fn check_typearg(extended: bool, ty: i32, n_types: u32) {
     // Arguments to type constructors can be primitive types or type indices
-    if !(is_primitive_type(ty) || (ty >= 0 && (ty as u32) < n_types)) {
+    if !(is_primitive_type(extended, ty) || (ty >= 0 && (ty as u32) < n_types)) {
         idl_trap_with("invalid type argument");
     }
 }
 
-unsafe fn parse_fields(buf: *mut Buf, n_types: u32) {
+unsafe fn parse_fields(extended: bool, buf: *mut Buf, n_types: u32) {
     let mut next_valid = 0;
     for n in (1..=leb128_decode(buf)).rev() {
         let tag = leb128_decode(buf);
@@ -90,7 +94,7 @@ unsafe fn parse_fields(buf: *mut Buf, n_types: u32) {
         }
         next_valid = tag + 1;
         let t = sleb128_decode(buf);
-        check_typearg(t, n_types);
+        check_typearg(extended, t, n_types);
     }
 }
 
@@ -161,32 +165,32 @@ unsafe fn parse_idl_header<M: Memory>(
             // internal
             // See Note [mutable stable values] in codegen/compile.ml
             let t = sleb128_decode(buf);
-            check_typearg(t, n_types);
+            check_typearg(extended, t, n_types);
         } else if ty >= 0 {
             idl_trap_with("illegal type table"); // illegal
-        } else if is_primitive_type(ty) {
+        } else if is_primitive_type(extended, ty) {
             // illegal
             idl_trap_with("primitive type in type table");
         } else if ty == IDL_CON_opt {
             let t = sleb128_decode(buf);
-            check_typearg(t, n_types);
+            check_typearg(extended, t, n_types);
         } else if ty == IDL_CON_vec {
             let t = sleb128_decode(buf);
-            check_typearg(t, n_types);
+            check_typearg(extended, t, n_types);
         } else if ty == IDL_CON_record {
-            parse_fields(buf, n_types);
+            parse_fields(extended, buf, n_types);
         } else if ty == IDL_CON_variant {
-            parse_fields(buf, n_types);
+            parse_fields(extended, buf, n_types);
         } else if ty == IDL_CON_func {
             // Arg types
             for _ in 0..leb128_decode(buf) {
                 let t = sleb128_decode(buf);
-                check_typearg(t, n_types);
+                check_typearg(extended, t, n_types);
             }
             // Ret types
             for _ in 0..leb128_decode(buf) {
                 let t = sleb128_decode(buf);
-                check_typearg(t, n_types);
+                check_typearg(extended, t, n_types);
             }
             // Annotations
             for _ in 0..leb128_decode(buf) {
@@ -225,7 +229,7 @@ unsafe fn parse_idl_header<M: Memory>(
 
                 // Type
                 let t = sleb128_decode(buf);
-                check_typearg(t, n_types);
+                check_typearg(extended, t, n_types);
             }
         } else {
             // Future type
@@ -272,7 +276,7 @@ unsafe fn parse_idl_header<M: Memory>(
     *main_types_out = (*buf).ptr;
     for _ in 0..leb128_decode(buf) {
         let t = sleb128_decode(buf);
-        check_typearg(t, n_types);
+        check_typearg(extended, t, n_types);
     }
 
     *typtbl_out = typtbl;
@@ -522,7 +526,7 @@ unsafe extern "C" fn skip_fields(tb: *mut Buf, buf: *mut Buf, typtbl: *mut *mut
 }
 
 unsafe fn is_opt_reserved(typtbl: *mut *mut u8, end: *mut u8, t: i32) -> bool {
-    if is_primitive_type(t) {
+    if is_primitive_type(false, t) {
         return t == IDL_PRIM_reserved;
     }
 
@@ -564,7 +568,7 @@ unsafe fn sub(
     };
 
     /* primitives reflexive */
-    if is_primitive_type(t1) && is_primitive_type(t2) && t1 == t2 {
+    if is_primitive_type(false, t1) && is_primitive_type(false, t2) && t1 == t2 {
         return true;
     }
 

test/bench/ok/bignum.drun-run-opt.ok

@@ -2,5 +2,5 @@ ingress Completed: Reply: 0x4449444c016c01b3c4b1f204680100010a000000000000000001
 ingress Completed: Reply: 0x4449444c0000
 debug.print: {cycles = 2_389_399; size = +59_652}
 ingress Completed: Reply: 0x4449444c0000
-debug.print: {cycles = 102_989_422; size = +1_817_872}
+debug.print: {cycles = 102_989_440; size = +1_817_872}
 ingress Completed: Reply: 0x4449444c0000

test/bench/ok/bignum.drun-run.ok

@@ -2,5 +2,5 @@ ingress Completed: Reply: 0x4449444c016c01b3c4b1f204680100010a000000000000000001
 ingress Completed: Reply: 0x4449444c0000
 debug.print: {cycles = 2_493_575; size = +59_652}
 ingress Completed: Reply: 0x4449444c0000
-debug.print: {cycles = 103_046_415; size = +1_817_872}
+debug.print: {cycles = 103_046_430; size = +1_817_872}
 ingress Completed: Reply: 0x4449444c0000

test/run-drun/issue-4229.mo

@@ -0,0 +1,12 @@
+import Prim  = "mo:prim";
+Prim.debugPrint(debug_show(to_candid(null)));
+assert "\44\49\44\4C\00\01\7F" == to_candid (null);
+// \80\7F (-128) is IDL_EXT_REGION descriptor,
+// and should not appear in a valid (pure) Candid type table
+assert ?null == (from_candid "\44\49\44\4C\00\01\7F" : ?Null);
+ignore from_candid "\44\49\44\4C\00\01\80\7F" : ?Null; // should trap
+assert false; // ohoh
+
+//SKIP run
+//SKIP run-low
+//SKIP run-ir

test/run-drun/ok/issue-4229.drun-run.ok

@@ -0,0 +1,3 @@
+ingress Completed: Reply: 0x4449444c016c01b3c4b1f204680100010a00000000000000000101
+debug.print: "\44\49\44\4C\00\01\7F"
+ingress Err: IC0503: Canister rwlgt-iiaaa-aaaaa-aaaaa-cai trapped explicitly: IDL error: invalid type argument