[Lldb-commits] [PATCH] D121030: [LLDB][NativePDB] Don't complete static members' types when completing a record type.

labath added a comment. This is cool, just one question about the test. Comment at: lldb/test/Shell/SymbolFile/NativePDB/local-variables.cpp:8-26 +class B; +class A { +public: +static const A constA; +static A a; +static B b; +int val = 1;

[Lldb-commits] [PATCH] D121443: [lldb] Add a getter for the process' system architecture

labath added inline comments. Comment at: lldb/source/Target/Process.cpp:2654 +ArchSpec Process::GetSystemArchitecture() { + return HostInfo::GetArchitecture(); +} This is only a good default for local process plugins. Since we have just one (windows) of those

[Lldb-commits] [PATCH] D120972: [lldb] Show progress events in the command line driver

labath added a comment. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120972/new/ https://reviews.llvm.org/D120972 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D121442: [lldb] Don't overwrite the host arch (from qHostInfo) with the process arch (from qProcessInfo)

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. sounds like a good idea to me CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121442/new/ https://reviews.llvm.org/D121442 ___ lldb-commits

[Lldb-commits] [PATCH] D121305: [lldb/gdb-remote] Remove ancient debugserver workaround

This revision was automatically updated to reflect the committed changes. Closed by commit rGed1a83befe65: [lldb/gdb-remote] Remove ancient debugserver workaround (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121305/new/

[Lldb-commits] [PATCH] D121348: Don't try to get memory returns for ABIMacOSX_arm64

labath added a comment. I don't think this is a peculiarity of the darwin ABI. I'm pretty sure i've ran into this on the "sysv" ABI as well. If we're going to disallow this, then I think we should do the same for ABISysV_arm64 as well -- it looks like an identical change to ABISysV_arm64

[Lldb-commits] [PATCH] D119508: [LLDB][NativePDB] Add support for S_DEFRANGE_REGISTER and S_DEFRANGE_SUBFIELD_REGISTER

labath accepted this revision. labath added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Symbol/Variable.cpp:251 +GetScopeRange().IsEmpty() || GetScopeRange().FindEntryThatContains( +

[Lldb-commits] [PATCH] D120972: [lldb] Show progress events in the command line driver

labath added a comment. In D120972#3370850 , @jingham wrote: > But using the standard event loop and "I want to handle async notifications > by hand" seem like they should be separable choices. Is the `show-progress` setting not enough of separation?

[Lldb-commits] [PATCH] D120972: [lldb] Show progress events in the command line driver

labath added inline comments. Comment at: lldb/source/Core/Debugger.cpp:1624 + if (!m_broadcaster.EventTypeHasListeners(Debugger::eBroadcastBitProgress)) { +listener_sp->StartListeningForEvents(_broadcaster, JDevlieghere wrote: > labath wrote: > > labath

[Lldb-commits] [PATCH] D121305: [lldb/gdb-remote] Remove ancient debugserver workaround

labath added a comment. In D121305#3370337 , @JDevlieghere wrote: > Assuming this isn't relevant anymore I was hoping you would be able to tell me that. :P I mean, it keys off of `GetThreadSuffixSupported() == false`, which means it must catch only

[Lldb-commits] [PATCH] D121305: [lldb/gdb-remote] Remove ancient debugserver workaround

labath added a comment. Phabricator didn't turn the commit reference into a link as I'd hoped, so here is it now: rG43c555dfcd76004d56ba03fd33ebbdba09f58d47 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D121305: [lldb/gdb-remote] Remove ancient debugserver workaround

labath created this revision. labath added reviewers: jingham, JDevlieghere, jasonmolenda. Herald added a subscriber: mgorny. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB. This workaround is the source of an awkwared Process->Platform

[Lldb-commits] [PATCH] D120810: [lldb] Remove the global platform list

labath added inline comments. Comment at: lldb/source/Plugins/Platform/gdb-server/CMakeLists.txt:10 lldbPluginProcessUtility +lldbPluginProcessGDBRemote ) thakis wrote: > It doesn't seem like lldb cares about this kind of thing, but this adds a >

[Lldb-commits] [PATCH] D120810: [lldb] Remove the global platform list

This revision was automatically updated to reflect the committed changes. Closed by commit rGffb9429b6f3c: [lldb] Remove the global platform list (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120810/new/

[Lldb-commits] [PATCH] D120810: [lldb] Remove the global platform list

labath added inline comments. Comment at: lldb/source/Target/Platform.cpp:1891 + +PlatformSP PlatformList::GetOrCreate(const ArchSpec , + ArchSpec *platform_arch_ptr, JDevlieghere wrote: > [Nit/pedantic] We have a few calls to

[Lldb-commits] [PATCH] D121290: [lldb] Create an enum to specify the kind of ArchSpec matching

labath created this revision. labath added a reviewer: JDevlieghere. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB. s/true/ArchSpec::ExactMatch s/false/ArchSpec::CompatibleMatch Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D120803: [lldb] Don't print *trailing* nuls in char arrays

This revision was automatically updated to reflect the committed changes. Closed by commit rGacf77bd2fd90: [lldb] Dont print *trailing* nuls in char arrays (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120803/new/

[Lldb-commits] [PATCH] D120320: [lldb/driver] Fix SIGTSTP handling

This revision was automatically updated to reflect the committed changes. Closed by commit rG4bcadd66864b: [lldb/driver] Fix SIGTSTP handling (authored by labath). Changed prior to commit: https://reviews.llvm.org/D120320?vs=412643=414078#toc Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [PATCH] D120892: [lldb] Warn when we fail to find dwo/dwp files

This revision was automatically updated to reflect the committed changes. Closed by commit rG8f6ee17f22a7: [lldb] Warn when we fail to find dwo/dwp files (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120892/new/

[Lldb-commits] [PATCH] D120972: [lldb] Show progress events in the command line driver

labath added inline comments. Comment at: lldb/source/Core/Debugger.cpp:1624 + if (!m_broadcaster.EventTypeHasListeners(Debugger::eBroadcastBitProgress)) { +listener_sp->StartListeningForEvents(_broadcaster, labath wrote: > Why is this conditional on

[Lldb-commits] [PATCH] D120972: [lldb] Show progress events in the command line driver

labath added inline comments. Comment at: lldb/source/Core/Debugger.cpp:1624 + if (!m_broadcaster.EventTypeHasListeners(Debugger::eBroadcastBitProgress)) { +listener_sp->StartListeningForEvents(_broadcaster, Why is this conditional on someone else

[Lldb-commits] [PATCH] D121161: [lldb] Avoid global constructor in LLDBLog.cpp

labath added a comment. Amusingly enough, the problem is the custom operator| used for bitmask enums. D121281 ought to fix that (by making the function constexpr). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121161/new/

[Lldb-commits] [PATCH] D121161: [lldb] Avoid global constructor in LLDBLog.cpp

labath added a comment. The class has a constexpr constructor. I thought that would be enough to avoid runtime initialization. Is this being flagged by something? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121161/new/ https://reviews.llvm.org/D121161

[Lldb-commits] [PATCH] D119797: Fix race condition when launching and attaching.

labath added a comment. It worked. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119797/new/ https://reviews.llvm.org/D119797 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D120755: Fix race condition when launching and attaching.This is a modified version of a previous patch that was reverted: https://reviews.llvm.org/D119797This version only wait

labath added a comment. Greg, this is still causing the TestVSCode_attach.py test to fail on lldb-x86_64-debian (it would probably fail elsewhere as well, but the test is already disabled everywhere else). Judging by the error message

[Lldb-commits] [PATCH] D120972: [lldb] Show progress events in the command line driver

labath added a comment. I like the feature, but why does it need to have a thread of it's own? Could this be done from inside the regular event handler thread? The event handler thread also prints to stdout, and it seems like a bad idea to have two threads trying to do the same... CHANGES

[Lldb-commits] [PATCH] D120892: [lldb] Warn when we fail to find dwo/dwp files

labath added a comment. In D120892#3357780 , @JDevlieghere wrote: > Might a `llvm::once_flag` + `llvm::call_once` be simpler? Simpler... maybe.. depends how you look at it. But, synchronization-wise, it is overkill. once_flag ensures that none of the

[Lldb-commits] [PATCH] D120892: [lldb] Warn when we fail to find dwo/dwp files

labath added a comment. In D120892#3357424 , @JDevlieghere wrote: > FWIW I'm planning some changes to the error reporting "soonish". The gist is > using error/warning events which Xcode (or VSCode) can subscribe to. I'll > send out an RFC with more

[Lldb-commits] [PATCH] D120892: [lldb] Warn when we fail to find dwo/dwp files

labath updated this revision to Diff 412774. labath added a comment. This code can be invoked concurrently (during indexing). Use atomic test-and-set to ensure correctness. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120892/new/

[Lldb-commits] [PATCH] D120892: [lldb] Warn when we fail to find dwo/dwp files

labath created this revision. labath added a reviewer: JDevlieghere. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB. This ensures that the user is aware that many commands will not work correctly. We print the warning only once (per module) to

[Lldb-commits] [PATCH] D120320: [lldb/driver] Fix SIGTSTP handling

labath updated this revision to Diff 412643. labath marked 3 inline comments as done. labath added a comment. Address mgorny's comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120320/new/ https://reviews.llvm.org/D120320 Files:

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

labath added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp:19 using namespace lldb; +using namespace dwarf; JDevlieghere wrote: > shafik wrote: > > JDevlieghere wrote: > > > Wouldn't it be more consistent to use

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

labath added a comment. In D120836#3355245 , @shafik wrote: > In D120836#3355167 , @labath wrote: > >> I think it's reasonable to be able to refer to the dwarf constants from >> within the dwarf plugin via their

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good. Thanks for the test. Comment at: lldb/test/API/iohandler/stdio/TestIOHandlerProcessSTDIO.py:16 +self.build() +

[Lldb-commits] [PATCH] D120836: [LLDB] Remove cases of using namespace llvm:: from header file

labath added a comment. I think it's reasonable to be able to refer to the dwarf constants from within the dwarf plugin via their base names alone. The implementation -- a top-level `using namespace llvm::dwarf` -- is not reasonable, but that's because the plugin is very old, and completely

[Lldb-commits] [PATCH] D120810: [lldb] Remove the global platform list

labath created this revision. labath added reviewers: JDevlieghere, clayborg, jingham. Herald added subscribers: mgorny, emaste. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB. This patch moves the platform creation and selection logic into the

[Lldb-commits] [PATCH] D119146: [lldb/Platform] Prepare decouple instance and plugin names

This revision was automatically updated to reflect the committed changes. Closed by commit rGd2edca6276d1: [lldb/Platform] Prepare decouple instance and plugin names (authored by labath). Herald added a project: All. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D120803: [lldb] Don't print *trailing* nuls in char arrays

labath created this revision. labath added reviewers: kastiglione, teemperor, jingham. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB. Embedded nul characters are still printed, and they don't terminate the string. See also D111634

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

labath added a comment. In D120762#3353686 , @JDevlieghere wrote: > In D120762#3353655 , @labath wrote: > >> Are you sure that we're still sending input to the process (I'm not sure how >> much test coverage

[Lldb-commits] [PATCH] D120762: [lldb] Avoid data race in IOHandlerProcessSTDIO

labath added a comment. This seems like a better approach, but I'm not sure how does it actually work (see inline comment). Are you sure that we're still sending input to the process (I'm not sure how much test coverage for this do we have)? Comment at:

[Lldb-commits] [PATCH] D120755: Fix race condition when launching and attaching.This is a modified version of a previous patch that was reverted: https://reviews.llvm.org/D119797This version only wait

labath added a comment. Herald added a project: All. I'm not exactly exactly thrilled by the sleep-based implementation, but otherwise, the patch seems fine. Comment at: lldb/tools/lldb-vscode/VSCode.cpp:551 + auto timeout_time = +

[Lldb-commits] [PATCH] D120718: Qualify DataExtractor with lldb_private

labath added a comment. In D120718#3352635 , @shafik wrote: > In D120718#3352450 , @JDevlieghere > wrote: > >> I think the better solution here is to get rid of the `using namespace >> llvm;` in the

[Lldb-commits] [PATCH] D120762: [lldb] Protect control variables in IOHandler with a mutex to avoid a data race

labath added a comment. These kinds of changes rarely fix a bug -- usually they just change a (detectable) data race into some nondeterministic runtime behavior. That's because, even though e.g. `IsActive` can compute its result in a race-free manner, there's nothing preventing someone from

[Lldb-commits] [PATCH] D117928: [lldb] Disable tests for x86 that uses write command on XMM registers

labath added a comment. In D117928#3349234 , @ljmf00 wrote: > Meanwhile, a commit will land Linux stable branches soon to fix this issue: > https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=44cad52cc14ae10062f142ec16ede489bccf4469 >

[Lldb-commits] [PATCH] D120595: [WIP][trace][intelpt] Add TSC to Nanosecond conversion for IntelPT traces

labath added inline comments. Comment at: lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h:74 +struct TraceIntelPTGetStateResponse : TraceGetStateResponse { + /// `nullptr` if no tsc conversion rate exists. + llvm::Optional tsc_rate; wallace wrote: >

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good, I'm sorry this took so long. Jim, do you have anything to add? Comment at: lldb/source/Symbol/Variable.cpp:439-440 +bool Variable::DumpLocations(Stream *s,

[Lldb-commits] [PATCH] D120578: [lldb] Implement ObjectFile::GetCStrFromSection

labath added a comment. I don't think this implementation is sound. If you look at the implementation of `ReadSectionData` you'll see that there is a lot more involved in reading data from a section than just incrementing a pointer. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[Lldb-commits] [PATCH] D120284: [lldb/test] Fix TestProgressReporting.py race issue with the event listener

labath added a comment. In D120284#3345994 , @JDevlieghere wrote: >> I'm not entirely sure what's the best fix here. @JDevlieghere, what do you >> think? Can we just remove the output arguments from the LLDB_INSTRUMENT_VA >> invocation (given how

[Lldb-commits] [PATCH] D120284: [lldb/test] Fix TestProgressReporting.py race issue with the event listener

labath added a comment. The backtrace is pretty much useless, but what is happening is that the LLDB_INSTRUMENT_VA macro inside SBDebugger::GetProgressFromEvent is trying to log the uninitialized output variable, which angers the gods of sanitation. I'm not entirely sure what's the best fix

[Lldb-commits] [PATCH] D119146: [lldb/Platform] Decouple instance and plugin names

labath updated this revision to Diff 411366. labath added a comment. Reduce the scope of the patch to GetName interface changes. Given the direction of the discourse thread, it seems unlikely that we will be able to determine the instance name at object creation time. We will likely need a

[Lldb-commits] [PATCH] D120320: [lldb/driver] Fix SIGTSTP handling

labath added inline comments. Comment at: lldb/tools/driver/Driver.cpp:673-674 +static void sigtstp_handler(int signo) { if (g_driver != nullptr) g_driver->GetDebugger().SaveInputTerminalState(); JDevlieghere wrote: > andcarminati wrote: > >

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

labath added inline comments. Comment at: lldb/source/Core/Address.cpp:739 - s->PutCString(", location = "); - var_sp->DumpLocationForAddress(s, *this); - s->PutCString(", decl = "); zequanwu wrote: > labath wrote: > > This

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

labath added inline comments. Comment at: lldb/source/Core/Address.cpp:739 - s->PutCString(", location = "); - var_sp->DumpLocationForAddress(s, *this); - s->PutCString(", decl = "); This place was the only caller of

[Lldb-commits] [PATCH] D120319: Set error message if ValueObjectRegister fails to write back to register

labath added a comment. So, it seems we were all correct. The mach-o core file implementation does permit writing registers, while the elf one doesn't. I guess we never got that feature request for elf. I don't think that's a blocker for writing this test though. If we decide to support

[Lldb-commits] [PATCH] D120320: [lldb/driver] Fix SIGTSTP handling

labath added inline comments. Comment at: lldb/tools/driver/Driver.cpp:673-674 +static void sigtstp_handler(int signo) { if (g_driver != nullptr) g_driver->GetDebugger().SaveInputTerminalState(); JDevlieghere wrote: > I see an opportunity for a little

[Lldb-commits] [PATCH] D120425: [lldb/host] Remove monitor_signals argument from process monitoring functions

This revision was automatically updated to reflect the committed changes. Closed by commit rG12c9c4a88537: [lldb/host] Remove monitor_signals argument from process monitoring functions (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D120425: [lldb/host] Remove monitor_signals argument from process monitoring functions

labath created this revision. labath added a reviewer: JDevlieghere. Herald added a subscriber: krytarowski. labath requested review of this revision. Herald added a project: LLDB. All current callers set the argument to false. monitor_signals=true used to be used in the Process plugins (which

[Lldb-commits] [PATCH] D120320: [lldb/driver] Fix SIGTSTP handling

labath marked 4 inline comments as done. labath added a comment. The behavior of the stop signals (SIGTTIN, SIGTTOU, SIGTSTP) is this: - if the process has a signal handler, then the kernel picks an arbitrary thread (which does not have the signal blocked) to handle it. As far as the kernel is

[Lldb-commits] [PATCH] D120319: Set error message if ValueObjectRegister fails to write back to register

labath added a comment. Core files have read-only registers. Would that help? (you could just throw in a SetValueFromCString call into one of the existing core file tests) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120319/new/

[Lldb-commits] [PATCH] D120320: [lldb/driver] Fix SIGTSTP handling

labath marked 4 inline comments as done. labath added inline comments. Comment at: lldb/test/API/driver/job_control/TestJobControl.py:9 +from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil +from lldbsuite.test.lldbpexpect import PExpectTest

[Lldb-commits] [PATCH] D120320: [lldb/driver] Fix SIGTSTP handling

labath updated this revision to Diff 410800. labath added a comment. Add comments, delete unused code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120320/new/ https://reviews.llvm.org/D120320 Files:

[Lldb-commits] [PATCH] D120322: [lldb] Simplify HostThreadMacOSX

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGf4568e12219f: [lldb] Simplify HostThreadMacOSX (authored by labath). Changed prior to commit:

[Lldb-commits] [PATCH] D120321: [lldb] Modernize ThreadLauncher

This revision was automatically updated to reflect the committed changes. Closed by commit rGd0810779b1f3: [lldb] Modernize ThreadLauncher (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120321/new/

[Lldb-commits] [PATCH] D120321: [lldb] Modernize ThreadLauncher

labath added inline comments. Comment at: lldb/source/Host/common/HostNativeThreadBase.cpp:55-56 HostNativeThreadBase::ThreadCreateTrampoline(lldb::thread_arg_t arg) { - ThreadLauncher::HostThreadCreateInfo *info = - (ThreadLauncher::HostThreadCreateInfo *)arg; -

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

labath added a comment. I'm not sure about `ModuleLookupShowRange::Single` option. It feels like it just complicates things. Did anyone request that? FWIW, I don't think that dumping a /single/ range is particularly verbose, so I could imagine even doing that by default. I think it should be

[Lldb-commits] [PATCH] D120322: [lldb] Simplify HostThreadMacOSX

labath created this revision. labath added reviewers: JDevlieghere, jingham. labath requested review of this revision. Herald added a project: LLDB. The class is using an incredibly elaborate setup to create and destroy an NSAutoreleasePool object. We can do it in a much simpler way by making

[Lldb-commits] [PATCH] D120321: [lldb] Modernize ThreadLauncher

labath created this revision. labath added a reviewer: JDevlieghere. Herald added a subscriber: mgorny. labath requested review of this revision. Herald added a project: LLDB. Accept a function object instead of a raw pointer. This avoids a bunch of boilerplate typically needed to pass arguments

[Lldb-commits] [PATCH] D120320: [lldb/driver] Fix SIGTSTP handling

labath created this revision. labath added reviewers: mgorny, DavidSpickett. labath requested review of this revision. Herald added a project: LLDB. Our SIGTSTP handler was working, but that was mostly accidental. The reason it worked is because lldb is multithreaded for most of its lifetime and

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

labath added a comment. Committed. Thanks for the great patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119548/new/ https://reviews.llvm.org/D119548 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

This revision was automatically updated to reflect the committed changes. Closed by commit rGa2c267e0c9d9: [lldb] Fix race condition between lldb-vscode and stop hooks executor (authored by ilya-nozhkin, committed by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D120100: [lldb/bindings] Expose the progress reporting machinery to the SWIG interface

labath added inline comments. Comment at: lldb/bindings/interface/SBDebugger.i:126-129 +%apply uint64_t& INOUT { uint64_t& progress_id }; +%apply uint64_t& INOUT { uint64_t& completed }; +%apply uint64_t& INOUT { uint64_t& total }; +%apply bool& INOUT { bool&

[Lldb-commits] [PATCH] D120100: [lldb/bindings] Expose the progress reporting machinery to the SWIG interface

labath added inline comments. Comment at: lldb/bindings/interface/SBDebugger.i:126-129 +%apply uint64_t& INOUT { uint64_t& progress_id }; +%apply uint64_t& INOUT { uint64_t& completed }; +%apply uint64_t& INOUT { uint64_t& total }; +%apply bool& INOUT { bool&

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

labath added a comment. Do you need someone to commit this for you? (I can probably do that tomorrow) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119548/new/ https://reviews.llvm.org/D119548 ___

[Lldb-commits] [PATCH] D120100: [lldb/bindings] Expose the progress reporting machinery to the SWIG interface

labath added a comment. In D120100#3332433 , @JDevlieghere wrote: > In D120100#3331656 , @labath wrote: > >> I'm not sure how we ended up with the .i files in the first place, but maybe >> a good solution to

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

labath added inline comments. Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s:28-29 # CHECK: Variable{{.*}}, name = "x0", {{.*}}, scope = parameter, location = # CHECK-NEXT: [0x, 0x0001): DW_OP_reg5 RDI # CHECK-NEXT:

[Lldb-commits] [PATCH] D119831: [lldb] Add support for a "global" lldbinit file

This revision was automatically updated to reflect the committed changes. Closed by commit rG6c99a3469d9c: [lldb] Add support for a global lldbinit file (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119831/new/

[Lldb-commits] [PATCH] D120100: [lldb] Expose eBroadcastBitProgress to the SWIG SBAPI

labath added a comment. I'm not sure how we ended up with the .i files in the first place, but maybe a good solution to this problem would be to ditch those and generate bindings from .h files directly. It may mean that we need to put `#ifndef SWIG` around some code, but I'd say that beats

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

labath added a comment. In D119963#3329973 , @jingham wrote: > There was a question on the dev list a while ago about to print out all the > valid ranges of a variable. That's a useful bit of info if you're trying to > figure out where you could break

[Lldb-commits] [PATCH] D111634: [lldb] Print embedded nuls in char arrays (PR44649)

labath added a comment. Yeah, I suppose that makes sense. Like, I don't think we can ever have a single solution that does exactly what people would expect in every possible situation. The only counterargument I can think of is "but gdb does it that way", but I don't think that's a

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

labath added a comment. In D119548#3330773 , @ilya-nozhkin wrote: > In D119548#3330226 , @clayborg > wrote: > >> Try out the https://reviews.llvm.org/D119797 patch and see if this fixes >> this issue? I have

[Lldb-commits] [PATCH] D120101: [lldb] Fix (unintentional) recursion in CommandObjectRegexCommand

labath added a comment. Wouldn't want to miss a good bikeshed. Logarithms are cool, but I think something like this (untested) snippet would be simpler Input.split(Parts, '%'); Result << Parts[0]; for (llvm::StringRef Part : drop_begin(Parts)) { if (!Part.consumeInteger(10, Idx) &&

[Lldb-commits] [PATCH] D119797: Fix race condition when launching and attaching.

labath added a comment. There's also a timeout in TestVSCode_coreFile.py Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119797/new/ https://reviews.llvm.org/D119797 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D119797: Fix race condition when launching and attaching.

labath added a comment. TestVSCode_attach.py is still failing. Are you able to fix that quickly or shall we revert? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119797/new/

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

labath added inline comments. Comment at: lldb/test/Shell/SymbolFile/DWARF/x86/debug_loc.s:28-29 # CHECK: Variable{{.*}}, name = "x0", {{.*}}, scope = parameter, location = # CHECK-NEXT: [0x, 0x0001): DW_OP_reg5 RDI # CHECK-NEXT:

[Lldb-commits] [PATCH] D119831: [lldb] Add support for a "global" lldbinit file

labath added a comment. In D119831#3329894 , @JDevlieghere wrote: > Specifying a directory instead of a file makes sense. Why omit the leading > dot though? I can think of a few reasons (you can specify a subdir unlike > home, it's probably not

[Lldb-commits] [PATCH] D119963: [LLDB] Dump valid ranges of variables

labath added a comment. I suppose one could ask the question whether this should be printing the entire set range of ranges, or just the one Comment at: lldb/source/Commands/Options.td:960 "target modules.">; + def target_modules_lookup_variables_ranges :

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

labath added a comment. In D119548#3327751 , @jingham wrote: > I don't think CreateProcess needs a HijackListener, does it? It doesn't > generate events, it just make the connection to the server. It is always > followed by Launch or DebugProcess,

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

labath added a comment. Doing it in the common code is a great idea, and it will make the code more robust. However, instead of "sneaking" the listener through a member variable, it would be better to pass it through function arguments instead (normally in the LaunchInfo struct, and as a

[Lldb-commits] [PATCH] D119831: [lldb] Add support for a "global" lldbinit file

labath updated this revision to Diff 409197. labath marked 4 inline comments as done. labath edited the summary of this revision. labath added a comment. And add the extra blank line. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119831/new/

[Lldb-commits] [PATCH] D119831: [lldb] Add support for a "system-wide" lldbinit file

labath updated this revision to Diff 409196. labath added a comment. Use the term "global" and rename functions to make it more consistent with existing code. It's worth noting that (as a part of the consistency), I have changed the cmake variable to point to a directory instead of a file. This

[Lldb-commits] [PATCH] D119831: [lldb] Add support for a "system-wide" lldbinit file

labath added a comment. In D119831#3324830 , @clayborg wrote: > This looks good to me. Just a few things to possibly think about: > > - Maybe we would want addition system init files for different workflows and > then we would start lldb with a new

[Lldb-commits] [PATCH] D119857: [lldb] Don't rely on unsigned integer wrapping in PutRawBytes and PutBytesAsRawHex8

labath added a comment. Another option would be to go full-STL and do something like ArrayRef data(ptr, len); if (reverse) for (uint8_t byte: llvm::reverse(data)) ... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119857/new/

[Lldb-commits] [PATCH] D119915: Replace use of double underscore in identifiers

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Yeah, we shouldn't be using these. Personally, I think the names are now unnecessarily long, and I'd go with something shorter. E.g. replacing the double underscore (btw, in python land they

[Lldb-commits] [PATCH] D119831: [lldb] Add support for a "system-wide" lldbinit file

labath created this revision. labath added reviewers: JDevlieghere, jingham, clayborg, wallace. Herald added a subscriber: mgorny. labath requested review of this revision. Herald added a project: LLDB. This patch adds introduces a new kind of an lldbinit file. Unlike the lldbinit in the home

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

labath added a comment. In D119548#3321397 , @ilya-nozhkin wrote: > In D119548#3321357 , @jingham wrote: > >> SBTarget.Launch calls Target::Launch. That sets up a hijacker for the "stop >> at the first

[Lldb-commits] [PATCH] D119797: Fix race condition when launching and attaching.

labath added a comment. Instead of polling, would it be possible for the event handler thread to send a notification (through a condition variable, std::future, etc.), and have the main thread wait for that? But in general, avoiding flipping the modes back and forth seems like a good idea.

[Lldb-commits] [PATCH] D119046: Add a repeat command option for "thread backtrace --count N".

labath added a comment. I've fixed the synchronization code in f8d42c55ec6e9 . Based on my experiments, lldb reports still reports a stop reason for suspended threads (regardless of the platform), so the code should expect

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

labath added a comment. Ah, I'm sorry. It seems I misunderstood the nature of the problem. Let me try again. In your description of the problem, I think the problematic part is the sequencing of steps 5 and 6. You're saying that the main thread returns from a "synchronous" launch operation

[Lldb-commits] [PATCH] D119548: [lldb] Fix race condition between lldb-vscode and stop hooks executor

labath added a comment. I'm not sure this is fixing the problem at the right level. I mean, even if you fix the race on the synchronicity flag, it still seems like there is a zillion of other things that the two processes (event handler thread and stop hook executor) race on. Isn't the right

[Lldb-commits] [PATCH] D119616: [lldb] Replace asserts on .Success() with assertSuccess()

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Thanks for doing this. > assertSuccess seems not to be well known That's probably because it was introduced only recently (for all the reasons that you mention). Repository: rG LLVM

<    3   4   5   6   7   8   9   10   11   12   >