Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Operand Visibility Bug #171

Open
fred26 opened this issue Mar 7, 2021 · 4 comments
Open

Operand Visibility Bug #171

fred26 opened this issue Mar 7, 2021 · 4 comments
Labels
A-decoder Area: Decoder C-bug-invalid Category: Invalid bug report
Milestone

Comments

@fred26
Copy link

fred26 commented Mar 7, 2021

I have tested 2 types of AND operations providing 2 different decoded results:

Just to test this, in the formatter01.c sample, I redefined ZyanU8 data[] to these:

ZyanU8 data[] =
{
0x25, 0xFF, 0x03, 0x00, 0x00, // and eax, 000003FF
0x83, 0xE0, 0x0F // and eax, 0F
};

The first decoded instruction (and eax, 000003FF) decodes first operand ZYDIS_REGISTER_EAX with visibility == ZYDIS_OPERAND_VISIBILITY_IMPLICIT

However, second instruction (and eax, 0F) decodes first operand ZYDIS_REGISTER_EAX with visibility == ZYDIS_OPERAND_VISIBILITY_EXPLICIT

To replicate this just use the formatter01.c sample and change the ZyanU8 data[] to the above.

Thanks

@athre0z
Copy link
Member

athre0z commented Mar 7, 2021

Hi @fred26!

This is expected behavior. ZYDIS_OPERAND_VISIBILITY_IMPLICIT and ZYDIS_OPERAND_VISIBILITY_EXPLICIT have a bit of a misleading name. What "implicit" means here is that the operand is specified in the instruction itself (the opcode) rather than being chosen through a generic means such as ModRM encoding.

For 0x25, 0xFF, 0x03, 0x00, 0x00 the instruction structure looks like this:

25 FF 03 00 00
:  :..IMM
:..OPCODE

For 0x83, 0xE0, 0x0F, it looks like this:

83 E0 0F
:  :  :..IMM
:  :..MODRM
:..OPCODE

The first instruction has the eax hardcoded in the opcode, the second one has it set via the ModRM portion of the instruction.

@flobernd since we've now heard this question twice in a rather short period of time, we should probably think about renaming these (and make the old constants a deprecated alias).

@athre0z athre0z added A-decoder Area: Decoder C-bug-invalid Category: Invalid bug report labels Mar 7, 2021
@fred26
Copy link
Author

fred26 commented Mar 7, 2021

@athre0z thanks for the explanation, makes sense.
Yes if you could rename this will be much clearer.
Regards,

@flobernd
Copy link
Member

flobernd commented Mar 8, 2021

@athre0z Sure, meanwhile we could improve the documentation for these values. Although I think it already is quite understandable 😛

/**
 * The operand is explicitly encoded in the instruction.
 */
ZYDIS_OPERAND_VISIBILITY_EXPLICIT,
/**
 * The operand is part of the opcode, but listed as an operand.
 */
ZYDIS_OPERAND_VISIBILITY_IMPLICIT,
/**
 * The operand is part of the opcode, and not typically listed as an operand.
 */
ZYDIS_OPERAND_VISIBILITY_HIDDEN,

@athre0z
Copy link
Member

athre0z commented Mar 10, 2021

Yeah, I think the documentation is fine -- it's just the names that are confusing.

@athre0z athre0z added this to the v4.0.0 milestone Nov 7, 2021
@athre0z athre0z modified the milestones: v4.0.0, v4.X.X Apr 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-decoder Area: Decoder C-bug-invalid Category: Invalid bug report
Projects
None yet
Development

No branches or pull requests

3 participants