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

Fix the displacement offset for moffset-encoded operands #1754

Merged

Conversation

jranieri-grammatech
Copy link
Contributor

This was initially introduced in dce7da9 but lost in the LLVM 7 sync
in 5a99624.

This was initially introduced in dce7da9 but lost in the LLVM 7 sync
in 5a99624.
@aquynh
Copy link
Collaborator

aquynh commented Jun 13, 2021

can you provide a testcase for this bug?

@jranieri-grammatech
Copy link
Contributor Author

Sure!

Where should I add the test to? I've looked in tests/ and suite/ and nothing really stands out as a place to add a test that disassembles a few instructions and verifies their displacement offsets.

@aquynh
Copy link
Collaborator

aquynh commented Jun 30, 2021

can you put the code exposing this bug here?

@jranieri-grammatech
Copy link
Contributor Author

#include <capstone/capstone.h>
#include <assert.h>
#include <string.h>

int main() {
  csh cs_handle;
  cs_err err = cs_open(CS_ARCH_X86, CS_MODE_32, &cs_handle);
  assert(err == 0);

  const char asm_bytes[] = { 0xA3, 0x00, 0x00, 0x00, 0x00 };

  cs_insn* insn;
  cs_option(cs_handle, CS_OPT_DETAIL, CS_OPT_ON);

  size_t count = cs_disasm(cs_handle, asm_bytes, sizeof(asm_bytes), 0, 0, &insn);

  assert(count == 1);
  assert(strcmp(insn[0].mnemonic, "mov") == 0);
  assert(insn[0].detail->x86.op_count == 2);
  assert(insn[0].detail->x86.operands[0].type == X86_OP_MEM);
  assert(insn[0].detail->x86.operands[1].type == X86_OP_REG);
  // The following assertions will fail without the bug fix.
  assert(insn[0].detail->x86.encoding.imm_offset == 0);
  assert(insn[0].detail->x86.encoding.imm_size == 0);
  assert(insn[0].detail->x86.encoding.disp_offset == 1);
  assert(insn[0].detail->x86.encoding.disp_size == 4);

  cs_free(insn, count);
  cs_close(&cs_handle);
}

@eschulte
Copy link
Contributor

Is Capstone actively maintained? I've noticed numerous forks which seem to primarily exist to hold un-merged bug fixes.

For example at my company, we maintain both a persistent fork at https://github.com/GrammaTech/capstone/tree/next and a PyPi package at https://pypi.org/project/capstone-gt/.

I appreciate that Capstone is open-source software and this sort of maintenance can require significant uncompensated effort, so please don't take this as a complaint, but rather a request for information on Capstone maintenance plans so that we can plan our development accordingly. In particular, should we continue to bother opening issues and creating merge requests?

Thanks!

@kabeor
Copy link
Member

kabeor commented Nov 10, 2021

Bug fix confirmed. Thanks for your contribution.
Merge now.

@kabeor kabeor merged commit c7538d4 into capstone-engine:next Nov 10, 2021
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 this pull request may close these issues.

None yet

4 participants