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

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil accepted this revision.
jankratochvil added a comment.
This revision is now accepted and ready to land.

In D82378#2111675 <https://reviews.llvm.org/D82378#2111675>, @labath wrote:

> It's not related, at least not directly.


My goal was to prove `UnwindAssembly_x86::AugmentUnwindPlanFromCallSite` is no longer useful nowadays.  It was trying to detect situation where both:

- `.eh_frame` exists
- `.eh_frame` does not unwind prologue

I cannot force clang/gcc to produce such `.eh_frame`, IMO it should be produced with `-funwind-tables -fno-asynchronous-unwind-tables` but that does produce the same as `-fasynchronous-unwind-tables`.

Whether the compiler has some unwind info incompleteness/bugs is not important, the quality of `.eh_frame` from recent clang+gcc is always better than any fallbacks LLDB has.

So given `UnwindAssembly_x86::AugmentUnwindPlanFromCallSite` is no longer useful and it can (I think it still can sometimes) falsely discard a good `.eh_frame` section it would be best to disable `UnwindAssembly_x86::AugmentUnwindPlanFromCallSite` for new compilers. The question is how to detect "new compilers" without needing DWARF, my only idea is to do it by checking existence of section `.gnu.build.attributes`. If some Linux distribution still does not have `.gnu.build.attributes` does not matter, it is only a backward compatible improvement of current situation. I will post a patch for that as I have hijacked this bug.

> And if there's one bug/incompleteness, there could very well be others, so I was also alluding to the fact that we might need to have some sort of an "augmentation framework" for the forseeable future. However, it could very well be the case that we no longer need to augment epilogue data, and if that's the case deleting the code for doing that would be great (but for that someone would need to investigate the epilogue behavior of different compilers more closely).

I do not see any specific case needing augmentation for code from new compilers.

"deleting the code" - if you mean one does not need to detect `.gnu.build.attributes` (or some other way) to detect new compilers and rather just assume any code is from a new enough compiler it is even better (and I agree).

> Well... I don't think that's an improvement. :/ This section seems to be present only on some flavours of linux (my distro doesn't have it),

When it somewhere is an improvement and somewhere it has no effect I find it still as an improvement.

> So, I don't see the reason for changing the current detection algorithm.

I mean to discard the current detection if there is possible a more safe detection.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82378