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

ARM asm fails with latest Clang, non-private labels cannot appear between .cfi_startproc / .cfi_endproc pairs #807

Open
mstorsjo opened this issue Nov 18, 2023 · 2 comments

Comments

@mstorsjo
Copy link
Contributor

The ARM assembly, in particular https://github.com/libffi/libffi/blob/master/src/arm/sysv.S#L139-L213 (the sequence of ffi_call_VFP followed by ffi_call_SYSV) fails to compile for Windows/ARM with the very latest Clang (the latest nightly since today).

The breaking change in Clang is this commit: https://reviews.llvm.org/D155245

[MC][AsmParser] Diagnose improperly nested .cfi frames

Compiling fails with an error like this:

../src/arm/sysv.S:156:37: error: non-private labels cannot appear between .cfi_startproc / .cfi_endproc pairs
.globl ffi_call_SYSV; ; .align 3; ; ffi_call_SYSV:
                                    ^
../src/arm/sysv.S:141:2: error: previous .cfi_startproc was here
 .cfi_startproc
 ^

For unknown reasons, this error doesn't seem to trigger on Linux targets, as mentioned in https://reviews.llvm.org/D155245#4657075. However the overall sentiment seems to be that this construct, having one CFI region cover two global functions, is invalid.

@mstorsjo
Copy link
Contributor Author

FWIW, the relevant commit in Clang was reverted in llvm/llvm-project@797b68c, and they're considering restricting the scope of when this is reported as an error in https://reviews.llvm.org/D155245#4657083.

@billatarm
Copy link
Contributor

It also seems that cfi_startproc could be moved below the label as well, testing seems to indicate that it's fine and we can just work around it. TBH in all other code ive looked at using CFI directives (which isn't much), its always below the label FWIW.

basilisk-dev pushed a commit to Basilisk-Development-Team/UXP-mirror that referenced this issue Sep 20, 2024
Due to the following LLVM change labels need to be outside .cfi_start/endprocs:
https://reviews.llvm.org/D155245
Solution pointed out here is to move the label above .cfi_startproc:
libffi/libffi#807
roytam1 pushed a commit to roytam1/UXP that referenced this issue Sep 22, 2024
…he following LLVM change labels need to be outside .cfi_start/endprocs: https://reviews.llvm.org/D155245 Solution pointed out here is to move the label above .cfi_startproc: libffi/libffi#807
roytam1 added a commit to roytam1/basilisk55 that referenced this issue Sep 22, 2024
…code 16. Due to the following LLVM change labels need to be outside .cfi_start/endprocs: https://reviews.llvm.org/D155245 Solution pointed out here is to move the label above .cfi_startproc: libffi/libffi#807 (27e66e3f)
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

No branches or pull requests

2 participants