[Lldb-commits] [PATCH] D131081: [lldb] Prevent race condition when fetching /proc/cpuinfo

2022-08-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/source/Plugins/Process/Linux/Procfs.cpp:22 Expected> lldb_private::process_linux::GetProcfsCpuInfo() { static Optional> cpu_info; + static Optional error; How about storing this as `ErrorOr>` (that's what

[Lldb-commits] [PATCH] D131075: [lldb] [gdb-remote] Send interrupt packets from async thread

2022-08-03 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good. Please pay special attention to the windows bot when landing. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:55-63

[Lldb-commits] [PATCH] D126614: [lldb] [gdb-remote] Client support for using the non-stop protocol

2022-08-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:138 +// and then drain the notification queue +// TODO: issue vCont;t to ensure that all threads have actually stopped +// (this is not needed for LLGS but for

[Lldb-commits] [PATCH] D130985: [lldb] Fix TestDeletedExecutable on linux

2022-08-03 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG69c39e2abc31: [lldb] Fix TestDeletedExecutable on linux (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130985/new/

[Lldb-commits] [PATCH] D126614: [lldb] [gdb-remote] Client support for using the non-stop protocol

2022-08-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Ok, I think I like this, but since this is pretty finicky code, could I ask you to split it up into smaller patches? The first could be just the interruption refactor. The second could contain all the `m_notification_packet_queue` and `ReadPacket` stuff (I think this

[Lldb-commits] [PATCH] D130942: [LLDB][DWARF] Set MSInheritanceAttr for record decl when it's using Microsoft compatible ABI.

2022-08-03 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Makes sense to me. For my education, what is the effect of not providing a specific inheritance attribute, like it's being done in the pdb code (which, I presume, is not possible because that

[Lldb-commits] [PATCH] D130796: [LLDB][NativePDB] Switch to use DWARFLocationList.

2022-08-03 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D130796#3692243 , @zequanwu wrote: > In D130796#3691227 , @zequanwu > wrote: > >>> If you find yourself needing to do extra work to work its limitations, we >>> should fix that

[Lldb-commits] [PATCH] D130803: [lldb] Allow SymbolTable regex search functions to match mangled name

2022-08-03 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. In D130803#3691363 , @augusto2112 wrote: > In D130803#3690754 , @labath wrote: > >> Who sets this flag? How do you intend to use it? > > I need this change

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

2022-08-02 Thread Pavel Labath 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 rG6093a77caf44: [lldb] Create an enum to specify the kind of ArchSpec matching (authored by labath). Changed prior to commit:

[Lldb-commits] [PATCH] D130985: [lldb] Fix TestDeletedExecutable on linux

2022-08-02 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 449282. labath added a comment. rename the size variable Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130985/new/ https://reviews.llvm.org/D130985 Files:

[Lldb-commits] [PATCH] D130985: [lldb] Fix TestDeletedExecutable on linux

2022-08-02 Thread Pavel Labath via Phabricator via lldb-commits
labath marked an inline comment as done. labath added a comment. In D130985#3693453 , @DavidSpickett wrote: > Tell me if I understand the strategy. > > If you're on arm64 but the process is actually arm32 and you ask for the > general registers, you'll

[Lldb-commits] [PATCH] D130985: [lldb] Fix TestDeletedExecutable on linux

2022-08-02 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added a reviewer: DavidSpickett. Herald added subscribers: JDevlieghere, kbarton, nemanjai. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB. Currently, lldb-server was opening the executable file to determine

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

2022-08-02 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/source/Core/Module.cpp:740 + bool user_provided_name_is_mangled = + Mangled::GetManglingScheme(m_name.GetStringRef()) != + Mangled::eManglingSchemeNone; bulbazord wrote: > labath wrote: > > labath wrote:

[Lldb-commits] [PATCH] D128989: [lldb] [llgs] Support resuming multiple processes via vCont w/ nonstop

2022-08-01 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good modulo the comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1786 +if (pid ==

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

2022-08-01 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/source/Core/Module.cpp:740 + bool user_provided_name_is_mangled = + Mangled::GetManglingScheme(m_name.GetStringRef()) != + Mangled::eManglingSchemeNone; labath wrote: > I think this is overly aggressive.

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

2022-08-01 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I like how you've extracted the name construction functionality. Just one (inline) comment about the filtering. Comment at: lldb/source/Core/Module.cpp:740 + bool user_provided_name_is_mangled = + Mangled::GetManglingScheme(m_name.GetStringRef())

[Lldb-commits] [PATCH] D128932: [lldb] [llgs] Improve stdio forwarding in multiprocess+nonstop

2022-08-01 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Herald added a subscriber: JDevlieghere. Flake here: https://lab.llvm.org/buildbot/#/builders/68/builds/36967. Presumably the same problem that cd18e2ea3f4e87f8804a7d6661d5596ef1f07b81 fixed for

[Lldb-commits] [PATCH] D130803: [lldb] Allow SymbolTable regex search functions to match mangled name

2022-08-01 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Who sets this flag? How do you intend to use it? Comment at: lldb/include/lldb/Core/Module.h:267 + SymbolContextList _list, + bool match_against_demangled = false);

[Lldb-commits] [PATCH] D130796: [LLDB][NativePDB] Switch to use DWARFLocationList.

2022-08-01 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. The general idea makes sense to me, although I haven't tried to understand the pdb parsing code. > (otherwise internal binary search may fail) The current search algorithm is buggy, but I think the `RangeDataVector` class has always intended to support overlapping

[Lldb-commits] [PATCH] D130674: Accurate watchpoint hit counts redux

2022-08-01 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. This makes sense to me. Thanks for doing this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130674/new/ https://reviews.llvm.org/D130674

[Lldb-commits] [PATCH] D126359: [LLDB] Add basic floating point ops to IR interpreter

2022-07-27 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/test/API/lang/c/fpeval/main.c:3-4 +{ +double a = 42.0; +double b = 2.0; +return 0; Set break point at this line. kpdev42 wrote: > clayborg wrote: > > We should be testing "float" and "long double"

[Lldb-commits] [PATCH] D126614: [lldb] [gdb-remote] Client support for using the non-stop protocol

2022-07-27 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Sorry about the delay. I've been putting this off because it's complicated, although that's a lousy excuse. The gist of it is that I don't think that the existing setup of the "async threads" and "continue locks" is going to work in this new world. The reason it has

[Lldb-commits] [PATCH] D130341: [lldb] [gdb-remote] Use vKill packet if multiprocess & available

2022-07-27 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:4268 +if (response.IsOKResponse()) + return SIGKILL; + This is a host signal number (and not really available on windows). Maybe we

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

2022-07-27 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Sorry for dropping off. Using the mangled name helped, but I am still moderately worried about that the name that's going to be matched here is different from the "real" name that gets computed later

[Lldb-commits] [PATCH] D129750: [lldb] Always use APFloat for FP dumping

2022-07-27 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D129750#3681676 , @DavidSpickett wrote: > @labath Did you forget about this? Yeah, pretty much that. Committed now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129750/new/

[Lldb-commits] [PATCH] D129750: [lldb] Always use APFloat for FP dumping

2022-07-27 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1400a3cb8d53: [lldb] Always use APFloat for FP dumping (authored by labath). Changed prior to commit: https://reviews.llvm.org/D129750?vs=444586=448010#toc Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D130045: Implement better path matching in FileSpecList::FindFileIndex(...).

2022-07-27 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D130045#3678054 , @clayborg wrote: > In D130045#3675738 , @labath wrote: > >> In D130045#310 , @JDevlieghere >> wrote: >> >>> I'm slightly

[Lldb-commits] [PATCH] D130401: Implement better path matching in FileSpecList::FindCompatibleIndex(...).

2022-07-27 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/include/lldb/Utility/FileSpec.h:219-224 + /// Get the path separator for the current path style. + /// + /// \return + /// A path separator character for this path. + char GetPathSeparator() const; +

[Lldb-commits] [PATCH] D130582: [lldb] Skip the new mte_core_file test like other MTE tests

2022-07-26 Thread Pavel Labath via Phabricator via lldb-commits
labath added a subscriber: DavidSpickett. labath added a comment. (@DavidSpickett ) I don't think this is correct, as we should be able to open the core file on non-arm machines as well. The x86 bot (https://lab.llvm.org/buildbot/#/builders/68) seems to be fine with this. Repository: rG

[Lldb-commits] [PATCH] D130401: Implement better path matching in FileSpecList::FindCompatibleIndex(...).

2022-07-25 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/include/lldb/Core/FileSpecList.h:140-141 + /// + /// \param[in] full + /// Should FileSpec::Equal be called with "full" true or false. + /// Is this ever being called with full=false? If not can we drop it?

[Lldb-commits] [PATCH] D130045: Implement better path matching in FileSpecList::FindFileIndex(...).

2022-07-25 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D130045#310 , @JDevlieghere wrote: > I'm slightly worried about the change to make the new "fuzzy" matching the > default. While it makes sense for the breakpoints, I wouldn't generally > expect `./a/b/c/main.cpp` to

[Lldb-commits] [PATCH] D130219: [lldb][NFCI] Refactor regex filtering logic in CommandObjectTypeFormatterList

2022-07-21 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Commands/CommandObjectType.cpp:1064 +// No regex means list everything. +return (regex == nullptr || (s == regex->GetText() ||

[Lldb-commits] [PATCH] D129814: Fix stepping over watchpoints in architectures that raise the exception before executing the instruction

2022-07-18 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. In D129814#3655878 , @jingham wrote: > In D129814#3654368 , @labath wrote: > >> In D129814#3654276

[Lldb-commits] [PATCH] D128710: [lldb] [llgs] Fix multi-resume bugs with nonstop mode

2022-07-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. flake here: https://lab.llvm.org/buildbot/#/builders/68/builds/35932 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128710/new/ https://reviews.llvm.org/D128710 ___ lldb-commits

[Lldb-commits] [PATCH] D129652: [lldb] [llgs] Convert m_debugged_processes into a map of structs

2022-07-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Herald added a subscriber: JDevlieghere. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1058-1060 // Terminate the main loop only if vKill has not been used. // When running in non-stop mode, wait

[Lldb-commits] [PATCH] D129814: Fix stepping over watchpoints in architectures that raise the exception before executing the instruction

2022-07-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D129814#3654276 , @jasonmolenda wrote: > In D129814#3654230 , @labath wrote: > >> Generally, this makes sense to me, but I do have one question (not >> necessarily for Jim). These

[Lldb-commits] [PATCH] D128893: [lldb] [llgs] Fix disabling non-stop mode

2022-07-15 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks much better. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128893/new/ https://reviews.llvm.org/D128893 ___ lldb-commits

[Lldb-commits] [PATCH] D129783: [lldb] Skip tests using int128 on ARM

2022-07-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I find the aarch64 part surprising, and it seems that the aarch64 bot did not have a problem with this patch (https://lab.llvm.org/buildbot/#/builders/96/builds/26089). The arm part is expected, and most likely applies to all 32 bit architectures (but I think we only

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

2022-07-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D129682#3653049 , @bulbazord wrote: > In D129682#3651175 , @labath wrote: > >> Well.. first of let me say that the performance gain is impressive. I >> wouldn't have expected to gain

[Lldb-commits] [PATCH] D129814: Fix stepping over watchpoints in architectures that raise the exception before executing the instruction

2022-07-15 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Generally, this makes sense to me, but I do have one question (not necessarily for Jim). These tests work on (arm) linux, and I am wondering why is that the case. Could it be related to the fact that lldb-server preserves the stop reason for threads that are not

[Lldb-commits] [PATCH] D128710: [lldb] [llgs] Fix multi-resume bugs with nonstop mode

2022-07-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1818-1821 +// 4. vCont on a running process that requests suspending a subset +//of running threads or resuming a subset of suspended threads. +

[Lldb-commits] [PATCH] D128893: [lldb] [llgs] Fix disabling non-stop mode

2022-07-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Actually, on second thought (:D) do we even need a per-process flag for this? Turning off non-stop should stop _all_ processes, so instead of checking whether there are any processes with the 'stopping-due-to-qnonstop` flag around, couldn't we just check if we have any

[Lldb-commits] [PATCH] D129652: [lldb] [llgs] Convert m_debugged_processes into a map of structs

2022-07-14 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1058-1060 // Terminate the main loop only if vKill has not been used.

[Lldb-commits] [PATCH] D129736: [lldb] Skip a float16 NaN test for RISC-V

2022-07-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. That code is horrible. Lemme see if I can remove it (=> D129750 ). If that doesn't work, then we can try skipping this... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129736/new/

[Lldb-commits] [PATCH] D129750: [lldb] Always use APFloat for FP dumping

2022-07-14 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: JDevlieghere, jingham, DavidSpickett, Emmmer. Herald added subscribers: jsji, pengfei. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB. The DumpDataExtractor function had two branches for

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

2022-07-14 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Well.. first of let me say that the performance gain is impressive. I wouldn't have expected to gain that much with a relatively simple change. Now, as for the implementation, I have two main questions: - Do we really need the `GetQualifiedNameWithParams` function? My

[Lldb-commits] [PATCH] D129724: [lldb] Remove ELF .zdebug support

2022-07-14 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:1610 static SectionType GetSectionTypeFromName(llvm::StringRef Name) { if

[Lldb-commits] [PATCH] D128893: [lldb] [llgs] Fix disabling non-stop mode

2022-07-13 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D128893#3621673 , @mgorny wrote: > FYI, I don't feel very strongly about this. If you really dislike adding > `m_inhibit_stop_reason_processes`, I suppose we could alternatively just > declare disabling non-stop as

[Lldb-commits] [PATCH] D129307: Add a new breakpoint partial match settings

2022-07-13 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D129307#3646623 , @clayborg wrote: >> - The fact that the breakpoint path `/src/foo/bar.cc` could match debug info >> path `/build/foo/bar.cc` is definitely strange. > > Not strange at all for production builds where you build

[Lldb-commits] [PATCH] D129490: [lldb/libc++] Simplify the libc++ string formatter

2022-07-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Thanks. I can see the problem. rGa72306e202e ought to fix it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129490/new/

[Lldb-commits] [PATCH] D128069: [lldb] add SBSection.alignment to python bindings

2022-07-12 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG4135abca897d: [lldb] add SBSection.alignment to python bindings (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128069/new/

[Lldb-commits] [PATCH] D129307: Add a new breakpoint partial match settings

2022-07-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I originally wanted to stay out of this, but then we got a path-resolving bug report which got me thinking about all of this. Generally I would say I agree with Jim, that this is a important problem to solve, but the implementation is somewhat unusual. I don't really

[Lldb-commits] [PATCH] D129521: Add the ability to run expressions that call fork() or vfork().

2022-07-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a subscriber: mgorny. labath added a comment. The general idea makes sense, as we don't stop for forks even during normal execution. I'll too defer to @jingham on the implementation. I am also wondering what is the current and expected behavior in the `follow-fork=child` mode. The

[Lldb-commits] [PATCH] D129490: [lldb/libc++] Simplify the libc++ string formatter

2022-07-12 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. labath marked an inline comment as done. Closed by commit rGd4381153ea63: [lldb/libc++] Simplify the libc++ string formatter (authored by labath). Changed prior to commit:

[Lldb-commits] [PATCH] D129490: [lldb/libc++] Simplify the libc++ string formatter

2022-07-12 Thread Pavel Labath via Phabricator via lldb-commits
labath marked an inline comment as done. labath added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:609 } else { -ValueObjectSP size_mode(dataval_sp->GetChildAtIndexPath({1, 0, 0})); -if (!size_mode) +if (ValueObjectSP size_mode =

[Lldb-commits] [PATCH] D129455: [lldb] Reduce the stack alignment requirements for the Windows x86_64 ABI

2022-07-12 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D129455#3642015 , @mstorsjo wrote: > In D129455#3641967 , @labath wrote: > >> You say that the issue is the lack of symtab in the "msvc" mode. What makes >> this test work then? > >

[Lldb-commits] [PATCH] D129490: [lldb/libc++] Simplify the libc++ string formatter

2022-07-11 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision. labath added reviewers: JDevlieghere, mib. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB. Precise string layout has changed a lot recently, but a long of these changes did not have any effect on the usages of its

[Lldb-commits] [PATCH] D129012: [lldb] [test] Improve stability of llgs vCont-threads tests

2022-07-11 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Well... I'm as puzzled as you are. Whatever the problem is, it seems to be limited to debian (kernel). I've filed a bug about that: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1014754. One good thing I learned is that this problem seems to be limited to SIGSTOPs,

[Lldb-commits] [PATCH] D129455: [lldb] Reduce the stack alignment requirements for the Windows x86_64 ABI

2022-07-11 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good. The patch is really trivial so I don't want to make big deal out of the test. However, I would like to continue the discussion started in the patch description, as I think it

[Lldb-commits] [PATCH] D50304: [lldb] Fix thread step until to not set breakpoint(s) on incorrect line numbers

2022-07-11 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I am not fully versed on the patch, but generally speaking, if you're reasonably confident that your change fixed the origianl problem, you can resubmit the patch (and watch the bot for problems). If you still don't know what's the issue, then we may need to find

[Lldb-commits] [PATCH] D129166: [lldb] Make sure we use the libc++ from the build dir

2022-07-11 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. In D129166#3636658 , @dblaikie wrote: >> The overall logic and the include and library paths make sense to me. The >> rpath thingy will not work on

[Lldb-commits] [PATCH] D81471: [lldb] Add support for using integral const static data members in the expression evaluator

2022-07-11 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D81471#3637057 , @werat wrote: > In D81471#3632297 , @labath wrote: > >> I guess the condition we really want to express here is "does this >> expression refer to a constexpr variable

[Lldb-commits] [PATCH] D128250: [LLDB][RISCV]Add initial support for lldb-server.

2022-07-08 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D128250#3598291 , @Emmmer wrote: > In D128250#3598223 , @DavidSpickett > wrote: > >> I'll give this a more thorough read later just one comment for now. >> >> In the meantime, what's

[Lldb-commits] [PATCH] D129272: [lldb][Windows] Fix memory region base addresses when a range is split

2022-07-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/test/API/functionalities/memory-region/TestMemoryRegion.py:91 + +def test_unique_base_addresses(self): +# In the past on Windows we were recording AllocationBase as the base address rename to

[Lldb-commits] [PATCH] D129012: [lldb] [test] Improve stability of llgs vCont-threads tests

2022-07-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/test/API/tools/lldb-server/vCont-threads/main.cpp:18 +std::atomic can_work = ATOMIC_VAR_INIT(false); +thread_local bool can_exit_now = false; mgorny wrote: > labath wrote: > > I guess this should technically be a

[Lldb-commits] [PATCH] D129272: [lldb][Windows] Fix memory region base addresses when a range is split

2022-07-07 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good, thanks. I'd consider strengthening the test to look for overlapping address ranges instead of just identical base addresses. Comment at:

[Lldb-commits] [PATCH] D128932: [lldb] [llgs] Improve stdio forwarding in multiprocess+nonstop

2022-07-07 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Ok, let's give this a shot. Comment at: lldb/test/API/tools/lldb-server/main.cpp:351 + break; +std::this_thread::sleep_for(std::chrono::milliseconds(125 *

[Lldb-commits] [PATCH] D129166: [lldb] Make sure we use the libc++ from the build dir

2022-07-07 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. The overall logic and the include and library paths make sense to me. The rpath thingy will not work on windows as it (afaik) has no equivalent feature (it has the equivalent of (DY)LD_LIBRARY_PATH though). Do you just want to make that unsupported. I don't think we're

[Lldb-commits] [PATCH] D129012: [lldb] [test] Improve stability of llgs vCont-threads tests

2022-07-07 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added inline comments. Comment at: lldb/test/API/tools/lldb-server/vCont-threads/main.cpp:18 +std::atomic can_work = ATOMIC_VAR_INIT(false); +thread_local bool can_exit_now = false; I guess this should technically be a

[Lldb-commits] [PATCH] D129166: [lldb] Make sure we use the libc++ from the build dir

2022-07-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D129166#3633116 , @JDevlieghere wrote: > Thanks for the thoughtful reply Pavel. The remote tests are something we care > about as well, so I'd like to have a solution for that. What do you think > about adding a "stdlib"

[Lldb-commits] [PATCH] D128932: [lldb] [llgs] Improve stdio forwarding in multiprocess+nonstop

2022-07-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1110 // does not interfere with our protocol. -StopSTDIOForwarding(); +if (!m_non_stop) + StopSTDIOForwarding(); mgorny

[Lldb-commits] [PATCH] D129012: [lldb] [test] Improve stability of llgs vCont-threads tests

2022-07-06 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added inline comments. This revision is now accepted and ready to land. Comment at: lldb/test/API/tools/lldb-server/vCont-threads/main.cpp:32 + get_thread_id()); +write(STDOUT_FILENO, buf, strlen(buf)); +

[Lldb-commits] [PATCH] D129166: [lldb] Make sure we use the libc++ from the build dir

2022-07-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I'm afraid this will not work on systems which do not default to libc++ (which includes at least linux and windows), because `-stdlib=libc++` is not "equivalent" to `-nostdlib++ -lc++` -- the former changes the include paths to use libc++, while the latter doesn't. And,

[Lldb-commits] [PATCH] D81471: [lldb] Add support for using integral const static data members in the expression evaluator

2022-07-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. My only complaint is about the restriction in CanTakeAddressOfLValue. It seems arbitrary. There's anything in C++ or DWARF that would prevent us from having constant-valued variables of class types. It's just that clang/llvm does not know how to emit the

[Lldb-commits] [PATCH] D129177: [lldb][AArch64] Use "+all" feature for the disassembler

2022-07-06 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Awesome. thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129177/new/ https://reviews.llvm.org/D129177

[Lldb-commits] [PATCH] D129012: [lldb] [test] Improve stability of llgs vCont-threads tests

2022-07-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Seems like an improvement. I'd like to hear what you make of the inline comment though. I can also imagine a setup where most of the verification happens inside the inferior. Like, each time a thread gets to run it increments a variable specific to that thread. Once

[Lldb-commits] [PATCH] D128932: [lldb] [llgs] Improve stdio forwarding in multiprocess+nonstop

2022-07-06 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. I have a feeling the semaphores will not work (compile) on darwin. I didn't find any sem_init call there -- just sem_open. Maybe instead of semaphores we could use files for the synchronization? Something similar to the `wait_for_file_on_target` function, just in the

[Lldb-commits] [PATCH] D126983: [lldb] [llgs] Support "t" vCont action

2022-07-01 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D126983#3624901 , @mgorny wrote: > In D126983#3624800 , @labath wrote: > >> Some flakyness here: >> https://lab.llvm.org/buildbot/#/builders/68/builds/35111/steps/6/logs/stdio > >

[Lldb-commits] [PATCH] D126983: [lldb] [llgs] Support "t" vCont action

2022-07-01 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. Some flakyness here: https://lab.llvm.org/buildbot/#/builders/68/builds/35111/steps/6/logs/stdio Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126983/new/ https://reviews.llvm.org/D126983

[Lldb-commits] [PATCH] D124155: [lldb] Add tests which simulate the various std::string layouts

2022-07-01 Thread Pavel Labath 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 rGb6b65403b382: [lldb] Add tests which simulate the various std::string layouts (authored by labath). Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [PATCH] D128264: [lldb/dyld-posix] Avoid reading the module list in inconsistent states

2022-07-01 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. labath marked an inline comment as done. Closed by commit rG553558292eae: [lldb/dyld-posix] Avoid reading the module list in inconsistent states (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE

[Lldb-commits] [PATCH] D128264: [lldb/dyld-posix] Avoid reading the module list in inconsistent states

2022-07-01 Thread Pavel Labath via Phabricator via lldb-commits
labath marked an inline comment as done. labath added inline comments. Comment at: lldb/source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp:425 // and all DT_NEEDED entries on *BSD. if (m_initial_modules_added) { I = m_rendezvous.loaded_begin();

[Lldb-commits] [PATCH] D128541: [WIP][lldb][windows] Handle OutputDebugString from debuggee

2022-06-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added a subscriber: jasonmolenda. labath added a comment. In D128541#3622433 , @alvinhochun wrote: > In D128541#3621364 , @labath wrote: > >> Well.. most OSes don't have this kind of functionality, so we

[Lldb-commits] [PATCH] D128694: [lldb/Dataformatters] Adapt C++ std::string dataformatter for D128285

2022-06-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp:637 if (location_sp->GetName() == g_size_name) - location_sp = short_sp->GetChildAtIndex(3, true); + location_sp = short_sp->GetChildAtIndex(2, true); if

[Lldb-commits] [PATCH] D128776: Handle a stop when another thread has hit a breakpoint with a failed condition

2022-06-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D128776#3620886 , @jingham wrote: > Use sleep_for instead of usleep. Man, that API isn't winning any beauty > contests! well.. normally one would write that as `sleep_for(std::chrono:microseconds(47))`, or (if he's into

[Lldb-commits] [PATCH] D128849: [lldb] [llgs] Send process output asynchronously in non-stop mode

2022-06-30 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:3931 +GDBRemoteCommunication::PacketResult

[Lldb-commits] [PATCH] D128069: [lldb] add SBSection.alignment to python bindings

2022-06-30 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. In D128069#3619736 , @dmlary wrote: > updated to include test cases for `SBSection.GetAlignment()` and > `SBSection.alignment` property. Cool. Looks

[Lldb-commits] [PATCH] D128832: [lldb-server] Skip shared regions for memory allocation

2022-06-30 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good, thanks for fixing this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128832/new/ https://reviews.llvm.org/D128832

[Lldb-commits] [PATCH] D128541: [WIP][lldb][windows] Handle OutputDebugString from debuggee

2022-06-30 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D128541#3608718 , @alvinhochun wrote: > I think this should be enabled by default, but none of the logging categories > for `LLDB_LOG` has default active. That's because it's not really meant for this use case. The logging

[Lldb-commits] [PATCH] D127999: [lldb] fix stepping through POSIX trampolines

2022-06-29 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGd8ad018869ae: [lldb] fix stepping through POSIX trampolines (authored by mdaniels, committed by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D128780: [lldb] [test] Use raise(SIGSTOP) instead of trap in fork tests

2022-06-29 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added inline comments. This revision is now accepted and ready to land. Comment at: lldb/packages/Python/lldbsuite/test/tools/lldb-server/fork_testbase.py:155 parent_expect = [ -"T05thread:p{}.{};.*".format(parent_pid,

[Lldb-commits] [PATCH] D128638: [lldb] [llgs] Add base nonstop fork/vfork tests

2022-06-29 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. This looks like a genuine problem, see https://lab.llvm.org/buildbot/#/builders/68/builds/34903. If I had to guess, I'd say the problem is you're overwriting the context object holding the first pid (here:

[Lldb-commits] [PATCH] D128776: Handle a stop when another thread has hit a breakpoint with a failed condition

2022-06-29 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. (Other than the inline comment, this seems fine to me) Comment at: lldb/test/API/functionalities/breakpoint/two_hits_one_actual/main.cpp:6 +void usleep_helper(useconds_t usec) { + usleep(usec); // Break here in the helper +} usleep

[Lldb-commits] [PATCH] D128678: [LLDB] Add PDB/calling-conventions.test for Arm/Windows

2022-06-29 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D128678#3616685 , @mstorsjo wrote: > Thanks, this looks more complete and consistent to me now! > > In D128678#3615531 , @labath wrote: > >> It seems like this is not actually running

[Lldb-commits] [PATCH] D123580: [libc++] Use bit field for checking if string is in long or short mode

2022-06-28 Thread Pavel Labath via Phabricator via lldb-commits
labath added inline comments. Comment at: libcxx/utils/gdb/libcxx/printers.py:192 class StdStringPrinter(object): """Print a std::string.""" Mordante wrote: > ldionne wrote: > > philnik wrote: > > > labath wrote: > > > > philnik wrote: > > > > > dblaikie

[Lldb-commits] [PATCH] D127999: [lldb] fix stepping through POSIX trampolines

2022-06-28 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. Cool, thanks. So do you have commit access, or you need someone to commit this for you? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127999/new/ https://reviews.llvm.org/D127999 ___

[Lldb-commits] [PATCH] D128264: [lldb/dyld-posix] Avoid reading the module list in inconsistent states

2022-06-28 Thread Pavel Labath via Phabricator via lldb-commits
labath marked an inline comment as done. labath added a comment. @mgorny, ping. Do you maybe want to try this out on freebsd? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128264/new/ https://reviews.llvm.org/D128264

[Lldb-commits] [PATCH] D128410: [lldb] Add a testcase for nested exceptions on Windows

2022-06-28 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. In D128410#3609185 , @mstorsjo wrote: > In D128410#3608190 , @labath wrote: > >> In D128410#3604927 , @alvinhochun >> wrote: >> >>> It may be

[Lldb-commits] [PATCH] D128678: [LLDB] Add PDB/calling-conventions.test for Arm/Windows

2022-06-28 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment. It seems like this is not actually running the code. Can we make it such that these tests are conditional on the appropriate llvm targets being enabled (instead of the host being of a specific type)? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128678/new/

[Lldb-commits] [PATCH] D128638: [lldb] [llgs] Add base nonstop fork/vfork tests

2022-06-28 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. In D128638#3611932 , @mgorny wrote: > BTW this test is getting a bit long as well but I don't have a good idea how > to split it, except for just

<    1   2   3   4   5   6   7   8   9   10   >