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

labath added a comment. In D82378#2113865 , @labath wrote: > You're right that the instruction emulation not handling multiple epilogues > is a bug. It's probably not about _all_ epilogues, only some special ones -- > for example, the function where we

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

labath marked an inline comment as done. labath added inline comments. Comment at: lldb/test/Shell/Unwind/eh-frame-augment-noop.test:18 +target modules show-unwind -n foo +# CHECK: Asynchronous (not restricted to call-sites) UnwindPlan is 'eh_frame CFI' +# CHECK: eh_frame

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

JDevlieghere added inline comments. Comment at: lldb/test/Shell/Unwind/eh-frame-augment-noop.test:18 +target modules show-unwind -n foo +# CHECK: Asynchronous (not restricted to call-sites) UnwindPlan is 'eh_frame CFI' +# CHECK: eh_frame augmented UnwindPlan:

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG5ed8765e2f00: [lldb/Unwind] Use eh_frame plan directly when it doesnt need to be augmented (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

jasonmolenda accepted this revision. jasonmolenda added a comment. Sorry yes this patch is fine. To be honest, on x86_64 at least, as you've pointed out, maybe we should just live on eh_frame completely without going through any of these augmentation checks at all. AFAIK gdb is living off of

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

labath added a comment. Thanks for replying Jason. I think having the augmentation string specify its validity would be great, though I am somewhat sceptical of being able to push that idea through -- given that eh_frame needs to be understood by a lot of consumers and is actually critical

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

jasonmolenda added a comment. Hi Pavel, sorry I've been doing a bunch of random things today and haven't really had a chance to look at this yet. eh_frame is so problematic for lldb, we never know what we're getting. I keep thinking we should take over a few augmentation string characters so

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

jankratochvil accepted this revision. jankratochvil added a comment. This revision is now accepted and ready to land. In D82378#2111675 , @labath wrote: > It's not related, at least not directly. My goal was to prove

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

labath updated this revision to Diff 273043. labath added a comment. - remove trailing whitespace - simplify test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82378/new/ https://reviews.llvm.org/D82378 Files:

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

labath added a comment. In D82378#2109467 , @jankratochvil wrote: > In D82378#2109330 , @labath wrote: > > > Now this situation is not actually handled by lldb's "augmenter", so the > > example somewhat shaky,

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

jankratochvil added a comment. I do not understand why the testcase has **two** epilogues. The `UnwindAssembly_x86::AugmentUnwindPlanFromCallSite()` code tests only beginning and end of CFI, it does not read anything in between. The simplified testcase works the same for me:

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

jankratochvil added a comment. In D82378#2109330 , @labath wrote: > 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. Good

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

labath added a comment. In 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

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

jankratochvil added a comment. 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

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

labath created this revision. labath added reviewers: jasonmolenda, jankratochvil. Herald added a project: LLDB. This fixes a bug in the logic for choosing the unwind plan. Based on the comment in UnwindAssembly-x86, the intention was that a plan which describes the function epilogue correctly