labath added a comment.
Committed as r305197. I needed to tweak the test logic a bit, as not even
thread.StepOut() did exactly what we needed there, because it was resuming also
all other threads even though it was not necessary. Take a look at the
resulting commit if you want to see the differ
This revision was automatically updated to reflect the committed changes.
Closed by commit rL305197: Introduce new command: thread backtrace unique
(authored by labath).
Changed prior to commit:
https://reviews.llvm.org/D33426?vs=101664&id=102192#toc
Repository:
rL LLVM
https://reviews.llvm
bgianfo added a comment.
Can someone commit this as I obviously don't have a svn commit bit?
https://reviews.llvm.org/D33426
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.
This looks fine to me.
https://reviews.llvm.org/D33426
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/
labath added a comment.
Cool. Sorry about all the back-and-forth and thanks for the patience.
https://reviews.llvm.org/D33426
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
bgianfo updated this revision to Diff 101664.
bgianfo added a comment.
Update test to user thread.StepOut() instead of thread.Resume().
https://reviews.llvm.org/D33426
Files:
include/lldb/Core/Debugger.h
include/lldb/Target/StackFrame.h
include/lldb/Target/StackFrameList.h
include/lldb/
labath added a comment.
In https://reviews.llvm.org/D33426#772174, @bgianfo wrote:
> @jingham @labath do you have any more feedback?
As Jim pointed out, the Resume command does not do what I thought it does, so
having it in the test makes no sense. One option would be to just leave out the
ca
bgianfo added a comment.
@jingham @labath do you have any more feedback?
https://reviews.llvm.org/D33426
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Woops, my bad. I was hoping that "resume" was to "StepInstruction",
which does have an immediate action, but i did not check what it
actually does.
In this case, we could probably just use "StepOut" for the test case,
as it should have the same effect - advance the thread until it
reaches the next
SBThread::Resume instructs lldb to set the resume state on a thread to
"eStateRunning", meaning that means the next time you continue the process,
that thread will get a chance to run. It has no effect on the StopReason for
the thread (it doesn't even start it running except maybe in the not we
clayborg accepted this revision.
clayborg added a comment.
Looks like a good starting point. Thanks for the changes.
https://reviews.llvm.org/D33426
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/list
labath added a comment.
In https://reviews.llvm.org/D33426#766525, @bgianfo wrote:
> Address Pavel and Greg's feedback on Diff 100365.
>
> Pavel: I took your suggestions to make the test case more readable,
> I really appreciate the guidance. I did have to tweak some of the
> functionality to m
bgianfo updated this revision to Diff 100830.
bgianfo added a comment.
Fix bug in "unique" backtrace output that Greg pointed out.
Introduced a new format for unique frames and plumbed that
through the stacks to be able to toggle between them both
depending on the calling arguments.
https://rev
DenseSet<> provides a method named find_as which lets you use an arbitrary
type as the lookup key. The only requirement is that your DenseMapInfo<>
that you've specialized provide two methods for each key type you want to
use. So using this approach, even if you have a DenseSet S,
you can still w
clayborg added a comment.
Zach made some comments via e-mail:
> Couple of things:
>
> 1. Unless you really really want iteration over this map to be in some
> deterministic order, better to use unordered_map. And if you do want it to
> be in some deterministic order, you should provide a comp
Couple of things:
1) Unless you really really want iteration over this map to be in some
deterministic order, better to use unordered_map. And if you do want it to
be in some deterministic order, you should provde a comparison function, as
the default one is probably not what you want.
2) std::ma
clayborg added a comment.
The other option for fixing the problem with showing the wrong variables in the
backtraces would be to make up a new frame-format string that is used for
uniqued stack frames and use that format when showing uniqued stack frames:
(lldb) settings show frame-format-uni
clayborg added a comment.
Clicked submit too early with last comments. After seeing how we previously
needed to create a temp UniqueStack just to do the lookup, and also how
UniqueStack make its thread index ID list mutable, a better solution is to use
a std::map as shown in the inlined comment
clayborg added a comment.
Much cleaner. After seeing how we must construct a UniqueStack just so we can
search for it, an even cleaner solution would be to make unique_stacks into:
std::map
https://reviews.llvm.org/D33426
___
lldb-commits mailin
bgianfo updated this revision to Diff 100529.
bgianfo marked 7 inline comments as done.
bgianfo added a comment.
Address Pavel and Greg's feedback on Diff 100365.
Pavel: I took your suggestions to make the test case more readable,
I really appreciate the guidance. I did have to tweak some of the
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
We should probably store the stacks as lldb::addr_t values that are load
addresses for quicker comparisons and searches. Inlined code details things
more clearly.
labath added a comment.
thanks for the effort. I found the logic of the test quite difficult to follow,
with multiple breakpoints and notify_calls(). Instead of trying to point out
each problem, I figured it will be easier to write my take on how the test
could look like: https://paste.ubuntu.c
bgianfo updated this revision to Diff 100365.
bgianfo added a comment.
Address Pavel's feedback, made the unit test more robust.
This update increases the robustness of the new test I added.
We ensure synchronization, and force the threads into the state
we want them to be in by manually stepping
labath added a comment.
Thanks for writing the test. We just need to make sure it runs in a stable
manner.
Comment at:
packages/Python/lldbsuite/test/functionalities/thread/num_threads/TestNumThreads.py:91
+# thread3 function. All of these threads should show as one s
bgianfo updated this revision to Diff 100047.
bgianfo marked an inline comment as done.
bgianfo added a comment.
Address Pavel/Jim/Greg's feedback with the addition of a new test.
I followed Jim's advice and extended the existing num_threads suite
so that we start more thread3's. The new test_uni
clayborg added a comment.
Add a test and fix the minor things as suggested and this will be good to go.
https://reviews.llvm.org/D33426
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-com
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.
Pavel's right, it would be good to add a test case. You could modify the test
case in packages/Python/lldbsuite/test/functionalities/thread/num_threads/ to
this end. Note this Te
labath added a comment.
Sounds like an awesome feature.
Could you please add a test for it as well?
https://reviews.llvm.org/D33426
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commit
28 matches
Mail list logo
