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 Casetest 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!
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 Casetest 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!
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 Casetest 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!
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 Casetest 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!
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 Casetest 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!
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 Casetest 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!
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.
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 Casetest 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: Dec 23 2024 at 12:05 UTC