[Lldb-commits] [PATCH] D82378: [lldb/Unwind] Use eh_frame plan directly when it doesn't need to be augmented

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D82378#2109137 <https://reviews.llvm.org/D82378#2109137>, @jankratochvil wrote:

> I do not like much this approach but it fixes this case. I would prefer checking `DW_AT_producer` together with `-grecord-gcc-switches`.
>  I was trying GCC and clang and all the IMO relevant options and I can get either no `.eh_frame`/`.debug_frame` or an asynchronous one.:
>
>   (set -ex;rm -rf udir;mkdir udir;cd udir;rm -f *;for e in -f{,no-}exceptions;do for u in -f{,no-}asynchronous-unwind-tables;do for a in -f{,no-}unwind-tables;do for c in clang gcc;do echo 'int main(void){return 0;}'|$c $e $a $u -o $c$e$u$a -x c -;done;done;done;done;for i in $(sha256sum *|sort|awk '{x[$1]=$2}END{for (y in x) print x[y]}');do nm $i|grep -w main;readelf -wf $i;done) 2>&1|less
>
>
> So I think current compilers no longer produce non-asynchronous unwind tables (that should be verified in the gcc+clang code which I did not) and a simple compiler version check in `DW_AT_producer` would be enough (even without checking compilation options).


I would be the first person to jump with joy of all compilers produced unwind info which is correct at every location, but I'm afraid we're just not there yet. The simplest way to disprove that is to look at the calls to __stdcall functions:

  $ bin/clang -m32 -x c - -o - -S <<<"__stdcall int e(int); void f() { e(47); }" -O3 -fasynchronous-unwind-tables
  ...
  f:                                      # @f
  	.cfi_startproc
  # %bb.0:                                # %entry
  	subl	$12, %esp
  	.cfi_def_cfa_offset 16
  	movl	$47, (%esp)
  	calll	e
  #        .cfi_def_cfa_offset 8  <=== THIS IS MISSING
    	addl	$8, %esp
  	.cfi_def_cfa_offset 4
  	retl
  ...

Clang does not correctly describe the fact that the __stdcall function pops its arguments, so unwind will be incorrect immediately upon return from the function. Gcc handles this correctly, though I don't guarantee that it doesn't miss some other corner case. Now this situation is not actually handled by lldb's "augmenter", so the example somewhat shaky, but it does show that there are gaps in clang/llvm modelling of unwind info.

As for the DW_AT_producer idea, the main gotcha I see there is that eh/debug_frame is supposed to be usable even without the rest of the debug info. Now, we could treat DW_AT_producer as optional and fall back to some default behavior in case it's not present -- that would mean that the unwind sections are still usable independently, but the need for a fallback makes the idea less appealing.

The other problem with that is that we would actually have to gather information about which compilers report the unwind info correctly. I think gathering that information would be very worthwhile, but what I would do with that data is just make a note of the releases which started producing reasonable epilogue info, and when those releases reach a certain age (5 years?), just delete the code to support them.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82378/new/

https://reviews.llvm.org/D82378