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

5.0.0rc4 branch instruction imm ambiguous #2072

Closed
nostrudexercitation opened this issue Jul 3, 2023 · 4 comments · Fixed by #2026
Closed

5.0.0rc4 branch instruction imm ambiguous #2072

nostrudexercitation opened this issue Jul 3, 2023 · 4 comments · Fixed by #2026

Comments

@nostrudexercitation
Copy link

nostrudexercitation commented Jul 3, 2023

>>> import capstone
>>> capstone.version_bind()
(5, 0, 1280)
>>> cs = capstone.Cs(capstone.CS_ARCH_ARM64, capstone.CS_MODE_ARM)
>>> cs.detail = True
>>> inst_bytes = b'\x07\x00\x00\x14'
>>> inst = cs.disasm(inst_bytes, 0xfffffff044444444)
>>> inst = next(inst)
>>> inst
<CsInsn 0xfffffff044444444 [07000014]: b #0xfffffff044444460>
>>> operand = inst.operands[0]
>>> hex(operand.imm)
'-0xfbbbbbba0'

operand.imm currently outputs as signed version of the branch label which is incorrect.
Should it be:

  • the unsigned label
  • the signed imm26 encoded in the instruction
    • multiply by 4
    • without multiply by 4
@nostrudexercitation nostrudexercitation changed the title 5.0.0rc4 branch instruction imm is signed 5.0.0rc4 branch instruction imm ambiguous Jul 3, 2023
@XVilka
Copy link
Contributor

XVilka commented Jul 4, 2023

Related #2056

@Rot127
Copy link
Collaborator

Rot127 commented Jul 4, 2023

#2056 is exactly the problem. Just scaled up to 64bit values.

In printAlignedLabel the address is correctly calculated as uint64_t

uint64_t imm = (MCOperand_getImm(Op) * 4) + MI->address;

and later assigned to an int64_t imm:

MI->flat_insn->detail->arm64.operands[MI->flat_insn->detail->arm64.op_count].imm = imm;

I doubt though that we going to fix this before the release?

@XVilka
Copy link
Contributor

XVilka commented Jul 4, 2023

@Rot127 it's a hard thing to fix properly without big changes AFAIK. It's better to do that during the breaking season of merging auto-sync PRs, IMHO.

@Rot127
Copy link
Collaborator

Rot127 commented Jul 4, 2023

Yes. Also I wouldn't count on LLVM having all the operand type information correct. This kind of info is often neglected.

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

Successfully merging a pull request may close this issue.

3 participants