Re: [PATCH] Fix dwarf2 unwinding through futex functions v2

["Carlos O'Donell" <carlos at redhat dot com>]

On 11/07/2013 07:22 PM, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> When profiling programs with lock problems with perf record -g dwarf, libunwind
> can currently not backtrace through the futex and unlock functions in pthread.
> This is because they use out of line sections, and those are not correctly
> described in dwarf2 (I believe needs dwarf3 or 4).
> 
> This patch first removes the out of line sections. They only save a single
> jump, but cause a lot of pain. Then it converts the now inline lock
> code to use the now standard gas .cfi_* commands.
> 
> Then the very complicated and long manual dwarf2 code can be removed. Right
> now I just #if 0ed it to make review easier, but it can be just
> removed for the final commit.
> 
> With these changes libunwind/perf can backtrace through the futex functions
> now.
> 
> Longer term it would be likely better to just use C futex() functions
> on x86 like all the other architectures. This would clean the code up
> even more.
> 
> v2: Removed #if 0 code (patch is a lot larger now)
>     Rebased to latest master and reran test suite. No new regressions.
>     I did not find an easy way to run the gdb test suite sorry

v2 looks good to me.

I'm OK with this patch as long as you've tried to at least debug a
program built with these new libpthread code and you can unwind through
them. That is to say build some application with the newly built glibc
e.g. -Wl,--dynamic-linker=xxx -Wl,-rpath=xxx:xxx, and you can break on
some instruction deep in the lock sequence and backtrace correctly.
 
> 2013-11-07  Andi Kleen  <ak@linux.intel.com>
> 
> 	* sysdeps/unix/sysv/linux/x86_64/lowlevellock.h (lll_lock, lll_robust_lock,
>         lll_cond_lock, lll_timedlock, lll_robust_cond_lock, lll_robust_timedlock,
> 	  lll_unlock, lll_robust_unlock): Remove out of line section. Use cfi
>         intrinsics.
>         (LLL_STUB_UNWIND_INFO*): Remove.
> 	* sysdeps/unix/sysv/linux/i386/lowlevellock.h (lll_lock, lll_robust_lock,
>         lll_cond_lock, lll_timedlock, lll_robust_cond_lock, lll_robust_timedlock,
> 	  lll_unlock, lll_robust_unlock): Remove out of line section. Use cfi
> 	  intrinsics.
>         (LLL_STUB_UNWIND_INFO*): Remove.

Take care that the start of a new line in the ChangeLog starts at the same level
of indentation as the *.

I'd like Andreas Schwab to sign off on this also since he commented on the first
revision of the patch and may have further comments.

Cheers,
Carlos.