Skip to content

Gabor/typtbl #5642

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from
Draft

Gabor/typtbl #5642

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
92604cd
tweaks
ggreif Nov 12, 2025
267acdd
more tweaks
ggreif Nov 12, 2025
92a0895
WIP: limit parsing or the type table
ggreif Nov 12, 2025
34c4371
pass in an estimate
ggreif Nov 12, 2025
b8ff2c4
WIP: this might pass
ggreif Nov 13, 2025
66182be
implement the linear rule
ggreif Nov 13, 2025
f703cca
tweak the linear rule to cover `run-drun/env-vars.mo`
ggreif Nov 13, 2025
c6a3d08
format
ggreif Nov 13, 2025
efb98d5
Updating `test/bench` numbers
github-actions[bot] Nov 13, 2025
0c0dd98
Update rts/motoko-rts/src/buf.rs
ggreif Nov 17, 2025
7835f37
Updating `test/bench` numbers
github-actions[bot] Nov 17, 2025
b3dc0a5
tweak and add more explanation
ggreif Nov 17, 2025
df2393b
add `get/setCandidTypeLimits`
ggreif Nov 18, 2025
f536cc7
compute!
ggreif Nov 18, 2025
1580442
Merge branch 'master' into gabor/typtbl
ggreif Nov 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions rts/motoko-rts/src/buf.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,17 @@ impl Buf {
pub(crate) unsafe fn advance(self: *mut Self, n: usize) {
advance(self, n)
}

/// Restrict the read area
#[cfg(feature = "ic")]
pub(crate) unsafe fn limit_size(self: *mut Self, lim: usize) -> *mut u8 {
let limit_ptr = (*self).ptr.add(lim);
let prev = (*self).end;
if limit_ptr < prev {
(*self).end = limit_ptr;
}
prev
}
}

/// Read a single byte
Expand All @@ -36,6 +47,7 @@ pub(crate) unsafe fn read_word(buf: *mut Buf) -> u32 {
const WORD_SIZE: usize = core::mem::size_of::<u32>();

if (*buf).ptr.add(WORD_SIZE - 1) >= (*buf).end {
// TODO: use add(WORD_SIZE) and >
Copy link
Contributor Author

@ggreif ggreif Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would aid CSE.

idl_trap_with("word read out of buffer");
}

Expand Down
21 changes: 14 additions & 7 deletions rts/motoko-rts/src/idl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ unsafe fn parse_idl_header<M: Memory>(
mem: &mut M,
extended: bool,
buf: *mut Buf,
typtbl_max: usize,
typtbl_out: *mut *mut *mut u8,
typtbl_size_out: *mut usize,
main_types_out: *mut *mut u8,
Expand Down Expand Up @@ -211,6 +212,9 @@ unsafe fn parse_idl_header<M: Memory>(
// Allocate the type table to be passed out
let typtbl: *mut *mut u8 = alloc(mem, Words(n_types as usize)) as *mut _;

// Set up a limited buffer to detect absurd type tables
let full_end = buf.limit_size(typtbl_max);

// Go through the table
for i in 0..n_types {
*typtbl.add(i as usize) = (*buf).ptr;
Expand Down Expand Up @@ -294,6 +298,9 @@ unsafe fn parse_idl_header<M: Memory>(
}
}

// Restore limit
(*buf).end = full_end;

// Now that we have the indices, we can go through it again
// and validate that all service method types are really function types
// (We could not do that in the first run because of possible forward
Expand Down Expand Up @@ -1258,7 +1265,7 @@ pub(crate) unsafe fn sub(

#[no_mangle]
unsafe extern "C" fn idl_sub_buf_words(typtbl_size1: usize, typtbl_size2: usize) -> usize {
return BitRel::words(typtbl_size1, typtbl_size2);
BitRel::words(typtbl_size1, typtbl_size2)
}

#[no_mangle]
Expand Down Expand Up @@ -1306,19 +1313,19 @@ unsafe extern "C" fn idl_sub(
t1: i32,
t2: i32,
) -> bool {
debug_assert!(rel_buf != (0 as *mut usize));
debug_assert!(typtbl1 != (0 as *mut *mut u8));
debug_assert!(typtbl2 != (0 as *mut *mut u8));
debug_assert!(typtbl_end1 != (0 as *mut u8));
debug_assert!(typtbl_end2 != (0 as *mut u8));
debug_assert!(rel_buf != 0 as *mut usize);
debug_assert!(typtbl1 != 0 as *mut *mut u8);
debug_assert!(typtbl2 != 0 as *mut *mut u8);
debug_assert!(typtbl_end1 != 0 as *mut u8);
debug_assert!(typtbl_end2 != 0 as *mut u8);

let rel = BitRel {
ptr: rel_buf,
end: rel_buf.add(idl_sub_buf_words(typtbl_size1, typtbl_size2) as usize),
size1: typtbl_size1,
size2: typtbl_size2,
};
debug_assert!(t1 < (typtbl_size1 as i32) && t2 < (typtbl_size2 as i32));
debug_assert!(t1 < typtbl_size1 as i32 && t2 < typtbl_size2 as i32);
return sub(
&rel,
true,
Expand Down
12 changes: 7 additions & 5 deletions src/codegen/compile_classical.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -761,7 +761,7 @@ let compile_op_const op i =
G.i (Binary (Wasm.Values.I32 op))
let compile_add_const = compile_op_const I32Op.Add
let compile_sub_const = compile_op_const I32Op.Sub
let compile_mul_const = compile_op_const I32Op.Mul
let compile_mul_const = compile_op_const I32Op.Mul (* TODO: use shift: compile_mul_const 2l *)
Copy link
Contributor Author

@ggreif ggreif Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #5651.

let compile_divU_const = compile_op_const I32Op.DivU
let compile_shrU_const = compile_op_const I32Op.ShrU
let compile_shrS_const = compile_op_const I32Op.ShrS
Expand All @@ -783,7 +783,7 @@ let compile_op64_const op i =
G.i (Binary (Wasm.Values.I64 op))
let compile_add64_const = compile_op64_const I64Op.Add
let compile_sub64_const = compile_op64_const I64Op.Sub
let compile_mul64_const = compile_op64_const I64Op.Mul
let compile_mul64_const = compile_op64_const I64Op.Mul (* TODO: use shift: compile_mul_const 2l *)
let _compile_divU64_const = compile_op64_const I64Op.DivU
let compile_shrU64_const = function
| 0L -> G.nop | n -> compile_op64_const I64Op.ShrU n
Expand Down Expand Up @@ -1072,7 +1072,7 @@ module RTS = struct
let system_imports env =
E.add_func_import env "rts" "memcmp" [I32Type; I32Type; I32Type] [I32Type];
E.add_func_import env "rts" "version" [] [I32Type];
E.add_func_import env "rts" "parse_idl_header" [I32Type; I32Type; I32Type; I32Type; I32Type] [];
E.add_func_import env "rts" "parse_idl_header" [I32Type; I32Type; I32Type; I32Type; I32Type; I32Type] [];
E.add_func_import env "rts" "idl_sub_buf_words" [I32Type; I32Type] [I32Type];
E.add_func_import env "rts" "idl_sub_buf_init" [I32Type; I32Type; I32Type] [];
E.add_func_import env "rts" "idl_sub"
Expand Down Expand Up @@ -7455,7 +7455,7 @@ module MakeSerialization (Strm : Stream) = struct
comparison.
*)
let coercion_error_value env : int32 =
Tagged.shared_static_obj env Tagged.CoercionFailure []
Tagged.(shared_static_obj env CoercionFailure [])

(* See Note [Candid subtype checks] *)
let with_rel_buf_opt env extended get_typtbl_size1 f =
Expand Down Expand Up @@ -8299,7 +8299,9 @@ module MakeSerialization (Strm : Stream) = struct
ReadBuf.set_size get_ref_buf (get_refs_size ^^ compile_mul_const Heap.word_size) ^^

(* Go! *)
Bool.lit extended ^^ get_data_buf ^^ get_typtbl_ptr ^^ get_typtbl_size_ptr ^^ get_maintyps_ptr ^^
let tydesc, _, _ = type_desc env ts in
let tydesc_len = Int32.of_int (String.length tydesc * 2 + 300) in
Bool.lit extended ^^ get_data_buf ^^ compile_unboxed_const tydesc_len ^^ get_typtbl_ptr ^^ get_typtbl_size_ptr ^^ get_maintyps_ptr ^^
E.call_import env "rts" "parse_idl_header" ^^

(* Allocate memo table, if necessary *)
Expand Down
8 changes: 5 additions & 3 deletions src/codegen/compile_enhanced.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -844,7 +844,7 @@ let compile_op_const op i =
G.i (Binary (Wasm_exts.Values.I64 op))
let compile_add_const = compile_op_const I64Op.Add
let compile_sub_const = compile_op_const I64Op.Sub
let compile_mul_const = compile_op_const I64Op.Mul
let compile_mul_const = compile_op_const I64Op.Mul (* TODO: use shift: compile_mul_const 2l *)
let compile_divU_const = compile_op_const I64Op.DivU
let compile_shrU_const = compile_op_const I64Op.ShrU
let compile_shrS_const = compile_op_const I64Op.ShrS
Expand Down Expand Up @@ -1150,7 +1150,7 @@ module RTS = struct
E.add_func_import env "rts" "get_upgrade_instructions" [] [I64Type];
E.add_func_import env "rts" "memcmp" [I64Type; I64Type; I64Type] [I32Type];
E.add_func_import env "rts" "version" [] [I64Type];
E.add_func_import env "rts" "parse_idl_header" [I32Type; I64Type; I64Type; I64Type; I64Type] [];
E.add_func_import env "rts" "parse_idl_header" [I32Type; I64Type; I64Type; I64Type; I64Type; I64Type] [];
E.add_func_import env "rts" "idl_alloc_typtbl" [I64Type; I64Type; I64Type; I64Type; I64Type] [];
E.add_func_import env "rts" "idl_sub_buf_words" [I64Type; I64Type] [I64Type];
E.add_func_import env "rts" "idl_sub_buf_init" [I64Type; I64Type; I64Type] [];
Expand Down Expand Up @@ -8781,7 +8781,9 @@ module Serialization = struct
ReadBuf.set_size get_ref_buf (get_refs_size ^^ compile_mul_const Heap.word_size) ^^

(* Go! *)
Bool.lit extended ^^ Bool.to_rts_int32 ^^ get_data_buf ^^ get_typtbl_ptr ^^ get_typtbl_size_ptr ^^ get_maintyps_ptr ^^
let tydesc, _, _ = type_desc env Candid ts in
let tydesc_len = Int64.of_int (String.length tydesc * 2 + 300) in
Copy link
Contributor Author

@ggreif ggreif Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't limit when extended.

Copy link
Contributor Author

@ggreif ggreif Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May do it in the RTS, but we have to be careful to not reuse functions across this flag.

Copy link
Contributor Author

@ggreif ggreif Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about it, we could leave the rule in, as the type table is not expected to shrink considerably for orthogonal persistence between upgrades. Needs more thinking.

Copy link
Contributor Author

@ggreif ggreif Nov 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aaah ok, they are separated already: @deserialize_extended and @deserialize.

Bool.(lit extended ^^ to_rts_int32) ^^ get_data_buf ^^ compile_unboxed_const tydesc_len ^^ get_typtbl_ptr ^^ get_typtbl_size_ptr ^^ get_maintyps_ptr ^^
E.call_import env "rts" "parse_idl_header" ^^

(* Allocate global type type, if necessary for subtype checks *)
Expand Down
2 changes: 1 addition & 1 deletion test/bench/ok/bignum.drun-run.ok
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ ingress Completed: Reply: 0x4449444c016c01b3c4b1f204680100010a000000000000000001
ingress Completed: Reply: 0x4449444c0000
debug.print: {cycles = 2_509_532; size = +25_528}
ingress Completed: Reply: 0x4449444c0000
debug.print: {cycles = 54_921_553; size = +830_464}
debug.print: {cycles = 54_921_579; size = +830_464}
Copy link
Contributor Author

@ggreif ggreif Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minimal penalty.

ingress Completed: Reply: 0x4449444c0000
ingress Completed: Reply: 0x4449444c0000
2 changes: 1 addition & 1 deletion test/bench/ok/candid-subtype-cost.drun-run.ok
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
ingress Completed: Reply: 0x4449444c016c01b3c4b1f204680100010a00000000000000000101
ingress Completed: Reply: 0x4449444c0000
debug.print: {cycles = 1_121_619; heap_bytes = +33_888}
debug.print: {cycles = 1_121_645; heap_bytes = +33_888}
ingress Completed: Reply: 0x4449444c0000
ingress Completed: Reply: 0x4449444c0000