On Tue, Jul 2, 2024 at 6:11 PM Josh Poimboeuf <jpoimboe@xxxxxxxxxx> wrote: > > On Tue, Jul 02, 2024 at 05:06:14PM -0700, Andrii Nakryiko wrote: > > > > Should it also check for ENDBR64? > > > > > > > > Sure, I can add a check for endbr64 as well. endbr64 probably can be > > used not just at function entry, is that right? So it might be another > > case of false positive (which I think is ok, see below). > > Yeah, at least theoretically they could happen in the middle of a > function for implementing C switch jump tables. > > > > > When compiled with -fcf-protection=branch, the first instruction of the > > > > function will almost always be ENDBR64. I'm not sure about other > > > > distros, but at least Fedora compiles its binaries like that. > > > > > > BTW, there are some cases (including leaf functions and some stack > > > alignment sequences) where a "push %rbp" can happen inside a function. > > > Then it would presumably add a bogus trace entry. Are such false > > > positives ok? > > > > I think such cases should be rare. People mostly seem to trace user > > function entry/exit, rarely if ever they trace something within the > > function, except for USDT cases, where it will be a nop instruction > > that they trace. > > > > In general, even with false positives, I think it's overwhelmingly > > better to get correct entry stack trace 99.9% of the time, and in the > > rest 0.01% cases it's fine having one extra bogus entry (but the rest > > should still be correct), which should be easy for humans to recognize > > and filter out, if necessary. > > Agreed, this is a definite improvement overall. Cool, I'll incorporate that into v3 and send it soon. > > BTW, soon there will be support for sframes instead of frame pointers, > at which point these checks should only be done for the frame pointer > case. Nice, this is one of the reasons I've been thinking about asynchronous stack trace capture in BPF (see [0] from recent LSF/MM). Few questions, while we are at it. Does it mean that perf_callchain_user() will support working from sleepable context and will wait for data to be paged in? Is anyone already working on this? Any pointers? [0] https://docs.google.com/presentation/d/1k10-HtK7pP5CMMa86dDCdLW55fHOut4co3Zs5akk0t4 > > -- > Josh
