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

arm64.S: use 32-bit loads to access caml_backtrace_active #8569

Merged
merged 3 commits into from Apr 2, 2019

Conversation

Projects
None yet
3 participants
@xavierleroy
Copy link
Contributor

commented Apr 2, 2019

caml_backtrace_active is declared with type int32_t, so it is
incorrect to access it with a 64-bit ldr instruction.
Either a link-time error occurs, as in issue #8567,
or the wrong value may be loaded.

This commit uses ldrsh ldrsw instructions (32-bit signed loads) to
access caml_backtrace_active.

arm64.S: use 32-bit load to access caml_backtrace_active
`caml_backtrace_active` is declared with type `int32_t`, so it is
incorrect to access it with a 64-bit "ldr" instruction.
Either a link-time error occurs, as in issue #8567,
or the wrong value may be loaded.

This commit uses `ldrsh` instructions (32-bit signed loads) to
access `caml_backtrace_active`.

Closes: #8567
@dra27

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2019

Not familiar enough with ARM64, and I can’t get the full file on my phone - but you refer to ldrsh above and use ldrsw in both places in the patch?

@xavierleroy

This comment has been minimized.

Copy link
Contributor Author

commented Apr 2, 2019

I did mean ldrsw (load 32-bit Word and Sign-extend) but my fat fingers typed ldrsh (load 16-bit Halfword and Sign-extend). Sorry for the typo and thanks for having spotted it.

@xavierleroy xavierleroy added this to the 4.08 milestone Apr 2, 2019

@mshinwell
Copy link
Contributor

left a comment

Reviewed also by Greta Yorsh.

@xavierleroy

This comment has been minimized.

Copy link
Contributor Author

commented Apr 2, 2019

Thanks for the quick review. I'll add a Changes entry and push it to trunk and 4.08.

xavierleroy added some commits Apr 2, 2019

@xavierleroy xavierleroy merged commit dc439ec into ocaml:trunk Apr 2, 2019

0 of 2 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@xavierleroy xavierleroy deleted the xavierleroy:issue-8567 branch Apr 2, 2019

xavierleroy added a commit that referenced this pull request Apr 2, 2019

arm64.S: use 32-bit loads to access caml_backtrace_active (#8569)
`caml_backtrace_active` is declared with type `int32_t`, so it is
incorrect to access it with a 64-bit "ldr" instruction.
Either a link-time error occurs, as in issue #8567,
or the wrong value may be loaded.

This commit uses `ldrsw` instructions (32-bit signed loads) to
access `caml_backtrace_active`.

Closes: #8567
@xavierleroy

This comment has been minimized.

Copy link
Contributor Author

commented Apr 2, 2019

Cherry-pick on 4.08: commit 9f7099b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.