Re: [Lldb-commits] [PATCH] D16814: Fix handling of the arm IT instruction in the unwinder

2016-02-10 Thread Tamas Berghammer via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL260368: Fix handling of the arm IT instruction in the unwinder (authored by tberghammer). Changed prior to commit: http://reviews.llvm.org/D16814?vs=46764&id=47432#toc Repository: rL LLVM http://rev

Re: [Lldb-commits] [PATCH] D16814: Fix handling of the arm IT instruction in the unwinder

2016-02-09 Thread Greg Clayton via lldb-commits
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good. http://reviews.llvm.org/D16814 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/li

Re: [Lldb-commits] [PATCH] D16814: Fix handling of the arm IT instruction in the unwinder

2016-02-09 Thread Tamas Berghammer via lldb-commits
tberghammer added a comment. Jason: Can you take at the change in the unwinding logic? Comment at: source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp:13653 @@ -13656,2 +13652,3 @@ const uint32_t cond = CurrentCond (m_opcode.GetOpcode32()); -return cond != 0xe && c

Re: [Lldb-commits] [PATCH] D16814: Fix handling of the arm IT instruction in the unwinder

2016-02-04 Thread Tamas Berghammer via lldb-commits
tberghammer added inline comments. Comment at: include/lldb/Core/EmulateInstruction.h:390 @@ +389,3 @@ +typedef uint32_t InstructionCondition; +static const InstructionCondition UnconditionalCondition = UINT32_MAX; + clayborg wrote: > It is nice to tell wh

Re: [Lldb-commits] [PATCH] D16814: Fix handling of the arm IT instruction in the unwinder

2016-02-03 Thread Greg Clayton via lldb-commits
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Yes, it was probably too complex. My main objection was the use of the UINT32_MAX as a magic number. Your UnconditionalCondition solution clears this up. Comme

Re: [Lldb-commits] [PATCH] D16814: Fix handling of the arm IT instruction in the unwinder

2016-02-03 Thread Pavel Labath via lldb-commits
labath added a comment. I agree that we should not over-engineer things. I'll leave the decision up to others... http://reviews.llvm.org/D16814 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo

Re: [Lldb-commits] [PATCH] D16814: Fix handling of the arm IT instruction in the unwinder

2016-02-03 Thread Tamas Berghammer via lldb-commits
tberghammer updated this revision to Diff 46764. tberghammer added a comment. Create a typedef for the condition type and a static const value for the unconditional condition. http://reviews.llvm.org/D16814 Files: include/lldb/Core/EmulateInstruction.h include/lldb/Symbol/UnwindPlan.h so

Re: [Lldb-commits] [PATCH] D16814: Fix handling of the arm IT instruction in the unwinder

2016-02-02 Thread Tamas Berghammer via lldb-commits
tberghammer added a comment. UINT32_MAX is a kind of a random value what is most likely won't be used on any architecture as a condition code (I can't imagine having so much different condition flags) and my current intention is to map the unconditional value to into it as it is already done on

Re: [Lldb-commits] [PATCH] D16814: Fix handling of the arm IT instruction in the unwinder

2016-02-02 Thread Greg Clayton via lldb-commits
clayborg added a comment. I like that llvm::Optional idea! http://reviews.llvm.org/D16814 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D16814: Fix handling of the arm IT instruction in the unwinder

2016-02-02 Thread Pavel Labath via lldb-commits
labath added a subscriber: labath. Comment at: include/lldb/Core/EmulateInstruction.h:406-410 @@ -405,4 +405,7 @@ -virtual bool -IsInstructionConditional() { return false; } +// Returns a condition code in the for of uint32_t where UINT32_MAX means that the instruct

Re: [Lldb-commits] [PATCH] D16814: Fix handling of the arm IT instruction in the unwinder

2016-02-02 Thread Greg Clayton via lldb-commits
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. It would be nice to not pick UINT32_MAX for the unconditional condition and let each emulator picks it desired values. See inlined comments. Let me know what you think. ===

[Lldb-commits] [PATCH] D16814: Fix handling of the arm IT instruction in the unwinder

2016-02-02 Thread Tamas Berghammer via lldb-commits
tberghammer created this revision. tberghammer added reviewers: omjavaid, jasonmolenda, clayborg. tberghammer added a subscriber: lldb-commits. Herald added subscribers: danalbert, tberghammer, rengolin, aemerson. Fix handling of the arm IT instruction in the unwinder The IT instruction can speci