| Message ID | 20191015175133.16598-4-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Update capstone module | expand |
On 15/10/2019 19.51, Richard Henderson wrote: > Capstone assumes any unknown instruction is 2 bytes. > Instead, use the ilen field in the first two bits of > the instruction to stay in sync with the insn stream. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > disas.c | 37 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > > diff --git a/disas.c b/disas.c > index 51c71534a3..2a000cbeb0 100644 > --- a/disas.c > +++ b/disas.c > @@ -178,6 +178,39 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info) > to share this across calls and across host vs target disassembly. */ > static __thread cs_insn *cap_insn; > > +/* > + * The capstone library always skips 2 bytes for S390X. > + * This is less than ideal, since we can tell from the first two bits > + * the size of the insn and thus stay in sync with the insn stream. > + */ > +static size_t CAPSTONE_API > +cap_skipdata_s390x_cb(const uint8_t *code, size_t code_size, > + size_t offset, void *user_data) > +{ > + size_t ilen; > + > + /* See get_ilen() in target/s390x/internal.h. */ > + switch (code[offset] >> 6) { > + case 0: > + ilen = 2; > + break; > + case 1: > + case 2: > + ilen = 4; > + break; > + default: > + ilen = 6; > + break; > + } > + > + return ilen; > +} The kernel has also a nice function to calculate this: static inline int insn_length(unsigned char code) { return ((((int) code + 64) >> 7) + 1) << 1; } ... but the switch-case is likely easier to read, so anyway: Reviewed-by: Thomas Huth <thuth@redhat.com>
On 10/15/19 11:46 AM, Thomas Huth wrote: >> +cap_skipdata_s390x_cb(const uint8_t *code, size_t code_size, >> + size_t offset, void *user_data) >> +{ >> + size_t ilen; >> + >> + /* See get_ilen() in target/s390x/internal.h. */ >> + switch (code[offset] >> 6) { >> + case 0: >> + ilen = 2; >> + break; >> + case 1: >> + case 2: >> + ilen = 4; >> + break; >> + default: >> + ilen = 6; >> + break; >> + } >> + >> + return ilen; >> +} > > The kernel has also a nice function to calculate this: > > static inline int insn_length(unsigned char code) > { > return ((((int) code + 64) >> 7) + 1) << 1; > } > > ... but the switch-case is likely easier to read, so anyway: Clever. I don't mind swapping to the kernel version, so long as we convert the target/s390x/internal.h function as well. r~
diff --git a/disas.c b/disas.c index 51c71534a3..2a000cbeb0 100644 --- a/disas.c +++ b/disas.c @@ -178,6 +178,39 @@ static int print_insn_od_target(bfd_vma pc, disassemble_info *info) to share this across calls and across host vs target disassembly. */ static __thread cs_insn *cap_insn; +/* + * The capstone library always skips 2 bytes for S390X. + * This is less than ideal, since we can tell from the first two bits + * the size of the insn and thus stay in sync with the insn stream. + */ +static size_t CAPSTONE_API +cap_skipdata_s390x_cb(const uint8_t *code, size_t code_size, + size_t offset, void *user_data) +{ + size_t ilen; + + /* See get_ilen() in target/s390x/internal.h. */ + switch (code[offset] >> 6) { + case 0: + ilen = 2; + break; + case 1: + case 2: + ilen = 4; + break; + default: + ilen = 6; + break; + } + + return ilen; +} + +static const cs_opt_skipdata cap_skipdata_s390x = { + .mnemonic = ".byte", + .callback = cap_skipdata_s390x_cb +}; + /* Initialize the Capstone library. */ /* ??? It would be nice to cache this. We would need one handle for the host and one for the target. For most targets we can reset specific @@ -208,6 +241,10 @@ static cs_err cap_disas_start(disassemble_info *info, csh *handle) /* "Disassemble" unknown insns as ".byte W,X,Y,Z". */ cs_option(*handle, CS_OPT_SKIPDATA, CS_OPT_ON); + if (info->cap_arch == CS_ARCH_SYSZ) { + cs_option(*handle, CS_OPT_SKIPDATA_SETUP, + (uintptr_t)&cap_skipdata_s390x); + } /* Allocate temp space for cs_disasm_iter. */ if (cap_insn == NULL) {
Capstone assumes any unknown instruction is 2 bytes. Instead, use the ilen field in the first two bits of the instruction to stay in sync with the insn stream. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- disas.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) -- 2.17.1