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.

Sign up for GitHub

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

Jump to bottom

LLVM's MCParser does not understand .cfi_label directives #97222

Closed
alexrp opened this issue Jun 30, 2024 · 1 comment · Fixed by #97922
Closed

LLVM's MCParser does not understand .cfi_label directives #97222

alexrp opened this issue Jun 30, 2024 · 1 comment · Fixed by #97922
Assignees
@MaskRay
Labels
mc Machine (object) code

Comments

@alexrp
Copy link
Member

alexrp commented Jun 30, 2024 •
edited
Loading 

❯ cat test.s
.globl foo
.align 2
.type foo,@function
.cfi_startproc
.cfi_label .Ldummy
.cfi_endproc
❯ riscv64-linux-gnu-gcc test.s -c
❯ /opt/llvm/bin/clang -target riscv64 test.s -c
test.s:5:1: error: unknown directive
.cfi_label .Ldummy
^
❯ /opt/llvm/bin/clang --version
clang version 18.1.6 (https://github.com/llvm/llvm-project 1118c2e05e67a36ed8ca250524525cdb66a55256)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /opt/llvm/bin

This is currently a blocker for compiling glibc with Clang for ARC, C-SKY, LoongArch, and RISC-V, as these all make use of .cfi_label in their start.S (example).
@alexrp alexrp mentioned this issue Jun 30, 2024
Open
@EugeneZelenko EugeneZelenko added mc Machine (object) code and removed new issue labels Jun 30, 2024
@MaskRay MaskRay self-assigned this Jul 6, 2024
@MaskRay MaskRay mentioned this issue Jul 6, 2024
Merged
@MaskRay
Copy link
Member

MaskRay commented Jul 6, 2024 •
edited
Loading

Created #97922 to support .cfi_label

In glibc, https://sourceware.org/PR23125 sysdeps/riscv/start.S utilizes .cfi_label to force
DW_CFA_undefined to be placed in a FDE. arc/csky/loongarch ports have
copied this use.

.cfi_startproc
// DW_CFA_undefined is allowed for CIE's initial instructions.
// Without .cfi_label, gas would place DW_CFA_undefined in a CIE.
.cfi_label .Ldummy
.cfi_undefined ra
.cfi_endproc

Technically, .cfi_label .Ldummy isn't really necessary. It moves the instruction from CIE to FDE and potentially decrease the number of CIEs, usually without making the FDE larger (since the FDE has to be padded).

ARC, C-SKY, LoongArch, and RISC-V, as these all make use of .cfi_label in their start.S ([example]

I believe ARC, C-SKY, LoongArch just copy RISC-V without understanding the rationale :)

/opt/llvm/bin/clang -target riscv64 test.s -c

Use --target=riscv64. -target has been deprecated for a long time since around Clang 3.4

@MaskRay MaskRay closed this as completed in 2718654 Jul 7, 2024
searlmc1 pushed a commit to ROCm/llvm-project that referenced this issue Jul 9, 2024
@MaskRay @ronlieb
[MC] Support .cfi_label
48f05fd 
GNU assembler 2.26 introduced the .cfi_label directive. It does not
expand to any CFI instructions, but defines a label in
.eh_frame/.debug_frame, which can be used by runtime patching code to
locate the FDE. .cfi_label is not allowed for CIE's initial
instructions, and can therefore be used to force the next instruction to
be placed in a FDE instead of a CIE.

In glibc since 2018, sysdeps/riscv/start.S utilizes .cfi_label to force
DW_CFA_undefined to be placed in a FDE. arc/csky/loongarch ports have
copied this use.
```
.cfi_startproc
// DW_CFA_undefined is allowed for CIE's initial instructions.
// Without .cfi_label, gas would place DW_CFA_undefined in a CIE.
.cfi_label .Ldummy
.cfi_undefined ra
.cfi_endproc
```

No CFI instruction is associated with .cfi_label, so the `case
MCCFIInstruction::OpLabel:` code in BOLT is unreachable and onlt to make
-Wswitch happy.

Close llvm#97222

Pull Request: llvm#97922

Change-Id: Ic236d33bf52ac5631bd202c2896d23463c8e099b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@MaskRay MaskRay

Labels
mc Machine (object) code
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[MC] Support .cfi_label llvm/llvm-project
3 participants
@alexrp @MaskRay @EugeneZelenko