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 crash due to bad shift in indirect addressing mode on aarch64 (fixes #302) #303

Merged
merged 1 commit into from
May 25, 2021

Conversation

simonis
Copy link
Contributor

@simonis simonis commented May 7, 2021

This is fix for #302. The problem is that on aarch64, indirect loads and stores can accept and index and a shift, but the shift has to correspond to the size of the load/store operation (e.g. shift==2 for 32-bit operations and shift==3 for 64-bit operations). Usage of sun.misc.Unsafe can lead to address expressions which violate this requirement. They might for example request a shift of "1" for a 32-bit operation. In the release build, the generated code will nevertheless perform a 2-bit shift in such a situation which can lead to arbitrary crashes at a later time. In a debug build, the problem will be detected by the following assertion:

assert(_ext.shift() == (int)size) failed: bad shift

Notice that this issue was resolved by a larger change in jdk9 (JDK-8154826: AArch64: take better advantage of base + shifted offset addressing mode) which completely reworked complex addressing modes.

This change only fixes the current issue in jdk8 by checking the shift amount against the size of the memory operation in the Matcher and rejects address modes where they don't conform.

Copy link
Contributor

@navyxliu navyxliu left a comment

Choose a reason for hiding this comment

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

LGTM.

@cliveverghese cliveverghese merged commit 7d6fafe into corretto:develop May 25, 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