trap_cfi(9): introduce amd64 implementation

[mchoo]

Summary

LLDB dev working on Darwin debugging recommends using CFI instead of trapframe sniffer callbacks. CFI directives itself doesn't any effects in actual code, but I splitted fork_trampoline() into fork_trampoline() and fork_trampoline_kthread() as CFI cannot express conditionals like PC equals to zero.

MFC after: 2 weeks
MFC to: stable/14, stable/15
Sponsored by: FreeBSD Foundation

[kib]

I do not quite understand what could be a subject for the review. It is standalone macro set, which I have no idea what supposed to do, and how are used.

sys/x86/include/frame.h

119

This comment is perhaps misplaced. Should it go somewhere where the 'trap frame CFI information' is applied?

[mchoo]

@kib, I was going to create revisions for actual changes once the macros are accepted, but since I tested this on amd64 with lldb, I'm posting second part of the change here.

[mchoo]

Post first part as well

[kib]

sys/amd64/amd64/exception.S

1397

ast should be handled for kthreads trampolines as well

1775–1790

FRED event entry points need the annotations

sys/amd64/include/trap_cfi.h

17

Shouldn't .cfi_sections directive specified once, at the beginning of the asm source? At least, this is my impression from reading the gas documentation for it.

31

I do not see this macro as useful, it only obfuscates the reading. And the passing of '.' as 'name' is confusing.

[mchoo]

Remove trap_cfi_end and use raw .cfi_endproc
Remove name from trap_cfi_entry
Remove .cfi_startproc from trap_cfi_entry. ENTRY() used in this patch doesn't have .cfi_startproc (the one in machine/asm.h does) but most architectures have only one ENTRY() which has the directive. For consistency between different architectures, omit it from trap_cfi_entry
.cfi_sections .eh_frame should only be emitted once (the original compiled and ran fine, but there's no need to have it under macro)
Added cfi directives to FRED code paths
kthread can return from fork_exit back to trampoline, so handle AST

[kib]

How do you validate the correctness of the .cfi annotations?

sys/amd64/include/trap_cfi.h

39

Did you checked this both with clang as and binutils as? I remember I having troubles with non-gprs, in particular segment registers names. See sys/amd64/amd64/sigtramp.S. If some #ifdef 0's can be enabled there, I think it is quite important for userspace debugging.

48

Might be call this macro 'trap_cfi_trapframe'?

[mchoo]

In D59649#1371990, @kib wrote:

How do you validate the correctness of the .cfi annotations?

I ran full kernel build with default buildkernel. I haven't tested gnu toolchain (although as I noted in the reply, current version of gas can correctly interpret non-gpr names except for %rflags). This eliminates having cfi directives outside .cfi_startproc and .cfi_endproc (This happened a few times while I was modifying macros).

So far, I verified identical backtrace and register info with LLDB (CFI) and KGDB (frame sniffer) for fork_trampoline_kthread() and nmi_calltrap. It's very tedious to check validity of every CFI directive because I need to obtain a dump (or dumps) that cover panics from all the relevant locations.

Because LLM is pretty good at repetitive chores, I let it check the kernel binary and see if CFIs are marked as signal frame, directives and disassmebly match, etc. I can manually go over all the symbols if you want, but I don't think llm's hallucination likely happened here.

sys/amd64/include/trap_cfi.h

39

      .text
      .globl  cfi_segments
      .type   cfi_segments, @function
cfi_segments:
      .cfi_startproc simple
      .cfi_def_cfa    %rsp, 40
      .cfi_offset     %cs, -32
      .cfi_offset     49, -24 /* rflags */
      .cfi_offset     %ss, -8
      ret
      .cfi_endproc
      .size   cfi_segments, . - cfi_segments

      .section .note.GNU-stack,"",@progbits

Both clang 21 and as 2.44 compile this code and emit same result (through llvm-dwarfdump --eh-frame). Both compiles to fail for %rflags instead of 49.

I have not tested on older versions though.

48

Do you want trap_cfi_entry state=full to be state=trapframe as well?

[kib]

In D59649#1372474, @mchoo wrote:

In D59649#1371990, @kib wrote:

How do you validate the correctness of the .cfi annotations?

I ran full kernel build with default buildkernel. I haven't tested gnu toolchain (although as I noted in the reply, current version of gas can correctly interpret non-gpr names except for %rflags). This eliminates having cfi directives outside .cfi_startproc and .cfi_endproc (This happened a few times while I was modifying macros).

So far, I verified identical backtrace and register info with LLDB (CFI) and KGDB (frame sniffer) for fork_trampoline_kthread() and nmi_calltrap. It's very tedious to check validity of every CFI directive because I need to obtain a dump (or dumps) that cover panics from all the relevant locations.

Because LLM is pretty good at repetitive chores, I let it check the kernel binary and see if CFIs are marked as signal frame, directives and disassmebly match, etc. I can manually go over all the symbols if you want, but I don't think llm's hallucination likely happened here.

I only wanted to understand how you do it. I do not ask about any additional actions.

sys/amd64/include/trap_cfi.h

16

Do we need cfi info in .eh_frame? It is loaded into the memory at runtime then. AFAIR there is no consumer of that data in kernel. Since it is for benefit of the offline debugger, can it be .debug_frame instead?

And I think that the .cfi_sections directive should go into the .S file directly. It is very surprising to have include file adjust code generation.

48

Perhaps. Also the default value for the macro arg does not need to be repeated at the use site.

[mchoo]

sys/amd64/include/trap_cfi.h

17

clang drops .cfi_signal_frame for .cfi_sections .dwarf_frame. I don't know if this is intended though.

On top of that, having .eh_frame in stripped kernel still gives correct backtrace when debug symbol file and full kernel are unavailable.

[mchoo]

sys/amd64/include/trap_cfi.h

16

clang drops .cfi_signal_frame for .cfi_sections .dwarf_frame. I don't know if this is intended though. On top of that, having .eh_frame in stripped kernel still gives correct backtrace when debug symbol file and full kernel are unavailable.

[mchoo]

• Move .cfi_sections to assembly files
• Rename trap_cfi_full to trap_cfi_trapframe
• Rename trap_cfi_entry's full state to trapframe
• Add missing CFI directives (e.g. amd64_syscall invalidating r10, r11)
• Add CFI directives to FRED handlers described in trap.c.
• fork_trampoline_kthread should run for P_KPROC, not TDP_KTHREAD
• Place nop after returning from fork_exit so return address sees terminal not trapframe
• Guard non-gpr regs from clang lower than 17

[kib]

sys/amd64/amd64/apic_vector.S

52

At least apic_vector.S and atpic_vector.S are not self-contained asm compilation units. They are not compiled alone, but included into exception.S.

sys/amd64/amd64/vm_machdep.c

172

I believe the old test for TDP_KTHREAD was somewhat more correct. Most of the code uses TDP_KTHREAD to detect kernel threads. I am not even sure that the other use of P_KPROC in copy_thread() is entirely correct, we are copying the thread state there.

sys/amd64/include/trap_cfi.h

17

Do we compile kernel with async exception tables? I remember that we do not, and then standard exception tables are not enough (because we want to unwind across interrupt frames), am I right?

[mchoo]

sys/amd64/include/trap_cfi.h

17

We don't for amd64, but explicit cfi directives still emit FDE in the compiled kernel. The only reason I used .cfi_signal_frame here is to use raw PC.

nop                         /* terminal CFI */
trap_cfi_trapframe
cmpb    $0, fred            /* trapframe CFI */

If exception happens at cmpb and the interrupt handler isn't marked as signal frame, it will calculate pc by subtracting 1 from return address and choose terminal frame. We want trapframe CFI here so unwinder shouldn't subtract 1 from saved PC.

[mchoo]

• No .cfi_sections .eh_frame for ia32_exception.S, apic_vector.S, and atpic_vector.S
• Restore TDP_KTHREAD
• fork_trampoline and fork_trampoline_kthread share post fork_exit code. Jump from fork_trampoline_kthread to .fork_trampoline_return after returning from fork_exit

[jhb]

Did you modify KGDB to handle fork_trampoline_kthread in its custom unwinder or was it able to use the CFI info?

[mchoo]

In D59649#1378673, @jhb wrote:

Did you modify KGDB to handle fork_trampoline_kthread in its custom unwinder or was it able to use the CFI info?

I havne't made any change to devel/gdb, but it seems like kgdb can recognize it.

(lldb) bt
* thread #802, name = '(pid 12) intr/irq60: nvme0:admin'
   * frame #0: 0xffffffff80c89041 kernel`sched_ule_sswitch(td=0xfffff80105370780, flags=<unavailable>) at sched_ule.c:2440:26
     frame #1: 0xffffffff80c5dfe2 kernel`mi_switch(flags=263) at kern_synch.c:526:2
     frame #2: 0xffffffff80c00c49 kernel`ithread_loop(arg=0xfffff801068a8e40) at kern_intr.c:1313:4
     frame #3: 0xffffffff80bfd032 kernel`fork_exit(callout=(kernel`ithread_loop at kern_intr.c:1237), arg=0xfffff801068a8e40, frame=0xfffffe019b9a9f40) at kern_fork.c:1240:2
     frame #4: 0xffffffff811bcefe kernel`fork_trampoline_kthread at exception.S:1404

(kgdb) bt
#0  sched_ule_sswitch (td=0xfffff80105370780, flags=<optimized out>) at /home/mchoo/freebsd/src/sys/kern/sched_ule.c:2440
#1  0xffffffff80c5dfe2 in mi_switch (flags=flags@entry=263) at /home/mchoo/freebsd/src/sys/kern/kern_synch.c:526
#2  0xffffffff80c00c49 in ithread_loop (arg=arg@entry=0xfffff801068a8e40) at /home/mchoo/freebsd/src/sys/kern/kern_intr.c:1313
#3  0xffffffff80bfd032 in fork_exit (callout=0xffffffff80c00bb0 <ithread_loop>, arg=0xfffff801068a8e40, frame=0xfffffe019b9a9f40) at /home/mchoo/freebsd/src/sys/kern/kern_fork.c:1240
#4  <signal handler called>

I'm not sure why kgdb can do this, but i guess gdb prioritizes cfi frame over our custom sniffer.

[jhb]

In D59649#1378709, @mchoo wrote:

In D59649#1378673, @jhb wrote:

Did you modify KGDB to handle fork_trampoline_kthread in its custom unwinder or was it able to use the CFI info?

I havne't made any change to devel/gdb, but it seems like kgdb can recognize it.

(lldb) bt
* thread #802, name = '(pid 12) intr/irq60: nvme0:admin'
   * frame #0: 0xffffffff80c89041 kernel`sched_ule_sswitch(td=0xfffff80105370780, flags=<unavailable>) at sched_ule.c:2440:26
     frame #1: 0xffffffff80c5dfe2 kernel`mi_switch(flags=263) at kern_synch.c:526:2
     frame #2: 0xffffffff80c00c49 kernel`ithread_loop(arg=0xfffff801068a8e40) at kern_intr.c:1313:4
     frame #3: 0xffffffff80bfd032 kernel`fork_exit(callout=(kernel`ithread_loop at kern_intr.c:1237), arg=0xfffff801068a8e40, frame=0xfffffe019b9a9f40) at kern_fork.c:1240:2
     frame #4: 0xffffffff811bcefe kernel`fork_trampoline_kthread at exception.S:1404

(kgdb) bt
#0  sched_ule_sswitch (td=0xfffff80105370780, flags=<optimized out>) at /home/mchoo/freebsd/src/sys/kern/sched_ule.c:2440
#1  0xffffffff80c5dfe2 in mi_switch (flags=flags@entry=263) at /home/mchoo/freebsd/src/sys/kern/kern_synch.c:526
#2  0xffffffff80c00c49 in ithread_loop (arg=arg@entry=0xfffff801068a8e40) at /home/mchoo/freebsd/src/sys/kern/kern_intr.c:1313
#3  0xffffffff80bfd032 in fork_exit (callout=0xffffffff80c00bb0 <ithread_loop>, arg=0xfffff801068a8e40, frame=0xfffffe019b9a9f40) at /home/mchoo/freebsd/src/sys/kern/kern_fork.c:1240
#4  <signal handler called>

I'm not sure why kgdb can do this, but i guess gdb prioritizes cfi frame over our custom sniffer.

The sniffer has priority, but it won't match on the different function name so it will then fall back to DWARF which then will work. This is what I expected and is a good test of the CFI info.

[mchoo]

@jhb Is this ready for commit?