[Lldb-commits] [PATCH] D85951: Add Python iterator & getitem for SBTypeEnumMemberList

2020-08-14 Thread Jim Ingham via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGb6db0a544df1: Add python enumerators for SBTypeEnumMemberList, and some tests for this API. (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D85951: Add Python iterator & getitem for SBTypeEnumMemberList

2020-08-13 Thread Jim Ingham via Phabricator via lldb-commits
jingham created this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. jingham requested review of this revision. Herald added a subscriber: JDevlieghere. This class represents the members of an enum type, but wasn't terribly convenient to use because it only had

[Lldb-commits] [PATCH] D85265: Add a setting to always run all threads when stepping

2020-08-07 Thread Jim Ingham via Phabricator via lldb-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd3dfd8cec440: Add a setting to force stepping to always run all threads. (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [PATCH] D85265: Add a setting to always run all threads when stepping

2020-08-07 Thread Jim Ingham via Phabricator via lldb-commits
jingham added inline comments. Comment at: lldb/source/Commands/CommandObjectThread.cpp:486-490 + // NonStopMode runs all threads down in the ProcessPlugin layer, but + // at this level we need to pretend we are actually only running this + // thread. So

[Lldb-commits] [PATCH] D85265: Add a setting to always run all threads when stepping

2020-08-07 Thread Jim Ingham via Phabricator via lldb-commits
jingham updated this revision to Diff 284031. jingham added a comment. reduce comment to the part appropriate to this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85265/new/ https://reviews.llvm.org/D85265 Files:

[Lldb-commits] [PATCH] D85265: Add a setting to always run all threads when stepping

2020-08-07 Thread Jim Ingham via Phabricator via lldb-commits
jingham added inline comments. Comment at: lldb/source/Commands/CommandObjectThread.cpp:486-490 + // NonStopMode runs all threads down in the ProcessPlugin layer, but + // at this level we need to pretend we are actually only running this + // thread. So

[Lldb-commits] [PATCH] D85265: Add a setting to always run all threads when stepping

2020-08-07 Thread Jim Ingham via Phabricator via lldb-commits
jingham updated this revision to Diff 283945. jingham added a comment. Clarify comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85265/new/ https://reviews.llvm.org/D85265 Files: lldb/bindings/interface/SBThreadPlan.i

[Lldb-commits] [PATCH] D85265: Add a setting to always run all threads when stepping

2020-08-06 Thread Jim Ingham via Phabricator via lldb-commits
jingham updated this revision to Diff 283769. jingham added a comment. Remove a file that got inadvertently included from another commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85265/new/ https://reviews.llvm.org/D85265 Files:

[Lldb-commits] [PATCH] D85265: Add a setting to always run all threads when stepping

2020-08-06 Thread Jim Ingham via Phabricator via lldb-commits
jingham updated this revision to Diff 283767. jingham added a comment. clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85265/new/ https://reviews.llvm.org/D85265 Files: lldb/bindings/interface/SBThreadPlan.i

[Lldb-commits] [PATCH] D85235: [lldb] Make SBTarget::LaunchSimple start form the target's LaunchInfo

2020-08-04 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85235/new/ https://reviews.llvm.org/D85235 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D85265: Add a setting to always run all threads when stepping

2020-08-04 Thread Jim Ingham via Phabricator via lldb-commits
jingham created this revision. jingham added a reviewer: friss. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. jingham requested review of this revision. Herald added a subscriber: JDevlieghere. There are some platforms (the xnu kernel being one) where trying to run just

[Lldb-commits] [PATCH] D85106: Move TestGuiBasicDebug.py to lldb/test and update it

2020-08-04 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D85106#2194840 , @clayborg wrote: > lldb/test/API is usually for testing the lldb::SB* interfaces IIRC. So not > sure this move makes sense? It's really for any test that wants to use the lldbtest test harness. There are

[Lldb-commits] [PATCH] D84527: Rename StoppointLocation to StoppointSite and drop its relationship with BreakpointLocation

2020-07-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. Remember to do the action thingie... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84527/new/ https://reviews.llvm.org/D84527 ___ lldb-commits

[Lldb-commits] [PATCH] D84809: [lldb] Fix invalid error message in TargetList::CreateTargetInternal

2020-07-28 Thread Jim Ingham via Phabricator via lldb-commits
jingham added inline comments. Comment at: lldb/source/Target/TargetList.cpp:245 } + error_strm.Printf("), specify an architecture to disambiguate"); + error.SetErrorString(error_strm.GetString()); jingham wrote: > I'm not sure why

[Lldb-commits] [PATCH] D84809: [lldb] Fix invalid error message in TargetList::CreateTargetInternal

2020-07-28 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. This LGTM with a couple of quibbles but I haven't been working on the platform support recently so we should wait till people who have done so weight in. Comment at: lldb/source/Target/TargetList.cpp:175-176 + if

[Lldb-commits] [PATCH] D84800: [lldb] Remove unused option '--platform-path' for 'target create'

2020-07-28 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. I'm a little curious how you would specify the path to your binary on a remote platform using the command line. But this way certainly wasn't it... Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D84527: Rename StoppointLocation to StoppointSite and drop its relationship with BreakpointLocation

2020-07-24 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. This overall change makes sense to me. It seems a little awkward that Target has to know that Watchpoints have a m_hit_counter. It's also a little weird that we're clearing watchpoint hit counts when a process dies, but not breakpoint hit counts. If we do one we

[Lldb-commits] [PATCH] D84475: [lldb] Inform every language runtime of the modified modules

2020-07-24 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84475/new/ https://reviews.llvm.org/D84475 ___

[Lldb-commits] [PATCH] D84257: [lldb] Don't use hardware index to determine whether a breakpoint site is hardware

2020-07-22 Thread Jim Ingham via Phabricator via lldb-commits
jingham added inline comments. Comment at: lldb/source/Breakpoint/BreakpointLocation.cpp:73 + if (m_bp_site_sp) +return m_bp_site_sp->IsHardware(); + tatyana-krasnukha wrote: > JDevlieghere wrote: > > Should we sanity check that this is true when

[Lldb-commits] [PATCH] D84307: [lldb/interpreter] Move the history subcommand to session (NFCI)

2020-07-22 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. The current lldb behavior is that when you start up lldb, it reads the editline command-history file (so the history of the previous session), and loads it into it's internal command history, which you can get to by up and down arrows or ^R searches. But it doesn't

[Lldb-commits] [PATCH] D84267: Thread ExecutionContextScope through GetByteSize where possible (NFC-ish)

2020-07-21 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. I think you missed two more places to pass a target, then this looks good to me. How tedious, thanks for doing it... Comment at:

[Lldb-commits] [PATCH] D84267: Thread ExecutionContextScope through GetByteSize where possible (NFC-ish)

2020-07-21 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. I pointed out a couple of places where you have either at hand or near at hand a useful exe_ctx that you could pass instead of nullptr. Other than that, this seems good to me. Thanks for pushing this through. Comment at:

[Lldb-commits] [PATCH] D84272: Add checks for ValueObjectSP in Cocoa summary providers

2020-07-21 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. The `!text` test is correct, since you intend to pass `*text` in as a `ValueObject &`. But I wouldn't add the GetValueAsUnsigned check, that seems confusing. The NSStringSummaryProvider is returning a bool to tell you whether it succeeded or not, so it seems odd to

[Lldb-commits] [PATCH] D84267: Thread ExecutionContextScope through GetByteSize where possible (NFC-ish)

2020-07-21 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. The patch lost seems to have lost everything but the PPC, SystemZ and MIPS code... CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84267/new/ https://reviews.llvm.org/D84267 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D84253: OptionValue::Clear should return void not bool

2020-07-21 Thread Jim Ingham via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8d6aa688eeff: Remove the bool return from OptionValue::Clear and its subclasses. (authored by jingham). Changed prior to commit: https://reviews.llvm.org/D84253?vs=279580=279594#toc Repository: rG

[Lldb-commits] [PATCH] D84253: OptionValue::Clear should return void not bool

2020-07-21 Thread Jim Ingham via Phabricator via lldb-commits
jingham created this revision. jingham added a reviewer: JDevlieghere. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. I forgot to add a "return true" to the new OptionValueFileColonLine::Clear method which the Windows compiler caught (thanks Jonas for fixing that!) But

[Lldb-commits] [PATCH] D83975: Add an option to "break set" and "source list" that takes a line spec in the form file:line:column

2020-07-20 Thread Jim Ingham via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. jingham marked 2 inline comments as done. Closed by commit rGbc0a9a17a4a6: Add an option (-y) to break set and source list that uses the same file… (authored by jingham). Changed prior to commit:

[Lldb-commits] [PATCH] D84210: [lldb] Use a weak pointer to hold on to the underlying thread plan in SBThreadPlan

2020-07-20 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84210/new/ https://reviews.llvm.org/D84210 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D84210: [lldb] Use a weak pointer to hold on to the underlying thread plan in SBThreadPlan

2020-07-20 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Yes, the only way you can get your hands on an SBThreadPlan is in the process of queueing one, so the ThreadPlan stack will always be the one to manage the lifecycle of a plan. And if an SBThreadPlan represents a ThreadPlan that has been discarded, it should convert

[Lldb-commits] [PATCH] D84083: [lldb/ObjectFileMachO] Correctly account for resolver symbols

2020-07-17 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84083/new/ https://reviews.llvm.org/D84083

[Lldb-commits] [PATCH] D83975: Add an option to "break set" and "source list" that takes a line spec in the form file:line:column

2020-07-17 Thread Jim Ingham via Phabricator via lldb-commits
jingham marked 4 inline comments as done. jingham added inline comments. Comment at: lldb/source/Interpreter/OptionValueFileColonLine.cpp:120-127 + // The setting value may have whitespace, double-quotes, or single-quotes + // around the file path to indicate that

[Lldb-commits] [PATCH] D83975: Add an option to "break set" and "source list" that takes a line spec in the form file:line:column

2020-07-17 Thread Jim Ingham via Phabricator via lldb-commits
jingham updated this revision to Diff 278928. jingham added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83975/new/ https://reviews.llvm.org/D83975 Files: lldb/include/lldb/Interpreter/OptionValue.h

[Lldb-commits] [PATCH] D83975: Add an option to "break set" and "source list" that takes a line spec in the form file:line:column

2020-07-16 Thread Jim Ingham via Phabricator via lldb-commits
jingham marked 2 inline comments as done. jingham added inline comments. Comment at: lldb/include/lldb/Interpreter/OptionValueFileColonLine.h:43 +m_line_number = LLDB_INVALID_LINE_NUMBER; +m_column_number = 0; + } JDevlieghere wrote: > This should be

[Lldb-commits] [PATCH] D83975: Add an option to "break set" and "source list" that takes a line spec in the form file:line:column

2020-07-16 Thread Jim Ingham via Phabricator via lldb-commits
jingham marked 8 inline comments as done. jingham added inline comments. Comment at: lldb/include/lldb/Interpreter/OptionValueFileColonLine.h:26 + + // Virtual subclass pure virtual overrides + JDevlieghere wrote: > I think these comments add very little, if

[Lldb-commits] [PATCH] D83975: Add an option to "break set" and "source list" that takes a line spec in the form file:line:column

2020-07-16 Thread Jim Ingham via Phabricator via lldb-commits
jingham updated this revision to Diff 278642. jingham added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83975/new/ https://reviews.llvm.org/D83975 Files: lldb/include/lldb/Interpreter/OptionValue.h

[Lldb-commits] [PATCH] D83975: Add an option to "break set" and "source list" that takes a line spec in the form file:line:column

2020-07-16 Thread Jim Ingham via Phabricator via lldb-commits
jingham created this revision. jingham added reviewers: JDevlieghere, teemperor. Herald added subscribers: lldb-commits, dang, mgorny. Herald added a project: LLDB. I think it is important to have separate the file & line specifiers in "break set" because that way no matter what you've named

[Lldb-commits] [PATCH] D83876: [lldb] Clean orphaned modules in Debugger::Destroy

2020-07-15 Thread Jim Ingham via Phabricator via lldb-commits
jingham requested changes to this revision. jingham added a comment. We certainly don't want to clear the shared module cache when the Debugger or the SBDebugger is destroyed. Most IDE's that use LLDB as a library use a debugger per debugging session, and destroy the debugger when the

[Lldb-commits] [PATCH] D83728: [lldb] Make `process connect` blocking in synchronous mode.

2020-07-13 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83728/new/ https://reviews.llvm.org/D83728 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D83728: [lldb] Make `process connect` blocking in synchronous mode.

2020-07-13 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Couple of minor comments. Comment at: lldb/include/lldb/Target/Platform.h:855 protected: + lldb::ProcessSP DoConnectProcess(llvm::StringRef connect_url, + llvm::StringRef plugin_name, Even though

[Lldb-commits] [PATCH] D83450: Delegate UpdateChildrenPointerType to the Root ValueObject

2020-07-10 Thread Jim Ingham via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGe337350be9d6: This is a refinement on 96601ec28b7efe5abf3479a1aa91bcedb235bbbd. The intent of… (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D83450: Delegate UpdateChildrenPointerType to the Root ValueObject

2020-07-10 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D83450#2143631 , @labath wrote: > Right. It does bug me a little that this has no test, but I don't think it's > the worst that could happen. And you're the one who's going to have to debug > this all over again if the next

[Lldb-commits] [PATCH] D83446: [WIP][lldb/Reproducers] Synchronize the command interpreter with asynchronous events

2020-07-10 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D83446#2143464 , @labath wrote: > In D83446#2142564 , @JDevlieghere > wrote: > > > Seems like my thoughts got lost a bit with all the inline replies: we can > > solve this particular

[Lldb-commits] [PATCH] D83433: Fix how we handle bit-fields for Objective-C when creating an AST

2020-07-09 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. IIRC from when Shafik and I were looking at this the ObjC runtime data lists the same offset for all the members of the bitfield: the offset of the first member of the bitfield. Apparently the ObjC runtime doesn't need to know where the individual members are

[Lldb-commits] [PATCH] D83446: [WIP][lldb/Reproducers] Synchronize the command interpreter with asynchronous events

2020-07-09 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. I want to be careful not to conflate lldb's general event behavior with the way the command interpreter happens to run things. For instance, there's no requirement that the debugger be the event listener for processes. You can have a listener per process under the

[Lldb-commits] [PATCH] D83446: [WIP][lldb/Reproducers] Synchronize the command interpreter with asynchronous events

2020-07-09 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D83446#2142350 , @JDevlieghere wrote: > In D83446#2142301 , @jingham wrote: > > > I think the proper way to gate this sort of "race" is to use the process > > state. If the current

[Lldb-commits] [PATCH] D83446: [WIP][lldb/Reproducers] Synchronize the command interpreter with asynchronous events

2020-07-09 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. I think the proper way to gate this sort of "race" is to use the process state. If the current process state is "stopped" then that means we're done with the stop processing and ready to allow commands that require the process to be stopped. When I originally did

[Lldb-commits] [PATCH] D83450: Delegate UpdateChildrenPointerType to the Root ValueObject

2020-07-09 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. I am not selling this as the correct long-term structure for ValueObjects. So far as I can see, in all cases it should be possible to tell a ValueObject where it's going to find both its own store and the storage for pointer children when the ValueObject is created.

[Lldb-commits] [PATCH] D83450: Delegate UpdateChildrenPointerType to the Root ValueObject

2020-07-08 Thread Jim Ingham via Phabricator via lldb-commits
jingham created this revision. jingham added a reviewer: labath. Herald added subscribers: lldb-commits, JDevlieghere. Herald added a project: LLDB. This is a refinement on 96601ec28b7efe5abf3479a1aa91bcedb235bbbd. The intent of that change was to do the same work for the computation of the

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

2020-07-07 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Other than a quibble about using _sp suffix, this is fine. Comment at: lldb/source/Core/ValueObject.cpp:693 + if (!valobj && synthetic_array_member) { +if

[Lldb-commits] [PATCH] D83327: [lldb/Core] Fix incomplete type variable dereferencing crash.

2020-07-07 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D83327#2136814 , @davide wrote: > Aside from cosmetics, I'm not entirely sure this is the correct fix. Why are > we calling this code _at all_ if the type is incomplete? Doing so allows one to write a synthetic child

[Lldb-commits] [PATCH] D83306: [lldb/API] Overwrite variables with SBLaunchInfo::SetEnvironment(append=true)

2020-07-07 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This is fine. If there were a universal convention (e.g. : separated lists) for environment variables, it would be reasonable for "append" behavior to extend rather than replace extant variables, but in the absence of the universality

[Lldb-commits] [PATCH] D81810: LLDB step-instruction gets stuck on jump to self

2020-06-16 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D81810#2095902 , @sirmc wrote: > Yes, I wasn't sure what the exact semantics were for step-inst for LLDB. I > think the issue is not mainly the mimicking of GDB behavior, but rather that > it is inconvenient for some

[Lldb-commits] [PATCH] D81810: LLDB step-instruction gets stuck on jump to self

2020-06-15 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Just to be clear, the lldb -> gdb command map doesn't prescribe behavior for lldb commands. It just suggests the analogous command in gdb. We are still free to implement lldb behavior however seems best to us. The logic in lldb doesn't change the logic of the

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-11 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80112/new/ https://reviews.llvm.org/D80112

[Lldb-commits] [PATCH] D74136: [LLDB] WIP: Follow DW_AT_decl_file when setting breakpoint

2020-06-11 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D74136#2087863 , @jingham wrote: > I need to read this in more detail at this point but I'm caught up in > something else. > > One of the not so great design decisions I made was to try to have "break > set" be a single

[Lldb-commits] [PATCH] D74136: [LLDB] WIP: Follow DW_AT_decl_file when setting breakpoint

2020-06-11 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. I need to read this in more detail at this point but I'm caught up in something else. One of the not so great design decisions I made was to try to have "break set" be a single command governed by flags. The way the command turned out, there are some flags that tell

[Lldb-commits] [PATCH] D81499: [Debugger] Use FileSystem instead of calling llvm::sys::fs::openFileForWrite directly.

2020-06-10 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM then. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81499/new/ https://reviews.llvm.org/D81499 ___

[Lldb-commits] [PATCH] D81499: [Debugger] Use FileSystem instead of calling llvm::sys::fs::openFileForWrite directly.

2020-06-10 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. This looks okay to me though I'm not very familiar with the llvm file system interfaces. Do we have any tests that tests that log output gets emitted to the file requested when you do "log enable -f somefile whatever"? If so and they still work, LGTM. If we don't

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-10 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Thanks, this is looking good. I have a bunch of nits, but nothing substantial. Comment at: lldb/source/Target/Process.cpp:3944 +bool Process::ProcessEventData::ShouldStop(Event *event_ptr, + bool

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-08 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. I thought I was just repeating your original description of the problem in a scenario orchestrated by breakpoint actions. I must have missed something in your description if it's true that the scenario I described doesn't match your initial problem. But regardless,

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-08 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Humm... So you'll have to test the continue behavior instead, which after all was your original issue. That shouldn't be too hard, however. Just make a breakpoint action that calls "thread suspend" on its thread and returns false for should_stop the first time it is

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-08 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D80112#2078574 , @fallkrum wrote: > In D80112#2075097 , @jingham wrote: > > > If I understand the problem you are describing, it is that you suspended a > > thread as a user-level

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-04 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D80112#2075097 , @jingham wrote: > Adding a ShouldStop to that if test doesn't seem right. You should still run > the stop actions even if the thread doesn't want to stop. > > If I understand the problem you are describing,

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-04 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Adding a ShouldStop to that if test doesn't seem right. You should still run the stop actions even if the thread doesn't want to stop. If I understand the problem you are describing, it is that you suspended a thread as a user-level suspend, so it's stop reason got

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-04 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Sorry, now that I'm thinking about this more, I'm confused as to why you are seeing the symptoms you describe. Thread::ShouldStop starts with: if (GetResumeState() == eStateSuspended) { LLDB_LOGF(log, "Thread::%s for tid = 0x%4.4" PRIx64 " 0x%4.4"

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-04 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. It bugs me a little bit that we are doing work masking the stop info when in fact the ShouldStop mechanism is the one that shouldn't be consulting threads that were suspended by the user during the last run. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D80112: Check if thread was suspended during previous stop added.

2020-06-03 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. The one scenario I can think of where this might do the wrong thing is: 1. Hit breakpoint location 1.1 on thread A 2. Switch to thread B 3. Run a function on thread B, has to only run on thread B and has to actually run code in the target, like: (lldb) expr -a 0 --

[Lldb-commits] [PATCH] D80848: [lldb/Bindings] Raise a Runtime error when using SBAddress properties that rely on lldb.target

2020-05-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Looks even better. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80848/new/ https://reviews.llvm.org/D80848 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D80848: [lldb/Bindings] Raise a Runtime error when using SBAddress properties that rely on lldb.target

2020-05-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Ooh, test would be good. Just use it in a python command. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80848/new/ https://reviews.llvm.org/D80848 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D80848: [lldb/Bindings] Raise a Runtime error when using SBAddress properties that rely on lldb.target

2020-05-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. I think "This resolves the SBAddress" is better than "This resolves SBAddress". Other than that LGTM. It would be convenient if you could reuse the _runtime_err_str in the doc strings,

[Lldb-commits] [PATCH] D78801: [LLDB] Add class WasmProcess for WebAssembly debugging

2020-05-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D78801#2062403 , @labath wrote: > In D78801#2050449 , @paolosev wrote: > > > Hello @clayborg , @labath, > > Any thoughts on this latest patch? :-) > > > Sorry about the delay. I think

[Lldb-commits] [PATCH] D74136: [LLDB] WIP: Follow DW_AT_decl_file when setting breakpoint

2020-05-28 Thread Jim Ingham via Phabricator via lldb-commits
jingham added inline comments. Comment at: lldb/source/Breakpoint/BreakpointResolverName.cpp:315 + if (filter_by_cu && filter_by_function) { +// Keep this symbol context if it is a function call to a function +// whose declaration is located in a file that

[Lldb-commits] [PATCH] D80680: save_crashlog should not be using the load_addr property of an SBAddress

2020-05-28 Thread Jim Ingham via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG723a1caa377b: Fix the crashlog.py scripts use of the load_address property. (authored by jingham). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80680/new/

[Lldb-commits] [PATCH] D79929: [lldb] Tab completion for process plugin name

2020-05-28 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D79929#2058939 , @MrHate wrote: > That does make sense. I will split it into `eArgTypeProcessPlugin` and > `eArgTypeDisassemblePlugin` along with the corresponding completion function > names at the time I implement the

[Lldb-commits] [PATCH] D80680: save_crashlog should not be using the load_addr property of an SBAddress

2020-05-27 Thread Jim Ingham via Phabricator via lldb-commits
jingham created this revision. jingham added a reviewer: clayborg. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. friss accepted this revision. friss added a comment. This revision is now accepted and ready to land. Thanks for adding a test! this LGTM The load_addr

[Lldb-commits] [PATCH] D79929: [lldb] Tab completion for process plugin name

2020-05-27 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. This change makes the eArgTypePlugin be the completion for process plugins. Shouldn't we change the enum name and completion name to indicate that? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79929/new/

[Lldb-commits] [PATCH] D79823: [lldb][Core] Remove dead codepath in Mangled

2020-05-26 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Note that there is no guarantee that ObjC names won't be mangled. They aren't on macOS because the macOS linker grew up along with objC so all the weird characters it introduces into the symbol namespace ([,],-,+, ,etc...) are allowed. Way back in the day when

[Lldb-commits] [PATCH] D80350: Handle the case where a thread exits while we were running a function on it

2020-05-21 Thread Jim Ingham via Phabricator via lldb-commits
jingham updated this revision to Diff 265632. jingham added a comment. Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80350/new/ https://reviews.llvm.org/D80350 Files: lldb/include/lldb/lldb-enumerations.h

[Lldb-commits] [PATCH] D80350: Handle the case where a thread exits while we were running a function on it

2020-05-21 Thread Jim Ingham via Phabricator via lldb-commits
jingham marked an inline comment as done. jingham added inline comments. Comment at: lldb/source/Target/Process.cpp:4671 + "the expression was running.", + thread_id); +return eExpressionThreadVanished; aprantl wrote: > Is it useful

[Lldb-commits] [PATCH] D80350: Handle the case where a thread exits while we were running a function on it

2020-05-21 Thread Jim Ingham via Phabricator via lldb-commits
jingham updated this revision to Diff 265608. jingham added a comment. clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80350/new/ https://reviews.llvm.org/D80350 Files: lldb/include/lldb/lldb-enumerations.h

[Lldb-commits] [PATCH] D80165: [lldb/Driver] Fix handling on positional arguments

2020-05-21 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D80165#2048458 , @labath wrote: > In D80165#2047241 , @jingham wrote: > > > In D80165#2045882 , @labath wrote: > > > > > In D80165#2044509

[Lldb-commits] [PATCH] D80350: Handle the case where a thread exits while we were running a function on it

2020-05-20 Thread Jim Ingham via Phabricator via lldb-commits
jingham created this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. It is possible that the thread we are running a function on could exit while we are waiting for the function call to return. We sort of handled that case before, but it really only worked by

[Lldb-commits] [PATCH] D80165: [lldb/Driver] Fix handling on positional arguments

2020-05-20 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D80165#2045882 , @labath wrote: > In D80165#2044509 , @jingham wrote: > > > We should make sure if we do exit that we don't output any other text that > > would obscure the error

[Lldb-commits] [PATCH] D80159: [lldb/Properties] Move OSPluginReportsAllThreads from Target to Process

2020-05-19 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Excellent! Thanks for doing this. Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80159/new/ https://reviews.llvm.org/D80159

[Lldb-commits] [PATCH] D80165: [lldb/Driver] Fix handling on positional arguments

2020-05-19 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. I agree with Pavel, reporting an error and exiting seems like a good behavior. When I mistype a command line option, I generally immediately quit lldb, up-arrow and fix and resubmit, so this would save keystrokes. We should make sure if we do exit that we don't

[Lldb-commits] [PATCH] D80165: [lldb/Driver] Fix handling on positional arguments

2020-05-18 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. LGTM. These were useful behaviors, thanks for restoring them! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80165/new/ https://reviews.llvm.org/D80165

[Lldb-commits] [PATCH] D80165: [lldb/Driver] Fix handling on positional arguments

2020-05-18 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Not sure why you need to track "beginning and end". Does libOption not pull out all the options (and their option values if they have them) before handing you the command line? If not, then this probably is not worth doing. But if it does, this should be

[Lldb-commits] [PATCH] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

2020-05-14 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. For expression evaluation in a REPL (which we only have for Swift, but if somebody wants to make such a thing for C++ we wouldn't be displeased) the model is compiling into a single translation unit. I think that's the only thing that makes sense. You don't want to

[Lldb-commits] [PATCH] D79789: [lldb] Don't dissasemble large functions by default

2020-05-13 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. If you have --force, then the address range is informational, in which case having the full command would be more confusing than anything. So this is fine, thanks for adding it. I thought briefly about whether we should also add a

[Lldb-commits] [PATCH] D79789: [lldb] Don't dissasemble large functions by default

2020-05-12 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. Instead of just printing out the range, can we print out the disassemble command you would run to actually disassemble the range? Also, I think we should add a --force option since if you were using this in a script you wouldn't get a chance to respond to the error.

[Lldb-commits] [PATCH] D79666: Complete breakpoint enable/disable/delete/modify with a list of breakpoint IDs

2020-05-11 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. This is great, but you can also specify breakpoints by name. Should be possible to also complete on the list of breakpoint names. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79666/new/ https://reviews.llvm.org/D79666

[Lldb-commits] [PATCH] D79659: [lldb/Commands] Add ability to run shell command on the host.

2020-05-08 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. We certainly shouldn't duplicate the code for CommandObjectPlatformShell. Why don't you just add a "m_use_host_platform" ivar to CommandObjectPlatformShell, and make two instances of CommandObjectPlatformShell, one with m_use_host_platform set to false and added as a

[Lldb-commits] [PATCH] D79659: [lldb/Commands] Add ability to run shell command on the host.

2020-05-08 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. I do something similar with CommandObjectThreadStepWithTypeAndScope so that I can share most of that command code, if you want to see an example of doing that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79659/new/

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-07 Thread Jim Ingham via Phabricator via lldb-commits
jingham added inline comments. Comment at: lldb/source/Core/ValueObject.cpp:2846 + if (llvm::isa(compiler_type.GetTypeSystem())) { +if (HasSyntheticValue()) { + TargetSP target_sp = GetTargetSP(); mib wrote: > friss wrote: > > I am

[Lldb-commits] [PATCH] D79554: [lldb/Dataformatter] Add support to CF{Dictionary, Set}Ref types

2020-05-07 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. The way the ValueObject code works w.r.t. Synthetic child providers is that you first look up and make a ValueObject for the actual value the user requested, (for instance a ValueObjectVariable or a ValueObjectChild or a ValueObjectConstResult for expressions, etc.),

[Lldb-commits] [PATCH] D79308: [lldb-server] Reset stop reason of all threads when resuming

2020-05-07 Thread Jim Ingham via Phabricator via lldb-commits
jingham accepted this revision. jingham added a comment. LGTM too! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79308/new/ https://reviews.llvm.org/D79308 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

2020-04-30 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D78972#2012796 , @labath wrote: > In D78972#2011571 , @jingham wrote: > > > That one does work all the way, including calling the function. That > > should be surprising. It

[Lldb-commits] [PATCH] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

2020-04-29 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D78972#2010033 , @labath wrote: > In D78972#2008129 , @jingham wrote: > > > In D78972#2007493 , @labath wrote: > > > > > > I don't have a way to

[Lldb-commits] [PATCH] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

2020-04-28 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment. In D78972#2007493 , @labath wrote: > > I don't have a way to write a C-based test for this as I don't know how to > > craft a user expression in C that will make such a thing. I asked around a > > bit and nobody had an easy way

[Lldb-commits] [PATCH] D78972: Treat hasWeakODRLinkage symbols as external in REPL computations

2020-04-27 Thread Jim Ingham via Phabricator via lldb-commits
jingham created this revision. jingham added a reviewer: JDevlieghere. jingham added a project: LLDB. Herald added a subscriber: lldb-commits. The way both the REPL and the --top-level mode of the expression parser work is that they compile and JIT the code the user provided, and then look

  1   2   3   4   5   6   7   8   9   10   >