Improperly nested .cfi region check should only fire on MachO targets

[jroelofs]

https://reviews.llvm.org/D155245#4657075

However, I noticed a different surprising detail here; it looks like this check fails to trigger on Linux (ELF in general I presume?) targets, while it does trigger on MachO and COFF. This can be tested with something like this:

.globl ffi_call_VFP
ffi_call_VFP:
.cfi_startproc
nop
.globl ffi_call_SYSV
ffi_call_SYSV:
nop
.cfi_endproc

$ clang -target aarch64-linux-gnu -c fallthrough.s 
$ clang -target aarch64-apple-darwin -c fallthrough.s 
fallthrough.s:7:1: error: non-private labels cannot appear between .cfi_startproc / .cfi_endproc pairs 
ffi_call_SYSV:
^
fallthrough.s:3:2: error: previous .cfi_startproc was here
.cfi_startproc 
^
$ clang -target aarch64-windows-gnu -c fallthrough.s 
fallthrough.s:7:1: error: non-private labels cannot appear between .cfi_startproc / .cfi_endproc pairs
ffi_call_SYSV:
^
fallthrough.s:3:2: error: previous .cfi_startproc was here
.cfi_startproc
^

[llvmbot]

@llvm/issue-subscribers-lld-elf

Author: Jon Roelofs (jroelofs)

https://reviews.llvm.org/D155245#4657075

> However, I noticed a different surprising detail here; it looks like this check fails to trigger on Linux (ELF in general I presume?) targets, while it does trigger on MachO and COFF. This can be tested with something like this:

.globl ffi_call_VFP
ffi_call_VFP:
.cfi_startproc
nop
.globl ffi_call_SYSV
ffi_call_SYSV:
nop
.cfi_endproc

$ clang -target aarch64-linux-gnu -c fallthrough.s 
$ clang -target aarch64-apple-darwin -c fallthrough.s 
fallthrough.s:7:1: error: non-private labels cannot appear between .cfi_startproc / .cfi_endproc pairs 
ffi_call_SYSV:
^
fallthrough.s:3:2: error: previous .cfi_startproc was here
.cfi_startproc 
^
$ clang -target aarch64-windows-gnu -c fallthrough.s 
fallthrough.s:7:1: error: non-private labels cannot appear between .cfi_startproc / .cfi_endproc pairs
ffi_call_SYSV:
^
fallthrough.s:3:2: error: previous .cfi_startproc was here
.cfi_startproc
^

[MaskRay]

Due to Mach-O's .subsections_via_symbols mechanism, non-private labels cannot appear between .cfi_startproc/.cfi_endproc. (https://reviews.llvm.org/D153167) However, this is not an issue for other object file formats that don't use .subsections_via_symbols.

Technically a non-local symbol can be defined in the middle of a function for ELF and other non-Mach-O object file formats. This is not commonly used. However, I am also not sure whether we need to impose this restriction.

---

If an assembly file is shared between ELF and Mach-O, I think it is fair to say that the user should be responsible for ensuring that there is no non-local symbol between a pair of .cfi_startproc/.cfi_endproc. However, it is probably not the assembler's responsibility to report an error for ELF.

[arichardson]

I have seen nested globals used e.g. in FreeBSD where the EENTRY() macro is used to declare an "extra entry" for functions without the .cfi_startproc that is normally added by the ENTRY() macro. I can't see it used much in the current source tree, currently only for symbol alias, but I'm pretty sure that in the past it was also used for nested entry points with a few set up instructions between the first and second entry point.

I think in most cases the nested global is an accident, so maybe a warning that can be silenced for legitimate use cases is the right thing?

[jroelofs]

The actual thing I'm trying to prevent is crashes from broken relocations, which occur when the .cfi region belongs to another symbol because of .subsections_via_symbols. IIUC under that constraint, EENTRY(foo) ENTRY(bar) is fine, but ENTRY(bar) EENTRY(foo) is not.

[zmodem]

I posted a follow-up on https://reviews.llvm.org/D155245#4657375 but I think maybe Phabricator never sent the email.

The situation with this vs. the compiler-rt builtins (which were patched in 7939ce3) seems a bit unfortunate. We hit that in https://crbug.com/1504532 but in general this means that it's no longer possible to build LLVM 17 with LLVM 18.

IIUC the builtins problem boils down to:

label:
.cfi_startproc
<stuff>

vs.

.cfi_startproc
label:   // We now error here.
<stuff>

To me these look equivalent at the binary level, but maybe with .subsections_via_symbols they are not? In that case, how did the builtins work before? Did they just have incorrect CFI?