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

Implement Replace::Replace properly for aarch64 #110

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mikedld
Copy link

@mikedld mikedld commented Jun 20, 2020

Add a separate implementation for aarch64 which boils down to

ldr x17, =0x1122334455667788
br x17

The x17 register is used as it's allowed to be modified during function call and doesn't require saving. Old approach under SOME_ARM ifdef that doesn't require changing any registers isn't possible since the pc register is no longer directly accessible.

Tested with qemu-aarch64 user mode emulation. I've also only tested it with cpp11 branch, but opening the PR against master; do tell if additional PR against cpp11 will be required or whether you'll merge it there yourself.

Potentially fixes #73, #57. CC @rzr for review.

Add a separate implementation for aarch64 which boils down to

    ldr x17, =0x1122334455667788
    br x17

The `x17` register is used as it's allowed to be modified during
function call and doesn't require saving. Old approach under `SOME_ARM`
ifdef that doesn't require changing any registers isn't possible since
the `pc` register is no longer directly accessible.

Tested with qemu-aarch64 user mode emulation.
@fzodana
Copy link

fzodana commented Nov 27, 2024

i know this is years old but i was looking into hippomocks on arm64 myself recently and thought this might be useful to someone:

not sure what was going on in qemu, but i tried this patch on both an apple m2 and a raspberry pi 4, and i found that while it does make it work in some cases, it's very brittle, to the point where hippomock's own tests don't pass as written.

that's because this solution requires the mocked function to consist of at least four instructions -- two to replace by the ldr + br, and two to store the address of the replacement. functions that immediately return or have been optimized by the compiler often have only two, in which case it essentially writes garbage into the beginning of the next function in memory. if that other function is called while the mock is still in effect, the program will (usually) crash. the effect compounds badly when mocking multiple short functions that are right next to each other.

i experimented with using adr with replacement - origFunc as the offset, but like ldr it can only look +/- 1 MiB around the pc, so although it makes the tests pass it still fails in a lot of real-world cases.

unfortunately i have very little knowledge of assembly or memory layouts myself so this is as far as i got. i just can't figure out how to work around that two-instruction limitation.

@rzr
Copy link
Contributor

rzr commented Nov 28, 2024

Not sure i understand the change but i can help testing it, i think gh can run some tests using qemu too

Unsure it will fix other plaforms as reported at

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=888883

@mikedld
Copy link
Author

mikedld commented Nov 29, 2024

@fzodana, mocking non-virtual functions is brittle regardless of the platform / architecture (approach taken in this PR isn't specific to aarch64) so I consider this a known limitation. There's a reason why other mocking frameworks don't even try. That said, I'm not an ARM expert so if you know of a better way — feel free to open a separate PR.

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.

[ANDROID] crash (SIGILL/ILL_ILLOPC) on arm64-v8a
3 participants