[Mesa-dev] [PATCH 2/2] gallium/util: libunwind support

[Thomas Hellstrom]

On 04/03/2017 11:09 PM, Rob Clark wrote:
> On Mon, Apr 3, 2017 at 4:57 PM, Rob Clark <robdclark at gmail.com> wrote:
>> On Mon, Apr 3, 2017 at 4:06 PM, Thomas Hellstrom <thellstrom at vmware.com> wrote:
>>> On 04/03/2017 07:33 PM, Thomas Hellstrom wrote:
>>>> On 04/03/2017 07:13 PM, Rob Clark wrote:
>>>>> On Mon, Apr 3, 2017 at 12:56 PM, Thomas Hellstrom <thellstrom at vmware.com> wrote:
>>>>>> Hi, Rob,
>>>>>>
>>>>>> On 03/24/2017 10:21 PM, Rob Clark wrote:
>>>>>>> It's kinda sad that (a) we don't have debug_backtrace support on !X86
>>>>>>> and that (b) we re-invent our own crude backtrace support in the first
>>>>>>> place.  If available, use libunwind instead.  The backtrace format is
>>>>>>> based on what xserver and weston use, since it is nice not to have to
>>>>>>> figure out a different format.
>>>>>>>
>>>>>>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>>>>>> Did you consider glibc "backtrace()", I think it's also available on ARM...
>>>>> I had not.. although xserver and weston are already using libunwind.
>>>>> I'm not sure about portability of libunwind to other libc
>>>>> implementations (but I guess it is at least not worse than using a
>>>>> glibc specific API).
>>>>>
>>>>> I suppose we could always add a fallback to backtrace().
>>>>>
>>> Hmm. This commit (bisected) appears to break svga/vmwgfx in DEBUG mode:
>>>
>>> *** Error in `glxgears': malloc(): memory corruption: 0x00000000025c09c0 ***
>>> Aborted (core dumped)
>>>
>>> The svga linux winsys makes extensive use of the backtrace functionality
>>> using u_debug_flush.c
>> ok, I can reproduce.. hopefully patch following shortly..
>>
> So, I found one other minor issue, but the big problem is
> debug_stack_frame size difference, because u_debug_flush.c doesn't
> #include config.h..  adding:
>
> #ifdef HAVE_CONFIG_H
> #include <config.h>
> #endif

Including config.h and also limiting the unwind frame-walk to nr_frames
in u_debug_stack.c fixes the problem on my side.

>
> in u_debug_stack.h solves that..  although I think usually folks
> prefer to have that in .c files.  (Not sure if I remember the reason.)
>
> So we should either add that in u_debug_stack.h or track down all the
> .c files that #include it and add there.
>
> (Or is there a reason we don't just do -include config.h globally in
> the build system?)

I guess the answer to both these is that an implementation might want to
have control over exactly which header files
config.h affects. Also as a side note, when adding config.h we need to
make sure we can build without.

But one more worrying thing is that with these fixes, debug_flush gets
too slow to be usable. I get about one frame every 5 seconds from Ubuntu
compiz. The culprit seems to be unw_get_proc_name(). Is there a way we
can save intermediate information during backtrace capture and call this
function at printout time?

/Thomas


>
> BR,
> -R