koute / polkavm

A fast and secure RISC-V based virtual machine
Apache License 2.0
199 stars 44 forks source link

Refactor of the disassembler out of `polkatool` to make it re-usable #109

Closed xermicus closed 2 months ago

xermicus commented 2 months ago

Quick refactor of the disassembler out of polkatool and into a dedicated crate so I can re-use it in the YUL compiler.

Mostly a linear refactor as I tried to not change any logic and keep syntactic/style changes to a minimum.

koute commented 2 months ago

Looks mostly good to me!

Hmm... disassembling pinky/doom might be a little bit of an overkill though, and those tests don't actually test the disassembly itself, just that it doesn't error out. Maybe instead you could dynamically assemble a simple program like this, disassemble that, and also assert_eq the exact output of the disassembly?

xermicus commented 2 months ago

Yeah I agree, I just wasn't aware of the super convenient program builder! Added a simple program with an explicit check against the disassembly.

I thought that pinky and doom probably contain a lot (most) instructions and if they don't panic and disassemble into an non empty string, the disassembler likely works fine.. If you'd like me to remove them I can do that, however I still think it wouldn't harm to exercise those anyways :) WDYT?

koute commented 2 months ago

I thought that pinky and doom probably contain a lot (most) instructions and if they don't panic and disassemble into an non empty string, the disassembler likely works fine.. If you'd like me to remove them I can do that, however I still think it wouldn't harm to exercise those anyways :) WDYT?

Well, they're probably unnecessary, but we can always easily remove them later so it's fine either way.

xermicus commented 2 months ago

@koute fair enough, I've removed them.

koute commented 2 months ago

Thanks!