zyantific / zydis-rs

Zydis Rust Bindings
MIT License
83 stars 14 forks source link

Enhance AccessedFlags struct #34

Closed r3bb1t closed 10 months ago

r3bb1t commented 11 months ago

Hi, i've noticed some strange stuff. When printing with dbg!() macro i noticed that some AccessedFlags looks weird. They are always wrapped in an Option enum, even though when working with my binary it was always evaluating to Some. Also, in many places we can see the 0x0 instead of something like UNUSED. I think that AccessedFlags probably should not be wrapped inside an Option enum, but instead it's fields should be.

Here is an example:

add r9b, 0xA9
[src/deep_taint_run.rs:76] insn.cpu_flags = Some(
    AccessedFlags {
        tested: CpuFlag(
            0x0,
        ),
        modified: CpuFlag(
            CF | PF | AF | ZF | SF | OF,
        ),
        set_0: CpuFlag(
            0x0,
        ),
        set_1: CpuFlag(
            0x0,
        ),
        undefined: CpuFlag(
            0x0,
        ),
    },
)

P.s. Idk honestly how to process all this flags btw. (i.e. log reads/writes to specific flags, etc)

athre0z commented 11 months ago

They are always wrapped in an Option enum, even though when working with my binary it was always evaluating to Some.

Ah yeah: the Option is due to a quirk in an earlier, in-dev version of v4 where Zydis would return NULL if no flags were set at all. That has been changed since, so we can get rid of the Option. Good catch!

Also, in many places we can see the 0x0 instead of something like UNUSED.

Yeah, we changed the representation in Zydis. It uses bitflags now. If you want to check for "unused", you just | together all the fields (tested | modified | ...) and & it with the flag that you want to check. Users had complained that with the previous approach it was very inefficient to quickly check whether some flag is accessed in some way, and the new representation solves that.

r3bb1t commented 10 months ago

All the field are private btw. I still can't track the flags state properly.

I want to implement dead store elimination in my program which involves the tracking of opaque flags writes/reads. That's why it's so important to me.

athre0z commented 10 months ago

Ohh, yeah, I see. That's by mistake and certainly explains your confusion. Fixed in 8c5a7a3bcfd45942a5272fb7c67c87260ef2aff1.

r3bb1t commented 10 months ago

Thanks, it's much better now.

I have one more question: do we really need to write flags in Option enum? I believe it always unwraps successfuly (at least from one case which i tested)

athre0z commented 10 months ago

Ah yeah, I forgot that over my surprise that these actually weren't pub. Fixed in c4ea4ae03734ca6f02deb8dc42356ed5372a2499.

r3bb1t commented 10 months ago

I guess we can probably close this issue