[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

aleksandr.urakov added a comment. @stella.stamenova I've fixed it with r374888, thanks again for pointing to that. The fix of unwind can fix some stepping issues, because stepping logic uses call stack information actively. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

Re: [Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

Sorry, I'm OOO now. I'll take a look tomorrow, thanks for catching that! On Mon, 14 Oct 2019, 19:20 Stella Stamenova via Phabricator, < revi...@reviews.llvm.org> wrote: > stella.stamenova added a comment. > > In D67347#1706405 , > @stella.stamenova wrote:

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

stella.stamenova added a comment. In D67347#1706405 , @stella.stamenova wrote: > It looks like this changed fixed at least one of the XFAILed tests on Windows: > > http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/9751 > > So now the test

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

stella.stamenova added a comment. It looks like this changed fixed at least one of the XFAILed tests on Windows: http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/9751 So now the test results would be red because of the unexpectedly passing test (if there wasn't another failure).

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

mstorsjo added a comment. In D67347#1705611 , @labath wrote: > In D67347#1705563 , @mstorsjo wrote: > > > Quick question here; will unwinding using DWARF debug info still work like > > before after this, for

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

labath added a comment. In D67347#1705563 , @mstorsjo wrote: > Quick question here; will unwinding using DWARF debug info still work like > before after this, for binaries that don't use SEH for exception unwinding? Do you mean debug_frame or eh_frame?

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

aleksandr.urakov added a comment. In D67347#1705563 , @mstorsjo wrote: > Quick question here; will unwinding using DWARF debug info still work like > before after this, for binaries that don't use SEH for exception unwinding? Hi Martin! Do I

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

mstorsjo added a comment. Quick question here; will unwinding using DWARF debug info still work like before after this, for binaries that don't use SEH for exception unwinding? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67347/new/

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

This revision was automatically updated to reflect the committed changes. Closed by commit rG30c2441a3262: [Windows] Use information from the PE32 exceptions directory to construct… (authored by aleksandr.urakov). Changed prior to commit: https://reviews.llvm.org/D67347?vs=220144=224554#toc

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

aleksandr.urakov added a comment. Thanks a lot for the review! Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67347/new/ https://reviews.llvm.org/D67347 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

jasonmolenda accepted this revision. jasonmolenda added a comment. This revision is now accepted and ready to land. Thanks for all of the work on this patch Aleksandr, I really appreciate the work and this will be a nice addition to lldb. Apologies again for not looking over the revised patch

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

amccarth added a comment. I'm just back from vacation. I agree with Pavel that we need to hear more from Jason at this point. I'm still very interested in helping this land in some form. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67347/new/

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

labath added a comment. I'm afraid I don't have much to add beyond what I've already said. I think @jasonmolenda should make the call as to how to move forward here. If the chosen direction requires any changes in how the breakpad unwind info handled, I'm happy to give you a hand there.

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

aleksandr.urakov added a comment. Gentle ping! What do you think of this? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67347/new/ https://reviews.llvm.org/D67347 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

labath added a comment. Sorry for the delay, I was OOO. I haven't yet read through all of the discussion, but I wanted to plug into this part. In D67347#1669877 , @aleksandr.urakov wrote: > But there is one more reason that makes it very difficult to

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

aleksandr.urakov marked 4 inline comments as done. aleksandr.urakov added a comment. Hi Jason, thanks for the review! Initially, the reason of these changes was to reuse the code that works with `eh_frame`, because on Windows we have a very similar compiler generated info designed to help with

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

aleksandr.urakov updated this revision to Diff 220144. aleksandr.urakov edited the summary of this revision. aleksandr.urakov added a comment. Update due to the requests. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67347/new/ https://reviews.llvm.org/D67347

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

amccarth added a comment. I have a couple more questions and some renaming requests. Comment at: lldb/include/lldb/Symbol/DWARFCallFrameInfo.h:74 - void ForEachFDEEntries( - const std::function ); + void ForEachEntries(const std::function ) override;

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

jasonmolenda added a comment. Hi Aleksandr, nice job getting this working. I read over the patch and had a couple of initial questions. I don't understand the creation of the ICallFrameInfo base class. All of the unwind info providers (things which parse their own format and output an

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

clayborg added a comment. Added Jason Molenda who owns the unwind stuff so he might be able to comment and help us with this patch. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67347/new/ https://reviews.llvm.org/D67347

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

clayborg added a comment. My opinion is we need to be able to ask both the SymbolFile and ObjectFile for unwind plans in the API, but we can always just ask the SymbolFile for the unwind plan and the default SymbolFile virtual function can just defer to the ObjectFile. We also need to say "I

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

aleksandr.urakov marked 4 inline comments as done. aleksandr.urakov added a comment. Thanks all for taking a look! In D67347#1666509 , @amccarth wrote: > This patch is pretty large. It might be easier to break it up into a series > of small steps to

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

aleksandr.urakov updated this revision to Diff 219853. aleksandr.urakov added a comment. Update diff due to Pavel's requests. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67347/new/ https://reviews.llvm.org/D67347 Files:

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

amccarth added a comment. I've been trying to figure out how to implement the same functionality you've got here, so I'm very interested in helping you land this in some form. I think Clayborg and Pavel makes some good points about where the right layer of abstraction is for the unwind plans.

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

labath added a comment. In D67347#1664640 , @aleksandr.urakov wrote: > It's a good point, thank you! I had the same thoughts when I done it, but I'm > not completely sure. The problem is that an object file can't be completely > responsible for

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

aleksandr.urakov added a comment. It's a good point, thank you! I had the same thoughts when I done it, but I'm not completely sure. The problem is that an object file can't be completely responsible for choosing an unwind plan, because some plans are produced by symbol files (and an object

[Lldb-commits] [PATCH] D67347: [Windows] Use information from the PE32 exceptions directory to construct unwind plans

clayborg added a comment. At the object file level I would love to see much less of this specific unwind info making it into the ObjectFile interface. Why can't we just ask the ObjectFile to provide an UnwindPlan for a given address. There is so much complexity within the UnwindPlan where it