Skip to content
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.

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

PCC: verification primitives for dynamic range checks. #7389

Merged
merged 6 commits into from
Oct 27, 2023

Conversation

cfallin
Copy link
Member

@cfallin cfallin commented Oct 27, 2023

This PR adds new kinds of facts on value dynamic_range, dynamic_mem, and compare; a new kind of memory type dynamic_memory; and a notion of symbolic expressions (a sum of either an SSA value or global value and a static offset) for min and max values in those ranges that are just rich enough to express the expected CLIF sequence for Wasm dynamic memory bounds-checks with Spectre mitigations (cmp/select rather than control-flow-based). It is sufficient to validate the following

; Equivalent to a Wasm `i64.load` from a dynamic memory.                                                                                                                                     
function %f0(i64 vmctx, i32) -> i64 {                                                                                                                                                         
    gv0 = vmctx                                                                                                                                                                               
    gv1 = load.i64 notrap aligned checked gv0+0 ;; base                                                                                                                                       
    gv2 = load.i64 notrap aligned checked gv0+8 ;; size                                                                                                                                       
                                                                                                                                                                                              
    ;; mock vmctx struct:                                                                                                                                                                     
    mt0 = struct 16 {                                                                                                                                                                         
        0: i64 readonly ! dynamic_mem(mt1, 0, 0),                                                                                                                                             
        8: i64 readonly ! dynamic_range(64, gv2, gv2),                                                                                                                                        
    }                                                                                                                                                                                         
    ;; mock dynamic memory: dynamic range, plus 2GiB guard                                                                                                                                    
    mt1 = dynamic_memory gv2 + 0x8000_0000                                                                                                                                                    
                                                                                                                                                                                              
block0(v0 ! mem(mt0, 0, 0): i64, v1 ! dynamic_range(32, v1, v1): i32):                                                                                                                        
    v2 ! dynamic_range(64, v1, v1)            = uextend.i64 v1       ;; extended Wasm offset                                                                                                  
    v3 ! dynamic_mem(mt1, 0, 0)               = global_value.i64 gv1 ;; base                                                                                                                  
    v4 ! dynamic_range(64, gv2, gv2)          = global_value.i64 gv2 ;; size                                                                                                                  
    v5 ! compare(uge, v1, gv2)                = icmp.i64 uge v2, v4  ;; bounds-check compare of extended Wasm offset to size                                                                  
    v6 ! dynamic_mem(mt1, v1, v1)              = iadd.i64 v3, v2      ;; compute access address: memory base plus extended Wasm offset                                                        
    v7 ! dynamic_mem(mt1, 0, 0, nullable)     = iconst.i64 0         ;; null pointer for speculative path                                                                                     
    v8 ! dynamic_mem(mt1, 0, gv2-1, nullable) = select_spectre_guard v5, v7, v6  ;; if OOB, pick null, otherwise the real address                                                             
    v9                                        = load.i64 checked v8                                                                                                                           
    return v9                                                                                                                                                                                 
}  

on both x86-64 and aarch64.

The first half of this was:

Co-authored-by: Nick Fitzgerald fitzgen@gmail.com

@cfallin cfallin requested a review from fitzgen October 27, 2023 06:37
@cfallin cfallin requested a review from a team as a code owner October 27, 2023 06:37
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:area:machinst Issues related to instruction selection and the new MachInst backend. cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:area:x64 Issues related to x64 codegen labels Oct 27, 2023
Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

r=me with some comments below

cranelift/codegen/src/ir/pcc.rs Outdated Show resolved Hide resolved
cranelift/codegen/src/ir/pcc.rs Show resolved Hide resolved
cranelift/codegen/src/ir/pcc.rs Outdated Show resolved Hide resolved
cranelift/codegen/src/ir/pcc.rs Outdated Show resolved Hide resolved
cranelift/codegen/src/ir/pcc.rs Outdated Show resolved Hide resolved
cranelift/codegen/src/ir/pcc.rs Outdated Show resolved Hide resolved
cranelift/codegen/src/ir/pcc.rs Outdated Show resolved Hide resolved
cranelift/codegen/src/isa/aarch64/pcc.rs Outdated Show resolved Hide resolved
cranelift/codegen/src/isa/x64/pcc.rs Outdated Show resolved Hide resolved
cranelift/filetests/filetests/pcc/succeed/dynamic.clif Outdated Show resolved Hide resolved
@cfallin cfallin enabled auto-merge October 27, 2023 21:18
@cfallin cfallin added this pull request to the merge queue Oct 27, 2023
Merged via the queue into bytecodealliance:main with commit 2340dff Oct 27, 2023
22 checks passed
@cfallin cfallin deleted the pcc-dynamic branch October 27, 2023 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:area:machinst Issues related to instruction selection and the new MachInst backend. cranelift:area:x64 Issues related to x64 codegen cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants