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

hawkinsw added a comment. I hope that these comments are helpful. If they are not, please feel free to tell me to stop! I appreciated learning from reading through your discussion with @labath ! Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:265 +:param d

[Lldb-commits] [PATCH] D132510: [RISCV][LLDB] Add initial SysV ABI support

kasper81 added a comment. @jasonmolenda, @Emmmer, any other feedback, or good to merge? i will work on `Plugins/Architecture/RISCV`, unless someone else beat me to it. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132510/new/ https://reviews.ll

[Lldb-commits] [PATCH] D132578: [lldb] [Core] Use thread for Communication::Write() as well

mgorny updated this revision to Diff 458017. mgorny added a comment. Rebase on top of D131160 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132578/new/ https://reviews.llvm.org/D132578 Files: lldb/include/lldb/Core/Communication.h lldb/source/Cor

[Lldb-commits] [PATCH] D132283: [lldb] [Core] Reimplement Communication::ReadThread using MainLoop

mgorny updated this revision to Diff 458016. mgorny added a comment. Rebase on top of D131160 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132283/new/ https://reviews.llvm.org/D132283 Files: lldb/include/lldb/Core/Communication.h lldb/source/Cor

[Lldb-commits] [PATCH] D131160: [WIP][lldb] Add "event" capability to the MainLoop class

mgorny accepted this revision. mgorny added a comment. This revision is now accepted and ready to land. LGTM. I've just rebased and tested my patches on top of this, and they seem to work. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131160/new/

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

labath accepted this revision. labath added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/make/Makefile.rules:381-386 +ifneq ($(and $(USE_LIBSTDCPP), $(USE_LIBCPP)),) + $(error Libcxx and Libstdc++ cannot be used together) +endif + +ifeq (1, $(USE_SYSTEM_

[Lldb-commits] [PATCH] D132395: [lldb] [gdb-remote] Use Communication::WriteAll() over Write()

labath added a comment. In D132395#3745227 , @mgorny wrote: > Yeah, I meant inside LLDB, since that's what we're talking about. Basically, > I'm wondering if we should perhaps rename `WriteAll()` to `Write()`, remove > the old `Write()` method and just

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

labath added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp:246-248 + DataExtractor data(&promise_addr, sizeof(promise_addr), + process_sp->GetByteOrder(), + process_sp->GetAddressByteSize()); --

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

labath added a comment. In D132954#3767083 , @dmlary wrote: > @labath it doesn't look like the relocations are applied during `lldb-test > object-file --contents`: I'm definitely seeing `ObjectFileELF::RelocateSection` be invoked with this command (I'm

[Lldb-commits] [PATCH] D133251: [lldb] [Core] Split read thread support into ThreadedCommunication

mgorny marked an inline comment as done. mgorny added inline comments. Comment at: lldb/source/Core/CMakeLists.txt:57 StreamFile.cpp + ThreadedCommunication.cpp UserSettingsController.cpp labath wrote: > I think you're missing this file. Indeed I were. Tha

[Lldb-commits] [PATCH] D133251: [lldb] [Core] Split read thread support into ThreadedCommunication

mgorny updated this revision to Diff 457984. mgorny added a comment. Add the missing file. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133251/new/ https://reviews.llvm.org/D133251 Files: lldb/include/lldb/API/SBCommunication.h lldb/include/lldb/Core/Communication.h lldb/include

[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] D132578: [lldb] [Core] Use thread for Communication::Write() as well

mgorny added a comment. In D132578#3770124 , @labath wrote: > Seems reasonable. Just be aware that the packet RTT is very important for > some, so we will either make sure that the extra thread is not used in the > base case (no processes running), or s

[Lldb-commits] [PATCH] D132283: [lldb] [Core] Reimplement Communication::ReadThread using MainLoop

labath added inline comments. Comment at: lldb/source/Core/Communication.cpp:427 // Notify the read thread. - m_connection_sp->InterruptRead(); mgorny wrote: > labath wrote: > > mgorny wrote: > > > labath wrote: > > > > Have you considered putting this code

[Lldb-commits] [PATCH] D132578: [lldb] [Core] Use thread for Communication::Write() as well

labath added a comment. In D132578#3769078 , @mgorny wrote: > Ok, so I guess I'm back the original plan of using async thread to do the > packet exchanges. Right now, the rough plan is to: > > 1. Try to clean up / streamline the comms API a bit. > 2. Int

[Lldb-commits] [PATCH] D133251: [lldb] [Core] Split read thread support into ThreadedCommunication

labath added a comment. I like this a lot. Comment at: lldb/source/Core/CMakeLists.txt:57 StreamFile.cpp + ThreadedCommunication.cpp UserSettingsController.cpp I think you're missing this file. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1332

[Lldb-commits] [PATCH] D131160: [WIP][lldb] Add "event" capability to the MainLoop class

labath added a comment. In D131160#3763936 , @mgorny wrote: > In D131160#3738805 , @labath wrote: > >> What's the reasoning behind `TriggerPendingCallbacks`? I was assuming that >> the addition of a callback woul

[Lldb-commits] [PATCH] D131160: [WIP][lldb] Add "event" capability to the MainLoop class

labath updated this revision to Diff 457949. labath added a comment. - Make TriggerPendingCallbacks protected - fix the windows version Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131160/new/ https://reviews.llvm.org/D131160 Files: lldb/includ

[Lldb-commits] [PATCH] D131160: [WIP][lldb] Add "event" capability to the MainLoop class

labath commandeered this revision. labath edited reviewers, added: mgorny; removed: labath. labath added a comment. Back to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131160/new/ https://reviews.llvm.org/D131160 ___ lldb-commits mailin

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

labath added inline comments. Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp:631 +if (name_components.empty() || +base_name != name_components.back()->toString()) + continue; What if the two symbols have the same address (