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

mishegos: Construct VEX/XOP/EVEX opcodes #1622

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aengelke
Copy link
Contributor

The VEX/XOP/EVEX "prefixes" actually behave like being part of the opcode (the opcode byte must immediately follow the "prefix"). Explicitly constructing these opcodes significantly increases the likelihood of correctly hitting vector instructions.

This is interesting not only for coverage, but also for the plenty of UD constraints in AVX-512.

I hope I didn't miss any occasions where implicit assumptions on the opcode size are made.

@woodruffw
Copy link
Member

Thanks for adding support for these! I'll take a closer look in a bit.

@woodruffw woodruffw self-requested a review December 21, 2022 16:17
src/mishegos/mutator.c Outdated Show resolved Hide resolved
@woodruffw
Copy link
Member

Thanks for adding this! There is indeed an implicit assumption about opcode size:

/* An x86 instruction's opcode is no longer than 3 bytes.
 */
typedef struct __attribute__((packed)) {
  uint8_t len;
  uint8_t op[3];
} opcode;
static_assert(sizeof(opcode) == 4, "opcode should be 4 bytes");

...so that opcode struct needs to be lengthened to accommodate these changes.

The VEX/XOP/EVEX "prefixes" actually behave like being part of the
opcode (the opcode byte must immediately follow the "prefix").
Explicitly constructing these opcodes significantly increases the
likelihood of correctly hitting vector instructions.

This is interesting not only for coverage, but also for the plenty of UD
constraints in AVX-512.
@woodruffw
Copy link
Member

JFYI: I haven't forgotten this! I'll have some time to review it again today or tomorrow.

@woodruffw
Copy link
Member

Sorry for the delay here. I just merged your performance refactor, so this needs deconflicting. The changes themselves LGTM, though!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants