Stream: git-wasmtime

Topic: wasmtime / issue #5927 cranelift-interpreter: `load`/`sto...

Wasmtime GitHub notifications bot (Mar 04 2023 at 13:26):

afonso360 opened issue #5927:

:wave: Hey,

@jan-justin found this test case (in #5921) where the interpreter does a stack access across stack slots, but it shouldn't!

StackSlots can be reordered (at least that's what I got from the docs) and we shouldn't allow accesses across them.

It should be noted that this doesn't trap because the entire access is within the interpreter stack.

In checked_{load,store} we should be checking if the access is fully within the stack slot, instead of the entire stack!

.clif Test Case

test interpret

function %test() -> i64 {
    ss0 = explicit_slot 8
    ss1 = explicit_slot 8

block0:
    v0 = stack_addr.i64 ss0
    v1 = iconst.i64 1
    store.i64 v1, v0+2
    return v1
}

; run: %test() == 1

Steps to Reproduce

• clif-util test ./the-above.clif

Expected Results

The test to fail with a out of bounds access.

Actual Results

It passes.

Versions and Environment

Cranelift version or commit: main

Operating system: Linux

Architecture: Interpreter (x86_64 host)

Extra Info

If anyone needs help working on this, let me know!

Wasmtime GitHub notifications bot (Mar 04 2023 at 13:26):

afonso360 labeled issue #5927:

:wave: Hey,

@jan-justin found this test case (in #5921) where the interpreter does a stack access across stack slots, but it shouldn't!

StackSlots can be reordered (at least that's what I got from the docs) and we shouldn't allow accesses across them.

It should be noted that this doesn't trap because the entire access is within the interpreter stack.

In checked_{load,store} we should be checking if the access is fully within the stack slot, instead of the entire stack!

.clif Test Case

test interpret

function %test() -> i64 {
    ss0 = explicit_slot 8
    ss1 = explicit_slot 8

block0:
    v0 = stack_addr.i64 ss0
    v1 = iconst.i64 1
    store.i64 v1, v0+2
    return v1
}

; run: %test() == 1

Steps to Reproduce

• clif-util test ./the-above.clif

Expected Results

The test to fail with a out of bounds access.

Actual Results

It passes.

Versions and Environment

Cranelift version or commit: main

Operating system: Linux

Architecture: Interpreter (x86_64 host)

Extra Info

If anyone needs help working on this, let me know!

Wasmtime GitHub notifications bot (Mar 04 2023 at 13:26):

afonso360 labeled issue #5927:

:wave: Hey,

@jan-justin found this test case (in #5921) where the interpreter does a stack access across stack slots, but it shouldn't!

StackSlots can be reordered (at least that's what I got from the docs) and we shouldn't allow accesses across them.

It should be noted that this doesn't trap because the entire access is within the interpreter stack.

In checked_{load,store} we should be checking if the access is fully within the stack slot, instead of the entire stack!

.clif Test Case

test interpret

function %test() -> i64 {
    ss0 = explicit_slot 8
    ss1 = explicit_slot 8

block0:
    v0 = stack_addr.i64 ss0
    v1 = iconst.i64 1
    store.i64 v1, v0+2
    return v1
}

; run: %test() == 1

Steps to Reproduce

• clif-util test ./the-above.clif

Expected Results

The test to fail with a out of bounds access.

Actual Results

It passes.

Versions and Environment

Cranelift version or commit: main

Operating system: Linux

Architecture: Interpreter (x86_64 host)

Extra Info

If anyone needs help working on this, let me know!

Wasmtime GitHub notifications bot (Mar 04 2023 at 13:26):

afonso360 labeled issue #5927:

:wave: Hey,

@jan-justin found this test case (in #5921) where the interpreter does a stack access across stack slots, but it shouldn't!

StackSlots can be reordered (at least that's what I got from the docs) and we shouldn't allow accesses across them.

It should be noted that this doesn't trap because the entire access is within the interpreter stack.

In checked_{load,store} we should be checking if the access is fully within the stack slot, instead of the entire stack!

.clif Test Case

test interpret

function %test() -> i64 {
    ss0 = explicit_slot 8
    ss1 = explicit_slot 8

block0:
    v0 = stack_addr.i64 ss0
    v1 = iconst.i64 1
    store.i64 v1, v0+2
    return v1
}

; run: %test() == 1

Steps to Reproduce

• clif-util test ./the-above.clif

Expected Results

The test to fail with a out of bounds access.

Actual Results

It passes.

Versions and Environment

Cranelift version or commit: main

Operating system: Linux

Architecture: Interpreter (x86_64 host)

Extra Info

If anyone needs help working on this, let me know!

Wasmtime GitHub notifications bot (Mar 04 2023 at 13:26):

afonso360 labeled issue #5927:

:wave: Hey,

@jan-justin found this test case (in #5921) where the interpreter does a stack access across stack slots, but it shouldn't!

StackSlots can be reordered (at least that's what I got from the docs) and we shouldn't allow accesses across them.

It should be noted that this doesn't trap because the entire access is within the interpreter stack.

In checked_{load,store} we should be checking if the access is fully within the stack slot, instead of the entire stack!

.clif Test Case

test interpret

function %test() -> i64 {
    ss0 = explicit_slot 8
    ss1 = explicit_slot 8

block0:
    v0 = stack_addr.i64 ss0
    v1 = iconst.i64 1
    store.i64 v1, v0+2
    return v1
}

; run: %test() == 1

Steps to Reproduce

• clif-util test ./the-above.clif

Expected Results

The test to fail with a out of bounds access.

Actual Results

It passes.

Versions and Environment

Cranelift version or commit: main

Operating system: Linux

Architecture: Interpreter (x86_64 host)

Extra Info

If anyone needs help working on this, let me know!

Wasmtime GitHub notifications bot (Mar 04 2023 at 13:27):

afonso360 edited issue #5927:

:wave: Hey,

@jan-justin found this test case (in #5921) where the interpreter does a stack access across stack slots, but it shouldn't!

StackSlots can be reordered (at least that's what I got from the docs) and we shouldn't allow accesses across them. They can also have padding between them which would cause a different form of UB.

It should be noted that this doesn't trap because the entire access is within the interpreter stack.

In checked_{load,store} we should be checking if the access is fully within the stack slot, instead of the entire stack!

.clif Test Case

test interpret

function %test() -> i64 {
    ss0 = explicit_slot 8
    ss1 = explicit_slot 8

block0:
    v0 = stack_addr.i64 ss0
    v1 = iconst.i64 1
    store.i64 v1, v0+2
    return v1
}

; run: %test() == 1

Steps to Reproduce

• clif-util test ./the-above.clif

Expected Results

The test to fail with a out of bounds access.

Actual Results

It passes.

Versions and Environment

Cranelift version or commit: main

Operating system: Linux

Architecture: Interpreter (x86_64 host)

Extra Info

If anyone needs help working on this, let me know!

Wasmtime GitHub notifications bot (Mar 29 2023 at 17:39):

jameysharp commented on issue #5927:

After quite a bit of discussion in #5950, we've determined that we don't actually want to prohibit cross-stack-slot accesses. That was not obvious though and we've learned a bunch about how we want Cranelift to work, so it was time well spent.

Wasmtime GitHub notifications bot (Mar 29 2023 at 17:39):

jameysharp closed issue #5927:

:wave: Hey,

@jan-justin found this test case (in #5921) where the interpreter does a stack access across stack slots, but it shouldn't!

StackSlots can be reordered (at least that's what I got from the docs) and we shouldn't allow accesses across them. They can also have padding between them which would cause a different form of UB.

It should be noted that this doesn't trap because the entire access is within the interpreter stack.

In checked_{load,store} we should be checking if the access is fully within the stack slot, instead of the entire stack!

.clif Test Case

test interpret

function %test() -> i64 {
    ss0 = explicit_slot 8
    ss1 = explicit_slot 8

block0:
    v0 = stack_addr.i64 ss0
    v1 = iconst.i64 1
    store.i64 v1, v0+2
    return v1
}

; run: %test() == 1

Steps to Reproduce

• clif-util test ./the-above.clif

Expected Results

The test to fail with a out of bounds access.

Actual Results

It passes.

Versions and Environment

Cranelift version or commit: main

Operating system: Linux

Architecture: Interpreter (x86_64 host)

Extra Info

If anyone needs help working on this, let me know!

Last updated: Nov 22 2024 at 16:03 UTC