Skip to content

Commit

Permalink
bugfix: fix Region.storeNat32/loadNat32 and Region.storeInt32/loadInt…
Browse files Browse the repository at this point in the history
…32 (#4335)

bugfix: fix broken implementations of `Region.loadNat32`, `Region.storeNat32`, `Region.loadInt32`, `Region.storeInt32` (#4335). The operations were writing the 32-bit vanilla representation of the Motoko values, not the decoded values. 

Values previously stored with the broken 32-bit operations must be loaded with care.
If bit 0 is clear, the original value can be obtained by an arithmetic shift right by 1 bit.
If bit 0 is set, the value cannot be trusted and should be ignored (it encodes some transient address of a boxed value).
  • Loading branch information
crusso authored Dec 20, 2023
1 parent c20e455 commit fe03888
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 2 deletions.
8 changes: 8 additions & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
# Motoko compiler changelog

* motoko (`moc`)

* bugfix: fix broken implementations of `Region.loadNat32`, `Region.storeNat32`, `Region.loadInt32`, `Region.storeInt32` (#4335).
Values previously stored with the broken 32-bit operations must be loaded with care.
If bit 0 is clear, the original value can be obtained by an arithmetic shift right by 1 bit.
If bit 0 is set, the value cannot be trusted and should be ignored
(it encodes some transient address of a boxed value).

## 0.10.2 (2023-11-12)

* motoko (`moc`)
Expand Down
4 changes: 2 additions & 2 deletions src/codegen/compile.ml
Original file line number Diff line number Diff line change
Expand Up @@ -10871,7 +10871,7 @@ and compile_prim_invocation (env : E.t) ae p es at =
Region.store_word16 env

| OtherPrim ("regionLoadNat32" | "regionLoadInt32"), [e0; e1] ->
SR.Vanilla,
SR.UnboxedWord32,
compile_exp_as env ae SR.Vanilla e0 ^^
compile_exp_as env ae SR.UnboxedWord64 e1 ^^
Region.load_word32 env
Expand All @@ -10880,7 +10880,7 @@ and compile_prim_invocation (env : E.t) ae p es at =
SR.unit,
compile_exp_as env ae SR.Vanilla e0 ^^
compile_exp_as env ae SR.UnboxedWord64 e1 ^^
compile_exp_as env ae SR.Vanilla e2 ^^
compile_exp_as env ae SR.UnboxedWord32 e2 ^^
Region.store_word32 env

| OtherPrim ("regionLoadNat64" | "regionLoadInt64"), [e0; e1] ->
Expand Down
2 changes: 2 additions & 0 deletions test/run-drun/ok/region-nat32-bug.drun-run.ok
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ingress Completed: Reply: 0x4449444c016c01b3c4b1f204680100010a00000000000000000101
ingress Completed: Reply: 0x4449444c0000
4 changes: 4 additions & 0 deletions test/run-drun/ok/region-nat32-bug.tc.ok
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
region-nat32-bug.mo:5.5-5.6: warning [M0145], this pattern of type
Nat64
does not cover value
1 or 2 or _
37 changes: 37 additions & 0 deletions test/run-drun/region-nat32-bug.mo
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import Prim "mo:⛔";
import Region "stable-region/Region";

let r = Region.new();
let 0 = Region.grow(r, 1);

do {
var i : Nat32 = 0;
while(i < 0xFFFF) {
Region.storeNat32(r, 0, i);
let i32 = Region.loadNat32(r, 0);
let i64 = Region.loadNat64(r, 0);
if (i32 != i or Prim.nat64ToNat32(i64) != i) {
Prim.debugPrint(debug_show({i;i64;i32;bytes=Region.loadBlob(r,0,4)}));
assert(false);
};
i += 1;
};
};

do {
var i : Nat32 = 0xFFFF_FFFF;
while(i > 0xFFFF_0000) {
Region.storeNat32(r, 0, i);
let i32 = Region.loadNat32(r, 0);
let i64 = Region.loadNat64(r, 0);
if (i32 != i or Prim.nat64ToNat32(i64) != i) {
Prim.debugPrint(debug_show({i;i64;i32;bytes=Region.loadBlob(r,0,4)}));
assert(false);
};
i -= 1;
};
}

//SKIP run-low
//SKIP run
//SKIP run-ir

0 comments on commit fe03888

Please sign in to comment.