-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Out of bounds memory access not consistent in WasmTime.Net #1431
Comments
Here's the full "Problem Report" from macOS:
|
Thanks for the report! Given your crash logs it says thread 20 crashed which has a stack trace of:
which looks like this is running into #1295 as an issue which is in turn slated to be fixed by #1315 |
NP, happy to help! Good that it's allready is on track to be fixed. A couple of things:
var memory = new Memory(Store, 2, 2); And I have a wasm module as follows: (module
(import "" "mem" (memory $m 1 1))
) Is there any way to grow the memory to 2 pages? I've tried to understand the spec, but haven't figured out how to use the |
In the wasm module you defined the maximum size to be 1 page. This means that you can't grow it to 2 pages. If you use |
@bjorn3 if you use |
I believe you can use |
Ok I think this should be fixed with #1315 now |
Still getting the same error after merging in master. Atleast for me it seems like it's the same stack trace (in thread 1 this time).
|
@havarnov that says that thread 1 crashed with a stack tha tlooks like:
are you sure that's related to segfaults? That looks like the application explicitly killed itself? |
Now I'm both confused and unsure about what a commented 2 days ago. Seems like a posted the wrong stack trace? Anyways, I'm sure the test is failing/passing inconsistently. Here's a new stack trace after running
|
Hm that also looks pretty normal, are you sure that was the thread which crashed? |
I don't know how I manage to mess this up every time :/ I run It has three different outcomes:
Crashed thread stack trace:
Full report:
Hope I manage to get this corret now. |
@havarnov ok that definitely looks like a bug! I don't think it's the same issue as before though because it doesn't look like there's any wasm code on the stack. This may perhaps mean there's a bug in the Would it be possible to reduce this to a small thing that can be reproduced locally? |
@alexcrichton not sure what you mean about a "small thing"? Isn't the test I provided ok for testing? Or were you thinking of a small console application? |
Hm so ideally we could get a reproduction with just the Rust API. Failing that a reproduction with just the C API would be great. Failing that a reproduction with a small example in the dotnet API would be great too. The dotnet extension has moved around a bit and it looks like the test mentioned here has either been renamed or no longer exists. Would it be possible to help reduce this to something standalone which can be reproduced today? |
@alexcrichton I'll look into reproducing this bug in rust or the c api. The test I refered to was one I created in my fork of this repo, https://github.com/havarnov/wasmtime/tree/outofboundsmemoryaccess. That's why I asked if you wanted a PR with the new test. I'll come back when I have something more to show, or I need more help. Ok? |
@alexcrichton I need some help on this one. I'm trying to reproduce this by using the wasmtime api directly as follows, which to the best of my knowledge mimics the test I've written from the dotnet side. #[test]
fn outofbounds_memory() -> Result<()> {
let store = Store::default();
let mut linker = Linker::new(&store);
let ty = MemoryType::new(Limits::new(1, Some(1)));
let memory = Memory::new(&store, ty);
let linker = linker.define("", "mem", memory).expect("Should be able to define memory item.");
let wat = r#"
(module
(import "" "mem" (memory $m 1 2))
(data (i32.const 65535) "\01")
(data (i32.const 65536) "\02")
)
"#;
let module = Module::new(&store, wat)?;
let trap= linker.instantiate(&module)
.err()
.unwrap()
.downcast::<Trap>()
.unwrap();
assert_eq!(
trap.message(),
"wasm trap: out of bounds memory access, source location: @-"
);
Ok(())
} As you can see I'm expecting this to be "trapped" with the "out of bounds memory access" message. This is after all what I'm seeing from the dotnet side of things when the test does pass. But what I'm getting is a linker error from
|
I believe that's the expected error from that test. I don't really know much about C# testing, but you've got two tests defined in that file, is it possible that they're sharing access to the same linker by accident? |
The Although, reusing the definitions seems like the probable cause of the failure. I can look into this. |
I'm not able to reproduce the failure of either of those two tests (both pass as-is). I verified that |
@peterhuene interesting that it does fail on your machine. Just so there's no misunderstandings, it's the test name I will try to get the |
I think I have pinpointed this a bit more. The public ModuleFixture()
{
Host = new HostBuilder()
.WithMultiValue(true)
.WithReferenceTypes(true)
.Build();
Module = Host.LoadModuleText(Path.Combine("Modules", ModuleFileName));
} EDIT: but to investigate this further I need some help from you guys; @alexcrichton @peterhuene If I comment out
|
@havarnov that's correct, the My steps to reproduce:
All test runs were successful. I'll see if I can reproduce it with your older |
However, I was able to reproduce the link error if support for reference types is disabled.
|
It appears to be a link-time error without reference types enabled by design. This is because the reference types feature implicitly enables the bulk-memory feature. Without it, you'll get the linker error. |
I've added two tests for out of bounds memory access to WasmTime.Net. See: havarnov@1b199cf.
The problem is that one of the test ("ItThrowsOnOutOfBoundsMemoryAccess") is not consistently passing on my machine. Some times it's passing, sometimes the dotnet runtime crashes and sometimes the process hangs.
The text was updated successfully, but these errors were encountered: