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

Document a limitation of the DW_CFA_AARCH64_negate_ra_state. #129

Merged

Conversation

DanielKristofKiss
Copy link
Contributor

DW_CFA_AARCH64_negate_ra_state depends on the previous state of the RA_SIGN_STATE
register which makes complex to handle in cases when the register is set by
DWARF expressions because expression are lasily evaulated in runtime.

@@ -582,6 +582,9 @@ This ABI defines one vendor call frame instruction

The ``DW_CFA_AARCH64_negate_ra_state`` operation negates bit[0] of the
RA_SIGN_STATE pseudo-register. It does not take any operands.
The ``DW_CFA_AARCH64_negate_ra_state`` shall not be mixed with other DWARF operations on the
Copy link
Contributor

Choose a reason for hiding this comment

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

i think we should use the "Register Rule Instruction" terminology from 6.4.2.3 of https://www.dwarfstd.org/doc/DWARF5.pdf

because we want to allow "Row State Instructions" from 6.4.2.4. to be mixed with negate_ra_state.

Copy link
Contributor

Choose a reason for hiding this comment

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

either that or we should call out DW_CFA_remember_state/restore_state explicitly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @nsz-arm, change is updated.

Copy link

@walkerkd walkerkd left a comment

Choose a reason for hiding this comment

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

In general this looks OK to me.

Minor wording nit-pick. The DWARF standard does not use "shall not", instead it uses "must not". Suggest we do the same here.

DW_CFA_AARCH64_negate_ra_state depends on the previous state of the RA_SIGN_STATE
register which makes complex to handle in cases when the register is set by
DWARF expressions because expression are lasily evaulated in runtime.
@DanielKristofKiss
Copy link
Contributor Author

@walkerkd Thanks, done.

@stuij stuij added this to the 2022Q1 milestone Feb 25, 2022
@stuij stuij merged commit 23e36bf into ARM-software:main Mar 14, 2022
@sallyarmneale
Copy link

Looks good to me

DanielKristofKiss added a commit to llvm/llvm-project that referenced this pull request May 13, 2022
Program may set the RA_SIGN_STATE pseudo register by expressions.
Libunwind expected only the DW_CFA_AARCH64_negate_ra_state could change the value
of the register which leads to runtime errors on PAC enabled systems.
In the recent version of the aadwarf64[1] a limitation is added[2] to forbid the mixing the
DW_CFA_AARCH64_negate_ra_state with other DWARF Register Rule Instructions.

[1] https://github.com/ARM-software/abi-aa/releases/tag/2022Q1
[2] ARM-software/abi-aa#129

Reviewed By: #libunwind, MaskRay

Differential Revision: https://reviews.llvm.org/D123692
DanielKristofKiss added a commit to llvm/llvm-project that referenced this pull request May 18, 2022
Program may set the RA_SIGN_STATE pseudo register by expressions.
Libunwind expected only the DW_CFA_AARCH64_negate_ra_state could change the value
of the register which leads to runtime errors on PAC enabled systems.
In the recent version of the aadwarf64[1] a limitation is added[2] to forbid the mixing the
DW_CFA_AARCH64_negate_ra_state with other DWARF Register Rule Instructions.

[1] https://github.com/ARM-software/abi-aa/releases/tag/2022Q1
[2] ARM-software/abi-aa#129

Reviewed By: #libunwind, MaskRay

Differential Revision: https://reviews.llvm.org/D123692
Reland: test moved because it depends on exceptions.
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this pull request Oct 7, 2022
Program may set the RA_SIGN_STATE pseudo register by expressions.
Libunwind expected only the DW_CFA_AARCH64_negate_ra_state could change the value
of the register which leads to runtime errors on PAC enabled systems.
In the recent version of the aadwarf64[1] a limitation is added[2] to forbid the mixing the
DW_CFA_AARCH64_negate_ra_state with other DWARF Register Rule Instructions.

[1] https://github.com/ARM-software/abi-aa/releases/tag/2022Q1
[2] ARM-software/abi-aa#129

Reviewed By: #libunwind, MaskRay

Differential Revision: https://reviews.llvm.org/D123692
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this pull request Oct 7, 2022
Program may set the RA_SIGN_STATE pseudo register by expressions.
Libunwind expected only the DW_CFA_AARCH64_negate_ra_state could change the value
of the register which leads to runtime errors on PAC enabled systems.
In the recent version of the aadwarf64[1] a limitation is added[2] to forbid the mixing the
DW_CFA_AARCH64_negate_ra_state with other DWARF Register Rule Instructions.

[1] https://github.com/ARM-software/abi-aa/releases/tag/2022Q1
[2] ARM-software/abi-aa#129

Reviewed By: #libunwind, MaskRay

Differential Revision: https://reviews.llvm.org/D123692
Reland: test moved because it depends on exceptions.
arichardson pushed a commit to CTSRD-CHERI/libunwind that referenced this pull request Sep 12, 2023
Program may set the RA_SIGN_STATE pseudo register by expressions.
Libunwind expected only the DW_CFA_AARCH64_negate_ra_state could change the value
of the register which leads to runtime errors on PAC enabled systems.
In the recent version of the aadwarf64[1] a limitation is added[2] to forbid the mixing the
DW_CFA_AARCH64_negate_ra_state with other DWARF Register Rule Instructions.

[1] https://github.com/ARM-software/abi-aa/releases/tag/2022Q1
[2] ARM-software/abi-aa#129

Reviewed By: #libunwind, MaskRay

Differential Revision: https://reviews.llvm.org/D123692
arichardson pushed a commit to CTSRD-CHERI/libunwind that referenced this pull request Sep 12, 2023
Program may set the RA_SIGN_STATE pseudo register by expressions.
Libunwind expected only the DW_CFA_AARCH64_negate_ra_state could change the value
of the register which leads to runtime errors on PAC enabled systems.
In the recent version of the aadwarf64[1] a limitation is added[2] to forbid the mixing the
DW_CFA_AARCH64_negate_ra_state with other DWARF Register Rule Instructions.

[1] https://github.com/ARM-software/abi-aa/releases/tag/2022Q1
[2] ARM-software/abi-aa#129

Reviewed By: #libunwind, MaskRay

Differential Revision: https://reviews.llvm.org/D123692
Reland: test moved because it depends on exceptions.
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.

5 participants