This revision was automatically updated to reflect the committed changes.
Closed by commit rL329889: [dotest] Use in-tree dsymutil on Darwin (authored by
JDevlieghere, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D45518?vs=142037=142137#toc
Repository:
rL LLVM
Author: jdevlieghere
Date: Thu Apr 12 02:25:32 2018
New Revision: 329889
URL: http://llvm.org/viewvc/llvm-project?rev=329889=rev
Log:
[dotest] Use in-tree dsymutil on Darwin
Summary:
With the upstream implementation of dsymutil containing almost all
functionality from the one shipped with Xcode,
labath added a comment.
In https://reviews.llvm.org/D45480#1063268, @zturner wrote:
> > This removes the last (direct) dependency from the Host module to
> > Interpreter, so I remove the Interpreter module from Host's dependency list.
>
> Big milestone! Kudos
Thanks. Host is still a member
labath added a comment.
I don't see myself using this command personally, but I agree that splitting
the "disable stats collection" and "dump accumulated stats" into two actions
seems a better choice (maybe disable could do a final auto-dump though). I also
think the header/footer is not
labath added a comment.
The fix seems simple enough, but Jim needs to say whether this is the right way
to fix this bug, as I am not sure about what are our assumptions about
Breakpoint object lifetimes.
Comment at:
labath accepted this revision.
labath added a comment.
This looks good.
Do you have any plans on how will this work with the XCode build (which is
kind of a prerequisite to start removing stuff from dotest)?
https://reviews.llvm.org/D45333
___
This revision was automatically updated to reflect the committed changes.
Closed by commit rL329891: Dont assume backing thread shares protocol ID.
(authored by JDevlieghere, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D45497?vs=141982=142139#toc
Repository:
rL LLVM
JDevlieghere added a comment.
In https://reviews.llvm.org/D45333#1065332, @labath wrote:
> This looks good.
>
> Do you have any plans on how will this work with the XCode build (which is
> kind of a prerequisite to start removing stuff from dotest)?
I haven't spend too much time worrying
Author: jdevlieghere
Date: Thu Apr 12 03:51:52 2018
New Revision: 329897
URL: http://llvm.org/viewvc/llvm-project?rev=329897=rev
Log:
Revert "Don't assume backing thread shares protocol ID."
This reverts r329891 because the test case is timing out on linux:
Author: jdevlieghere
Date: Thu Apr 12 02:58:20 2018
New Revision: 329891
URL: http://llvm.org/viewvc/llvm-project?rev=329891=rev
Log:
Don't assume backing thread shares protocol ID.
When we're dealing with virtual (memory) threads created by the OS
plugins, there's no guarantee that the real
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.
LGTM module comment
Comment at: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:3809
}
// E01 code from vAttach means
eugene added inline comments.
Comment at:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c:13
{
+printf("Observable side effect\n");
return 0; // Set break point at this line.
labath wrote:
> Why did you need to add
xiaobai added subscribers: sas, xiaobai.
xiaobai added a comment.
I really like this idea! It will be very helpful for @sas and I. I'd like to +1
creating a separate `stats dump` subcommand instead of dumping stats on `stats
disable`.
Comment at:
davide added inline comments.
Comment at:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c:14-15
+// This line adds a real body to the function, so we can set more than one
+// breakpoint in it.
+printf("Observable side effect\n");
eugene updated this revision to Diff 142209.
eugene marked 2 inline comments as done.
eugene added a comment.
add comment to the test
https://reviews.llvm.org/D45554
Files:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
labath created this revision.
labath added reviewers: jingham, JDevlieghere.
If the remote stub sends a specific error message instead of just a E??
code, we can use this to display a more informative error message
instead of just the generic "unable to attach" message.
I write a test for this
davide added a comment.
lgtm
https://reviews.llvm.org/D45573
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
davide added inline comments.
Comment at:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/main.c:14-15
+// This line adds a real body to the function, so we can set more than one
+// breakpoint in it.
+printf("Observable side effect\n");
davide added a comment.
I'm under the impression that we should either merge `log timers` with `stats`
or just remove log timers altogether and start from scratch.
From what I hear from Jim, it was really useful for a few people, so maybe a
fresh start would be a better way of handling things.
davide added inline comments.
Comment at: lldb/source/Commands/CommandObjectStats.cpp:43-46
+target->RegisterStats(StatisticKind::ExpressionSuccessful);
+target->RegisterStats(StatisticKind::ExpressionFailure);
+target->RegisterStats(StatisticKind::FrameVarSuccess);
JDevlieghere updated this revision to Diff 142239.
JDevlieghere added a comment.
Make things a little more consistent.
https://reviews.llvm.org/D45586
Files:
source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp
Index:
clayborg added inline comments.
Comment at: packages/Python/lldbsuite/test/lldbutil.py:579
test.assertTrue(
-file_name == out_file_name,
+out_file_name in file_name,
"Breakpoint file name '%s' doesn't match resultant name '%s'." %
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.
Looks good.
https://reviews.llvm.org/D45592
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
clayborg created this revision.
clayborg added reviewers: jingham, jasonmolenda, labath, lldb-commits.
Herald added subscribers: JDevlieghere, aprantl.
Many IDEs set breakpoints using absolute paths and this causes problems when
the full path of the source file path doesn't match what is in the
jingham added a comment.
Except for a minor comment, this seems fine to me, and is a useful addition!
Comment at: packages/Python/lldbsuite/test/lldbutil.py:579
test.assertTrue(
-file_name == out_file_name,
+out_file_name in file_name,
clayborg updated this revision to Diff 142289.
clayborg added a comment.
Fixed the breakpoint verification in lldbutil to make sure the file name ends
with the right thing
https://reviews.llvm.org/D45592
Files:
include/lldb/Breakpoint/BreakpointResolverFileLine.h
jingham added a comment.
Timers seemed like they would be really useful for collection of data about
operations in lldb, but for most things I think they end up being hard to use
because actual wall-clock time is so variable from run to run, and especially
for disk and inter-process heavy
JDevlieghere created this revision.
JDevlieghere added reviewers: clayborg, jingham, labath.
When performing a synchronous resume, the API mutex is held until the
process is stopped. This is fine, except for when the OS plugins are processing
an event before the main thread is aware of it, in
jingham added a comment.
Calling ClearAllBreakpointSites twice does no harm, it just sees the list is
empty and goes on. So even though Target::RemoveBreakpointByID clears the
breakpoint sites by disabling them and then calls BreakpointList::Remove, it is
fine for BreakpointList::Remove to
davide added inline comments.
Comment at: lldb/source/Commands/CommandObjectStats.cpp:37
+
+if (target->GetCollectingStats() == true) {
+ result.AppendError("stats already enabled");
xiaobai wrote:
> nit: You can drop the `== true`
Thanks, I'll fix
davide added inline comments.
Comment at: lldb/source/Commands/CommandObjectExpression.cpp:626
+ Target *target = m_interpreter.GetExecutionContext().GetTargetPtr();
+ if (!target)
jingham wrote:
> This is what CommandObject::GetSelectedOrDummy target is
eugene updated this revision to Diff 142319.
eugene added a comment.
Got rid of the printf in the test
https://reviews.llvm.org/D45554
Files:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
davide updated this revision to Diff 142301.
https://reviews.llvm.org/D45547
Files:
lldb/include/lldb/Target/Target.h
lldb/include/lldb/lldb-private-enumerations.h
lldb/packages/Python/lldbsuite/test/functionalities/stats/Makefile
davide added inline comments.
Comment at: source/Breakpoint/BreakpointResolverFileLine.cpp:219-221
+ if (is_relative) {
+search_file_spec.GetDirectory().Clear();
+ }
For consistency with the rest of the codestyle (and what you use above), can
you remove
davide added a comment.
Thanks for this! Just one minor inline.
Comment at: source/Breakpoint/BreakpointResolverFileLine.cpp:132
+if (is_relative) {
+ // If the path was reltive, make sure any matches match as long as the
+ // relative parts of the path match the
eugene marked 2 inline comments as done.
eugene added a comment.
There is an ownership cycle between BreakpointSite::m_owners and
BreakpointLocation::m_bp_site_sp.
We should probably make m_owners a collection of weak references.
But currently most of the code just works it around by calling
jingham accepted this revision.
jingham added a comment.
That looks fine.
https://reviews.llvm.org/D45573
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
davide added a comment.
In https://reviews.llvm.org/D45547#1066348, @jingham wrote:
> Timers seemed like they would be really useful for collection of data about
> operations in lldb, but for most things I think they end up being hard to use
> because actual wall-clock time is so variable from
jingham added a comment.
I made a few comments all to the same effect, you can use
CommandObject::GetSelectedOrDummyTarget to get the target that the command is
operating on, or you can get is straight from m_exe_ctx if you want to make
sure that you are getting a real target. In the case of
davide added a comment.
I prefer having it as a top level command rather than part of log. If you think
about it LLVM does the same distinction and it worked quite well in practice.
We plan to collect more metrics to the command so I'd very much like to have
this living as a separate entity.
davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.
No objections from me.
https://reviews.llvm.org/D45480
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
labath added a comment.
In https://reviews.llvm.org/D45547#1065661, @davide wrote:
> I prefer having it as a top level command rather than part of log. If you
> think about it LLVM does the same distinction and it worked quite well in
> practice.
> We plan to collect more metrics to the
42 matches
Mail list logo
