-
-
Notifications
You must be signed in to change notification settings - Fork 483
Consider reworking branch types in the encoder #268
Description
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.