Wasmtime GitHub notifications bot (Aug 19 2022 at 19:40): 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.
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?
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)
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_assertrather thanassertso that we don't have any runtime impact (the debug assert will still be tested in the fuzzing configuration).
Last updated: Oct 23 2024 at 20:03 UTC