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

[Rob Clark]

On Tue, Apr 4, 2017 at 5:00 AM, Thomas Hellstrom <thellstrom at vmware.com> wrote:
> 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
> Hmm,
>
> Actually it seems we don't have a config.h, but rather -DHAVE_LIBUNWIND
> is included in the compile command.
> So the memory corruption should be solely due to not limiting the stack
> walk.

possibly that is build system dependent?  The limiting the stack walk
was the other fix that I found, but with autotools build I needed to
add the #include <config.h>.

As far as speed, my original idea was to store the unw_cursor_t in the
debug_stack_frame.  But that is only valid while the unw_context_t is
still valid, and we didn't have a very good place to stash the
unw_context_t without refactoring the u_debug_stack API.  (And also
the unw_context_t can be fairly large on some archs.)  I'll think
about alternatives after I have some caffeine in me this morning..

BR,
-R