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

maybe error in objc_msgSend.arm.S #151

Open
ClarkGuan opened this issue Mar 26, 2020 · 4 comments
Open

maybe error in objc_msgSend.arm.S #151

ClarkGuan opened this issue Mar 26, 2020 · 4 comments

Comments

@ClarkGuan
Copy link

ClarkGuan commented Mar 26, 2020

It's macro byte3, Thumb code is

ubfx   \dst, \src, #16, #8

ARM code is

and \dst, \src, #0xff00
lsr \dst, \dst, 16

Is it equivalent?
Why the #0xff00 isn't #0xff0000?

@davidchisnall
Copy link
Member

I think we can't materialise a constant that large in the and, so the sequence should actually be:

lsr \dst, \src, 16
and \dst, \dst, 0xff

Do you have an ARM system to test this on? I suspect that we've never hit that bug because programs with more than 2^16 distinct selectors are unlikely to fit in the RAM of such a machine (and are very rare - that path was broken on x86 for years without anyone noticing and, to-date, I'm only aware of one real-world program that hit this case). There is a test called ManyManySelectors that handles creating this many selectors, it should be quite simple to extend it to calling a method with the last selector value.

@ClarkGuan
Copy link
Author

I think we can't materialise a constant that large in the and, so the sequence should actually be:

lsr \dst, \src, 16
and \dst, \dst, 0xff

Do you have an ARM system to test this on? I suspect that we've never hit that bug because programs with more than 2^16 distinct selectors are unlikely to fit in the RAM of such a machine (and are very rare - that path was broken on x86 for years without anyone noticing and, to-date, I'm only aware of one real-world program that hit this case). There is a test called ManyManySelectors that handles creating this many selectors, it should be quite simple to extend it to calling a method with the last selector value.

I agree with you that this is an extremely rare situation. What I can say now is to use an Android device, then annotate the irrelevant path, force the wrong logic, and see the results.

@davidchisnall
Copy link
Member

I'm happy to take a PR that fixes this, if you can test it.

@davidchisnall
Copy link
Member

Note that this code path is used only on ARMv6 without thumb support and earlier. The original Raspberry Pi and anything newer will use the bitfield extract instructions. We have these code paths for some dirt cheap Chinese Android phones that were still around five years ago, with ARMv5 CPUs.

Given that most of the devices that this code was intended to support are likely to be dead, it’s probably simpler to just drop support for suck old hardware. It’s unlikely any of them shipped with enough RAM to run a program that would trigger this bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants