Skip to content

Commit

Permalink
cranelift-wasm: Add a bounds-checking optimization for dynamic memori…
Browse files Browse the repository at this point in the history 
…es and guard pages (bytecodealliance#6031)

* cranelift-wasm: Add a bounds-checking optimization for dynamic memories and guard pages

This is a new special case for when we know that there are enough guard pages to
cover the memory access's offset and access size.

The precise should-we-trap condition is

    index + offset + access_size > bound

However, if we instead check only the partial condition

    index > bound

then the most out of bounds that the access can be, while that partial check
still succeeds, is `offset + access_size`.

However, when we have a guard region that is at least as large as `offset +
access_size`, we can rely on the virtual memory subsystem handling these
out-of-bounds errors at runtime. Therefore, the partial `index > bound` check is
sufficient for this heap configuration.

Additionally, this has the advantage that a series of Wasm loads that use the
same dynamic index operand but different static offset immediates -- which is a
common code pattern when accessing multiple fields in the same struct that is in
linear memory -- will all emit the same `index > bound` check, which we can GVN.

* cranelift: Add WAT tests for accessing dynamic memories with the same index but different offsets

The bounds check comparison is GVN'd but we still branch on values we should
know will always be true if we get this far in the code. This is actual `br_if`s
in the non-Spectre code and `select_spectre_guard`s that we should know will
always go a certain way if we have Spectre mitigations enabled.

Improving the non-Spectre case is pretty straightforward: walk the dominator
tree and remember which values we've already branched on at this point, and
therefore we can simplify any further conditional branches on those same values
into direct jumps.

Improving the Spectre case requires something that is morally the same, but has
a few snags:

* We don't have actual `br_if`s to determine whether the bounds checking
  condition succeeded or not. We need to instead reason about dominating
  `select_spectre_guard; {load, store}` instruction pairs.

* We have to be SUPER careful about reasoning "through" `select_spectre_guard`s.
  Our general rule is never to do that, since it could break the speculative
  execution sandboxing that the instruction is designed for.
  • Loading branch information
@fitzgen
fitzgen authored Mar 17, 2023
1 parent 73cc433 commit 2e48bab
Show file tree
Hide file tree
Showing 109 changed files with 2,076 additions and 2,151 deletions.
22 changes: 10 additions & 12 deletions ...asm/load_store_dynamic_kind_i32_index_0xffffffff_guard_no_spectre_i32_access_0_offset.wat
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,13 @@

;; function u0:0:
;; block0:
;; mov w7, w0
;; ldr x8, [x2, #8]
;; sub x8, x8, #4
;; subs xzr, x7, x8
;; mov w6, w0
;; ldr x7, [x2, #8]
;; subs xzr, x6, x7
;; b.hi label3 ; b label1
;; block1:
;; ldr x9, [x2]
;; str w1, [x9, w0, UXTW]
;; ldr x8, [x2]
;; str w1, [x8, w0, UXTW]
;; b label2
;; block2:
;; ret
Expand All @@ -57,14 +56,13 @@
;;
;; function u0:1:
;; block0:
;; mov w7, w0
;; ldr x8, [x1, #8]
;; sub x8, x8, #4
;; subs xzr, x7, x8
;; mov w6, w0
;; ldr x7, [x1, #8]
;; subs xzr, x6, x7
;; b.hi label3 ; b label1
;; block1:
;; ldr x9, [x1]
;; ldr w0, [x9, w0, UXTW]
;; ldr x8, [x1]
;; ldr w0, [x8, w0, UXTW]
;; b label2
;; block2:
;; ret
Expand Down
28 changes: 12 additions & 16 deletions ...oad_store_dynamic_kind_i32_index_0xffffffff_guard_no_spectre_i32_access_0x1000_offset.wat
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,14 @@

;; function u0:0:
;; block0:
;; mov w9, w0
;; ldr x10, [x2, #8]
;; movn x8, #4099
;; add x10, x10, x8
;; subs xzr, x9, x10
;; mov w7, w0
;; ldr x8, [x2, #8]
;; subs xzr, x7, x8
;; b.hi label3 ; b label1
;; block1:
;; ldr x11, [x2]
;; add x11, x11, #4096
;; str w1, [x11, w0, UXTW]
;; ldr x9, [x2]
;; add x9, x9, #4096
;; str w1, [x9, w0, UXTW]
;; b label2
;; block2:
;; ret
Expand All @@ -59,16 +57,14 @@
;;
;; function u0:1:
;; block0:
;; mov w9, w0
;; ldr x10, [x1, #8]
;; movn x8, #4099
;; add x10, x10, x8
;; subs xzr, x9, x10
;; mov w7, w0
;; ldr x8, [x1, #8]
;; subs xzr, x7, x8
;; b.hi label3 ; b label1
;; block1:
;; ldr x11, [x1]
;; add x10, x11, #4096
;; ldr w0, [x10, w0, UXTW]
;; ldr x9, [x1]
;; add x8, x9, #4096
;; ldr w0, [x8, w0, UXTW]
;; b label2
;; block2:
;; ret
Expand Down
34 changes: 14 additions & 20 deletions ...store_dynamic_kind_i32_index_0xffffffff_guard_no_spectre_i32_access_0xffff0000_offset.wat
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,15 @@

;; function u0:0:
;; block0:
;; mov w10, w0
;; movn w9, #65531
;; adds x11, x10, x9
;; b.lo 8 ; udf
;; ldr x12, [x2, #8]
;; subs xzr, x11, x12
;; mov w8, w0
;; ldr x9, [x2, #8]
;; subs xzr, x8, x9
;; b.hi label3 ; b label1
;; block1:
;; ldr x13, [x2]
;; movz x14, #65535, LSL #16
;; add x13, x14, x13
;; str w1, [x13, w0, UXTW]
;; ldr x10, [x2]
;; movz x11, #65535, LSL #16
;; add x10, x11, x10
;; str w1, [x10, w0, UXTW]
;; b label2
;; block2:
;; ret
Expand All @@ -61,18 +58,15 @@
;;
;; function u0:1:
;; block0:
;; mov w10, w0
;; movn w9, #65531
;; adds x11, x10, x9
;; b.lo 8 ; udf
;; ldr x12, [x1, #8]
;; subs xzr, x11, x12
;; mov w8, w0
;; ldr x9, [x1, #8]
;; subs xzr, x8, x9
;; b.hi label3 ; b label1
;; block1:
;; ldr x13, [x1]
;; movz x12, #65535, LSL #16
;; add x12, x12, x13
;; ldr w0, [x12, w0, UXTW]
;; ldr x10, [x1]
;; movz x9, #65535, LSL #16
;; add x9, x9, x10
;; ldr w0, [x9, w0, UXTW]
;; b label2
;; block2:
;; ret
Expand Down
28 changes: 12 additions & 16 deletions ...load_store_dynamic_kind_i32_index_0xffffffff_guard_no_spectre_i8_access_0x1000_offset.wat
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,14 @@

;; function u0:0:
;; block0:
;; mov w9, w0
;; ldr x10, [x2, #8]
;; movn x8, #4096
;; add x10, x10, x8
;; subs xzr, x9, x10
;; mov w7, w0
;; ldr x8, [x2, #8]
;; subs xzr, x7, x8
;; b.hi label3 ; b label1
;; block1:
;; ldr x11, [x2]
;; add x11, x11, #4096
;; strb w1, [x11, w0, UXTW]
;; ldr x9, [x2]
;; add x9, x9, #4096
;; strb w1, [x9, w0, UXTW]
;; b label2
;; block2:
;; ret
Expand All @@ -59,16 +57,14 @@
;;
;; function u0:1:
;; block0:
;; mov w9, w0
;; ldr x10, [x1, #8]
;; movn x8, #4096
;; add x10, x10, x8
;; subs xzr, x9, x10
;; mov w7, w0
;; ldr x8, [x1, #8]
;; subs xzr, x7, x8
;; b.hi label3 ; b label1
;; block1:
;; ldr x11, [x1]
;; add x10, x11, #4096
;; ldrb w0, [x10, w0, UXTW]
;; ldr x9, [x1]
;; add x8, x9, #4096
;; ldrb w0, [x8, w0, UXTW]
;; b label2
;; block2:
;; ret
Expand Down
34 changes: 14 additions & 20 deletions ..._store_dynamic_kind_i32_index_0xffffffff_guard_no_spectre_i8_access_0xffff0000_offset.wat
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,15 @@

;; function u0:0:
;; block0:
;; mov w10, w0
;; movn w9, #65534
;; adds x11, x10, x9
;; b.lo 8 ; udf
;; ldr x12, [x2, #8]
;; subs xzr, x11, x12
;; mov w8, w0
;; ldr x9, [x2, #8]
;; subs xzr, x8, x9
;; b.hi label3 ; b label1
;; block1:
;; ldr x13, [x2]
;; movz x14, #65535, LSL #16
;; add x13, x14, x13
;; strb w1, [x13, w0, UXTW]
;; ldr x10, [x2]
;; movz x11, #65535, LSL #16
;; add x10, x11, x10
;; strb w1, [x10, w0, UXTW]
;; b label2
;; block2:
;; ret
Expand All @@ -61,18 +58,15 @@
;;
;; function u0:1:
;; block0:
;; mov w10, w0
;; movn w9, #65534
;; adds x11, x10, x9
;; b.lo 8 ; udf
;; ldr x12, [x1, #8]
;; subs xzr, x11, x12
;; mov w8, w0
;; ldr x9, [x1, #8]
;; subs xzr, x8, x9
;; b.hi label3 ; b label1
;; block1:
;; ldr x13, [x1]
;; movz x12, #65535, LSL #16
;; add x12, x12, x13
;; ldrb w0, [x12, w0, UXTW]
;; ldr x10, [x1]
;; movz x9, #65535, LSL #16
;; add x9, x9, x10
;; ldrb w0, [x9, w0, UXTW]
;; b label2
;; block2:
;; ret
Expand Down
36 changes: 17 additions & 19 deletions ...sm/load_store_dynamic_kind_i32_index_0xffffffff_guard_yes_spectre_i32_access_0_offset.wat
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,32 +41,30 @@

;; function u0:0:
;; block0:
;; mov w10, w0
;; ldr x11, [x2, #8]
;; sub x11, x11, #4
;; ldr x12, [x2]
;; add x12, x12, x0, UXTW
;; movz x9, #0
;; subs xzr, x10, x11
;; csel x12, x9, x12, hi
;; mov w9, w0
;; ldr x10, [x2, #8]
;; ldr x11, [x2]
;; add x11, x11, x0, UXTW
;; movz x8, #0
;; subs xzr, x9, x10
;; csel x11, x8, x11, hi
;; csdb
;; str w1, [x12]
;; str w1, [x11]
;; b label1
;; block1:
;; ret
;;
;; function u0:1:
;; block0:
;; mov w10, w0
;; ldr x11, [x1, #8]
;; sub x11, x11, #4
;; ldr x12, [x1]
;; add x12, x12, x0, UXTW
;; movz x9, #0
;; subs xzr, x10, x11
;; csel x12, x9, x12, hi
;; mov w9, w0
;; ldr x10, [x1, #8]
;; ldr x11, [x1]
;; add x11, x11, x0, UXTW
;; movz x8, #0
;; subs xzr, x9, x10
;; csel x11, x8, x11, hi
;; csdb
;; ldr w0, [x12]
;; ldr w0, [x11]
;; b label1
;; block1:
;; ret
;; ret
42 changes: 19 additions & 23 deletions ...ad_store_dynamic_kind_i32_index_0xffffffff_guard_yes_spectre_i32_access_0x1000_offset.wat
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,36 +41,32 @@

;; function u0:0:
;; block0:
;; mov w12, w0
;; ldr x13, [x2, #8]
;; movn x11, #4099
;; add x13, x13, x11
;; ldr x14, [x2]
;; add x14, x14, x0, UXTW
;; add x14, x14, #4096
;; movz x11, #0
;; subs xzr, x12, x13
;; csel x14, x11, x14, hi
;; mov w10, w0
;; ldr x11, [x2, #8]
;; ldr x12, [x2]
;; add x12, x12, x0, UXTW
;; add x12, x12, #4096
;; movz x9, #0
;; subs xzr, x10, x11
;; csel x12, x9, x12, hi
;; csdb
;; str w1, [x14]
;; str w1, [x12]
;; b label1
;; block1:
;; ret
;;
;; function u0:1:
;; block0:
;; mov w12, w0
;; ldr x13, [x1, #8]
;; movn x11, #4099
;; add x13, x13, x11
;; ldr x14, [x1]
;; add x14, x14, x0, UXTW
;; add x14, x14, #4096
;; movz x11, #0
;; subs xzr, x12, x13
;; csel x14, x11, x14, hi
;; mov w10, w0
;; ldr x11, [x1, #8]
;; ldr x12, [x1]
;; add x12, x12, x0, UXTW
;; add x12, x12, #4096
;; movz x9, #0
;; subs xzr, x10, x11
;; csel x12, x9, x12, hi
;; csdb
;; ldr w0, [x14]
;; ldr w0, [x12]
;; b label1
;; block1:
;; ret
;; ret
Loading

0 comments on commit 2e48bab

Please sign in to comment.