Repository navigation
Conversation
LegNeato
changed the title
|
Note: written by AI |
|
Oops, didn't see #21. @Firestar99 , do you think you can use something like this or is your PR better? This one seems straightforward and smaller but I bet it is missing something |
Firestar99
left a comment
Firestar99
left a comment
There was a problem hiding this comment.
I've shrunk the other PR in half by deleting an old stale branch, due to which github changed the base of the PR to main, throwing out half of the commits which were already merged...
And now the other PR is actually smaller :D
| #[inline] | ||
| fn dis_fmt(&self, f: &mut Formatter<'_>, ctx: &OperandDisContext<'_>) -> std::fmt::Result { | ||
| if self | ||
| .words | ||
| .len() | ||
| .is_multiple_of(switch_case_word_len::<SwitchLiteral32>()) | ||
| { | ||
| for case in self.cases::<SwitchLiteral32>().expect("validated above") { | ||
| case.dis_fmt(f, ctx)?; | ||
| } | ||
| } else { | ||
| for case in self | ||
| .cases::<SwitchLiteral64>() | ||
| .expect("validated on construction") | ||
| { | ||
| case.dis_fmt(f, ctx)?; | ||
| } | ||
| } | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
The biggest problem would be that it just tries to decode it as 32bit, so if you encounter a a 6-word switch it'll just assume it's 32bit and disassemble it like that without even checking. Which is really bad if you want to use disassembly for tooling.
The other PR doesn't properly solve this case either yet, there I was assuming that OpSwitch had a ResultIdType operand but it doesn't, which is why it's hitting unwraps. The only way to properly implement it would be to accumulate a def-use chain in DisContext (or rather a ResultId (def) -> PrimitiveType would be sufficient). I've somewhat started that in the other branch it seems like without finishing it off...
|
Superseded by #21 |
2 participants
Support decoding OpSwitch case tables whose selector uses 64-bit literals. The grammar operand is context-dependent, so generated OpSwitch now uses a lossless SwitchTargets operand with typed 32-bit and 64-bit views instead of assuming LiteralInteger is always one word.
This also gates grammar-parser codegen reexports behind the codegen feature so the package test set compiles without that optional feature.
Validation: