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

Fixed Android 11 incompatibility with ValueSlot ptr tagging #104

Merged
merged 1 commit into from Aug 4, 2021

Conversation

snej
Copy link
Contributor

@snej snej commented Jul 30, 2021

The prior size optimization in ValueSlot used the high byte of the
_pointer field to distinguish between an actual pointer and an
inline 7-bit Value; it assumed if the upper byte is zero then it's a
pointer.

This isn't true anymore in Android: "Starting in Android 11, for 64-
bit processes, all heap allocations have an implementation defined tag
set in the top byte of the pointer on devices with kernel support for
ARM Top-byte Ignore (TBI)."

Switched to using the low byte as a tag, with a value of 0xFF
denoting inline, and anything else being a pointer. This works because
any Value* we store there must have at least one zero bit in the
LSB; see the comment in ValueSlot.hh for details.

The prior size optimization in ValueSlot used the high byte of the
`_pointer` field to distinguish between an actual pointer and an
inline 7-bit Value; it assumed if the upper byte is zero then it's a
pointer.

This isn't true anymore in Android: "Starting in Android 11, for 64-
bit processes, all heap allocations have an implementation defined tag
set in the top byte of the pointer on devices with kernel support for
ARM Top-byte Ignore (TBI)."

Switched to using the _low_ byte as a tag, with a value of 0xFF
denoting inline, and anything else being a pointer. This works because
any Value* we store there must have at least one zero bit in the
LSB; see the comment in ValueSlot.hh for details.
@snej snej requested review from borrrden and bmeike July 30, 2021 22:43
@snej
Copy link
Contributor Author

snej commented Jul 30, 2021

Tested on macOS, needs testing on ARM obviously, esp. with Android 11!

Copy link
Member

@borrrden borrrden left a comment

Choose a reason for hiding this comment

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

Looks sane to me, but @bmeike has the appropriate hardware for testing.

Copy link

@bmeike bmeike left a comment

Choose a reason for hiding this comment

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

I just got a clean run of all Android tests, after cherry-picking this commit onto Core master.

I think it is good to go.

Thanks!

@snej snej merged commit b820e03 into master Aug 4, 2021
@snej snej deleted the fix/valueslot-tagging branch August 4, 2021 20:59
@snej
Copy link
Contributor Author

snej commented Aug 4, 2021

OK, I'll update the Fleece submodule in LiteCore now.

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

3 participants