zyantific / zydis

Fast and lightweight x86/x86-64 disassembler and code generation library
https://zydis.re
MIT License
3.4k stars 434 forks source link

Question about ZydisEncoderDecodedInstructionToEncoderRequest's operand_count parameter #458

Closed asesidaa closed 11 months ago

asesidaa commented 12 months ago

This parameter is called operand_count, however, there is a runtime check at https://github.com/zyantific/zydis/blob/master/src/Encoder.c#L4639 that requires it to be less than or equal to instruction->operand_count_visible. So maybe it should be renamed to operand_count_visible to reduce confusion?

An example is shl rcx, 0x07. This would fail if using ZydisEncoderDecodedInstructionToEncoderRequest( &insn.info, insn.operands, insn.info.operand_count, &req);

athre0z commented 12 months ago

Sounds good to me. We used to have a similar check in the formatter, but then made it more lenient a while ago. For the encoder I think renaming is the better approach though -- simply ignoring trailing operands might be even more confusing for the encoder. @flobernd @mappzor opinions?

mappzor commented 12 months ago

Renaming sounds good to me.

flobernd commented 12 months ago

@athre0z +1 for renaming