[Lldb-commits] [PATCH] D135768: Counterexample for D134849

labath created this revision. labath added a reviewer: zequanwu. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB. Herald added a subscriber: lldb-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D135768 Files:

[Lldb-commits] [PATCH] D135631: [lldb] Copy log files into diagnostic directory (RFC)

labath added a comment. seems fine to me. Comment at: lldb/test/Shell/Diagnostics/TestCopyLogs.test:3 +# RUN: mkdir -p %t +# The "ll''db" below is not a typo but a way to prevent lit from substituting 'lldb'. +# RUN: %lldb -o 'log enable ll''db commands -f %t/commands.log' -o

[Lldb-commits] [PATCH] D135621: [lldb] Add an always-on log channel

labath added a comment. I'm not sure if this will do what you wanted it to do. Despite the title, this patch is not adding a new log *channel* -- it is adding a new log *category* to the "lldb" channel. Our logging infra, perhaps somewhat misleadingly, manages all of the log state on a

[Lldb-commits] [PATCH] D135413: [lldb][CPlusPlusLanguage] Respect the step-avoid-regex for functions with auto return types

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. > There are longer-term plans of replacing the hand-rolled C++ parser with an > alternative that uses the mangle tree API to do the parsing for us. You may be aware of this, but I feel I

[Lldb-commits] [PATCH] D134849: [LLDB][NativePDB] Fix struct layout when it has anonymous unions.

labath added a comment. The test looks great, and the comments have helped. I still have a couple of questions about the algorithm though. Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:431 +uint64_t offset = pair.first; +auto =

[Lldb-commits] [PATCH] D135516: [lldb] [MainLoopPosix] Fix crash upon adding lots of pending callbacks

labath added a comment. I never expected we would have so many callbacks that we'd have to worry about this, but yes, this is one way to fix the problem. Another (slightly simpler, but also less performant) would be to make the write pipe nonblocking, and do not treat EAGAIN as an error.

[Lldb-commits] [PATCH] D134882: [lldb] Move breakpoint hit reset code to Target::CleanupProcess

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 rG08c4a6795ac4: [lldb] Move breakpoint hit reset code to Target::CleanupProcess (authored by labath). Repository: rG

[Lldb-commits] [PATCH] D134754: [lldb/gdb-server] Better reporting of launch errors

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 rG8d1de7b34af4: [lldb/gdb-server] Better reporting of launch errors (authored by labath). Repository: rG LLVM Github

[Lldb-commits] [PATCH] D134570: [lldb] Skip check for conflicting filter/synth when adding a new regex.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Let's just skip this check. Though it might be fun to implement an emptyness-of-intersection check for two regular expressions, that's: a) overkill; b) impossible if you include

[Lldb-commits] [PATCH] D134771: [NFCI] Simplify TypeCategoryImpl for-each callbacks.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. (it looks like I did not click "submit" for some reason) Comment at: lldb/include/lldb/DataFormatters/TypeCategory.h:169 +template ForEachCallback(Callable c) :

[Lldb-commits] [PATCH] D134849: [LLDB][NativePDB] Fix struct layout when it has anonymous unions.

labath added inline comments. Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:331 + clang::DeclContext *parent_decl_ctx) { + static lldb::user_id_t anonymous_id = LLDB_INVALID_UID - 1; + clang::FieldDecl *field_decl =

[Lldb-commits] [PATCH] D134906: Have MemoryCache cache addresses that were unreadable, so duplicate read attempts can be suppressed

labath added a comment. In D134906#3835260 , @jasonmolenda wrote: > In D134906#3832642 , @labath wrote: > >> I don't know about debugserver, but both lldb-server and gdbserver currently >> return an error when

[Lldb-commits] [PATCH] D135031: [lldb] [llgs] Move client-server communication into a separate thread (WIP)

labath added a comment. I think we should get some measurements (e.g. from the `process plugin packet speed-test` cmd) of the overhead of this approach. The latency/RTT of the connection is very important for some users, and I fear that all of this ping pong could significantly regress that.

[Lldb-commits] [PATCH] D135029: [lldb] [gdb-remote] Isolate ClientBase from Communication

labath added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h:116-118 + GDBRemoteCommunication () { +return m_comm; + } mgorny wrote: > labath wrote: > > Is the plan to make this private/protected at some point, or

[Lldb-commits] [PATCH] D134882: [lldb] Move breakpoint hit reset code to Target::CleanupProcess

labath added a comment. In D134882#3831137 , @jingham wrote: > That seems fine. I think it's useful to be able to see breakpoint hit counts > up to the point where you start a new process. From looking at the code, it > looks like putting the clear

[Lldb-commits] [PATCH] D134882: [lldb] Move breakpoint hit reset code to Target::CleanupProcess

labath updated this revision to Diff 465012. labath added a comment. Check breakpoint hit counts after termination Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134882/new/ https://reviews.llvm.org/D134882 Files: lldb/source/Target/Process.cpp

[Lldb-commits] [PATCH] D134906: Have MemoryCache cache addresses that were unreadable, so duplicate read attempts can be suppressed

labath added a comment. I don't know about debugserver, but both lldb-server and gdbserver currently return an error when the memory is partially accessible, even though the protocol explicitly allows the possibility of truncated reads. It is somewhat hard to reproduce, because the caching

[Lldb-commits] [PATCH] D134882: [lldb] Move breakpoint hit reset code to Target::CleanupProcess

labath added a comment. Here's an even better idea. I've now doing it in Target::CleanupProcess, right next to the code for resetting **watch**point hit counts. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134882/new/

[Lldb-commits] [PATCH] D134882: [lldb] Move breakpoint hit reset code to Target::CreateProcess

labath updated this revision to Diff 464700. labath added a comment. use CleanupProcess instead Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134882/new/ https://reviews.llvm.org/D134882 Files: lldb/source/Target/Process.cpp

[Lldb-commits] [PATCH] D134656: [LLDB][NativePDB] Add class/union layout bit size.

labath added a comment. In D134656#3819025 , @zequanwu wrote: >>> For members from parent classes, pdb only has information saying that its >>> direct parents class are at some offsets for this class. For class without >>> vtable, it's easy to

[Lldb-commits] [PATCH] D135029: [lldb] [gdb-remote] Isolate ClientBase from Communication

labath added inline comments. Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteClientBase.h:116-118 + GDBRemoteCommunication () { +return m_comm; + } Is the plan to make this private/protected at some point, or something like that? Otherwise,

[Lldb-commits] [PATCH] D134991: [lldb] Add diagnostics

labath added a subscriber: rupprecht. labath added a comment. Adding @rupprecht, as he might be interested in following this. I don't want to get too involved in this, but the first thought on my mind is "do you really want to do all this from within a signal handler?". The fact that we're

[Lldb-commits] [PATCH] D134877: [lldb] Get rid of __STDC_LIMIT_MACROS and __STDC_CONSTANT_MACROS

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

[Lldb-commits] [PATCH] D134882: [lldb] Move breakpoint hit reset code to Target::CreateProcess

labath added a comment. I'm not sure what kind of guarantees are you looking for. `m_process_sp` is a private member of the Target class, so it's not like just anyone can come in and change it. There's no way to stop code from inside the Target class from changing it without going through the

[Lldb-commits] [PATCH] D134877: [lldb] Fixes for swig-4.1.0 macro definition correction

labath added a comment. sure, sounds good. TBH, I am not sure if either of those is really needed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134877/new/ https://reviews.llvm.org/D134877 ___

[Lldb-commits] [PATCH] D134882: [lldb] Move breakpoint hit reset code to Target::CreateProcess

labath created this revision. labath added reviewers: fdeazeve, jingham. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB. This ensures it is run regardless of the method we use to initiate the session (previous version did not handle connects).

[Lldb-commits] [PATCH] D134877: [lldb] Fixes for swig-4.1.0 macro definition correction

labath added a comment. Looking at https://bugzilla.redhat.com/show_bug.cgi?id=2128646, I'd say that the real bug is that we're defining this macro in two places. How about we leave these definitions as they are, and remove the one at `bindings/interfaces.swig:5` ? Repository: rG LLVM

[Lldb-commits] [PATCH] D134842: Improve dynamic loader support in DynamicLoaderPOSIXDYLD when using core files.

labath added a comment. In D134842#3822740 , @yinghuitan wrote: > I am surprised other major companies did not hit this issue. That could be because this is something specific to your environment. Just to be clear, is this happening for *all* core

[Lldb-commits] [PATCH] D134849: [LLDB][NativePDB] Fix struct layout when it has anonymous unions.

labath added a comment. The fact that MSVC does not store all of the anonymous union data is unfortunate, though not entirely surprising, given that the goal of debug info never was to offer an exact reconstruction of the source AST. That, I'm not sure if checking for matching initial offsets

[Lldb-commits] [PATCH] D134768: [NFCI] More TypeCategoryImpl refactoring.

labath added a comment. FWIW, phabricator allows you to create stacked patches. Just base your diff on top of the previous patch, and then list that patch as a "parent revision" for the new one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D134771: [NFCI] Simplify TypeCategoryImpl for-each callbacks.

labath added inline comments. Comment at: lldb/include/lldb/DataFormatters/TypeCategory.h:136 + void ForEach(std::function)> + callback) { did you want to add a reference here? A const by-value argument is not particularly useful.

[Lldb-commits] [PATCH] D134656: [LLDB][NativePDB] Add class/union layout bit size.

labath added a comment. In D134656#3818759 , @zequanwu wrote: > In D134656#3818234 , @labath wrote: > >> That said, I am kinda surprised that this is the whole thing. I would have >> expected to see more here.

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

labath added a comment. In D133858#3818755 , @jingham wrote: > If we use Target::CreateProcess to handle this task, how do we ensure that > that will always get called any time we make a new process? I'm not really sure how to answer that. Clearly

[Lldb-commits] [PATCH] D134754: [lldb/gdb-server] Better reporting of launch errors

labath created this revision. labath added reviewers: mgorny, jasonmolenda. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB. Use our "rich error" facility to propagate error reported by the stub to the user. lldb-server reports rich launch

[Lldb-commits] [PATCH] D134570: [lldb] Skip check for conflicting filter/synth when adding a new regex.

labath added subscribers: jingham, labath. labath added a reviewer: jingham. labath added a comment. The current behavior is clearly not useful, but I don't have any context to be able to determine whether there's something else that can be done about that. @jingham might have it. CHANGES

[Lldb-commits] [PATCH] D134518: [lldb][COFF] Add note to forwarder export symbols in symtab

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. In D134518#3811382 , @alvinhochun wrote: > In D134518#3811218 , @labath wrote: > >> Ok, so is there any

[Lldb-commits] [PATCH] D134426: [lldb][COFF] Match symbols from COFF symbol table to export symbols

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Seems fine to me from a general perspective. I think others have already checked the windows parts. Comment at:

[Lldb-commits] [PATCH] D134636: [lldb][Windows] Always call SetExecutableModule on debugger connected

labath accepted this revision. labath added inline comments. This revision is now accepted and ready to land. Comment at: lldb/test/Shell/Target/dependent-modules-nodupe-windows.test:21-22 # CHECK-NEXT: .main.exe -# CHECK-NEXT: .shlib.dll +# CHECK-NEXT: ntdll.dll +# CHECK-NEXT:

[Lldb-commits] [PATCH] D134661: [lldb][TypeSystemClang] Honor DW_AT_rvalue_reference when creating C++ FunctionPrototypes

labath accepted this revision. labath added a subscriber: zequanwu. labath added a comment. This revision is now accepted and ready to land. Looks straight forward enough. Tagging @zequanwu, as he might want to do something similar for PDB. Comment at:

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

labath added a comment. In D133858#3816105 , @fdeazeve wrote: > In D133858#3805630 , @labath wrote: > >> I am afraid that this patch misses one method of initiating a debug session >> -- connecting to a running

[Lldb-commits] [PATCH] D134333: When there are variable errors, display an error in VS Code's local variables view.

labath added a comment. The vscode change seems fine to me, but I don't consider myself an authority on that. In D134333#3812653 , @jingham wrote: > In D134333#3812448 , @clayborg > wrote: > >> I just did a

[Lldb-commits] [PATCH] D134581: [lldb] Prevent re-adding a module that is already loaded

labath added a comment. In D134581#3817457 , @alvinhochun wrote: > In D134581#3815766 , @jasonmolenda > wrote: > >> In D134581#3813757 , @alvinhochun >> wrote: >> >>>

[Lldb-commits] [PATCH] D134656: [LLDB][NativePDB] Add class/union layout bit size.

labath added a comment. That said, I am kinda surprised that this is the whole thing. I would have expected to see more here. In dwarf we specify the offsets of individual class members. Does PDB encode that information? If not, how does it handle the case when the definition of some class is

[Lldb-commits] [PATCH] D134656: [LLDB][NativePDB] Add class/union layout bit size.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. LGTM modulo formatting. Comment at: lldb/test/Shell/SymbolFile/NativePDB/packed_class_layout.cpp:24 + +union __attribute__((packed, aligned(1))) U { + char c[2];

[Lldb-commits] [PATCH] D134041: [LLDB] Enable non-trivial types in EmulateInstruction::Context

labath added a comment. In D134041#3806994 , @jasonmolenda wrote: > In D134041#3805941 , @labath wrote: > >> In D134041#3805034 , >> @DavidSpickett wrote: >> That

[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

labath added a comment. lol, I knew about the last two parts (not that I agree with them, but I think we have about an equal amount of code which relies on it, and that which tries to work around it), but the first one is really weird. I think we have invented a middle ground between sign- and

[Lldb-commits] [PATCH] D134333: When there are variable errors, display an error in VS Code's local variables view.

labath added a comment. In D134333#3807511 , @clayborg wrote: > In D134333#3805945 , @labath wrote: > >> Do we actually promise that strings returned by the SB API will live >> forever? That's not something *I*

[Lldb-commits] [PATCH] D134518: [lldb][COFF] Skip forwarder export symbols in symtab

labath added a comment. In D134518#3811206 , @alvinhochun wrote: > In D134518#3811160 , @labath wrote: > >> Where is the "dll description string" that they point to? Could they be made >> to point to that? > >

[Lldb-commits] [PATCH] D132815: [LLDB] Do not dereference promise pointer in `coroutine_handle` pretty printer

labath accepted this revision. labath added a comment. Seems fine (with the caveat that all I know about coroutines was learned in this review). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132815/new/ https://reviews.llvm.org/D132815

[Lldb-commits] [PATCH] D134426: [lldb][COFF] Match symbols from COFF symbol table to export symbols

labath added inline comments. Comment at: lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp:892 } + std::sort(export_list.begin(), export_list.end(), RVASymbolListCompareRVA); + return export_list; Can you have multiple symbols pointing to the same

[Lldb-commits] [PATCH] D134509: [LLDB][NativePDB] Let native pdb use class layout in debug info.

labath added a comment. It's not clear to me how much of this patch is pure refactoring, and how much of it is adding new features. It would have been better to split that out into two patches. I see some layout handling code in `UdtRecordCompleter` constructor, but that's just two lines of

[Lldb-commits] [PATCH] D134518: [lldb][COFF] Skip forwarder export symbols in symtab

labath added a comment. Where is the "dll description string" that they point to? Could they be made to point to that? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134518/new/ https://reviews.llvm.org/D134518

[Lldb-commits] [PATCH] D134518: [lldb][COFF] Skip forwarder export symbols in symtab

labath added a comment. They may not be useful (at the moment), but if they're not actively causing harm (e.g. stopping some other feature from functioning, or if we're badly misrepresenting them in the symtab output), then I think we should keep them. You never know -- maybe someone will find

[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Ok, sounds good then. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134493/new/ https://reviews.llvm.org/D134493

[Lldb-commits] [PATCH] D134344: [WIP][lldb][test] 1 - Remove gmodules debug_info variant: add decorator for API tests that explicitly test gmodules

labath added a comment. In D134344#3805953 , @Michael137 wrote: > that would require an audit of each API test right? Not really. I think this could be a purely mechanical change which replaces `NO_DEBUG_INFO_TESTCASE = False` with something

[Lldb-commits] [PATCH] D134493: [lldb][TypeSystemClang] Deduce lldb::eEncodingUint for unsigned enum types

labath added a comment. In D134493#3810290 , @Michael137 wrote: > Wasn't sure how to properly test this since the original reproducer > technically relies on implementation-defined behaviour (i.e., initialising a > bitfield with an out-of-range

[Lldb-commits] [PATCH] D134333: When there are variable errors, display an error in VS Code's local variables view.

labath added a comment. Do we actually promise that strings returned by the SB API will live forever? That's not something *I* would want to promote. I always though that we're ConstStringifying the return values only when we don't know any better... Repository: rG LLVM Github Monorepo

[Lldb-commits] [PATCH] D134041: [LLDB] Enable non-trivial types in EmulateInstruction::Context

labath added a comment. In D134041#3805034 , @DavidSpickett wrote: >> That said I would *love* is someone changed the RegisterInfo structs into >> something saner, but I think that will need to be more elaborate than simply >> stuffing a std::vector

[Lldb-commits] [PATCH] D134344: [WIP][lldb][test] 1 - Remove gmodules debug_info variant: add decorator for API tests that explicitly test gmodules

labath added a comment. celebration_balloons I like this a lot. Incidentally, I believe the main reason these tests don't work on non-darwin platforms is because their system

[Lldb-commits] [PATCH] D133858: [lldb] Reset breakpoint hit count before new runs

labath added a comment. I am afraid that this patch misses one method of initiating a debug session -- connecting to a running debug server (`process connect`, `SBTarget::ConnectRemote`). The breakpoint hit counts don't get reset in case of a reconnect. This isn't a particularly common use

[Lldb-commits] [PATCH] D133910: [NFCI] Refactor FormatterContainerPair into TieredFormatterContainer.

labath added inline comments. Comment at: lldb/include/lldb/DataFormatters/TypeCategory.h:76 +uint32_t result = 0; +for (auto sc : m_subcontainers) { + result += sc->GetCount(); According to

[Lldb-commits] [PATCH] D134196: [lldb][COFF] Rewrite ParseSymtab to list both export and symbol tables

labath added a comment. I don't know how coff symbol tables work, but this seems reasonable to me. Comment at: lldb/test/Shell/ObjectFile/PECOFF/symbols-export-table.yaml:10 +# CHECK-NEXT: D Code 0x0001800010100x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} exportfunc +#

[Lldb-commits] [PATCH] D134033: [lldb/Plugins] Improve error reporting with reading/writing memory in a Scripted Process (WIP)

labath added inline comments. Comment at: lldb/include/lldb/API/SBError.h:95 private: - std::unique_ptr m_opaque_up; + std::shared_ptr m_opaque_sp; This is technically an ABI break (changes `sizeof(SBError)`). I don't care, but someone might.

[Lldb-commits] [PATCH] D133906: [lldb] Generate lldb-forward with .def file

labath added a comment. In D133906#3792666 , @JDevlieghere wrote: > In D133906#3792230 , @labath wrote: > >> // no .def file >> #define LLDB_FORWARD_CLASS(cls) \ >> namespace lldb_private { class cls; }

[Lldb-commits] [PATCH] D134041: [LLDB] Enable non-trivial types in EmulateInstruction::Context

labath added a comment. I think the main reason that the RegisterInfo struct is such as it is, is because it wants to be trivially constructible (or whatever the c++ term for that is). I'm pretty sure that adding a std::string member will make it stop being that, and I don't think we suddenly

[Lldb-commits] [PATCH] D134111: [lldb] Add newline in output of `target modules lookup`

labath accepted this revision. labath added inline comments. This revision is now accepted and ready to land. Comment at: lldb/test/Shell/Commands/command-target-modules-lookup.test:13 +# CHECK-NEXT: Summary: [[MODULE]]`someOtherFunc(double) +# CHECK-NOT: ignoreThisFunction

[Lldb-commits] [PATCH] D134066: [LLDB][NativePDB] Forcefully complete a record type it has incomplete type debug info.

labath added inline comments. Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:681 +auto = llvm::cast(*ct.GetTypeSystem()); +ts.GetMetadata()->SetIsForcefullyCompleted(); + } zequanwu wrote: > rnk wrote: > > Is this

[Lldb-commits] [PATCH] D133906: [lldb] Generate lldb-forward with .def file

labath added a comment. In D133906#3791352 , @JDevlieghere wrote: > In D133906#3791153 , @jingham wrote: > >> This patch makes me a little sad because it breaks the "Jump to Definition" >> chain in Xcode (and I

[Lldb-commits] [PATCH] D133446: [LLDB][NativePDB] Global ctor and dtor should be global decls.

labath added a comment. In D133446#3787362 , @zequanwu wrote: > In D133446#3786561 , @labath wrote: > >> In D133446#3780961 , @zequanwu >> wrote: >> >>> In

[Lldb-commits] [PATCH] D132954: lldb: Add support for R_386_32 relocations to ObjectFileELF

labath added a comment. Yeah, we can enable them. I've done that now in d079bf33 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132954/new/ https://reviews.llvm.org/D132954

[Lldb-commits] [PATCH] D132954: lldb: Add support for R_386_32 relocations to ObjectFileELF

This revision was automatically updated to reflect the committed changes. Closed by commit rGf4fc4056319d: lldb: Add support for R_386_32 relocations to ObjectFileELF (authored by dmlary, committed by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[Lldb-commits] [PATCH] D133446: [LLDB][NativePDB] Global ctor and dtor should be global decls.

labath added a comment. In D133446#3780961 , @zequanwu wrote: > In D133446#3779600 , @labath wrote: > >> I believe that this fixes the crash, but the names of generated functions >> still look fairly weird.

[Lldb-commits] [PATCH] D133763: WIP strawman building perf.cpp on an older kernel

labath added a comment. (This file is only used for the processor trace feature, and one can debug just fine without it. So, I'd just conditionally compile out the processor trace support on older kernels -- it's highly questionable whether this works there anyway). Repository: rG LLVM

[Lldb-commits] [PATCH] D133240: [Formatters][NFCI] Replace 'is_regex' arguments with an enum.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. This seems in line with what was being discussed. Jim, do you have any thoughts on this? Comment at: lldb/include/lldb/lldb-enumerations.h:841 + + kNumFormatterMatchTypes,

[Lldb-commits] [PATCH] D133129: [lldb] Add boilerplate for debugger interrupts

labath added a comment. In D133129#3778125 , @jingham wrote: > To be clear, I'm not trying to implement a preemptive interrupt using these > callbacks. There are so many places where lldb is doing work that you really > can't interrupt - e.g. we can't

[Lldb-commits] [PATCH] D133427: [gdb-remote] Move broadcasting logic down to GDBRemoteClientBase

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. cool. Comment at: lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationTest.cpp:20 public: - TestClient() - : GDBRemoteCommunication("test.client",

[Lldb-commits] [PATCH] D132954: lldb: Add support for R_386_32 relocations to ObjectFileELF

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Cool. Thanks for figuring this out. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132954/new/ https://reviews.llvm.org/D132954 ___

[Lldb-commits] [PATCH] D133352: [lldb-server] Report launch error in vRun packets

This revision was automatically updated to reflect the committed changes. Closed by commit rG681d0d9e5f05: [lldb-server] Report launch error in vRun packets (authored by labath). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133352/new/

[Lldb-commits] [PATCH] D133410: [lldb] Fix ThreadedCommunication races

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

[Lldb-commits] [PATCH] D133164: Add the ability to show when variables fails to be available when debug info is valid.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks good. Thanks. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:4161 + if (command) { +if (command->contains(" -gline-tables-only")) +

[Lldb-commits] [PATCH] D133461: [LLDB][NativePDB] Set block address range.

labath added inline comments. Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:372 +func->GetAddressRange().GetBaseAddress().GetFileAddress(); +Block::Range range = Block::Range(block_base - func_base, block.CodeSize); +if (block_base

[Lldb-commits] [PATCH] D133534: Complete support of loading a darwin kernel over a live gdb-remote connection given the address of a mach-o fileset

labath added inline comments. Comment at: lldb/unittests/Interpreter/CMakeLists.txt:17-18 lldbInterpreter + lldbPluginDynamicLoaderDarwinKernel + lldbPluginObjectContainerMachOFileset lldbPluginPlatformMacOSX These dependencies should be

[Lldb-commits] [PATCH] D133519: Document some of the clang-specific DWARF extensions supported by LLDB

labath accepted this revision. labath added a comment. cool Comment at: lldb/docs/use/extensions.rst:84 + DW_TAG_compile_unit + DW_AT_GNU_dwo_id (0xabcdef) + DW_AT_GNU_dwo_name("M.pcm") Michael137 wrote: > Is it worth commenting on the

[Lldb-commits] [PATCH] D133530: [lldb] Add zstd support

labath added a comment. Seems fine to me, though you may want to have a llvm test for the new functionality, given that the patch is exclusively changing llvm code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133530/new/

[Lldb-commits] [PATCH] D133393: [test] Use either `127.0.0.1` or `[::1]` to run in ipv6-only environments.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. In D133393#3775995 , @rupprecht wrote: > In D133393#3773793 , @labath wrote: > >> I believe the reasons are

[Lldb-commits] [PATCH] D133446: [LLDB][NativePDB] Global ctor and dtor should be global decls.

labath added a comment. I believe that this fixes the crash, but the names of generated functions still look fairly weird. Could we create them using their mangled name instead? That way, someone might actually call them, if he was so inclined. Repository: rG LLVM Github Monorepo CHANGES

[Lldb-commits] [PATCH] D133164: Add the ability to show when variables fails to be available when debug info is valid.

labath added inline comments. Comment at: lldb/include/lldb/Target/StackFrame.h:264 /// A pointer to a list of variables. - VariableList *GetVariableList(bool get_file_globals); + VariableList *GetVariableList(bool get_file_globals, Status *error_ptr);

[Lldb-commits] [PATCH] D133410: [lldb] Fix ThreadedCommunication races

labath added inline comments. Comment at: lldb/source/Core/ThreadedCommunication.cpp:113 - if (event_type & eBroadcastBitReadThreadDidExit) { -// If the thread exited of its own accord, it either means it -// hit an end-of-file condition or an error. -

[Lldb-commits] [PATCH] D133352: [lldb-server] Report launch error in vRun packets

labath updated this revision to Diff 458408. labath added a comment. Use `pos` instead of the previous size in the resize expression. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133352/new/ https://reviews.llvm.org/D133352 Files:

[Lldb-commits] [PATCH] D133352: [lldb-server] Report launch error in vRun packets

labath added inline comments. Comment at: lldb/source/Host/posix/ProcessLauncherPosixFork.cpp:293 +r = llvm::sys::RetryAfterSignal(-1, read, pipe.GetReadFileDescriptor(), pos, +buf.end() - pos); + } while (r > 0);

[Lldb-commits] [PATCH] D133352: [lldb-server] Report launch error in vRun packets

labath updated this revision to Diff 458405. labath added a comment. dynamically resize error buffer Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133352/new/ https://reviews.llvm.org/D133352 Files:

[Lldb-commits] [PATCH] D133129: [lldb] Add boilerplate for debugger interrupts

labath added a comment. I've added you to D133410 , as I think it demonstrates very well the difficulties in getting two threads to talk to one another. This code has been here since forever (and may have contributed to your desire to introduce interrupts),

[Lldb-commits] [PATCH] D133410: [lldb] Fix ThreadedCommunication races

labath created this revision. labath added reviewers: mgorny, JDevlieghere. Herald added a project: All. labath requested review of this revision. Herald added a project: LLDB. The Read function could end up blocking if data (or EOF) arrived just as it was about to start waiting for the events.

[Lldb-commits] [PATCH] D133410: [lldb] Fix ThreadedCommunication races

labath added a comment. I've only seen one flake on linux so far, but the windows bot seems more susceptible to this

[Lldb-commits] [PATCH] D133181: [test] Remove problematic thread from MainLoopTest to fix flakiness

labath added a comment. In D133181#3772830 , @rupprecht wrote: > In D133181#3771747 , @labath wrote: > >> (I've reverted the pthread_kill part, as the mac build did not like it.) > > Thanks! I didn't get any

[Lldb-commits] [PATCH] D133129: [lldb] Add boilerplate for debugger interrupts

labath added a comment. I don't know how this is intended to be used, but I find it a rather heavyweight approach towards implementing, well... anything. The idea that something as innocuous as SBAddress::IsValid() will end up iterating through _all_ SBDebugger objects seems wrong, regardless

[Lldb-commits] [PATCH] D133243: [LLDB][NativePDB] Fix PdbAstBuilder::GetParentDeclContextForSymbol when ICF happens.

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Well, I still don't claim to understand this code, but I am happy to stamp anything that fixes bugs by deleting code. Just as an idea for future work, you may want to check that we're

[Lldb-commits] [PATCH] D133393: [test] Use localhost in place of 127.0.0.1 to run in ipv6-only environments.

labath added a comment. I believe the reasons are still relevant. Basically the problem is that listening on `localhost:x` creates two sockets (one for 127.0.0.1, one for ::1), and there's no way to guarantee that we'll be able to grab the same port for both (one could be taken by some other

[Lldb-commits] [PATCH] D132940: [lldb] Use just-built libcxx for tests when available

labath added inline comments. Comment at: lldb/test/API/commands/expression/fixits/TestFixIts.py:54 +# FIXME: LLDB struggles with this when stdlib has debug info. +if not lldbutil.has_debug_info_in_libcxx(target): +

[Lldb-commits] [PATCH] D133352: [lldb-server] Report launch error in vRun packets

labath added inline comments. Comment at: lldb/test/API/tools/lldb-server/TestGdbRemoteLaunch.py:18 +args = [exe_path, "stderr:arg1", "stderr:arg2", "stderr:arg3"] +hex_args = [seven.hexlify(x) for x in args] + mgorny wrote: > Since we no longer

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