Stream: git-wasmtime

Topic: wasmtime / issue #4738 Fix Invalid Instruction format in ...


view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2022 at 19:40):

fitzgen commented on issue #4738:

Is there ever a case where someone should be allowed to use a different instruction-format constructor than the standard format for an opcode?

That should never happen. Would break our ISLE extractors for matching clif instructions, at the very least. But a ton of helpers only look for certain opcodes in certain formats, etc.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2022 at 19:44):

jameysharp commented on issue #4738:

That should never happen. Would break our ISLE extractors for matching clif instructions, at the very least. But a ton of helpers only look for certain opcodes in certain formats, etc.

Just to be clear: If it should never happen, then a runtime assertion that it never happens is a good idea, right?

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2022 at 19:50):

fitzgen commented on issue #4738:

Yeah, probably a debug assertion. (I haven't looked at this patch at all)

view this post on Zulip Wasmtime GitHub notifications bot (Aug 19 2022 at 19:52):

cfallin commented on issue #4738:

Indeed it should never happen, and this is usually enforced by the generated builder interface's per-instruction methods, but nothing currently stops someone from manually putting the wrong pieces together in the data structure or invoking the per-instruction-format method with an opcode as in the test here. That will break a lot of things (including ISLE matchers as Nick notes). Our best defense is to guard against this at construction time as best we can.

An assert in the per-instruction-format builder API seems totally reasonable, but let's make it a debug_assert rather than assert so that we don't have any runtime impact (the debug assert will still be tested in the fuzzing configuration).


Last updated: Dec 23 2024 at 13:07 UTC