amd64: Add CFI directives for libc syscall stubs

[cem]

Summary

This DWARF metadata allows llvm-libunwind to unwind program stacks when the
program is executing the function. The goal is to collect accurate
userspace stacktraces when programs have entered syscalls.

(The motivation for "Call Frame Information," or CFI for short -- not to be
confused with Control Flow Integrity -- is to sufficiently annotate assembly
functions such that stack unwinders can unwind out of the local frame
without the requirement of a dedicated framepointer register; i.e.,
-fomit-frame-pointer. This is necessary for C++ exception handling or
collecting backtraces.)

[cem]

Fix a number of ENTRY()s without matched END()s.

[emaste]

I think you could go ahead and commit the END() additions independently in any case?

Do you have an example you can paste that shows the correct unwinding after this addition?

[kib]

I dislike the fact that you now have to strictly match ENTRY with END, and umtx_op change demonstrates the reason.

[emaste]

I dislike the fact that you now have to strictly match ENTRY with END

What alternative would you prefer though?

[kib]

In D22122#483452, @emaste wrote:

I dislike the fact that you now have to strictly match ENTRY with END

What alternative would you prefer though?

I do not see any other variant except a new macros.

In fact, I suspect that enforcement of .cfi_startproc might make things worse for functions which e.g. push anything, esp. %rbp but do not annotate cfa changes. Normally unwinder would only use dwarf in that case, while without .cfi_start_proc they fall back to the stack-frame unwinder. The later works, while dwarf-based does not.

[emaste]

I do not see any other variant except a new macros.

I.e., something like BEGIN_CFI and END_CFI? I suppose that'd be fine.

[cem]

Thanks for taking a look, folks!

In D22122#483447, @emaste wrote:

I think you could go ahead and commit the END() additions independently in any case?

Maybe? I don't see the downside, but I'm not sure what kib's concern is yet.

Edit: I submitted the libm changes, but not the libthr one that I think kib may object to, in r353929.

Do you have an example you can paste that shows the correct unwinding after this addition?

Before:

#0  0x00080041396a at /lib/libc/libc.so.7:__sys_select+0xa

After:

#0  0x00080041396a at /obj/usr/home/conrad/src/freebsd/amd64.amd64/lib/libc/libc.so.7.full:__sys_select+0xa
#1  0x0008006604b2 at /lib/libthr.so.3:_pthread_suspend_all_np+0x1632
#2  0x0008008baffc at /usr/local/lib/libpython3.6m.so.1.0:PyInit_time+0x61c
#3  0x0008007c3a36 at /usr/local/lib/libpython3.6m.so.1.0:_PyCFunction_FastCallDict+0x246
#4  0x000800844355 at /usr/local/lib/libpython3.6m.so.1.0:PyEval_GetFuncDesc+0x225
#5  0x0008008419f1 at /usr/local/lib/libpython3.6m.so.1.0:_PyEval_EvalFrameDefault+0x61e1
#6  0x000800844bf0 at /usr/local/lib/libpython3.6m.so.1.0:PyEval_GetFuncDesc+0xac0
#7  0x00080083b763 at /usr/local/lib/libpython3.6m.so.1.0:PyEval_EvalCode+0x33
#8  0x00080086fd2d at /usr/local/lib/libpython3.6m.so.1.0:PyRun_StringFlags+0x8d
#9  0x00080086fc65 at /usr/local/lib/libpython3.6m.so.1.0:PyRun_SimpleStringFlags+0x45
#10 0x00080088a5b1 at /usr/local/lib/libpython3.6m.so.1.0:Py_Main+0xa41
#11 0x0000002013c8 at /usr/local/bin/python:main+0xf8
#12 0x00000020110f at /usr/local/bin/python:_start+0x10f

(Frames 1 and 2 are probably incorrectly resolved static functions, so ignore the nonsense, but it unwinds out of libc now, which is what matters. Python was python -c 'import time; time.sleep(5)'.)

In D22122#483449, @kib wrote:

I dislike the fact that you now have to strictly match ENTRY with END, and umtx_op change demonstrates the reason.

Can you elaborate on the latter? What do you dislike about the umtx_op change?

In D22122#483464, @kib wrote:

In fact, I suspect that enforcement of .cfi_startproc might make things worse for functions which e.g. push anything, esp. %rbp but do not annotate cfa changes. Normally unwinder would only use dwarf in that case, while without .cfi_start_proc they fall back to the stack-frame unwinder. The later works, while dwarf-based does not.

FWIW, LLVM's unwinder cannot unwind even functions that push nothing without CFI directives. It has no functional fallback for the libc stubs, which do not have an ordinary framepointer. (See paste in earlier comment.)

I don't think there are uses of this version of ENTRY/END outside of libc, libm, and libthr. Do we consider the machine/asm.h macros an API for 3rd party ports? The kernel uses the similar file asmacros.h, not this one.

In D22122#483468, @emaste wrote:

I.e., something like BEGIN_CFI and END_CFI? I suppose that'd be fine.

If we are going to divorce this from BEGIN/END, we might as well just skip straight to the pass-through capitalized names, i.e., CFI_STARTPROC and CFI_ENDPROC.

(Edit: combined comments.)

[cem]

This comment was removed by cem.

[cem]

sys/amd64/include/asm.h

62–77 ↗

(On Diff #63570)

So one concern I have is that anything that actually uses ALTENTRY now has two cfi_startprocs followed by a single cfi_endproc. I don't know if as tolerates that in a sane way.

On the other hand, I don't see any uses of ALTENTRY in all of userspace, so maybe we can just remove the definition.