[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