From 298d413bbc996b09549067b4acb63ea26cde2d68 Mon Sep 17 00:00:00 2001 From: flyingsymbols Date: Mon, 30 Jun 2014 01:45:40 -0400 Subject: [PATCH] * added a test file to suite for testing invalid and valid instruction sequences * fixed and added a test for a thumb-2 invalid sequence that was incorrectly allowed before these changes (pop.w with sp argument included) * fixed and added a test for a blx from thumb to ARM that had its immediate argument incorrect (misaligned) * eliminated some warnings by explicitly casting so I could turn on treat warnings as errors locally General notes: * probably worth turning on treat all warnings as errors in the msvc project files, had a subtle bug that resulted from a missing declaration causing differences in dll and static compilation modes ( code was working incorrectly in dll form because of missing declaration in arch/ARM/ARMMapping.h for new function ARM_blx_to_arm_mode. Something about the linking was confusing ld when making the dll, and the resulting offsets were wonky (e.g. the added ble test would show up as #0x1fc instead of #0x1fe like it should have ) * the invalid pop was being treated as a soft fail which then gets coerced to a success because it is != MCDisassembler_Fail in Thumb_getInstruction what are the semantics of a soft fail? Maybe we should be able to set up whether or not we want a soft fail to be a real fail in the csh struct? --- SStream.c | 2 +- arch/ARM/ARMDisassembler.c | 17 +- arch/ARM/ARMInstPrinter.c | 17 +- arch/ARM/ARMMapping.c | 23 ++- arch/ARM/ARMMapping.h | 2 + arch/PowerPC/PPCModule.c | 4 +- arch/Sparc/SparcModule.c | 2 +- arch/SystemZ/SystemZModule.c | 2 +- cs.c | 2 +- suite/test_arm_regression.c | 380 +++++++++++++++++++++++++++++++++++ 10 files changed, 438 insertions(+), 13 deletions(-) create mode 100644 suite/test_arm_regression.c diff --git a/SStream.c b/SStream.c index 9f7eb38b..462aff29 100644 --- a/SStream.c +++ b/SStream.c @@ -23,7 +23,7 @@ void SStream_concat0(SStream *ss, char *s) { #ifndef CAPSTONE_DIET strcpy(ss->buffer + ss->index, s); - ss->index += strlen(s); + ss->index += (int) strlen(s); #endif } diff --git a/arch/ARM/ARMDisassembler.c b/arch/ARM/ARMDisassembler.c index 524ec0e1..6b3ed119 100644 --- a/arch/ARM/ARMDisassembler.c +++ b/arch/ARM/ARMDisassembler.c @@ -1231,10 +1231,13 @@ static DecodeStatus DecodeRegListOperand(MCInst *Inst, unsigned Val, { unsigned i; DecodeStatus S = MCDisassembler_Success; - + unsigned opcode = 0; + bool NeedDisjointWriteback = false; unsigned WritebackReg = 0; - switch (MCInst_getOpcode(Inst)) { + + opcode = MCInst_getOpcode(Inst); + switch (opcode) { default: break; case ARM_LDMIA_UPD: @@ -1262,6 +1265,16 @@ static DecodeStatus DecodeRegListOperand(MCInst *Inst, unsigned Val, } } + if (opcode == ARM_t2LDMIA_UPD && WritebackReg == ARM_SP) { + if ( + Val & (1 << ARM_SP) + || ((Val & (1 << ARM_PC)) && (Val & (1 << ARM_LR)))) { + // invalid thumb2 pop + // needs no sp in reglist and not both pc and lr set at the same time + return MCDisassembler_Fail; + } + } + return S; } diff --git a/arch/ARM/ARMInstPrinter.c b/arch/ARM/ARMInstPrinter.c index 4097bb84..ae564d87 100644 --- a/arch/ARM/ARMInstPrinter.c +++ b/arch/ARM/ARMInstPrinter.c @@ -591,6 +591,9 @@ static void printOperand(MCInst *MI, unsigned OpNo, SStream *O) } } } else if (MCOperand_isImm(Op)) { + unsigned int opc = 0; + opc = MCInst_getOpcode(MI); + imm = (int32_t)MCOperand_getImm(Op); // relative branch only has relative offset, so we have to update it @@ -598,12 +601,20 @@ static void printOperand(MCInst *MI, unsigned OpNo, SStream *O) // Note: in ARM, PC is always 2 instructions ahead, so we have to // add 8 in ARM mode, or 4 in Thumb mode // printf(">> opcode: %u\n", MCInst_getOpcode(MI)); - if (ARM_rel_branch(MI->csh, MCInst_getOpcode(MI))) { + if (ARM_rel_branch(MI->csh, opc)) { // only do this for relative branch - if (MI->csh->mode & CS_MODE_THUMB) + if (MI->csh->mode & CS_MODE_THUMB) { imm += (int32_t)MI->address + 4; - else + if (ARM_blx_to_arm_mode(MI->csh, opc)) { + // here need to align down to the nearest 4-byte + // address +#define _ALIGN_DOWN(v, align_width) ((v/align_width)*align_width) + imm = _ALIGN_DOWN(imm, 4); +#undef _ALIGN_DOWN + } + } else { imm += (int32_t)MI->address + 8; + } if (imm >= 0) { if (imm > HEX_THRESHOLD) diff --git a/arch/ARM/ARMMapping.c b/arch/ARM/ARMMapping.c index 479e3133..16371d59 100644 --- a/arch/ARM/ARMMapping.c +++ b/arch/ARM/ARMMapping.c @@ -13908,17 +13908,36 @@ static unsigned int insn_rel[] = { 0 }; +static unsigned int insn_blx_rel_to_arm[] = { + ARM_tBLXi, + 0 +}; + // check if this insn is relative branch bool ARM_rel_branch(cs_struct *h, unsigned int id) { int i; - for (i = 0; insn_rel[i]; i++) - if (id == insn_rel[i]) + for (i = 0; insn_rel[i]; i++) { + if (id == insn_rel[i]) { return true; + } + } // not found return false; } +bool ARM_blx_to_arm_mode(cs_struct *h, unsigned int id) { + int i; + + for (i = 0; insn_blx_rel_to_arm[i]; i++) + if (id == insn_blx_rel_to_arm[i]) + return true; + + // not found + return false; + +} + #endif diff --git a/arch/ARM/ARMMapping.h b/arch/ARM/ARMMapping.h index 7553e06d..bf7e231e 100644 --- a/arch/ARM/ARMMapping.h +++ b/arch/ARM/ARMMapping.h @@ -18,4 +18,6 @@ const char *ARM_insn_name(csh handle, unsigned int id); // check if this insn is relative branch bool ARM_rel_branch(cs_struct *h, unsigned int insn_id); +bool ARM_blx_to_arm_mode(cs_struct *h, unsigned int insn_id); + #endif diff --git a/arch/PowerPC/PPCModule.c b/arch/PowerPC/PPCModule.c index 88a7fe86..46bca87d 100644 --- a/arch/PowerPC/PPCModule.c +++ b/arch/PowerPC/PPCModule.c @@ -18,7 +18,7 @@ static cs_err init(cs_struct *ud) CS_MODE_BIG_ENDIAN)) return CS_ERR_MODE; - mri = cs_mem_malloc(sizeof(*mri)); + mri = (MCRegisterInfo *) cs_mem_malloc(sizeof(*mri)); PPC_init(mri); ud->printer = PPC_printInst; @@ -37,7 +37,7 @@ static cs_err init(cs_struct *ud) static cs_err option(cs_struct *handle, cs_opt_type type, size_t value) { if (type == CS_OPT_SYNTAX) - handle->syntax = value; + handle->syntax = (int) value; return CS_ERR_OK; } diff --git a/arch/Sparc/SparcModule.c b/arch/Sparc/SparcModule.c index a1dc05e7..3f25936b 100644 --- a/arch/Sparc/SparcModule.c +++ b/arch/Sparc/SparcModule.c @@ -36,7 +36,7 @@ static cs_err init(cs_struct *ud) static cs_err option(cs_struct *handle, cs_opt_type type, size_t value) { if (type == CS_OPT_SYNTAX) - handle->syntax = value; + handle->syntax = (int) value; return CS_ERR_OK; } diff --git a/arch/SystemZ/SystemZModule.c b/arch/SystemZ/SystemZModule.c index 267c0461..8f5390ab 100644 --- a/arch/SystemZ/SystemZModule.c +++ b/arch/SystemZ/SystemZModule.c @@ -32,7 +32,7 @@ static cs_err init(cs_struct *ud) static cs_err option(cs_struct *handle, cs_opt_type type, size_t value) { if (type == CS_OPT_SYNTAX) - handle->syntax = value; + handle->syntax = (int) value; return CS_ERR_OK; } diff --git a/cs.c b/cs.c index 3f51d8f0..0b776f6b 100644 --- a/cs.c +++ b/cs.c @@ -532,7 +532,7 @@ size_t cs_disasm_ex(csh ud, const uint8_t *buffer, size_t size, uint64_t offset, // we have to skip some amount of data, depending on arch & mode insn_cache->id = 0; // invalid ID for this "data" instruction insn_cache->address = offset; - insn_cache->size = skipdata_bytes; + insn_cache->size = (uint16_t) skipdata_bytes; memcpy(insn_cache->bytes, buffer, skipdata_bytes); strncpy(insn_cache->mnemonic, handle->skipdata_setup.mnemonic, sizeof(insn_cache->mnemonic) - 1); diff --git a/suite/test_arm_regression.c b/suite/test_arm_regression.c new file mode 100644 index 00000000..341979a2 --- /dev/null +++ b/suite/test_arm_regression.c @@ -0,0 +1,380 @@ +/* Capstone Disassembler Engine */ +/* By Nguyen Anh Quynh , 2013> */ + +// the following must precede stdio (woo, thanks msft) +#ifdef _MSC_VER +#define _CRT_SECURE_NO_WARNINGS +#define snprintf _snprintf +#endif +#include +#include +#include +#include + + + +#include + +static csh handle; + +struct platform { + cs_arch arch; + cs_mode mode; + unsigned char *code; + size_t size; + char *comment; + int syntax; +}; + +static char* hex_string(unsigned char *str, int len) { + // returns a malloced string that has the hex version of the string in it + // null if failed to malloc + char * hex_out = NULL; + size_t i = 0; + hex_out = (char *) malloc(len*2 + 1); // two ascii characters per input character, plus trailing null + if (!hex_out) { goto Exit; } + + for (i = 0; i < len; ++i) { + snprintf(hex_out + (i*2), 2, "%02x", str[i]); + } + + hex_out[len*2] = 0; // trailing null + +Exit: + return hex_out; +} + +static void snprint_insn_detail( + char * buf, size_t * cur, size_t * left, cs_insn *ins +) { + size_t used = 0; + +#define _this_printf(...) \ + { \ + size_t used = 0; \ + used = snprintf(buf + *cur, *left, __VA_ARGS__); \ + *left -= used; \ + *cur += used; \ + } + + cs_arm *arm; + int i; + + // detail can be NULL on "data" instruction if SKIPDATA option is turned ON + if (ins->detail == NULL) + return; + + arm = &(ins->detail->arm); + + if (arm->op_count) + _this_printf("\top_count: %u\n", arm->op_count); + + for (i = 0; i < arm->op_count; i++) { + cs_arm_op *op = &(arm->operands[i]); + switch((int)op->type) { + default: + break; + case ARM_OP_REG: + _this_printf("\t\toperands[%u].type: REG = %s\n", i, cs_reg_name(handle, op->reg)); + break; + case ARM_OP_IMM: + _this_printf("\t\toperands[%u].type: IMM = 0x%x\n", i, op->imm); + break; + case ARM_OP_FP: + _this_printf("\t\toperands[%u].type: FP = %f\n", i, op->fp); + break; + case ARM_OP_MEM: + _this_printf("\t\toperands[%u].type: MEM\n", i); + if (op->mem.base != X86_REG_INVALID) + _this_printf("\t\t\toperands[%u].mem.base: REG = %s\n", + i, cs_reg_name(handle, op->mem.base)); + if (op->mem.index != X86_REG_INVALID) + _this_printf("\t\t\toperands[%u].mem.index: REG = %s\n", + i, cs_reg_name(handle, op->mem.index)); + if (op->mem.scale != 1) + _this_printf("\t\t\toperands[%u].mem.scale: %u\n", i, op->mem.scale); + if (op->mem.disp != 0) + _this_printf("\t\t\toperands[%u].mem.disp: 0x%x\n", i, op->mem.disp); + + break; + case ARM_OP_PIMM: + _this_printf("\t\toperands[%u].type: P-IMM = %u\n", i, op->imm); + break; + case ARM_OP_CIMM: + _this_printf("\t\toperands[%u].type: C-IMM = %u\n", i, op->imm); + break; + } + + if (op->shift.type != ARM_SFT_INVALID && op->shift.value) { + if (op->shift.type < ARM_SFT_ASR_REG) { + // shift with constant value + _this_printf("\t\t\tShift: %u = %u\n", op->shift.type, op->shift.value); + } else { + // shift with register + _this_printf("\t\t\tShift: %u = %s\n", op->shift.type, + cs_reg_name(handle, op->shift.value)); + } + } + } + + if (arm->cc != ARM_CC_AL && arm->cc != ARM_CC_INVALID) { + _this_printf("\tCode condition: %u\n", arm->cc); + } + + if (arm->update_flags) { + _this_printf("\tUpdate-flags: True\n"); + } + + if (arm->writeback) { + _this_printf("\tWrite-back: True\n"); + } + +#undef _this_printf + +} + +static void print_insn_detail(cs_insn *ins) +{ + char a_buf[2048]; + size_t cur=0, left=2048; + snprint_insn_detail(a_buf, &cur, &left, ins); + printf("%s\n", a_buf); +} + +struct invalid_code { + unsigned char *code; + size_t size; + char *comment; +}; + +#define MAX_INVALID_CODES 16 + +struct invalid_instructions { + cs_arch arch; + cs_mode mode; + char *platform_comment; + int num_invalid_codes; + struct invalid_code invalid_codes[MAX_INVALID_CODES]; +}; + +static void test_invalids() { + struct invalid_instructions invalids[] = {{ + CS_ARCH_ARM, + CS_MODE_THUMB, + "Thumb", + 1, + {{ + "\xbd\xe8\x1e\xff", + 4, + "invalid thumb2 pop because sp used and because both pc and lr are " + "present at the same time" + }}, + }}; + + struct invalid_instructions * invalid = NULL; + + uint64_t address = 0x1000; + cs_insn *insn; + int i; + int j; + size_t count; + + printf("\nShould be invalid\n" + "-----------------\n"); + + for (i = 0; i < sizeof(invalids)/sizeof(invalids[0]); i++) { + cs_err err; + + invalid = invalids + i; + err = cs_open(invalid->arch, invalid->mode, &handle); + + if (err) { + printf("Failed on cs_open() with error returned: %u\n", err); + continue; + } + + cs_option(handle, CS_OPT_DETAIL, CS_OPT_ON); + + for (j = 0; j < invalid->num_invalid_codes; ++j) { + struct invalid_code * invalid_code = NULL; + char * hex_str = NULL; + + invalid_code = invalid->invalid_codes + j; + + hex_str = hex_string(invalid_code->code, invalid_code->size); + + printf("%s %s: %s\n", invalid->platform_comment, hex_str, invalid_code->comment); + + free(hex_str); + + count = cs_disasm_ex(handle, + invalid_code->code, invalid_code->size, address, 0, &insn + ); + + if (count) { + size_t k; + printf(" ERROR:\n"); + + for (k = 0; k < count; k++) { + printf(" 0x%"PRIx64":\t%s\t%s\n", + insn[k].address, insn[k].mnemonic, insn[k].op_str); + print_insn_detail(&insn[k]); + } + cs_free(insn, count); + + } else { + printf(" SUCCESS: invalid\n"); + } + } + + cs_close(&handle); + } +} + +struct valid_code { + unsigned char *code; + size_t size; + uint32_t start_addr; + char* expected_out; + char *comment; +}; + +#define MAX_VALID_CODES 16 +struct valid_instructions { + cs_arch arch; + cs_mode mode; + char *platform_comment; + int num_valid_codes; + struct valid_code valid_codes[MAX_VALID_CODES]; +}; + +static void test_valids() { + struct valid_instructions valids[] = {{ + CS_ARCH_ARM, + CS_MODE_THUMB, + "Thumb", + 2, + {{ "\x00\xf0\x26\xe8", 4, 0x352, + + "0x352:\tblx\t#0x3a0\n" + "\top_count: 1\n" + "\t\toperands[0].type: IMM = 0x3a0\n", + + "thumb2 blx with misaligned immediate" + + }, { "\x05\xdd", 2, 0x1f0, + + "0x1f0:\tble\t#0x1fe\n" + "\top_count: 1\n" + "\t\toperands[0].type: IMM = 0x1fe\n" + "\tCode condition: 14\n", + + "thumb b cc with thumb-aligned target" + }} + }}; + + struct valid_instructions * valid = NULL; + + uint64_t address = 0x1000; + cs_insn *insn; + int i; + int j; + size_t count; + + + for (i = 0; i < sizeof(valids)/sizeof(valids[0]); i++) { + cs_err err; + + valid = valids + i; + err = cs_open(valid->arch, valid->mode, &handle); + + if (err) { + printf("Failed on cs_open() with error returned: %u\n", err); + continue; + } + + cs_option(handle, CS_OPT_DETAIL, CS_OPT_ON); + +#define _this_printf(...) \ + { \ + size_t used = 0; \ + used = snprintf(tmp_buf + cur, left, __VA_ARGS__); \ + left -= used; \ + cur += used; \ + } + printf("\nShould be valid\n" + "---------------\n"); + + for (j = 0; j < valid->num_valid_codes; ++j) { + char tmp_buf[2048]; + size_t left = 2048; + size_t cur = 0; + size_t used = 0; + int success = 0; + char * hex_str = NULL; + + struct valid_code * valid_code = NULL; + valid_code = valid->valid_codes + j; + + hex_str = hex_string(valid_code->code, valid_code->size); + + printf("%s %s @ 0x%04x: %s\n %s", + valid->platform_comment, hex_str, valid_code->start_addr, + valid_code->comment, valid_code->expected_out); + + count = cs_disasm_ex(handle, + valid_code->code, valid_code->size, + valid_code->start_addr, 0, &insn + ); + + if (count) { + size_t k; + size_t max_len = 0; + size_t tmp_len = 0; + + for (k = 0; k < count; k++) { + _this_printf( + "0x%"PRIx64":\t%s\t%s\n", + insn[k].address, insn[k].mnemonic, + insn[k].op_str + ); + + snprint_insn_detail(tmp_buf, &cur, &left, &insn[k]); + } + + max_len = strlen(tmp_buf); + tmp_len = strlen(valid_code->expected_out); + if (tmp_len > max_len) { + max_len = tmp_len; + } + + if (memcmp(tmp_buf, valid_code->expected_out, max_len)) { + printf( + " ERROR: '''\n%s''' does not match" + " expected '''\n%s'''\n", + tmp_buf, valid_code->expected_out + ); + } else { + printf(" SUCCESS: valid\n"); + } + + cs_free(insn, count); + + } else { + printf("ERROR: invalid\n"); + } + } + + cs_close(&handle); + } + +#undef _this_prinf +} + +int main() +{ + test_invalids(); + test_valids(); + return 0; +} +