jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.
Ravitheja and I had some discussions over email about a possible alternate
approach to this issue - I've committed that approach as r274700 after testing
help from Ravi. This patc
ravitheja updated this revision to Diff 62831.
ravitheja added a comment.
Removing other files.
http://reviews.llvm.org/D21221
Files:
packages/Python/lldbsuite/test/functionalities/unwind/ehframe/
packages/Python/lldbsuite/test/functionalities/unwind/ehframe/Makefile
packages/Python/lldb
ravitheja updated this revision to Diff 61936.
ravitheja added a comment.
Renaming testcase
http://reviews.llvm.org/D21221
Files:
packages/Python/lldbsuite/test/functionalities/unwind/ehframe/
packages/Python/lldbsuite/test/functionalities/unwind/ehframe/Makefile
packages/Python/lldbsuit
ravitheja updated this revision to Diff 61668.
ravitheja added a comment.
Adding testcase
http://reviews.llvm.org/D21221
Files:
packages/Python/lldbsuite/test/functionalities/unwind/nonabi/
packages/Python/lldbsuite/test/functionalities/unwind/nonabi/Makefile
packages/Python/lldbsuite/te
clayborg resigned from this revision.
clayborg removed a reviewer: clayborg.
clayborg added a comment.
Jason Molenda should be able to adequately review any patches. If he is OK,
then I am OK.
http://reviews.llvm.org/D21221
___
lldb-commits mailing
ravitheja added a comment.
@jasonmolenda The approach suggested seems promising, I look forward to it, if
u want I can test it at my end ? once I see it.
http://reviews.llvm.org/D21221
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http:
jasonmolenda added a comment.
Regarding the part of the patch where eh_frame is used unconditionally on Linux
systems, I'm hesitant to make a change like that (beyond the notes I mentioned
earlier about how I'd do it in FuncUnwinders instead of down in
RegisterContextLLDB). Modern gcc's put ou
jasonmolenda added a comment.
Thanks Ravi, I see the problem here and I agree that lldb should use eh_frame
to unwind from this function - that's the only way this is going to work. I'm
surprised that there is accurate eh_frame instructions for this function, it's
great to see it.
I'm wonderi
labath added a comment.
In http://reviews.llvm.org/D21221#457329, @ravitheja wrote:
> @labath In order to reproduce this situation without the help of standard
> library, I would have to write handwritten assembly and the CFI directives
> for that, is that fine ?
Yes, I think that's fine. Obv
ravitheja added a comment.
@labath In order to reproduce this situation without the help of standard
library, I would have to write handwritten assembly and the CFI directives for
that, is that fine ?
http://reviews.llvm.org/D21221
___
lldb-commit
ravitheja added a comment.
so regarding this particular situation I want to give little more insight ->
It starts out from here
0x40143a <+346>: movabsq $0x403e32, %rdi ; imm = 0x403E32
0x401444 <+356>: movb $0x0, %al
0x401446 <+358>: callq 0x400d30 ; symbol s
ravitheja added inline comments.
Comment at: source/Plugins/Process/Utility/RegisterContextLLDB.cpp:1602
@@ -1610,3 +1601,3 @@
// isn't going to do any better.
-if (m_full_unwind_plan_sp->GetSourcedFromCompiler() == eLazyBoolYes)
-return false;
+//if (m_full_u
jasonmolenda added a comment.
Hi Ravi, sorry for not having time to look at this patch yet - I was out of the
office Friday and was catching up on everything today. I'll look at this
tomorrow. My initial reactions are to be a little worried. It sounds like you
have functions that are non-ABI
labath added a comment.
I think this idea in general is more acceptable, but we'll need an OK from
Jason on the details.
Also, we should figure out a way to add a test for this. We should definitely
add a more deterministic test and not rely on TestPrintStackTraces to check
this functionality.
ravitheja updated this revision to Diff 60539.
ravitheja added a comment.
Checks for nullptr
http://reviews.llvm.org/D21221
Files:
source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
source/Plugins/Process/Uti
ravitheja updated this revision to Diff 60533.
ravitheja added a comment.
Making EH Frame CFI the full unwinder
when artificial symbols are found.
http://reviews.llvm.org/D21221
Files:
source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
source/Plugins/DynamicLoader/POSIX-DYLD
ravitheja added a comment.
Well its not possible for the assembly unwinder to do the correct thing in this
situation , as the function is entered through a stub function (which has push
instruction) and then a jump. The point of this patch is to catch when to use
the CFI or the assembly unwinde
jingham added a subscriber: jingham.
jingham added a comment.
I agree with Pavel. Remember that most stops in the course of stepping will
require an unwind (to tell step-in vrs. lateral step vrs. step out). This can
happen many times in the course of a "user level" step. So for stepping,
par
labath added a comment.
So, I'll let Jason be the definitive judge here, but I have to say I don't like
this solution.
Computing 10 frames significantly increases the amount of processing needed to
get some result. Most importantly, it increases the latency, as it will cause a
lot more data to
19 matches
Mail list logo
