Skip to content

Commit

Permalink
* added a test file to suite for testing invalid and valid instructio…
Browse files Browse the repository at this point in the history
…n 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?
  • Loading branch information
symbols committed Jul 15, 2014
1 parent aa791a2 commit 298d413
Show file tree
Hide file tree
Showing 10 changed files with 438 additions and 13 deletions.
2 changes: 1 addition & 1 deletion SStream.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
17 changes: 15 additions & 2 deletions arch/ARM/ARMDisassembler.c
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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;
}

Expand Down
17 changes: 14 additions & 3 deletions arch/ARM/ARMInstPrinter.c
Original file line number Diff line number Diff line change
Expand Up @@ -591,19 +591,30 @@ 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
// to reflect absolute address.
// 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)
Expand Down
23 changes: 21 additions & 2 deletions arch/ARM/ARMMapping.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions arch/ARM/ARMMapping.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions arch/PowerPC/PPCModule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion arch/Sparc/SparcModule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion arch/SystemZ/SystemZModule.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion cs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading

0 comments on commit 298d413

Please sign in to comment.