[PATCH v4] Backtrace via call detail

José Fonseca jose.r.fonseca at gmail.com
Mon May 13 03:13:59 PDT 2013


On Mon, May 13, 2013 at 10:34 AM, Alexander Monakov <amonakov at ispras.ru>wrote:

> Hello,
>
> On Wed, 8 May 2013, José Fonseca wrote:
>
> > - please confirm that Android was not busted with my changes (as I only
> > tested on Linux)
>
> It needs the following patch to unbreak the build:
>
> diff --git a/common/trace_backtrace.cpp b/common/trace_backtrace.cpp
> index ef19aa9..d0bdd5a 100644
> --- a/common/trace_backtrace.cpp
> +++ b/common/trace_backtrace.cpp
> @@ -243,7 +243,7 @@ public:
>          while (*rawBacktrace_it != '\0') {
>              RawStackFrame stackFrame;
>              // TODO: Keep a cache of stack frames
> -            stackFrame->id = nextFrameId++;
> +            stackFrame.id = nextFrameId++;
>              /* skip leading space */
>              while (*rawBacktrace_it == ' ') {
>                  rawBacktrace_it++;
>
>
>
Applied thanks.


> > - please make default not capture backtraces on linux (I'm fine leaving
> > backtraces on by default on Android if nobody else opposes) -- this is
> > temporarily until backtrace capture is fast enough that the overhead is
> > negligble (which I think it should be possible)
>
> We (both Eugene and I) don't quite understand what happened here.  You have
> eliminated usage of 'backtrace_is_needed', removing the ability to select
> the
> subset of "backtraced" calls.  Was that intentional?  This was not raised
> in
> preceding review, and you did not prune the resulting dead code.
>

No, it was not intentional. I removed by mistake, and somehow thought that
it was only used in Android. I've re-added backtrace_is_needed() now.


> Non-zero overhead from backtrace collection was the reason Eugene
> implemented
> that subsetting in the first place.
>

OK. It seems good enough for now. I'll merge to master shortly after sanity
check on windows/macosx.


BTW, another issue is that the "offset" values on linux are not really the
offset from start of shared object, but the absolute address. Anyway, I
think that using glibc backtrace for this is unsustainable long term, as it
requires executables to be built with -rdynamic, and shared objects not to
be built with -fvisibility=hidden, the exact opposite of common practice
these days.  I don't know if there is a better alternative for runtime
symbol resolution, otherwise I think we'll need to do offline symbol
resolution on linux by default.


One last request -- could you make it so that when one selects a call on
qapitrace, the default lower pane is the "Details View" (instead of the
"Backtrace").


Jose
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/apitrace/attachments/20130513/ffdb7335/attachment.html>


More information about the apitrace mailing list