[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info

labath added a comment. I am trying to get rid of the friendship declaration and the forwarding of protected methods. I have no issue with making `GetCompileUnitAtIndex` virtual -- that is pretty much a necessity for this feature (*). However, `SetCompileUnitAtIndex` seems like an internal

[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info

labath added a comment. I'm looking at the SymbolFileOnDemand friendship and the forwarding of protected methods (mostly dealing with compile unit lists). Does this by any chance have something to do with the fact that there are now two compile unit lists (one in the actual symbol file, and

[Lldb-commits] [PATCH] D123128: don't extra notify ModulesDidLoad() from LoadModuleAtAddress()

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Cool. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123128/new/ https://reviews.llvm.org/D123128 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

labath added a comment. In D123020#3437246 , @llunak wrote: > In D123020#3426867 , @JDevlieghere > wrote: > >> FWIW the official policy is outlined here: >> https://llvm.org/docs/CodeReview.html > > I'm aware

[Lldb-commits] [PATCH] D123401: [lldb] Fix debug_info decorators for NO_DEBUG_INFO_TESTCASE

labath added a comment. I think this is actually a wider problem. I also ran into this recently (with triples), but didn't have time to create a patch yet. I don't think that `None` as the "actual" value should ever be considered a match (except when the pattern is `None`, I guess),

[Lldb-commits] [PATCH] D123340: Applying clang-tidy modernize-use-override over LLDB

labath added a comment. The reason for the funny `/*override*/` thingy in the mock classes is that it is impossible (in the gmock version that we use) to add the override keyword to the methods overridden by the MOCK macros. Then, having override keywords on hand-written methods triggered

[Lldb-commits] [PATCH] D91835: [lldb] Add Python bindings to print stack traces on crashes.

labath added inline comments. Comment at: lldb/source/API/SBDebugger.cpp:215-216 + llvm::EnablePrettyStackTrace(); + // We don't have a meaningful argv[0] to use, so use "SBDebugger" as a + // substitute. + llvm::sys::PrintStackTraceOnErrorSignal("SBDebugger");

[Lldb-commits] [PATCH] D123203: [lldb] Silence GCC warnings about missing returns after fully covered switches. NFC.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Yes, that's the way we usually deal with this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123203/new/ https://reviews.llvm.org/D123203

[Lldb-commits] [PATCH] D123205: [lldb] Silence GCC/glibc warnings about ignoring the return value of write(). NFC.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Herald added a subscriber: JDevlieghere. The gcc behavior is quite annoying here... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123205/new/

[Lldb-commits] [PATCH] D123202: [lldb] Fix detecting warning options for GCC

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Herald added a subscriber: JDevlieghere. This is great. I've been bothered by this for quite a while, but I didn't realize the fix is that easy. Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D122856: [lldb] Refactor DataBuffer so we can map files as read-only

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good. Thanks for your patience. Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2636-2638 + assert(llvm::isa(data_buffer_sp.get())); +

[Lldb-commits] [PATCH] D123128: don't extra notify ModulesDidLoad() from LoadModuleAtAddress()

labath added a comment. > Note that this patch could have been shorter if I simply changed > LoadModuleAtAddress() to always pass false for notify (all callers currently > call ModulesDidLoad() afterwards) and added a note to LoadModuleAtAddress() > docs about it, but I don't know if that's a

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

labath updated this revision to Diff 420507. labath added a comment. Reopening, since the last attempt was quite some time ago, and the rebase was non-trivial. Functionally, there are no real changes. The one-liner in dotest.py is somewhat interesting, as it was necessary for (emu|simu)lator

[Lldb-commits] [PATCH] D122975: parallelize module loading in DynamicLoaderPOSIXDYLD()

labath added a comment. In D122975#3426731 , @llunak wrote: > In D122975#3426575 , @labath wrote: > >> I'd like to understand what is the precise thing that you're trying to >> optimise. If there is like a

[Lldb-commits] [PATCH] D122660: [lldb] Avoid duplicate vdso modules when opening core files

This revision was automatically updated to reflect the committed changes. Closed by commit rGe67cee09499c: [lldb] Avoid duplicate vdso modules when opening core files (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122660/new/

[Lldb-commits] [PATCH] D122716: [lldb/linux] Handle main thread exits

This revision was automatically updated to reflect the committed changes. Closed by commit rG4384c96fe7eb: [lldb/linux] Handle main thread exits (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122716/new/

[Lldb-commits] [PATCH] D122898: [lldb] Move host platform implementations into the base class

This revision was automatically updated to reflect the committed changes. Closed by commit rG331150a47dd5: [lldb] Move host platform implementations into the base class (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122898/new/

[Lldb-commits] [PATCH] D122856: [lldb] Refactor DataBuffer so we can map files as read-only

labath added inline comments. Comment at: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:361 // Update the data to contain the entire file if it doesn't already if (data_sp->GetByteSize() < length) { +data_sp = MapFileDataWritable(*file, length, file_offset);

[Lldb-commits] [PATCH] D123073: [lldb] Change CreateMemoryInstance to take a WritableDataBuffer

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. cool. thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123073/new/ https://reviews.llvm.org/D123073 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D123092: [LLDB][NativePDB] Fix inline line info in line table

labath added a comment. As unusual, I don't understand pdbs, but the resulting line table seems reasonable. I don't think we can assert this level of detail in a compilation-based test though. Could you put these checks into the non-"live" version of the test? Repository: rG LLVM Github

[Lldb-commits] [PATCH] D122856: [lldb] Refactor DataBuffer so we can map files as read-only

labath added inline comments. Comment at: lldb/include/lldb/Utility/DataBuffer.h:58 + /// if the object contains no bytes. + virtual const uint8_t *GetBytesImpl() const = 0; + Are you sure this should be public? Comment at:

[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

labath added a comment. If it's just a matter of setting the default timeout value, then I don't think it would be that bad -- we already set a different timeout for debug and release builds, so it we could conceivably put this there. But yes, my "setting the initial timeout value to a

[Lldb-commits] [PATCH] D122975: parallelize module loading in DynamicLoaderPOSIXDYLD()

labath requested changes to this revision. labath added a comment. This revision now requires changes to proceed. This is an interesting area for optimisation, but I don't think it's going to be that easy. A couple of years ago, I experimented with a similar approach, but I could not come up

[Lldb-commits] [PATCH] D123001: make 'step out' step out of the selected frame

labath added a comment. This should definitely have a test case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123001/new/ https://reviews.llvm.org/D123001 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

labath added a comment. > BTW, does it make sense to get even things like this reviewed, or is it ok if > I push these directly if I'm reasonably certain I know what I'm doing? I feel > like I'm spamming you by now. Generally, I would say yes. I'm not even sure that some of your other patches

[Lldb-commits] [PATCH] D122980: make ConstStringTable use DenseMap rather than std::map

labath accepted this revision. labath added a comment. In D122980#3424636 , @JDevlieghere wrote: > I feel like this came up in the past and there was a reason an unordered map > couldn't work? Maybe I'm confusing this with something else. Added Pavel

[Lldb-commits] [PATCH] D122974: prevent ConstString from calling djbHash() more than once

labath added subscribers: dblaikie, labath. labath added a reviewer: dblaikie. labath added a comment. In principle, I like this a lot, though I'm not sure if this is the best way to integrate with with the StringMap class. Adding @dblaikie for thoughts on that. Some background: LLDB uses a

[Lldb-commits] [PATCH] D122944: [lldb] Prevent object file plugins from messing with the data buffer if they don't match

labath added a comment. Yes, that's the general idea, but you forgot the `CreateMemoryInstance` callback -- we should fix that too. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122944/new/ https://reviews.llvm.org/D122944 ___ lldb-commits

[Lldb-commits] [PATCH] D122943: [LLDB][NativePDB] Fix a crash when S_DEFRANGE_SUBFIELD_REGISTER descirbes a simple type

labath added a comment. Seems reasonable, but I don't know much about pdb's. @rnk, do you want to take a look? Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp:54 struct FindMembersSize : public TypeVisitorCallbacks { - FindMembersSize(std::vector> _info, +

[Lldb-commits] [PATCH] D122944: [lldb] Prevent object file plugins from messing with the data buffer if they don't match

labath added a comment. It would be better if we could automatically "reset" these variables by having the plugins accept them as values instead of references. Does anyone I actually need the new values of the variables after these functions are done? I know of at least one place

[Lldb-commits] [PATCH] D122898: [lldb] Move host platform implementations into the base class

labath created this revision. labath added reviewers: JDevlieghere, jingham. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB. About half of our host platform code was implemented in the Platform class, while the rest was it RemoteAwarePlatform.

[Lldb-commits] [PATCH] D122680: Add a setting to force overwriting commands in "command script add"

labath added inline comments. Comment at: lldb/source/Commands/CommandObjectCommands.cpp:1650 std::string m_short_help; - bool m_overwrite = false; + bool m_overwrite = eLazyBoolCalculate; ScriptedCommandSynchronicity m_synchronicity = This doesn't seem

[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good thanks. I didn't really check the pdb code, but it looks like @rnk already reviewed that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D122856: [lldb] Refactor DataBuffer so we can map files as read-only

labath added a comment. Yeah, I tried doing this (well, i used `const DataBuffer` to represent read-only data) last time this topic came up, and ran into the same problem. Since there was no pressing need for it, I put the patch away. I think the best way to fix this is to allow the object

[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

labath added a comment. In D121967#3403886 , @zequanwu wrote: >> Can you say (in english) what are the properties that you are trying to >> check there? Maybe we can find a better way to do that... > > I'm trying to check for local variables values in

[Lldb-commits] [PATCH] D117559: [lldb] Remove forward-connect ability from lldb-server tests

labath added a comment. (Thanks for trying this out, I'm still thinking about the best way to move this forward.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117559/new/ https://reviews.llvm.org/D117559

[Lldb-commits] [PATCH] D122716: [lldb/linux] Handle main thread exits

labath created this revision. labath added reviewers: mgorny, DavidSpickett. Herald added a subscriber: krytarowski. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB. This patch handles the situation where the main thread exits (through the

[Lldb-commits] [PATCH] D122710: [lldb-vscode] Avoid a -Wunused-but-set-variable warning. NFC.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Herald added a subscriber: JDevlieghere. I think this qualifies as an obvious fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122710/new/

[Lldb-commits] [PATCH] D121844: Applying clang-tidy modernize-use-equals-default over LLDB

labath added inline comments. Comment at: lldb/source/Core/Value.cpp:667 -const ValueList ::operator=(const ValueList ) { +const ValueList ::operator=(const ValueList ) { // NOLINT(modernize-use-equals-default) m_values = rhs.m_values; shafik wrote: >

[Lldb-commits] [PATCH] D122684: [lldb] Use the selected and host platform to disambiguate between fat binary architectures

labath accepted this revision. labath added a comment. Just this morning, I was (again) contemplating the idea of introducing some sort of numeric platform priorities. (We currently have a sort of a ternary system -- exact match, compatible match, no match). That would allow us to express

[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info

labath added inline comments. Comment at: lldb/test/Shell/SymbolFile/OnDemand/shared-lib-globals-hydration.test:4 + +# REQUIRES: system-linux + yinghuitan wrote: > labath wrote: > > yinghuitan wrote: > > > labath wrote: > > > > Is this here because there is no

[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info

labath added inline comments. Comment at: lldb/test/Shell/SymbolFile/OnDemand/shared-lib-globals-hydration.test:4 + +# REQUIRES: system-linux + yinghuitan wrote: > labath wrote: > > Is this here because there is no portable way to create shared libraries in >

[Lldb-commits] [PATCH] D122660: [lldb] Avoid duplicate vdso modules when opening core files

labath created this revision. labath added reviewers: mgorny, clayborg, JDevlieghere. Herald added subscribers: pmatos, asb, sunfish, sbc100. Herald added a project: All. labath requested review of this revision. Herald added a subscriber: aheejin. Herald added a project: LLDB. When opening core

[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info

labath added a comment. In D121631#3387077 , @clayborg wrote: > In D121631#3386244 , @labath wrote: > >> In D121631#3384124 , @yinghuitan >> wrote: >> unify with

[Lldb-commits] [PATCH] D122411: [lldb][AArch64] Fix corefile memory reads when there are non-address bits

labath added a comment. In D122411#3408204 , @DavidSpickett wrote: > - Corefile down to 24k with some tweaking of coredump_filter 24k is pretty good. At this point, if you wanted to go down further, you'd probably have to ditch libc, which may not be

[Lldb-commits] [PATCH] D121999: [lldb][AArch64] Update disassembler feature list and add tests for all extensions

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Wow. It sounds like we should figure out a way to enable all extensions with a single stroke. Otherwise, this will be an endless cat-and-mouse. Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [PATCH] D122411: [lldb][AArch64] Fix corefile memory reads when there are non-address bits

labath added a comment. Can we make the core file smaller? Since it's essentially static data, do you really need all the mmap, debug info and everything? Could you just take an address that is known to be in the core file (smallest you can make), and then do a `memory read 0xwhatever` ?

[Lldb-commits] [PATCH] D122461: [lldb] Add a fuzzer for target create

labath added subscribers: cmtice, labath. labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. I like this. Comment at: lldb/tools/lldb-fuzzer/lldb-fuzzer-target.cpp:24 +extern "C" int LLVMFuzzerTestOneInput(uint8_t *data,

[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

labath added a comment. In D121967#3400827 , @zequanwu wrote: >> I'd consider writing the live test case in c++ (with judicious use of >> always_inline, noinline, etc. attributes) > > I think it's better to just add live test case on compiled

[Lldb-commits] [PATCH] D120485: [lldb][Process/FreeBSD] Add support for address masks on aarch64

labath added a comment. Can you detect it from inside the debugee? You could have it exit with some predefined error code if the feature is not available (and then skip the test based on that)... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D122283: [lldb] Add a Stream variant that escapes null bytes

labath added a comment. Although I can't say exactly what would be the problem with this, this seems like a strange thing to do, and I'm not aware of any other stream class which would handle nul (and _only_ nul) characters specially. CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D122193: [lldb/test] Add events listener helper function to lldbtest

labath accepted this revision. labath added inline comments. This revision is now accepted and ready to land. Comment at: lldb/packages/Python/lldbsuite/test/lldbutil.py:1265 + +def fetch_next_event(test, listener, broadcaster, timeout=1): +"""Fetch one event from the

[Lldb-commits] [PATCH] D122193: Reland "[lldb/test] Add events listener helper class to lldbtest"

labath added a comment. In D122193#3399921 , @mib wrote: > Hey @labath, thanks for the feedback! Here are some other questions ? > > In D122193#3399109 , @labath wrote: > >> I like this implementation a lot. The

[Lldb-commits] [PATCH] D122193: Reland "[lldb/test] Add events listener helper class to lldbtest"

labath added a comment. I like this implementation a lot. The less threads we use, the better. I have a couple of remarks/questions though: - given how simple the new approach is, is a dedicated test class really needed? It seems like it should be sufficient to add two utility functions to:

[Lldb-commits] [PATCH] D122025: [lldb] Remove lldbassert from CommandInterpreter::PrintCommandOutput

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Agreed we generally don't want to have embedded nuls in the command output, but I don't think we have to go to such great lengths to enforce it. > The latter doesn't trigger this bug because

[Lldb-commits] [PATCH] D121967: [LLDB][NativePDB] Create inline function decls

labath added a comment. I don't know much about PDBs, but overall, the patch seems fine to me. In D121967#3393556 , @zequanwu wrote: > I think adding a live debugging test case is necessary. I found several bugs > when working on this via live

[Lldb-commits] [PATCH] D121977: [lldb/test] Add events listener helper class to lldbtest

labath added a comment. I've reverted this patch because it introduces races in the event processing. See inline comment for details. Comment at: lldb/packages/Python/lldbsuite/test/eventlistener.py:38-41 +# Broadcast a eBroadcastBitStopListenerThread` event so the

[Lldb-commits] [PATCH] D121999: [lldb][AArch64] Update disassembler feature list and add tests for all extensions

labath added a comment. Can you make this a shell test? Something like llvm-mc | lldb -o "disassemble -n function_with_funky_insns" | FileCheck ? Presumably, llvm-mc can assemble anything that we are able to disassemble... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D121900: [lldb] Add more documentation on test variants

This revision was automatically updated to reflect the committed changes. Closed by commit rGc7fc7205bb45: [lldb] Add more documentation on test variants (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121900/new/

[Lldb-commits] [PATCH] D121912: [lldb] Fix ^C handling in IOHandlerProcessSTDIO

labath added a comment. In D121912#3389378 , @DavidSpickett wrote: > Do you know if the pexpect test will cleanup the debugee process if some part > of the test fails? Or will we have that hanging around in its infinite loop? It won't, but the

[Lldb-commits] [PATCH] D121977: [lldb/test] Add progress events listener helper class to lldbutil

labath accepted this revision. labath added inline comments. This revision is now accepted and ready to land. Comment at: lldb/packages/Python/lldbsuite/test/lldbutil.py:1635 + +def __fetch_events__(self): +event = lldb.SBEvent() This contains way

[Lldb-commits] [PATCH] D121912: [lldb] Fix ^C handling in IOHandlerProcessSTDIO

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. D120762 accidentally moved the interrupt check into the block which was reading stdio. This

[Lldb-commits] [PATCH] D121900: [lldb] Add more documentation on test variants

labath created this revision. labath added reviewers: JDevlieghere, clayborg, jingham. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB. This formalizes some of the discussion on D121631 . Repository: rG

[Lldb-commits] [PATCH] D121844: Applying clang-tidy modernize-use-equals-default over LLDB

labath added a comment. (Please run clang-format before submitting) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121844/new/ https://reviews.llvm.org/D121844 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[Lldb-commits] [PATCH] D121844: Applying clang-tidy modernize-use-equals-default over LLDB

labath accepted this revision. labath added inline comments. Comment at: lldb/source/Core/Value.cpp:667 -const ValueList ::operator=(const ValueList ) { +const ValueList ::operator=(const ValueList ) { // NOLINT(modernize-use-equals-default) m_values = rhs.m_values;

[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info

labath added a comment. In D121631#3384124 , @yinghuitan wrote: >> unify with `target.preload-symbols` feature > > I do not have strong opinion on this. One concern is that > `target.preload-symbols` provides full experience but >

[Lldb-commits] [PATCH] D121802: [LLDB] Fix typos in LLDB help output.

labath added a comment. Yeah, the overhead is pretty large for trivial patches like this, but it's the best we have, so just keep them coming. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121802/new/ https://reviews.llvm.org/D121802

[Lldb-commits] [PATCH] D121800: [LLDB] Change enumaration to enumeration

This revision was automatically updated to reflect the committed changes. Closed by commit rGd58ef6df0d4a: [LLDB] Change enumaration to enumeration (authored by hawkinsw, committed by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D121802: [LLDB] Fix typos in LLDB help output.

This revision was automatically updated to reflect the committed changes. Closed by commit rGfe93395b92d5: [LLDB] Fix typos in LLDB help output. (authored by hawkinsw, committed by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121802/new/

[Lldb-commits] [PATCH] D121511: [lldb] Report debugger diagnostics as events

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. cool Comment at: lldb/unittests/Core/DiagnosticEventTest.cpp:53-54 +TEST_F(DiagnosticEventTest, Warning) { + ArchSpec arch("x86_64-apple-macosx-"); +

[Lldb-commits] [PATCH] D121802: [LLDB] Fix typos in LLDB help output.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. I think you're doing everything right. Thanks for the fixes. I can commit this together with the other patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D121800: [LLDB] Change enumaration to enumeration

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good. I presume you need someone to commit this for you (?) What's the name I can use for commit attribution? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. awesome Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121030/new/ https://reviews.llvm.org/D121030

[Lldb-commits] [PATCH] D121605: [lldb/test] Make category-skipping logic "platform"-independent

labath added a comment. Yeah, I'm pretty sure its due to this patch. Though the failures don't look near as bad as I feared. :) I've reverted the patch while I work on a fix. Do let me know if you see some other failures related to this, particularly in some more exotic

[Lldb-commits] [PATCH] D121605: [lldb/test] Make category-skipping logic "platform"-independent

This revision was automatically updated to reflect the committed changes. Closed by commit rGdddf4ce034a8: [lldb/test] Make category-skipping logic platform-independent (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121605/new/

[Lldb-commits] [PATCH] D121631: Introduce new symbol on-demand for debug info

labath added a comment. I agree with everything Jim said. One of the interesting 'strategy' questions would be the interaction between this feature and the `target.preload-symbols` feature. I don't think the interaction between the two is obvious or intuitive, and I'm even sure that

[Lldb-commits] [PATCH] D121537: [lldb] Synchronize the output through the IOHandler

labath accepted this revision. labath added inline comments. This revision is now accepted and ready to land. Comment at: lldb/include/lldb/Interpreter/CommandInterpreter.h:659 + void PrintCommandOutput(IOHandler _handler, llvm::StringRef str, + bool

[Lldb-commits] [PATCH] D121444: [lldb] Fix platform selection on Apple Silicon (again)

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Cool. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121444/new/ https://reviews.llvm.org/D121444 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D121511: [lldb] Report debugger diagnostics as events

labath added a comment. Heh, I didn't realize that "debugger-less" events are implemented by iterating over all debugger instances, but since we're doing that for progress events anyway, we might as well copy the pattern. Any chance of a test case for some of this stuff? Maybe a unittest which

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

labath added inline comments. 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; zequanwu wrote: > labath wrote: > > Would it be

[Lldb-commits] [PATCH] D117559: [lldb] Remove forward-connect ability from lldb-server tests

labath added a comment. Sorry about the delay. Here's the version which switches to reverse connect everywhere. Please try it out. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117559/new/ https://reviews.llvm.org/D117559

[Lldb-commits] [PATCH] D117559: [lldb] Remove remote testing ability from lldb**-server** tests

labath updated this revision to Diff 415120. labath added a comment. Herald added a project: All. Remove just the forward-connect logic instead. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117559/new/ https://reviews.llvm.org/D117559 Files:

[Lldb-commits] [PATCH] D121605: [lldb/test] Make category-skipping logic "platform"-independent

labath added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/lldbplatformutil.py:125 def getPlatform(): """Returns the target platform which the tests are running on.""" I'm not sure if this will work for all apple platforms. Can I

[Lldb-commits] [PATCH] D121484: [lldb] Plumb host architecture through platform selection

labath added a comment. In D121484#3379532 , @JDevlieghere wrote: > In D121484#3378923 , @labath wrote: > >> I'm not sure what would be a good way to denote that the new argument >> represents the architecture

[Lldb-commits] [PATCH] D121605: [lldb/test] Make category-skipping logic "platform"-independent

labath created this revision. labath added reviewers: JDevlieghere, jingham, clayborg. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB. The decision which categories are relevant for a particular test run happen very early in the test setup

[Lldb-commits] [PATCH] D121444: [lldb] Fix platform selection on Apple Silicon (again)

labath added a comment. I don't suppose we could have a (gdb-client?) test for some of this stuff? Comment at: lldb/source/Plugins/Platform/MacOSX/PlatformMacOSX.cpp:158 +// tell them apart and mark the host platform as compatible or not. +if (host_arch.IsValid()) { +

[Lldb-commits] [PATCH] D121484: [lldb] Plumb host architecture through platform selection

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. I'm not sure what would be a good way to denote that the new argument represents the architecture of the host running the debugged process, and not the one running lldb. This isn't helped by

[Lldb-commits] [PATCH] D121511: [lldb] Report debugger diagnostics as events

labath added a comment. It seems to me that it would be better to decentralize the warning tracking. Also due to a central list of all possible warnings not being modular, but mainly because I can imagine that some warnings may want to be reported with different "scopes" -- once per target,

[Lldb-commits] [PATCH] D121500: [lldb] Protect the debugger's output and error stream

labath added a comment. In D121500#3377434 , @JDevlieghere wrote: > In D121500#3377321 , @labath wrote: > >> Are you sure we don't want to handle this at a higher level? The way I >> understand it, the main

[Lldb-commits] [PATCH] D121536: [lldb] Use the IOHandler's output/error stream instead of the debugger one in PrintAsync

labath added a comment. (I suspect this has something to do with the ability to override the output of a particular iohandler in the stack. The idea may have been that the iohandler prints whereever it prints, but the asynchronous output always goes to the main output. But I don't know if we

[Lldb-commits] [PATCH] D121536: [lldb] Use the IOHandler's output/error stream instead of the debugger one in PrintAsync

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. > In that context it didn't make much sense (to me) that this is using the > debugger's output and error stream rather than the one from the IOHandler. I don't know why this is set up the way

[Lldb-commits] [PATCH] D121537: [lldb] Synchronize the output through the IOHandler

labath added a comment. That is roughly what I had in mind, *but* if you look inside `Editline` class, you'll see that it already declares a `std::mutex m_output_mutex` variable. It was introduced for the same reason as this mutex here, although it only controls stdout access during command

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

labath accepted this revision. labath added a comment. In D121348#3373396 , @jingham wrote: > In D121348#3373170 , @labath wrote: > >> I don't think this is a peculiarity of the darwin ABI. I'm pretty sure i've

[Lldb-commits] [PATCH] D121487: [lldb] Require native for command-thread-siginfo.test

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good, although I would be surprised if this was the only test which suffered from the same problem. Another possibility would be to make the %clang_host substitution smarter and have

[Lldb-commits] [PATCH] D121481: Applying clang-tidy modernize-use-default-member-init over LLDB

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. cool CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121481/new/ https://reviews.llvm.org/D121481 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D121500: [lldb] Protect the debugger's output and error stream

labath added a reviewer: clayborg. labath added a comment. Are you sure we don't want to handle this at a higher level? The way I understand it, the main reason for the existence of PrintAsync and StreamAsynchronousIO machinery is to provide precise control about when the various bits of

[Lldb-commits] [PATCH] D121444: [lldb] Fix platform selection on Apple Silicon (again)

labath added a comment. In D121444#3375445 , @JDevlieghere wrote: > In D121444#3374854 , @labath wrote: > >>> I wish I could make this distinction in the platform, but you need a >>> connected process to do

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

labath accepted this revision. labath added inline comments. This revision is now accepted and ready to land. Comment at: lldb/source/Plugins/Process/Windows/Common/NativeProcessWindows.h:75 + ArchSpec Process::GetSystemArchitecture() override; + This should

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

labath added a comment. (I'm not going to land this immediately, since I've also learned that this breaks running the test suite against the qemu platform -- which works right now, but it seems that is mostly accidental. I will switch gears and make the qemu platform more test-suite compatible

[Lldb-commits] [PATCH] D121444: [lldb] Fix platform selection on Apple Silicon (again)

labath added a comment. > I wish I could make this distinction in the platform, but you need a > connected process to do this. Basically, what you're saying is that the ArchSpec alone is not sufficient to select the right platform. Instead of punching right through the layers, could we just

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