Skip to content

Consider reworking branch types in the encoder #268

@athre0z

Description

@athre0z

This is a continuation of the discussion here. Thinking about this for a bit longer and having gained some usage experience with the encoder, I withdraw my initial suggestion.

When encoding branches, a user must currently explicitly choose a very specific branch type like ZYDIS_ENCODABLE_BRANCH_TYPE_NEAR64 in the request, which is quite inconvenient for code generation. I think that it would be preferable to instead keep the decoder type as-is, and instead split the ZydisEncodableBranchType type into two separate fields in the encoder request such as:

enum ZydisBranchWidth {
  ZYDIS_BRANCH_WIDTH_NONE,
  ZYDIS_BRANCH_WIDTH_16,
  ZYDIS_BRANCH_WIDTH_32,
  ZYDIS_BRANCH_WIDTH_64
};

struct ZydisEncoderRequest {
  ZydisBranchType branch_type; // reusing the decoder type
  ZydisBranchWidth branch_width;
  // remaining fields omitted
};

ZYDIS_BRANCH_TYPE_NONE and ZYDIS_BRANCH_WIDTH_NONE would mean "infer" to the encoder. ZYDIS_BRANCH_TYPE_NONE specifically would mean "infer whether near or short", but never consider "far", which must be requested explicitly. I'd argue that in 99% of cases, users will want some variation of a near/short branch. In a semantic encoder interface, I'd further argue that it should be the encoder that picks a good branch variant for the provided offset.

The upside with the current approach is that it's impossible to specify invalid combinations, reducing the amount of validation required. I assume that's what you meant with "needing a tuple type" in your comment here. However, I personally don't think this warrants the downsides. If such a tuple type is truly required, we should keep it internal to the encoder, abstracting it away from the user.

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-encoderArea: EncoderC-enhancementCategory: Enhancement of existing featuresP-mediumPriority: Medium

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions