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

Add support for PC-relative LDRi12 #1157

Merged
merged 1 commit into from
Oct 4, 2017
Merged

Add support for PC-relative LDRi12 #1157

merged 1 commit into from
Oct 4, 2017

Conversation

dsrbecky
Copy link
Contributor

@dsrbecky dsrbecky commented Oct 4, 2017

Sample disassembly. It looks ok to me, and the functions seem to trace (glScissor in particular).

libhwgl.so`hw_glReadPixels: (original)
    0xed62b604 <+0>:   0xe16d42f4   strd   r4, r5, [sp, #-36]!
    0xed62b608 <+4>:   0xe59f413c   ldr    r4, [pc, #0x13c]
    0xed62b60c <+8>:   0xe1a05000   mov    r5, r0
    ...
    0xed62b74c <+328>: 0x0000f930   andeq  pc, r0, r0, lsr r9

libhwgl.so`hw_glReadPixels: (patched)
    0xed62b604 <+0>:  0xe51ff004   ldr    pc, [pc, #-0x4]
    0xed62b608 <+4>:  0xcf3810fd   svcgt  #0x3810fd

libgapii.so`::glReadPixels(gapii::GLint, gapii::GLint, gapii::GLsizei, gapii::GLsizei, uint32_t, uint32_t, void *):
    0xcf3810fd <+1>:  lsls   r5, r6, #0xe

(lldb) dis -b -c 8 -s callback
    0xebbea000: 0xe16d42f4   strd   r4, r5, [sp, #-36]!
    0xebbea004: 0xe59f4000   ldr    r4, [pc]
    0xebbea008: 0xe59ff000   ldr    pc, [pc]
    0xebbea00c: 0x0000f930   andeq  pc, r0, r0, lsr r9
    0xebbea010: 0xed62b60c   .long  0xed62b60c                ; unknown opcode

@dsrbecky dsrbecky requested a review from ben-clayton October 4, 2017 15:49
@dsrbecky
Copy link
Contributor Author

dsrbecky commented Oct 4, 2017

PS: Relevant MSInst encoder:
case ARM::LDRBi12:
case ARM::LDRi12:
case ARM::STRBi12:
case ARM::STRi12: {
// op: p
op = getMachineOpValue(MI, MI.getOperand(3), Fixups, STI);
Value |= (op & UINT64_C(15)) << 28;
// op: Rt
op = getMachineOpValue(MI, MI.getOperand(0), Fixups, STI);
Value |= (op & UINT64_C(15)) << 12;
// op: addr
op = getAddrModeImm12OpValue(MI, 1, Fixups, STI);
Value |= (op & UINT64_C(4096)) << 11;
Value |= (op & UINT64_C(122880)) << 3;
Value |= op & UINT64_C(4095);
break;
}

@@ -114,6 +114,7 @@ static void *calculatePcRelativeAddressArm(void *data, size_t pc_offset,

data_addr += pc_offset; // Add the PC
data_addr += 8; // Add the 8 byte implicit offset
data_addr += offset; // Add the offset
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a nasty bug you've fixed. Was nothing passing offset before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only ARM::Bcc seems to have been affected (non-Thumb).
The Thumb calculation is correct.

Copy link
Contributor

@ben-clayton ben-clayton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thank you!

@dsrbecky dsrbecky merged commit 9492539 into google:master Oct 4, 2017
@dsrbecky dsrbecky deleted the hw2 branch October 4, 2017 17:24
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.

2 participants