Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARM LDR instruction doesn't sets mem.lshift in the detailed struct #576

Closed
radare opened this issue Jan 5, 2016 · 12 comments
Closed

ARM LDR instruction doesn't sets mem.lshift in the detailed struct #576

radare opened this issue Jan 5, 2016 · 12 comments

Comments

@radare
Copy link
Contributor

radare commented Jan 5, 2016

See radareorg/radare2#3893

03f19f97   ldrls pc, [pc, r3, lsl 2]

The value of insn->detail->arm.operands[1].mem.lshift should be 2 and it is 0

@aquynh
Copy link
Collaborator

aquynh commented Jan 5, 2016

is this report for "master" branch or "next" branch?

@radare
Copy link
Contributor Author

radare commented Jan 5, 2016

I always use next in r2

On 05 Jan 2016, at 09:38, Nguyen Anh Quynh notifications@github.com wrote:

is this report for "master" branch or "next" branch?


Reply to this email directly or view it on GitHub.

@aquynh
Copy link
Collaborator

aquynh commented Jan 7, 2016

here is the disasm output of your instruction. what is wrong??

0x100000c:  ldrls   pc, [pc, r3, lsl #2]
    op_count: 2

    operands[0].type: REG = pc
    operands[0].access: WRITE

    operands[1].type: MEM
        operands[1].mem.base: REG = pc
        operands[1].mem.index: REG = r3
        Shift: LSL = 2

@radare
Copy link
Contributor Author

radare commented Jan 7, 2016

How are you accessing the shift value? Because for me: mem.lshift is 0

On 07 Jan 2016, at 02:47, Nguyen Anh Quynh notifications@github.com wrote:

here is the disasm output of your instruction. what is wrong??

0x100000c: ldrls pc, [pc, r3, lsl #2]
op_count: 2

operands[0].type: REG = pc
operands[0].access: WRITE

operands[1].type: MEM
    operands[1].mem.base: REG = pc
    operands[1].mem.index: REG = r3
    Shift: LSL = 2


Reply to this email directly or view it on GitHub.

@radare
Copy link
Contributor Author

radare commented Jan 7, 2016

Test case in C:

#include <stdio.h>
#include <stdlib.h>
#include <capstone/capstone.h>

int main() {
        int count;
        static csh handle;
        cs_insn *insn;
        uint8_t fun[] = "\x03\xf1\x9f\x97";
        int mode = CS_MODE_LITTLE_ENDIAN | CS_MODE_ARM;
        cs_err err = cs_open (CS_ARCH_ARM, mode, &handle);
        cs_option (handle, CS_OPT_DETAIL, CS_OPT_ON);
        if (err) {
                printf("Failed on cs_open() with error returned: %u\n", err);
                abort();
        }
        count = cs_disasm(handle, fun, 4, 0, 1, &insn);
        printf ("(%s) %d\n", insn->mnemonic, insn->detail->arm.operands[1].mem.lshift);
        return 0;
}

the output is 0

@radare
Copy link
Contributor Author

radare commented Jan 7, 2016

Oh i see your code is doing op->shift.type and .value ... isn't the mem.lshift suposed to be handling this? isnt bypassing the .mem union a design flaw?

@aquynh
Copy link
Collaborator

aquynh commented Jan 8, 2016

currently mem.lshift is for other thing.

yes looks like this should be reimplemented so the shift only involves the memory operand.

@radare
Copy link
Contributor Author

radare commented Jan 8, 2016

which is the purpose of the current shift field?

this change must be done before the next release, when do you have plans for this to happen?

On 08 Jan 2016, at 17:32, Nguyen Anh Quynh notifications@github.com wrote:

currently mem.lshift is for other thing.

yes looks like this should be reimplemented so the shift only involves the memory operand.


Reply to this email directly or view it on GitHub #576 (comment).

@aquynh
Copy link
Collaborator

aquynh commented Jan 9, 2016

will do this soon

@radare
Copy link
Contributor Author

radare commented Jan 9, 2016

Awesome 👍

On 09 Jan 2016, at 18:32, Nguyen Anh Quynh notifications@github.com wrote:

will do this soon


Reply to this email directly or view it on GitHub.

@aquynh
Copy link
Collaborator

aquynh commented Jan 24, 2016

there is no need to fix anything, as you are looking into the wrong info.

see the code below:

#include <stdio.h>
#include <stdlib.h>
#include <inttypes.h>
#include <capstone/capstone.h>

int main() {
    int count;
    static csh handle;
    cs_insn *insn;
    uint8_t fun[] = "\x03\xf1\x9f\x97";
    int mode = CS_MODE_LITTLE_ENDIAN | CS_MODE_ARM;
    cs_err err = cs_open (CS_ARCH_ARM, mode, &handle);
    cs_option (handle, CS_OPT_DETAIL, CS_OPT_ON);
    if (err) {
        printf("Failed on cs_open() with error returned: %u\n", err);
        abort();
    }
    count = cs_disasm(handle, fun, 4, 0, 1, &insn);

    //printf("0x%"PRIx64":\t%s\t\t%s\n", insn[0].address, insn[0].mnemonic, insn[0].op_str);
    printf ("(%s) %d\n", insn[0].mnemonic, insn[0].detail->arm.operands[1].shift.value);

    cs_free(insn, count);
    return 0;
}

the output is:

$ ./test
(ldrls) 2

@radare
Copy link
Contributor Author

radare commented Feb 16, 2016

Ok thanks. r2 code was updated with this. sorry for the confussion

@radare radare closed this as completed Feb 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants