On 09-02-19 22:07, Ian Lance Taylor wrote: > On Fri, Feb 8, 2019 at 9:26 AM Thomas Schwinge <tho...@codesourcery.com> > wrote: >> >> On Fri, 8 Feb 2019 10:41:47 +0100, Tom de Vries <tdevr...@suse.de> wrote: >>> The backtrace functions backtrace_full, backtrace_print and backtrace_simple >>> walk the call stack, but make sure to skip the first entry, in order to skip >>> over the functions themselves, and start the backtrace at the caller of the >>> functions. >>> >>> When compiling with -flto, the functions may be inlined, causing them to >>> skip >>> over the caller instead. >> >> So, when recently working on the OpenACC Profiling Interface >> implementation in libgomp, where I'm using libbacktrace to figure out the >> caller of certain libgomp functions, I recently wondered about the very >> same issue, that we reliably have to skip a few initial frames. >> >> So, "noinline" is how to do that reliably... ;-/ That might be >> non-obvious for the casual reader, so they might not understand... >> >>> Fix this by declaring the functions with __attribute__((noinline)). >> >> ... this alone. >> >> I'd suggest to have a common "#define LIBBACKTRACE_NOINLINE [...]" (or >> similar), together with the explanatory comment given above, and use that >> at the respective definition (or declaration?) sites. Can that go into >> the public libbacktrace "*.h" file, so that it can also be used >> elsewhere, as described above? >> >> If you agree, want me to prepare a patch? > > I think that at least for backtrace_full and backtrace_print we are > arguably looking at the SKIP parameter in the wrong place. We > shouldn't look at it in unwind before calling backtrace_pcinfo. We > should count the inlined functions found by backtrace_pcinfo against > the SKIP parameter.
That change makes sense to me. It would stabilise the cut-off point independent of whether inlining happens or not. Though the documentation of both functions list SKIP as "SKIP is the number of frames to skip", and inlined function do not have their own frame, so AFAIU changing things in the way described above would require changing this documentation as well. Btw, I think we'd still need the inline attributes, in order to make libbacktrace behave the same with and without debug info. Thanks, - Tom
