Wasmtime GitHub notifications bot (Jun 25 2020 at 19:00): Wasmtime GitHub notifications bot (Jun 25 2020 at 19:00):github-actions[bot] commented on Issue #1923:
Subscribe to Label Action
cc @bnjbvr, @kubkon
<details>
This issue or pull request has been labeled: "cranelift", "cranelift:meta", "cranelift:wasm", "wasi"Thus the following users have been cc'd because of the following labels:
- bnjbvr: cranelift
- kubkon: wasi
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
Wasmtime GitHub notifications bot (Jun 25 2020 at 20:23):fitzgen commented on Issue #1923:
Could this add some more tests too? Some things like:
All of these cases are in the spec tests (except explicitly targeting replacing a
ref_count == 1element), and enabled in this PR. I don't think it makes sense to duplicate them (again, excpet forref_count == 1).
Wasmtime GitHub notifications bot (Jun 25 2020 at 20:41):alexcrichton commented on Issue #1923:
Hm so I do think that it's worthwhile to add some more tests here. I'm a bit hesitant to rely on pure wast tests since all the interesting bits are about how host code interacts with this.
For example this program will hit "attempt to subtract with overflow" because the reference count is decremented in wasm but then
drop_in_placetries to decrement it again. If that's fixed then the program segfaults since it tries to dropAtwice. The problem is that we're in the process of dropping a slot in the table when at the same time we still have access to read that slot. What needs to happen is we need to move out of the table, move the new entry into the table, then drop the previous entry. That way if in the destructor of the previous value we read the table slot we get the new value.I do realize that the spec tests try to be somewhat exhaustive, but given that this was pretty easy to segfault I think it's worthwhile trying to be at least somewhat flavorful with tests that exercise the embedding where we have more control over GC and such.
Wasmtime GitHub notifications bot (Jun 25 2020 at 20:46):fitzgen commented on Issue #1923:
I do realize that the spec tests try to be somewhat exhaustive, but given that this was pretty easy to segfault I think it's worthwhile trying to be at least somewhat flavorful with tests that exercise the embedding where we have more control over GC and such.
Yeah I aleady had a fix for the
drop_in_placebit, but the destructor-touching-the-table bit needs some tests for sure. I have a half written one already.I don't think we need to duplicate anything that's already in the spec tests, just the
ref_count == 1paths, and dtors that touch the table.
Wasmtime GitHub notifications bot (Jun 26 2020 at 19:53):fitzgen commented on Issue #1923:
Okay, I've added a bunch of tests for pathological gc-in-externref-destructor-while-sweeping behaviors and other various re-entry into GC.
Working on a fuzz target specifically for exercising
table.get,table.set, and GC now, but it will probably be a follow up PR.
Wasmtime GitHub notifications bot (Jun 26 2020 at 23:26):fitzgen commented on Issue #1923:
Alright, I got the fuzz target written and passing as well!
Wasmtime GitHub notifications bot (Jun 26 2020 at 23:39):github-actions[bot] commented on Issue #1923:
Subscribe to Label Action
cc @fitzgen, @peterhuene
<details>
This issue or pull request has been labeled: "fuzzing", "wasmtime:api"Thus the following users have been cc'd because of the following labels:
- fitzgen: fuzzing
- peterhuene: wasmtime:api
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
Last updated: Dec 23 2024 at 12:05 UTC