[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)
https://github.com/SuibianP updated https://github.com/llvm/llvm-project/pull/121269 >From c08879c346a1f127324669fc6b74ce5a7407a30d Mon Sep 17 00:00:00 2001 From: Jialun Hu Date: Mon, 24 Feb 2025 22:10:17 +0800 Subject: [PATCH] [lldb-dap] Implement runInTerminal for Windows Currently, the named pipe is passed by name and a transient ofstream is constructed at each I/O request. This assumes, - Blocking semantics: FIFO I/O waits for the other side to connect. - Buffered semantics: Closing one side does not discard existing data. The former can be replaced by WaitNamedPipe/ConnectNamedPipe on Win32, but the second cannot be easily worked around. It is also impossible to have another "keep-alive" pipe server instance, as server-client pairs are fixed on connection on Win32 and the client may get connected to it instead of the real one. Refactor FifoFile[IO] to use an open file handles rather than file name. --- Win32 provides no way to replace the process image. Under the hood exec* actually creates a new process with a new PID. DebugActiveProcess also cannot get notified of process creations. Create the new process in a suspended state and resume it after attach. --- lldb/packages/Python/lldbsuite/test/dotest.py | 2 +- .../runInTerminal/TestDAP_runInTerminal.py| 3 - .../API/tools/lldb-dap/runInTerminal/main.c | 6 +- lldb/tools/lldb-dap/FifoFiles.cpp | 131 +++--- lldb/tools/lldb-dap/FifoFiles.h | 35 +++-- .../tools/lldb-dap/Handler/RequestHandler.cpp | 8 +- lldb/tools/lldb-dap/RunInTerminal.cpp | 52 --- lldb/tools/lldb-dap/RunInTerminal.h | 11 +- lldb/tools/lldb-dap/lldb-dap.cpp | 61 ++-- 9 files changed, 234 insertions(+), 75 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py b/lldb/packages/Python/lldbsuite/test/dotest.py index 681ea1638f2d6..538cece8b4913 100644 --- a/lldb/packages/Python/lldbsuite/test/dotest.py +++ b/lldb/packages/Python/lldbsuite/test/dotest.py @@ -545,7 +545,7 @@ def setupSysPath(): lldbDir = os.path.dirname(lldbtest_config.lldbExec) -lldbDAPExec = os.path.join(lldbDir, "lldb-dap") +lldbDAPExec = os.path.join(lldbDir, "lldb-dap.exe" if os.name == "nt" else "lldb-dap") if is_exe(lldbDAPExec): os.environ["LLDBDAP_EXEC"] = lldbDAPExec diff --git a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py index ac96bcc1364a2..18a92dd3fa335 100644 --- a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py +++ b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py @@ -43,7 +43,6 @@ def isTestSupported(self): except: return False -@skipIfWindows @skipIf(archs=no_match(["x86_64"])) def test_runInTerminal(self): if not self.isTestSupported(): @@ -113,7 +112,6 @@ def test_runInTerminalWithObjectEnv(self): self.assertIn("FOO", request_envs) self.assertEqual("BAR", request_envs["FOO"]) -@skipIfWindows @skipIf(archs=no_match(["x86_64"])) def test_runInTerminalInvalidTarget(self): if not self.isTestSupported(): @@ -132,7 +130,6 @@ def test_runInTerminalInvalidTarget(self): response["message"], ) -@skipIfWindows @skipIf(archs=no_match(["x86_64"])) def test_missingArgInRunInTerminalLauncher(self): if not self.isTestSupported(): diff --git a/lldb/test/API/tools/lldb-dap/runInTerminal/main.c b/lldb/test/API/tools/lldb-dap/runInTerminal/main.c index 676bd830e657b..9cc25b2e45a49 100644 --- a/lldb/test/API/tools/lldb-dap/runInTerminal/main.c +++ b/lldb/test/API/tools/lldb-dap/runInTerminal/main.c @@ -1,11 +1,13 @@ #include #include -#include + +#include +#include int main(int argc, char *argv[]) { const char *foo = getenv("FOO"); for (int counter = 1;; counter++) { -sleep(1); // breakpoint +thrd_sleep(&(struct timespec){.tv_sec = 1}, NULL); // breakpoint } return 0; } diff --git a/lldb/tools/lldb-dap/FifoFiles.cpp b/lldb/tools/lldb-dap/FifoFiles.cpp index 1f1bba80bd3b1..8069049d78519 100644 --- a/lldb/tools/lldb-dap/FifoFiles.cpp +++ b/lldb/tools/lldb-dap/FifoFiles.cpp @@ -8,8 +8,15 @@ #include "FifoFiles.h" #include "JSONUtils.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/Path.h" +#include "llvm/Support/WindowsError.h" -#if !defined(_WIN32) +#if defined(_WIN32) +#include +#include +#include +#else #include #include #include @@ -24,27 +31,76 @@ using namespace llvm; namespace lldb_dap { -FifoFile::FifoFile(StringRef path) : m_path(path) {} +FifoFile::FifoFile(std::string path, FILE *f) : m_path(path), m_file(f) {} + +Expected FifoFile::create(StringRef path) { + auto file = fopen(path.data(), "r+"); + if (file == nullptr) +return createStringError(inconvertibleErrorCode(), + "Failed to
[Lldb-commits] [lldb] [lldb] Prepare UnwindPlans for discontinuous functions (PR #127661)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/127661 >From 40a29a7ba10caeae2a02355abddb9469246cdd3e Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Tue, 18 Feb 2025 16:14:30 +0100 Subject: [PATCH] [lldb] Prepare UnwindPlans for discontinuous functions The main changes are: - changing the internal row representation from vector to a map. This wasn't strictly necessary, but I'm doing it because due the functions not starting at the lowest address, the Row for the function entry point (offset zero) may no longer be the first (zeroth) entry in the vector, so we will need to (binary) search for it. This would have been possible with a (sorted) vector representation as well, but that's somewhat tricky because the unwind plans are constructed in place, so there isn't a good place to insert a sort operations (the plans are currently implicitly sorted due to the sequential nature of their construction, but that will be harder to ensure when jumping between multiple regions. - changing the valid address range from singular to plural The changes to other files are due to the address range pluralization and due to the removal of `AppendRow` (as there isn't really an "append" operation in a map). --- lldb/include/lldb/Symbol/UnwindPlan.h | 32 +++--- .../Plugins/ABI/AArch64/ABIMacOSX_arm64.cpp | 4 +- .../Plugins/ABI/AArch64/ABISysV_arm64.cpp | 4 +- lldb/source/Plugins/ABI/ARC/ABISysV_arc.cpp | 2 +- lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp | 4 +- lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp | 4 +- .../Plugins/ABI/Hexagon/ABISysV_hexagon.cpp | 4 +- .../ABI/LoongArch/ABISysV_loongarch.cpp | 4 +- .../Plugins/ABI/MSP430/ABISysV_msp430.cpp | 4 +- lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp | 4 +- .../Plugins/ABI/Mips/ABISysV_mips64.cpp | 4 +- .../Plugins/ABI/PowerPC/ABISysV_ppc.cpp | 4 +- .../Plugins/ABI/PowerPC/ABISysV_ppc64.cpp | 4 +- .../Plugins/ABI/RISCV/ABISysV_riscv.cpp | 4 +- .../Plugins/ABI/SystemZ/ABISysV_s390x.cpp | 2 +- .../source/Plugins/ABI/X86/ABIMacOSX_i386.cpp | 4 +- lldb/source/Plugins/ABI/X86/ABISysV_i386.cpp | 4 +- .../source/Plugins/ABI/X86/ABISysV_x86_64.cpp | 4 +- .../Plugins/ABI/X86/ABIWindows_x86_64.cpp | 4 +- .../Instruction/ARM/EmulateInstructionARM.cpp | 2 +- .../ARM64/EmulateInstructionARM64.cpp | 2 +- .../MIPS/EmulateInstructionMIPS.cpp | 2 +- .../MIPS64/EmulateInstructionMIPS64.cpp | 2 +- .../PPC64/EmulateInstructionPPC64.cpp | 2 +- .../ObjectFile/PECOFF/PECallFrameInfo.cpp | 6 +- .../Plugins/Platform/Linux/PlatformLinux.cpp | 2 +- .../Breakpad/SymbolFileBreakpad.cpp | 18 ++-- .../x86/x86AssemblyInspectionEngine.cpp | 10 +- lldb/source/Symbol/ArmUnwindInfo.cpp | 2 +- lldb/source/Symbol/CompactUnwindInfo.cpp | 16 +-- lldb/source/Symbol/DWARFCallFrameInfo.cpp | 14 +-- lldb/source/Symbol/UnwindPlan.cpp | 98 +++ lldb/unittests/Symbol/CMakeLists.txt | 1 + lldb/unittests/Symbol/UnwindPlanTest.cpp | 76 ++ .../x86/Testx86AssemblyInspectionEngine.cpp | 24 ++--- 35 files changed, 209 insertions(+), 168 deletions(-) create mode 100644 lldb/unittests/Symbol/UnwindPlanTest.cpp diff --git a/lldb/include/lldb/Symbol/UnwindPlan.h b/lldb/include/lldb/Symbol/UnwindPlan.h index 462c1a52b01db..e8846cf111c9b 100644 --- a/lldb/include/lldb/Symbol/UnwindPlan.h +++ b/lldb/include/lldb/Symbol/UnwindPlan.h @@ -438,7 +438,7 @@ class UnwindPlan { // Performs a deep copy of the plan, including all the rows (expensive). UnwindPlan(const UnwindPlan &rhs) - : m_plan_valid_address_range(rhs.m_plan_valid_address_range), + : m_plan_valid_ranges(rhs.m_plan_valid_ranges), m_register_kind(rhs.m_register_kind), m_return_addr_register(rhs.m_return_addr_register), m_source_name(rhs.m_source_name), @@ -448,9 +448,8 @@ class UnwindPlan { m_plan_is_for_signal_trap(rhs.m_plan_is_for_signal_trap), m_lsda_address(rhs.m_lsda_address), m_personality_func_addr(rhs.m_personality_func_addr) { -m_row_list.reserve(rhs.m_row_list.size()); -for (const RowSP &row_sp : rhs.m_row_list) - m_row_list.emplace_back(new Row(*row_sp)); +for (const auto &[offset, row_sp] : rhs.m_rows) + m_rows.emplace(offset, std::make_shared(*row_sp)); } UnwindPlan(UnwindPlan &&rhs) = default; UnwindPlan &operator=(const UnwindPlan &rhs) { @@ -462,9 +461,7 @@ class UnwindPlan { void Dump(Stream &s, Thread *thread, lldb::addr_t base_addr) const; - void AppendRow(const RowSP &row_sp); - - void InsertRow(const RowSP &row_sp, bool replace_existing = false); + void InsertRow(const RowSP &row_sp, bool replace_existing = true); // Returns a pointer to the best row for the given offset into the function's // instructions. If offset is -1 it indicates tha
[Lldb-commits] [lldb] [lldb] Build the API unittests with -Wdocumentation (PR #128893)
https://github.com/labath approved this pull request. A slightly unusual setup, but I sort of like it. https://github.com/llvm/llvm-project/pull/128893 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (PR #128750)
https://github.com/ashgti updated https://github.com/llvm/llvm-project/pull/128750 >From b3f60a464deedae59d902f7417c74a64c5cbf5a1 Mon Sep 17 00:00:00 2001 From: John Harrison Date: Mon, 24 Feb 2025 15:07:13 -0800 Subject: [PATCH 1/4] [lldb-dap] Use existing lldb::IOObjectSP for IO (NFC). This simplifies the IOStream.cpp implementation by building on top of the existing lldb::IOObjectSP. Additionally, this should help ensure clients connected of a `--connection` specifier properly detect shutdown requests when the Socket is closed. Previously, the StreamDescriptor was just accessing the underyling native handle and was not aware of the `Close()` call to the Socket itself. --- lldb/tools/lldb-dap/DAP.cpp | 3 +- lldb/tools/lldb-dap/DAP.h| 9 +-- lldb/tools/lldb-dap/IOStream.cpp | 117 --- lldb/tools/lldb-dap/IOStream.h | 38 ++ lldb/tools/lldb-dap/lldb-dap.cpp | 36 +- 5 files changed, 45 insertions(+), 158 deletions(-) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 01f294e14de6a..95f2e5c6521c2 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -17,6 +17,7 @@ #include "lldb/API/SBListener.h" #include "lldb/API/SBProcess.h" #include "lldb/API/SBStream.h" +#include "lldb/Utility/IOObject.h" // IWYU pragma: keep #include "lldb/Utility/Status.h" #include "lldb/lldb-defines.h" #include "lldb/lldb-enumerations.h" @@ -59,7 +60,7 @@ const char DEV_NULL[] = "/dev/null"; namespace lldb_dap { DAP::DAP(std::string name, llvm::StringRef path, std::ofstream *log, - StreamDescriptor input, StreamDescriptor output, ReplMode repl_mode, + lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode, std::vector pre_init_commands) : name(std::move(name)), debug_adaptor_path(path), log(log), input(std::move(input)), output(std::move(output)), diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index f87841a56f4d3..b4e77b78c5273 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -29,6 +29,7 @@ #include "lldb/API/SBThread.h" #include "lldb/API/SBValue.h" #include "lldb/API/SBValueList.h" +#include "lldb/lldb-forward.h" #include "lldb/lldb-types.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" @@ -125,21 +126,21 @@ struct Variables { struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {}; + explicit StartDebuggingRequestHandler(DAP &d) : dap(d){}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit ReplModeRequestHandler(DAP &d) : dap(d) {}; + explicit ReplModeRequestHandler(DAP &d) : dap(d){}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; struct SendEventRequestHandler : public lldb::SBCommandPluginInterface { DAP &dap; - explicit SendEventRequestHandler(DAP &d) : dap(d) {}; + explicit SendEventRequestHandler(DAP &d) : dap(d){}; bool DoExecute(lldb::SBDebugger debugger, char **command, lldb::SBCommandReturnObject &result) override; }; @@ -211,7 +212,7 @@ struct DAP { std::string last_nonempty_var_expression; DAP(std::string name, llvm::StringRef path, std::ofstream *log, - StreamDescriptor input, StreamDescriptor output, ReplMode repl_mode, + lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode, std::vector pre_init_commands); ~DAP(); DAP(const DAP &rhs) = delete; diff --git a/lldb/tools/lldb-dap/IOStream.cpp b/lldb/tools/lldb-dap/IOStream.cpp index 7d0f363440f53..d8543b560c050 100644 --- a/lldb/tools/lldb-dap/IOStream.cpp +++ b/lldb/tools/lldb-dap/IOStream.cpp @@ -7,125 +7,34 @@ //===--===// #include "IOStream.h" +#include "lldb/Utility/IOObject.h" // IWYU pragma: keep +#include "lldb/Utility/Status.h" #include #include -#if defined(_WIN32) -#include -#else -#include -#include -#include -#endif - using namespace lldb_dap; -StreamDescriptor::StreamDescriptor() = default; - -StreamDescriptor::StreamDescriptor(StreamDescriptor &&other) { - *this = std::move(other); -} - -StreamDescriptor::~StreamDescriptor() { - if (!m_close) -return; - - if (m_is_socket) -#if defined(_WIN32) -::closesocket(m_socket); -#else -::close(m_socket); -#endif - else -::close(m_fd); -} - -StreamDescriptor &StreamDescriptor::operator=(StreamDescriptor &&other) { - m_close = other.m_close; - other.m_close = false; - m_is_socket = other.m_is_socket; - if (m_is_socket) -m_socket = other.m_socket; - else -m_fd = other.m_fd; - return *this; -} - -StreamDescriptor Strea
[Lldb-commits] [lldb] [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (PR #128750)
ashgti wrote: > To reliably force termination, I'd suggest changing the read thread to use a > MainLoop object reading from the socket, and then terminating it via > `loop.AddPendingCallback([&](MainLoopBase &loop) { > loop.RequestTermination(); });`. It sounds a bit like overkill, but the main > loop class is currently our multiplexing primitive with the widest platform > support. It also goes along nicely with the IOObject classes you're > introducing here. And when/if we have a way (see the Pipe patch) for it to > handle pipes as well, we could have it handle stdout/err forwarding as well, > and ditch the forwarding threads. The one issue with this is that MainLoop on Windows only supports sockets, not Files. At the moment we primarily work over stdin/stdout for communication, although the `--connection` flag is in now, so we can support sockets. > > (I can also imagine a setup where we reuse the top-level MainLoop object (the > one that currently accepts connections). In this world the main thread would > pull json data from (all) connections, and enqueue them to > connection-specific request queues. The nice part about this approach is that > there are fewer threads flying around, so it's easier to understand what's > happening and easier to shut things down. The disadvantage is that the main > thread becomes a bit of a choke point -- though I don't expect this to matter > much in practice since we will usually only have one connection and I doubt > that json (de)serialization is going to be the bottleneck) I do think we could merge some of these threads if MainLoop supported Files on Windows. That may be worth tackling specifically to unblock this behavior. https://github.com/llvm/llvm-project/pull/128750 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 317461e - [lldb-dap] Avoid a std::string allocation for the help output (NFC)
Author: Jonas Devlieghere Date: 2025-02-26T11:56:23-06:00 New Revision: 317461ed61002de7f6e54ab0a26780c6d2726bb0 URL: https://github.com/llvm/llvm-project/commit/317461ed61002de7f6e54ab0a26780c6d2726bb0 DIFF: https://github.com/llvm/llvm-project/commit/317461ed61002de7f6e54ab0a26780c6d2726bb0.diff LOG: [lldb-dap] Avoid a std::string allocation for the help output (NFC) Don't create a temporary `std::string` for the help output, just write it to `llvm::outs()` directly. Added: Modified: lldb/tools/lldb-dap/lldb-dap.cpp Removed: diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index 3b029b2ef047a..1c6bd7e903409 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -161,7 +161,7 @@ static void printHelp(LLDBDAPOptTable &table, llvm::StringRef tool_name) { std::string usage_str = tool_name.str() + " options"; table.printHelp(llvm::outs(), usage_str.c_str(), "LLDB DAP", false); - std::string examples = R"___( + llvm::outs() << R"___( EXAMPLES: The debug adapter can be started in two modes. @@ -176,7 +176,6 @@ static void printHelp(LLDBDAPOptTable &table, llvm::StringRef tool_name) { lldb-dap -g )___"; - llvm::outs() << examples; } // If --launch-target is provided, this instance of lldb-dap becomes a ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)
https://github.com/oontvoo updated https://github.com/llvm/llvm-project/pull/128534 >From 236ab76b7c9433f8fed6586a4ee18351779cc191 Mon Sep 17 00:00:00 2001 From: Vy Nguyen Date: Mon, 24 Feb 2025 11:32:03 -0500 Subject: [PATCH 01/15] [llvm][telemetr]Change Telemetry-disabling mechanism. Details: - Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any Telemetry code will be built. This has proven to cause more nuisance to both users of the Telemetry and any further extension of it. (Eg., we needed to put #ifdef around caller/user code) - So the new approach is to: + Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by default. + If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would still be built BUT Telemetry cannot be enabled. And no data can be collected. The benefit of this is that it simplifies user (and extension) code since we just need to put the check on Config::EnableTelemetry. Besides, the Telemetry library itself is very small, hence the additional code to be built would not cause any difference in build performance. --- lldb/source/Core/CMakeLists.txt | 4 +--- lldb/source/Core/Telemetry.cpp | 7 ++- lldb/unittests/Core/CMakeLists.txt | 6 +++--- llvm/CMakeLists.txt | 3 ++- llvm/cmake/modules/LLVMConfig.cmake.in | 1 + llvm/include/llvm/Config/llvm-config.h.cmake | 2 ++ llvm/include/llvm/Telemetry/Telemetry.h | 12 ++-- llvm/lib/CMakeLists.txt | 5 ++--- llvm/unittests/CMakeLists.txt| 4 +--- .../gn/secondary/llvm/include/llvm/Config/BUILD.gn | 1 + utils/bazel/llvm_configs/llvm-config.h.cmake | 2 ++ 11 files changed, 27 insertions(+), 20 deletions(-) diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt index 82fb5f42f9f4b..a3c12e4c1bdbc 100644 --- a/lldb/source/Core/CMakeLists.txt +++ b/lldb/source/Core/CMakeLists.txt @@ -16,9 +16,7 @@ if (LLDB_ENABLE_CURSES) endif() endif() -if (LLVM_BUILD_TELEMETRY) - set(TELEMETRY_DEPS Telemetry) -endif() +set(TELEMETRY_DEPS Telemetry) # TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore add_lldb_library(lldbCore diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp index 5222f76704f91..51a860ea5313b 100644 --- a/lldb/source/Core/Telemetry.cpp +++ b/lldb/source/Core/Telemetry.cpp @@ -8,8 +8,6 @@ #include "llvm/Config/llvm-config.h" -#ifdef LLVM_BUILD_TELEMETRY - #include "lldb/Core/Telemetry.h" #include "lldb/Core/Debugger.h" #include "lldb/Utility/LLDBLog.h" @@ -57,7 +55,8 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const { } TelemetryManager::TelemetryManager(std::unique_ptr config) -: m_config(std::move(config)) {} +: m_config(std::move(config)) +} llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) { // Do nothing for now. @@ -75,5 +74,3 @@ void TelemetryManager::setInstance(std::unique_ptr manager) { } // namespace telemetry } // namespace lldb_private - -#endif // LLVM_BUILD_TELEMETRY diff --git a/lldb/unittests/Core/CMakeLists.txt b/lldb/unittests/Core/CMakeLists.txt index d4d3764b67ae3..e8df299631e2e 100644 --- a/lldb/unittests/Core/CMakeLists.txt +++ b/lldb/unittests/Core/CMakeLists.txt @@ -1,6 +1,6 @@ -if (LLVM_BUILD_TELEMETRY) - set(TELEMETRY_DEPS Telemetry) -endif() + +set(TELEMETRY_DEPS Telemetry) + add_lldb_unittest(LLDBCoreTests CommunicationTest.cpp diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt index 88512d0f1dd96..9188fb93b7846 100644 --- a/llvm/CMakeLists.txt +++ b/llvm/CMakeLists.txt @@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm API documentation." OF option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF) option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON) option (LLVM_ENABLE_BINDINGS "Build bindings." ON) -option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not enable telemetry." ON) +option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable the telemetry library. If set to OFF, library cannot be enabled after build (eg., at runtime)" ON) +option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF, library cannot be enabled after build (eg., at runtime)" ON) set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html" CACHE STRING "Doxygen-generated HTML documentation install directory") diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in b/llvm/cmake/modules/LLVMConfig.cmake.in index 28655ee3ab87d..134d107ce79ba 100644 --- a/llvm/cmake/modules/LLVMConfig.cmake.in +++ b/llvm/cmake/modules/LLVMConfig.cmake.in @@ -101,6 +101,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@) set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@) set(LLVM_BUILD_TELEMETRY @LLVM
[Lldb-commits] [lldb] [LLDB-DAP] SBDebugger don't prefix title on progress updates (PR #124648)
@@ -40,7 +40,10 @@ SBProgress::~SBProgress() = default; void SBProgress::Increment(uint64_t amount, const char *description) { LLDB_INSTRUMENT_VA(amount, description); - m_opaque_up->Increment(amount, description); + std::optional description_opt; + if (description) clayborg wrote: ``` if (description && description[0]) ``` https://github.com/llvm/llvm-project/pull/124648 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB-DAP] SBDebugger don't prefix title on progress updates (PR #124648)
@@ -41,8 +41,136 @@ def test_output(self): for event in self.dap_server.progress_events: event_type = event["event"] if "progressStart" in event_type: +title = event["body"]["title"] +self.assertIn("Progress tester", title) start_found = True if "progressUpdate" in event_type: +message = event["body"]["message"] +print(f"Progress update: {message}") +self.assertNotIn("Progres tester", message) +update_found = True + +self.assertTrue(start_found) +self.assertTrue(update_found) + +@skipIfWindows +def test_output_nodetails(self): +program = self.getBuildArtifact("a.out") +self.build_and_launch(program) +progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py") +print(f"Progress emitter path: {progress_emitter}") +source = "main.cpp" +breakpoint_ids = self.set_source_breakpoints( +source, [line_number(source, "// break here")] +) +self.continue_to_breakpoints(breakpoint_ids) +self.dap_server.request_evaluate( +f"`command script import {progress_emitter}", context="repl" +) +self.dap_server.request_evaluate( +"`test-progress --total 3 --seconds 1 --no-details", context="repl" +) + +self.dap_server.wait_for_event("progressEnd", 15) +# Expect at least a start, an update, and end event +# However because the underlying Progress instance is an RAII object and we can't guaruntee +# it's deterministic destruction in the python API, we verify just start and update +# otherwise this test could be flakey. clayborg wrote: Don't rely on RAII, either send the end event manually or set the progress object to `None` to force it to finish up. Might be worth adding documentation notes in the SBProcess*.i files to let users know how to make sure the finish event will get sent, or document to send it manually. https://github.com/llvm/llvm-project/pull/124648 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB-DAP] SBDebugger don't prefix title on progress updates (PR #124648)
@@ -40,7 +40,10 @@ SBProgress::~SBProgress() = default; void SBProgress::Increment(uint64_t amount, const char *description) { LLDB_INSTRUMENT_VA(amount, description); - m_opaque_up->Increment(amount, description); + std::optional description_opt; + if (description) clayborg wrote: This still can be fixed, no need to send an empty string if it isn't needed. https://github.com/llvm/llvm-project/pull/124648 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)
llvm-ci wrote: LLVM Buildbot has detected a new failure on builder `sanitizer-ppc64le-linux` running on `ppc64le-sanitizer` while building `lldb,llvm,utils` at step 2 "annotate". Full details are available at: https://lab.llvm.org/buildbot/#/builders/72/builds/8614 Here is the relevant piece of the build log for the reference ``` Step 2 (annotate) failure: 'python ../sanitizer_buildbot/sanitizers/zorg/buildbot/builders/sanitizers/buildbot_selector.py' (failure) ... [733/4137] Building DiagnosticSemaKinds.inc... [734/4137] Building CXX object lib/Frontend/OpenACC/CMakeFiles/LLVMFrontendOpenACC.dir/ACC.cpp.o [735/4137] Building CXX object lib/CodeGenTypes/CMakeFiles/LLVMCodeGenTypes.dir/LowLevelType.cpp.o [736/4137] Building riscv_vector_builtins.inc... [737/4137] Linking CXX static library lib/libLLVMFrontendOpenACC.a [738/4137] Linking CXX static library lib/libLLVMCodeGenTypes.a [739/4137] Building riscv_vector_builtin_cg.inc... [740/4137] Building arm_sve_builtin_cg.inc... [741/4137] Building riscv_vector_builtin_sema.inc... [742/4137] Building CXX object lib/Telemetry/CMakeFiles/LLVMTelemetry.dir/Telemetry.cpp.o FAILED: lib/Telemetry/CMakeFiles/LLVMTelemetry.dir/Telemetry.cpp.o CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /home/buildbots/llvm-external-buildbots/workers/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm_build0/bin/clang++ -DGTEST_HAS_RTTI=0 -DLLVM_EXPORTS -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-sanitizer/sanitizer-ppc64le/build/build_default/lib/Telemetry -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/llvm/lib/Telemetry -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-sanitizer/sanitizer-ppc64le/build/build_default/include -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/llvm/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG -std=c++17 -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -MD -MT lib/Telemetry/CMakeFiles/LLVMTelemetry.dir/Telemetry.cpp.o -MF lib/Telemetry/CMakeFiles/LLVMTelemetry.dir/Telemetry.cpp.o.d -o lib/Telemetry/CMakeFiles/LLVMTelemetry.dir/Telemetry.cpp.o -c /home/buildbots/llvm-external-buildbots/workers/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/llvm/lib/Telemetry/Telemetry.cpp In file included from /home/buildbots/llvm-external-buildbots/workers/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/llvm/lib/Telemetry/Telemetry.cpp:14: /home/buildbots/llvm-external-buildbots/workers/ppc64le-sanitizer/sanitizer-ppc64le/build/llvm-project/llvm/include/llvm/Telemetry/Telemetry.h:66:8: error: 'llvm::telemetry::Config' has virtual functions but non-virtual destructor [-Werror,-Wnon-virtual-dtor] 66 | struct Config { |^ 1 error generated. [743/4137] Building arm_sve_sema_rangechecks.inc... [744/4137] Building arm_sve_builtins.inc... [745/4137] Building IntrinsicsHexagon.h... [746/4137] Building IntrinsicsAArch64.h... [747/4137] Building IntrinsicsS390.h... [748/4137] Building IntrinsicEnums.inc... [749/4137] Building IntrinsicsX86.h... [750/4137] Building IntrinsicsAMDGPU.h... [751/4137] Building IntrinsicsWebAssembly.h... [752/4137] Building IntrinsicsLoongArch.h... [753/4137] Building IntrinsicsMips.h... [754/4137] Building IntrinsicsNVPTX.h... [755/4137] Building IntrinsicsARM.h... [756/4137] Building IntrinsicsSPIRV.h... [757/4137] Building IntrinsicsPowerPC.h... [758/4137] Building IntrinsicsR600.h... [759/4137] Building IntrinsicsRISCV.h... [760/4137] Building IntrinsicsBPF.h... [761/4137] Building IntrinsicsXCore.h... [762/4137] Building IntrinsicsVE.h... [763/4137] Building IntrinsicImpl.inc... [764/4137] Building IntrinsicsDirectX.h... [765/4137] Building ARMTargetParserDef.inc... ninja: build stopped: subcommand failed. How to reproduce locally: https://github.com/google/sanitizers/wiki/SanitizerBotReproduceBuild @@@STEP_FAILURE@@@ @@@STEP_FAILURE@@@ Step 8 (build compiler-rt debug) failure: build compiler-rt debug (failure) ... [733/4137] Building DiagnosticSemaKinds.inc... [734/4137] Building CXX object lib/Frontend/OpenACC/CMakeFiles/LLVMFrontendOpenACC.dir/ACC.cpp.o [735/4137] Building CXX object lib/CodeGenTypes/CMakeFiles/LLVMCodeGenTypes.dir/LowLevelType.cpp.o [736/4137] Building riscv_vector_builtins.inc... [737/4137] Linking CXX static
[Lldb-commits] [lldb] [WIP] [lldb][TypeSystemClang] Create clang::SourceLocation from DWARF and attach to AST (PR #127829)
adrian-prantl wrote: It might be worth exploring a SourceManager subclass that has in addition to a MemoryBuffer-backed location also a source file type that is a file/line variant that is only converted into a MemoryBuffer-backed location if we do an operation that really needs it. https://github.com/llvm/llvm-project/pull/127829 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB-DAP] SBDebugger don't prefix title on progress updates (PR #124648)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/124648 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 1e24670 - [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (#128750)
Author: John Harrison Date: 2025-02-26T11:29:13-08:00 New Revision: 1e246704e23e3dcae16adbf68cc10b668a8db680 URL: https://github.com/llvm/llvm-project/commit/1e246704e23e3dcae16adbf68cc10b668a8db680 DIFF: https://github.com/llvm/llvm-project/commit/1e246704e23e3dcae16adbf68cc10b668a8db680.diff LOG: [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (#128750) This simplifies the IOStream.cpp implementation by building on top of the existing lldb::IOObjectSP. Additionally, this should help ensure clients connected of a `--connection` specifier properly detect shutdown requests when the Socket is closed. Previously, the StreamDescriptor was just accessing the underlying native handle and was not aware of the `Close()` call to the Socket itself. This is both nice to have for simplifying the existing code and this unblocks an upcoming refactor to support the cancel request. - Co-authored-by: Jonas Devlieghere Added: Modified: lldb/tools/lldb-dap/DAP.cpp lldb/tools/lldb-dap/DAP.h lldb/tools/lldb-dap/IOStream.cpp lldb/tools/lldb-dap/IOStream.h lldb/tools/lldb-dap/lldb-dap.cpp Removed: diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 37bc1f68e4b08..cd53e2aca3fb6 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -18,6 +18,7 @@ #include "lldb/API/SBListener.h" #include "lldb/API/SBProcess.h" #include "lldb/API/SBStream.h" +#include "lldb/Utility/IOObject.h" #include "lldb/Utility/Status.h" #include "lldb/lldb-defines.h" #include "lldb/lldb-enumerations.h" @@ -61,7 +62,7 @@ const char DEV_NULL[] = "/dev/null"; namespace lldb_dap { DAP::DAP(std::string name, llvm::StringRef path, std::ofstream *log, - StreamDescriptor input, StreamDescriptor output, ReplMode repl_mode, + lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode, std::vector pre_init_commands) : name(std::move(name)), debug_adaptor_path(path), log(log), input(std::move(input)), output(std::move(output)), diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 7ceb1d114a57d..a7c7e5d9bbc19 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -30,6 +30,7 @@ #include "lldb/API/SBThread.h" #include "lldb/API/SBValue.h" #include "lldb/API/SBValueList.h" +#include "lldb/lldb-forward.h" #include "lldb/lldb-types.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" @@ -210,7 +211,7 @@ struct DAP { std::string last_nonempty_var_expression; DAP(std::string name, llvm::StringRef path, std::ofstream *log, - StreamDescriptor input, StreamDescriptor output, ReplMode repl_mode, + lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode, std::vector pre_init_commands); ~DAP(); DAP(const DAP &rhs) = delete; diff --git a/lldb/tools/lldb-dap/IOStream.cpp b/lldb/tools/lldb-dap/IOStream.cpp index 7d0f363440f53..c6f1bfaf3b799 100644 --- a/lldb/tools/lldb-dap/IOStream.cpp +++ b/lldb/tools/lldb-dap/IOStream.cpp @@ -7,125 +7,34 @@ //===--===// #include "IOStream.h" +#include "lldb/Utility/IOObject.h" +#include "lldb/Utility/Status.h" #include #include -#if defined(_WIN32) -#include -#else -#include -#include -#include -#endif - using namespace lldb_dap; -StreamDescriptor::StreamDescriptor() = default; - -StreamDescriptor::StreamDescriptor(StreamDescriptor &&other) { - *this = std::move(other); -} - -StreamDescriptor::~StreamDescriptor() { - if (!m_close) -return; - - if (m_is_socket) -#if defined(_WIN32) -::closesocket(m_socket); -#else -::close(m_socket); -#endif - else -::close(m_fd); -} - -StreamDescriptor &StreamDescriptor::operator=(StreamDescriptor &&other) { - m_close = other.m_close; - other.m_close = false; - m_is_socket = other.m_is_socket; - if (m_is_socket) -m_socket = other.m_socket; - else -m_fd = other.m_fd; - return *this; -} - -StreamDescriptor StreamDescriptor::from_socket(SOCKET s, bool close) { - StreamDescriptor sd; - sd.m_is_socket = true; - sd.m_socket = s; - sd.m_close = close; - return sd; -} - -StreamDescriptor StreamDescriptor::from_file(int fd, bool close) { - StreamDescriptor sd; - sd.m_is_socket = false; - sd.m_fd = fd; - sd.m_close = close; - return sd; -} - bool OutputStream::write_full(llvm::StringRef str) { - while (!str.empty()) { -int bytes_written = 0; -if (descriptor.m_is_socket) - bytes_written = ::send(descriptor.m_socket, str.data(), str.size(), 0); -else - bytes_written = ::write(descriptor.m_fd, str.data(), str.size()); - -if (bytes_written < 0) { - if (errno == EINTR || errno == EAGAIN) -continue; - return false; -} -str = str.drop_front(bytes_written); - } + if (!descriptor) +return false;
[Lldb-commits] [lldb] [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (PR #128750)
https://github.com/ashgti closed https://github.com/llvm/llvm-project/pull/128750 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (PR #128750)
ashgti wrote: > Is it worth keeping `InputStream` and `OutputStream` around? Can we use a > `(Stream)File` directly? I'll take a look at removing them as part of the refactoring to support 'cancel' requests. I think it makes sense. https://github.com/llvm/llvm-project/pull/128750 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Telemetry]Define DebuggerTelemetryInfo and related methods (PR #127696)
https://github.com/oontvoo updated https://github.com/llvm/llvm-project/pull/127696 >From 24e9f78744f98ecf3ac01f1f719f1eac9b3479f0 Mon Sep 17 00:00:00 2001 From: Vy Nguyen Date: Tue, 18 Feb 2025 15:58:08 -0500 Subject: [PATCH 1/7] [LLDB][Telemetry]Define DebuggerTelemetryInfo and related methods - This type of entry is used to collect data about the debugger startup/exit - Tests will be added (They may need to be shell test with a "test-only" TelemetryManager plugin defined. I'm trying to figure out how to get that linked only when tests are running and not to the LLDB binary all the time. --- lldb/include/lldb/Core/Telemetry.h | 78 +++ lldb/source/Core/Debugger.cpp | 40 ++ lldb/source/Core/Telemetry.cpp | 115 + 3 files changed, 220 insertions(+), 13 deletions(-) diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h index b72556ecaf3c9..d6eec5dc687be 100644 --- a/lldb/include/lldb/Core/Telemetry.h +++ b/lldb/include/lldb/Core/Telemetry.h @@ -13,6 +13,7 @@ #include "lldb/Interpreter/CommandReturnObject.h" #include "lldb/Utility/StructuredData.h" #include "lldb/lldb-forward.h" +#include "llvm/ADT/DenseMap.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/JSON.h" @@ -29,6 +30,9 @@ namespace telemetry { struct LLDBEntryKind : public ::llvm::telemetry::EntryKind { static const llvm::telemetry::KindType BaseInfo = 0b11000; + static const llvm::telemetry::KindType DebuggerInfo = 0b11001; + // There are other entries in between (added in separate PRs) + static const llvm::telemetry::KindType MiscInfo = 0b0; }; /// Defines a convenient type for timestamp of various events. @@ -56,6 +60,71 @@ struct LLDBBaseTelemetryInfo : public llvm::telemetry::TelemetryInfo { void serialize(llvm::telemetry::Serializer &serializer) const override; }; +/// Describes the exit status of a debugger. +struct ExitDescription { + int exit_code; + std::string description; +}; + +struct DebuggerTelemetryInfo : public LLDBBaseTelemetryInfo { + std::string username; + std::string lldb_git_sha; + std::string lldb_path; + std::string cwd; + std::optional exit_desc; + + DebuggerTelemetryInfo() = default; + + // Provide a copy ctor because we may need to make a copy before + // sanitizing the data. + // (The sanitization might differ between different Destination classes). + DebuggerTelemetryInfo(const DebuggerTelemetryInfo &other) { +username = other.username; +lldb_git_sha = other.lldb_git_sha; +lldb_path = other.lldb_path; +cwd = other.cwd; + }; + + llvm::telemetry::KindType getKind() const override { +return LLDBEntryKind::DebuggerInfo; + } + + static bool classof(const llvm::telemetry::TelemetryInfo *T) { +return T->getKind() == LLDBEntryKind::DebuggerInfo; + } + + void serialize(llvm::telemetry::Serializer &serializer) const override; +}; + +/// The "catch-all" entry to store a set of non-standard data, such as +/// error-messages, etc. +struct MiscTelemetryInfo : public LLDBBaseTelemetryInfo { + /// If the event is/can be associated with a target entry, + /// this field contains that target's UUID. + /// otherwise. + std::string target_uuid; + + /// Set of key-value pairs for any optional (or impl-specific) data + std::map meta_data; + + MiscTelemetryInfo() = default; + + MiscTelemetryInfo(const MiscTelemetryInfo &other) { +target_uuid = other.target_uuid; +meta_data = other.meta_data; + } + + llvm::telemetry::KindType getKind() const override { +return LLDBEntryKind::MiscInfo; + } + + static bool classof(const llvm::telemetry::TelemetryInfo *T) { +return T->getKind() == LLDBEntryKind::MiscInfo; + } + + void serialize(llvm::telemetry::Serializer &serializer) const override; +}; + /// The base Telemetry manager instance in LLDB. /// This class declares additional instrumentation points /// applicable to LLDB. @@ -63,6 +132,11 @@ class TelemetryManager : public llvm::telemetry::Manager { public: llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override; + const llvm::telemetry::Config *getConfig(); + + void atDebuggerStartup(DebuggerTelemetryInfo *entry); + void atDebuggerExit(DebuggerTelemetryInfo *entry); + virtual llvm::StringRef GetInstanceName() const = 0; static TelemetryManager *getInstance(); @@ -73,6 +147,10 @@ class TelemetryManager : public llvm::telemetry::Manager { private: std::unique_ptr m_config; + // Each debugger is assigned a unique ID (session_id). + // All TelemetryInfo entries emitted for the same debugger instance + // will get the same session_id. + llvm::DenseMap session_ids; static std::unique_ptr g_instance; }; diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index 18569e155b517..b458abc798a9e 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -62,6 +62,7 @@
[Lldb-commits] [lldb] [LLDB-DAP] SBDebugger don't prefix title on progress updates (PR #124648)
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/124648 >From 0587ba8239dbbd22aa40bde23d61f9504975817d Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Mon, 27 Jan 2025 13:41:58 -0800 Subject: [PATCH 1/9] Only include title on the first message --- lldb/include/lldb/Core/DebuggerEvents.h | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h index 49a4ecf8e537e..52e4f77d7637d 100644 --- a/lldb/include/lldb/Core/DebuggerEvents.h +++ b/lldb/include/lldb/Core/DebuggerEvents.h @@ -44,12 +44,15 @@ class ProgressEventData : public EventData { uint64_t GetCompleted() const { return m_completed; } uint64_t GetTotal() const { return m_total; } std::string GetMessage() const { -std::string message = m_title; -if (!m_details.empty()) { - message.append(": "); - message.append(m_details); -} -return message; +if (m_completed == 0) { + std::string message = m_title; + if (!m_details.empty()) { +message.append(": "); +message.append(m_details); + } + return message; +} else + return !m_details.empty() ? m_details : std::string(); } const std::string &GetTitle() const { return m_title; } const std::string &GetDetails() const { return m_details; } >From b64fe53741486ea9b6cde2e658b766aa68bf9389 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Mon, 27 Jan 2025 14:48:01 -0800 Subject: [PATCH 2/9] Add comment explaining if and add a test --- lldb/include/lldb/Core/DebuggerEvents.h | 1 + lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py | 5 + 2 files changed, 6 insertions(+) diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h index 52e4f77d7637d..ca06ee835fde3 100644 --- a/lldb/include/lldb/Core/DebuggerEvents.h +++ b/lldb/include/lldb/Core/DebuggerEvents.h @@ -44,6 +44,7 @@ class ProgressEventData : public EventData { uint64_t GetCompleted() const { return m_completed; } uint64_t GetTotal() const { return m_total; } std::string GetMessage() const { +// Only put the title in the message of the progress create event. if (m_completed == 0) { std::string message = m_title; if (!m_details.empty()) { diff --git a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py index 36c0cef9c4714..945c3f7633364 100755 --- a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py +++ b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py @@ -41,8 +41,13 @@ def test_output(self): for event in self.dap_server.progress_events: event_type = event["event"] if "progressStart" in event_type: +title = event["body"]["title"] +self.assertIn("Progress tester", title) start_found = True if "progressUpdate" in event_type: +message = event["body"]["message"] +print(f"Progress update: {message}") +self.assertNotIn("Progres tester", message) update_found = True self.assertTrue(start_found) >From 96f0ebb43f83cce80c76915f5412b9bb31dd3f12 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Thu, 30 Jan 2025 10:04:04 -0800 Subject: [PATCH 3/9] Migrate to structured data Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D68927453 --- lldb/include/lldb/Core/DebuggerEvents.h | 16 ++-- .../Handler/InitializeRequestHandler.cpp | 77 --- 2 files changed, 73 insertions(+), 20 deletions(-) diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h index ca06ee835fde3..49a4ecf8e537e 100644 --- a/lldb/include/lldb/Core/DebuggerEvents.h +++ b/lldb/include/lldb/Core/DebuggerEvents.h @@ -44,16 +44,12 @@ class ProgressEventData : public EventData { uint64_t GetCompleted() const { return m_completed; } uint64_t GetTotal() const { return m_total; } std::string GetMessage() const { -// Only put the title in the message of the progress create event. -if (m_completed == 0) { - std::string message = m_title; - if (!m_details.empty()) { -message.append(": "); -message.append(m_details); - } - return message; -} else - return !m_details.empty() ? m_details : std::string(); +std::string message = m_title; +if (!m_details.empty()) { + message.append(": "); + message.append(m_details); +} +return message; } const std::string &GetTitle() const { return m_title; } const std::string &GetDetails() const { return m_details; } diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp index e9041f3
[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)
llvm-ci wrote: LLVM Buildbot has detected a new failure on builder `clang-ppc64le-rhel` running on `ppc64le-clang-rhel-test` while building `lldb,llvm,utils` at step 5 "build-unified-tree". Full details are available at: https://lab.llvm.org/buildbot/#/builders/145/builds/5321 Here is the relevant piece of the build log for the reference ``` Step 5 (build-unified-tree) failure: build (failure) ... 8.767 [5618/160/647] Copying clang's avxvnniint8intrin.h... 8.767 [5618/159/648] Copying clang's avxvnniintrin.h... 8.768 [5618/158/649] Copying clang's bmi2intrin.h... 8.768 [5618/157/650] Copying clang's bmiintrin.h... 8.768 [5618/156/651] Copying clang's cetintrin.h... 8.768 [5618/155/652] Copying clang's cldemoteintrin.h... 8.768 [5618/154/653] Copying clang's clflushoptintrin.h... 8.768 [5618/153/654] Copying clang's cmpccxaddintrin.h... 8.768 [5618/152/655] Copying clang's crc32intrin.h... 8.770 [5618/151/656] Building CXX object lib/Telemetry/CMakeFiles/LLVMTelemetry.dir/Telemetry.cpp.o FAILED: lib/Telemetry/CMakeFiles/LLVMTelemetry.dir/Telemetry.cpp.o ccache /home/buildbots/llvm-external-buildbots/clang.19.1.7/bin/clang++ --gcc-toolchain=/gcc-toolchain/usr -DGTEST_HAS_RTTI=0 -DLLVM_EXPORTS -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/lib/Telemetry -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/lib/Telemetry -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/include -I/home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -O3 -DNDEBUG -std=c++17 -fPIC -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -MD -MT lib/Telemetry/CMakeFiles/LLVMTelemetry.dir/Telemetry.cpp.o -MF lib/Telemetry/CMakeFiles/LLVMTelemetry.dir/Telemetry.cpp.o.d -o lib/Telemetry/CMakeFiles/LLVMTelemetry.dir/Telemetry.cpp.o -c /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/lib/Telemetry/Telemetry.cpp In file included from /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/lib/Telemetry/Telemetry.cpp:14: /home/buildbots/llvm-external-buildbots/workers/ppc64le-clang-rhel-test/clang-ppc64le-rhel/llvm-project/llvm/include/llvm/Telemetry/Telemetry.h:66:8: error: 'llvm::telemetry::Config' has virtual functions but non-virtual destructor [-Werror,-Wnon-virtual-dtor] 66 | struct Config { |^ 1 error generated. 8.771 [5618/150/657] Building IntrinsicsLoongArch.h... 8.771 [5618/149/658] Copying clang's clwbintrin.h... 8.771 [5618/148/659] Copying clang's clzerointrin.h... 8.771 [5618/147/660] Copying clang's emmintrin.h... 8.771 [5618/146/661] Copying clang's enqcmdintrin.h... 8.771 [5618/145/662] Copying clang's f16cintrin.h... 8.771 [5618/144/663] Copying clang's fma4intrin.h... 8.771 [5618/143/664] Copying clang's fmaintrin.h... 8.772 [5618/142/665] Copying clang's fxsrintrin.h... 8.772 [5618/141/666] Copying clang's gfniintrin.h... 8.772 [5618/140/667] Copying clang's hresetintrin.h... 8.772 [5618/139/668] Copying clang's ia32intrin.h... 8.772 [5618/138/669] Copying clang's immintrin.h... 8.772 [5618/137/670] Copying clang's invpcidintrin.h... 8.772 [5618/136/671] Copying clang's keylockerintrin.h... 8.772 [5618/135/672] Copying clang's lwpintrin.h... 8.772 [5618/134/673] Copying clang's lzcntintrin.h... 8.772 [5618/133/674] Copying clang's mm3dnow.h... 8.772 [5618/132/675] Copying clang's mmintrin.h... 8.772 [5618/131/676] Copying clang's movdirintrin.h... 8.772 [5618/130/677] Copying clang's movrs_avx10_2_512intrin.h... 8.773 [5618/129/678] Copying clang's movrs_avx10_2intrin.h... 8.773 [5618/128/679] Copying clang's movrsintrin.h... 8.773 [5618/127/680] Linking CXX shared library lib/libLLVMCodeGenTypes.so.21.0git 8.773 [5618/126/681] Linking CXX shared library lib/libLLVMFrontendOpenACC.so.21.0git 8.773 [5618/125/682] Copying clang's mwaitxintrin.h... 8.773 [5618/124/683] Copying clang's nmmintrin.h... 8.773 [5618/123/684] Copying clang's pconfigintrin.h... 8.773 [5618/122/685] Copying clang's pkuintrin.h... 8.774 [5618/121/686] Copying clang's pmmintrin.h... 8.774 [5618/120/687] Copying clang's popcntintrin.h... 8.77
[Lldb-commits] [lldb] [LLDB][Telemetry]Define DebuggerTelemetryInfo and related methods (PR #127696)
oontvoo wrote: I think this is ready for review now. New changes: - Incorporate the enable-telemetry flag from the other PR to get rid of all the ifdefs - remove process-exit/startup data from this - stick to debugger's exit/startup only. + I'd like to find the right place to report debugger's crashes (ie., in the code where it'd normally print "Please file a bug report ") - add some RAII helper to reduce boilerplate https://github.com/llvm/llvm-project/pull/127696 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Deindent UnwindAssemblyInstEmulation (PR #128874)
https://github.com/JDevlieghere approved this pull request. ⬅️ https://github.com/llvm/llvm-project/pull/128874 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)
https://github.com/oontvoo updated https://github.com/llvm/llvm-project/pull/128534 >From 236ab76b7c9433f8fed6586a4ee18351779cc191 Mon Sep 17 00:00:00 2001 From: Vy Nguyen Date: Mon, 24 Feb 2025 11:32:03 -0500 Subject: [PATCH 01/11] [llvm][telemetr]Change Telemetry-disabling mechanism. Details: - Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any Telemetry code will be built. This has proven to cause more nuisance to both users of the Telemetry and any further extension of it. (Eg., we needed to put #ifdef around caller/user code) - So the new approach is to: + Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by default. + If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would still be built BUT Telemetry cannot be enabled. And no data can be collected. The benefit of this is that it simplifies user (and extension) code since we just need to put the check on Config::EnableTelemetry. Besides, the Telemetry library itself is very small, hence the additional code to be built would not cause any difference in build performance. --- lldb/source/Core/CMakeLists.txt | 4 +--- lldb/source/Core/Telemetry.cpp | 7 ++- lldb/unittests/Core/CMakeLists.txt | 6 +++--- llvm/CMakeLists.txt | 3 ++- llvm/cmake/modules/LLVMConfig.cmake.in | 1 + llvm/include/llvm/Config/llvm-config.h.cmake | 2 ++ llvm/include/llvm/Telemetry/Telemetry.h | 12 ++-- llvm/lib/CMakeLists.txt | 5 ++--- llvm/unittests/CMakeLists.txt| 4 +--- .../gn/secondary/llvm/include/llvm/Config/BUILD.gn | 1 + utils/bazel/llvm_configs/llvm-config.h.cmake | 2 ++ 11 files changed, 27 insertions(+), 20 deletions(-) diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt index 82fb5f42f9f4b..a3c12e4c1bdbc 100644 --- a/lldb/source/Core/CMakeLists.txt +++ b/lldb/source/Core/CMakeLists.txt @@ -16,9 +16,7 @@ if (LLDB_ENABLE_CURSES) endif() endif() -if (LLVM_BUILD_TELEMETRY) - set(TELEMETRY_DEPS Telemetry) -endif() +set(TELEMETRY_DEPS Telemetry) # TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore add_lldb_library(lldbCore diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp index 5222f76704f91..51a860ea5313b 100644 --- a/lldb/source/Core/Telemetry.cpp +++ b/lldb/source/Core/Telemetry.cpp @@ -8,8 +8,6 @@ #include "llvm/Config/llvm-config.h" -#ifdef LLVM_BUILD_TELEMETRY - #include "lldb/Core/Telemetry.h" #include "lldb/Core/Debugger.h" #include "lldb/Utility/LLDBLog.h" @@ -57,7 +55,8 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const { } TelemetryManager::TelemetryManager(std::unique_ptr config) -: m_config(std::move(config)) {} +: m_config(std::move(config)) +} llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) { // Do nothing for now. @@ -75,5 +74,3 @@ void TelemetryManager::setInstance(std::unique_ptr manager) { } // namespace telemetry } // namespace lldb_private - -#endif // LLVM_BUILD_TELEMETRY diff --git a/lldb/unittests/Core/CMakeLists.txt b/lldb/unittests/Core/CMakeLists.txt index d4d3764b67ae3..e8df299631e2e 100644 --- a/lldb/unittests/Core/CMakeLists.txt +++ b/lldb/unittests/Core/CMakeLists.txt @@ -1,6 +1,6 @@ -if (LLVM_BUILD_TELEMETRY) - set(TELEMETRY_DEPS Telemetry) -endif() + +set(TELEMETRY_DEPS Telemetry) + add_lldb_unittest(LLDBCoreTests CommunicationTest.cpp diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt index 88512d0f1dd96..9188fb93b7846 100644 --- a/llvm/CMakeLists.txt +++ b/llvm/CMakeLists.txt @@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm API documentation." OF option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF) option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON) option (LLVM_ENABLE_BINDINGS "Build bindings." ON) -option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not enable telemetry." ON) +option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable the telemetry library. If set to OFF, library cannot be enabled after build (eg., at runtime)" ON) +option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF, library cannot be enabled after build (eg., at runtime)" ON) set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html" CACHE STRING "Doxygen-generated HTML documentation install directory") diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in b/llvm/cmake/modules/LLVMConfig.cmake.in index 28655ee3ab87d..134d107ce79ba 100644 --- a/llvm/cmake/modules/LLVMConfig.cmake.in +++ b/llvm/cmake/modules/LLVMConfig.cmake.in @@ -101,6 +101,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@) set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@) set(LLVM_BUILD_TELEMETRY @LLVM
[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)
https://github.com/oontvoo updated https://github.com/llvm/llvm-project/pull/128534 >From 236ab76b7c9433f8fed6586a4ee18351779cc191 Mon Sep 17 00:00:00 2001 From: Vy Nguyen Date: Mon, 24 Feb 2025 11:32:03 -0500 Subject: [PATCH 01/12] [llvm][telemetr]Change Telemetry-disabling mechanism. Details: - Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any Telemetry code will be built. This has proven to cause more nuisance to both users of the Telemetry and any further extension of it. (Eg., we needed to put #ifdef around caller/user code) - So the new approach is to: + Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by default. + If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would still be built BUT Telemetry cannot be enabled. And no data can be collected. The benefit of this is that it simplifies user (and extension) code since we just need to put the check on Config::EnableTelemetry. Besides, the Telemetry library itself is very small, hence the additional code to be built would not cause any difference in build performance. --- lldb/source/Core/CMakeLists.txt | 4 +--- lldb/source/Core/Telemetry.cpp | 7 ++- lldb/unittests/Core/CMakeLists.txt | 6 +++--- llvm/CMakeLists.txt | 3 ++- llvm/cmake/modules/LLVMConfig.cmake.in | 1 + llvm/include/llvm/Config/llvm-config.h.cmake | 2 ++ llvm/include/llvm/Telemetry/Telemetry.h | 12 ++-- llvm/lib/CMakeLists.txt | 5 ++--- llvm/unittests/CMakeLists.txt| 4 +--- .../gn/secondary/llvm/include/llvm/Config/BUILD.gn | 1 + utils/bazel/llvm_configs/llvm-config.h.cmake | 2 ++ 11 files changed, 27 insertions(+), 20 deletions(-) diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt index 82fb5f42f9f4b..a3c12e4c1bdbc 100644 --- a/lldb/source/Core/CMakeLists.txt +++ b/lldb/source/Core/CMakeLists.txt @@ -16,9 +16,7 @@ if (LLDB_ENABLE_CURSES) endif() endif() -if (LLVM_BUILD_TELEMETRY) - set(TELEMETRY_DEPS Telemetry) -endif() +set(TELEMETRY_DEPS Telemetry) # TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore add_lldb_library(lldbCore diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp index 5222f76704f91..51a860ea5313b 100644 --- a/lldb/source/Core/Telemetry.cpp +++ b/lldb/source/Core/Telemetry.cpp @@ -8,8 +8,6 @@ #include "llvm/Config/llvm-config.h" -#ifdef LLVM_BUILD_TELEMETRY - #include "lldb/Core/Telemetry.h" #include "lldb/Core/Debugger.h" #include "lldb/Utility/LLDBLog.h" @@ -57,7 +55,8 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const { } TelemetryManager::TelemetryManager(std::unique_ptr config) -: m_config(std::move(config)) {} +: m_config(std::move(config)) +} llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) { // Do nothing for now. @@ -75,5 +74,3 @@ void TelemetryManager::setInstance(std::unique_ptr manager) { } // namespace telemetry } // namespace lldb_private - -#endif // LLVM_BUILD_TELEMETRY diff --git a/lldb/unittests/Core/CMakeLists.txt b/lldb/unittests/Core/CMakeLists.txt index d4d3764b67ae3..e8df299631e2e 100644 --- a/lldb/unittests/Core/CMakeLists.txt +++ b/lldb/unittests/Core/CMakeLists.txt @@ -1,6 +1,6 @@ -if (LLVM_BUILD_TELEMETRY) - set(TELEMETRY_DEPS Telemetry) -endif() + +set(TELEMETRY_DEPS Telemetry) + add_lldb_unittest(LLDBCoreTests CommunicationTest.cpp diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt index 88512d0f1dd96..9188fb93b7846 100644 --- a/llvm/CMakeLists.txt +++ b/llvm/CMakeLists.txt @@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm API documentation." OF option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF) option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON) option (LLVM_ENABLE_BINDINGS "Build bindings." ON) -option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not enable telemetry." ON) +option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable the telemetry library. If set to OFF, library cannot be enabled after build (eg., at runtime)" ON) +option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF, library cannot be enabled after build (eg., at runtime)" ON) set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html" CACHE STRING "Doxygen-generated HTML documentation install directory") diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in b/llvm/cmake/modules/LLVMConfig.cmake.in index 28655ee3ab87d..134d107ce79ba 100644 --- a/llvm/cmake/modules/LLVMConfig.cmake.in +++ b/llvm/cmake/modules/LLVMConfig.cmake.in @@ -101,6 +101,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@) set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@) set(LLVM_BUILD_TELEMETRY @LLVM
[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)
@@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm API documentation." OF option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF) option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON) option (LLVM_ENABLE_BINDINGS "Build bindings." ON) -option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not enable telemetry." ON) +option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]" ON) oontvoo wrote: This was removed. https://github.com/llvm/llvm-project/pull/128534 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)
@@ -21,6 +21,8 @@ void TelemetryInfo::serialize(Serializer &serializer) const { } Error Manager::dispatch(TelemetryInfo *Entry) { + assert(Config::BuildTimeEnableTelemetry && + "Telemetry should have been enabled"); oontvoo wrote: done https://github.com/llvm/llvm-project/pull/128534 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)
https://github.com/oontvoo updated https://github.com/llvm/llvm-project/pull/128534 >From 236ab76b7c9433f8fed6586a4ee18351779cc191 Mon Sep 17 00:00:00 2001 From: Vy Nguyen Date: Mon, 24 Feb 2025 11:32:03 -0500 Subject: [PATCH 01/14] [llvm][telemetr]Change Telemetry-disabling mechanism. Details: - Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any Telemetry code will be built. This has proven to cause more nuisance to both users of the Telemetry and any further extension of it. (Eg., we needed to put #ifdef around caller/user code) - So the new approach is to: + Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by default. + If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would still be built BUT Telemetry cannot be enabled. And no data can be collected. The benefit of this is that it simplifies user (and extension) code since we just need to put the check on Config::EnableTelemetry. Besides, the Telemetry library itself is very small, hence the additional code to be built would not cause any difference in build performance. --- lldb/source/Core/CMakeLists.txt | 4 +--- lldb/source/Core/Telemetry.cpp | 7 ++- lldb/unittests/Core/CMakeLists.txt | 6 +++--- llvm/CMakeLists.txt | 3 ++- llvm/cmake/modules/LLVMConfig.cmake.in | 1 + llvm/include/llvm/Config/llvm-config.h.cmake | 2 ++ llvm/include/llvm/Telemetry/Telemetry.h | 12 ++-- llvm/lib/CMakeLists.txt | 5 ++--- llvm/unittests/CMakeLists.txt| 4 +--- .../gn/secondary/llvm/include/llvm/Config/BUILD.gn | 1 + utils/bazel/llvm_configs/llvm-config.h.cmake | 2 ++ 11 files changed, 27 insertions(+), 20 deletions(-) diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt index 82fb5f42f9f4b..a3c12e4c1bdbc 100644 --- a/lldb/source/Core/CMakeLists.txt +++ b/lldb/source/Core/CMakeLists.txt @@ -16,9 +16,7 @@ if (LLDB_ENABLE_CURSES) endif() endif() -if (LLVM_BUILD_TELEMETRY) - set(TELEMETRY_DEPS Telemetry) -endif() +set(TELEMETRY_DEPS Telemetry) # TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore add_lldb_library(lldbCore diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp index 5222f76704f91..51a860ea5313b 100644 --- a/lldb/source/Core/Telemetry.cpp +++ b/lldb/source/Core/Telemetry.cpp @@ -8,8 +8,6 @@ #include "llvm/Config/llvm-config.h" -#ifdef LLVM_BUILD_TELEMETRY - #include "lldb/Core/Telemetry.h" #include "lldb/Core/Debugger.h" #include "lldb/Utility/LLDBLog.h" @@ -57,7 +55,8 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const { } TelemetryManager::TelemetryManager(std::unique_ptr config) -: m_config(std::move(config)) {} +: m_config(std::move(config)) +} llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) { // Do nothing for now. @@ -75,5 +74,3 @@ void TelemetryManager::setInstance(std::unique_ptr manager) { } // namespace telemetry } // namespace lldb_private - -#endif // LLVM_BUILD_TELEMETRY diff --git a/lldb/unittests/Core/CMakeLists.txt b/lldb/unittests/Core/CMakeLists.txt index d4d3764b67ae3..e8df299631e2e 100644 --- a/lldb/unittests/Core/CMakeLists.txt +++ b/lldb/unittests/Core/CMakeLists.txt @@ -1,6 +1,6 @@ -if (LLVM_BUILD_TELEMETRY) - set(TELEMETRY_DEPS Telemetry) -endif() + +set(TELEMETRY_DEPS Telemetry) + add_lldb_unittest(LLDBCoreTests CommunicationTest.cpp diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt index 88512d0f1dd96..9188fb93b7846 100644 --- a/llvm/CMakeLists.txt +++ b/llvm/CMakeLists.txt @@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm API documentation." OF option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF) option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON) option (LLVM_ENABLE_BINDINGS "Build bindings." ON) -option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not enable telemetry." ON) +option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable the telemetry library. If set to OFF, library cannot be enabled after build (eg., at runtime)" ON) +option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF, library cannot be enabled after build (eg., at runtime)" ON) set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html" CACHE STRING "Doxygen-generated HTML documentation install directory") diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in b/llvm/cmake/modules/LLVMConfig.cmake.in index 28655ee3ab87d..134d107ce79ba 100644 --- a/llvm/cmake/modules/LLVMConfig.cmake.in +++ b/llvm/cmake/modules/LLVMConfig.cmake.in @@ -101,6 +101,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@) set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@) set(LLVM_BUILD_TELEMETRY @LLVM
[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)
https://github.com/oontvoo updated https://github.com/llvm/llvm-project/pull/128534 >From 236ab76b7c9433f8fed6586a4ee18351779cc191 Mon Sep 17 00:00:00 2001 From: Vy Nguyen Date: Mon, 24 Feb 2025 11:32:03 -0500 Subject: [PATCH 01/13] [llvm][telemetr]Change Telemetry-disabling mechanism. Details: - Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any Telemetry code will be built. This has proven to cause more nuisance to both users of the Telemetry and any further extension of it. (Eg., we needed to put #ifdef around caller/user code) - So the new approach is to: + Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by default. + If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would still be built BUT Telemetry cannot be enabled. And no data can be collected. The benefit of this is that it simplifies user (and extension) code since we just need to put the check on Config::EnableTelemetry. Besides, the Telemetry library itself is very small, hence the additional code to be built would not cause any difference in build performance. --- lldb/source/Core/CMakeLists.txt | 4 +--- lldb/source/Core/Telemetry.cpp | 7 ++- lldb/unittests/Core/CMakeLists.txt | 6 +++--- llvm/CMakeLists.txt | 3 ++- llvm/cmake/modules/LLVMConfig.cmake.in | 1 + llvm/include/llvm/Config/llvm-config.h.cmake | 2 ++ llvm/include/llvm/Telemetry/Telemetry.h | 12 ++-- llvm/lib/CMakeLists.txt | 5 ++--- llvm/unittests/CMakeLists.txt| 4 +--- .../gn/secondary/llvm/include/llvm/Config/BUILD.gn | 1 + utils/bazel/llvm_configs/llvm-config.h.cmake | 2 ++ 11 files changed, 27 insertions(+), 20 deletions(-) diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt index 82fb5f42f9f4b..a3c12e4c1bdbc 100644 --- a/lldb/source/Core/CMakeLists.txt +++ b/lldb/source/Core/CMakeLists.txt @@ -16,9 +16,7 @@ if (LLDB_ENABLE_CURSES) endif() endif() -if (LLVM_BUILD_TELEMETRY) - set(TELEMETRY_DEPS Telemetry) -endif() +set(TELEMETRY_DEPS Telemetry) # TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore add_lldb_library(lldbCore diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp index 5222f76704f91..51a860ea5313b 100644 --- a/lldb/source/Core/Telemetry.cpp +++ b/lldb/source/Core/Telemetry.cpp @@ -8,8 +8,6 @@ #include "llvm/Config/llvm-config.h" -#ifdef LLVM_BUILD_TELEMETRY - #include "lldb/Core/Telemetry.h" #include "lldb/Core/Debugger.h" #include "lldb/Utility/LLDBLog.h" @@ -57,7 +55,8 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const { } TelemetryManager::TelemetryManager(std::unique_ptr config) -: m_config(std::move(config)) {} +: m_config(std::move(config)) +} llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) { // Do nothing for now. @@ -75,5 +74,3 @@ void TelemetryManager::setInstance(std::unique_ptr manager) { } // namespace telemetry } // namespace lldb_private - -#endif // LLVM_BUILD_TELEMETRY diff --git a/lldb/unittests/Core/CMakeLists.txt b/lldb/unittests/Core/CMakeLists.txt index d4d3764b67ae3..e8df299631e2e 100644 --- a/lldb/unittests/Core/CMakeLists.txt +++ b/lldb/unittests/Core/CMakeLists.txt @@ -1,6 +1,6 @@ -if (LLVM_BUILD_TELEMETRY) - set(TELEMETRY_DEPS Telemetry) -endif() + +set(TELEMETRY_DEPS Telemetry) + add_lldb_unittest(LLDBCoreTests CommunicationTest.cpp diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt index 88512d0f1dd96..9188fb93b7846 100644 --- a/llvm/CMakeLists.txt +++ b/llvm/CMakeLists.txt @@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm API documentation." OF option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF) option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON) option (LLVM_ENABLE_BINDINGS "Build bindings." ON) -option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not enable telemetry." ON) +option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable the telemetry library. If set to OFF, library cannot be enabled after build (eg., at runtime)" ON) +option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF, library cannot be enabled after build (eg., at runtime)" ON) set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html" CACHE STRING "Doxygen-generated HTML documentation install directory") diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in b/llvm/cmake/modules/LLVMConfig.cmake.in index 28655ee3ab87d..134d107ce79ba 100644 --- a/llvm/cmake/modules/LLVMConfig.cmake.in +++ b/llvm/cmake/modules/LLVMConfig.cmake.in @@ -101,6 +101,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@) set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@) set(LLVM_BUILD_TELEMETRY @LLVM
[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff eacbcbe47744a496ad1651ebd65914f9e6a66f85 a72c7db93aa9c04fbe48e058f5e2c829a608252f --extensions h,cpp -- lldb/source/Core/Telemetry.cpp lldb/unittests/Core/TelemetryTest.cpp llvm/include/llvm/Telemetry/Telemetry.h llvm/lib/Telemetry/Telemetry.cpp llvm/unittests/Telemetry/TelemetryTest.cpp `` View the diff from clang-format here. ``diff diff --git a/lldb/unittests/Core/TelemetryTest.cpp b/lldb/unittests/Core/TelemetryTest.cpp index 5bb46c65ac..22fade5866 100644 --- a/lldb/unittests/Core/TelemetryTest.cpp +++ b/lldb/unittests/Core/TelemetryTest.cpp @@ -72,7 +72,6 @@ public: #define TELEMETRY_TEST(suite, test) TEST(DISABLED_##suite, test) #endif - TELEMETRY_TEST(TelemetryTest, PluginTest) { // This would have been called by the plugin reg in a "real" plugin // For tests, we just call it directly. diff --git a/llvm/unittests/Telemetry/TelemetryTest.cpp b/llvm/unittests/Telemetry/TelemetryTest.cpp index b96ec0503d..c1b9f38bc1 100644 --- a/llvm/unittests/Telemetry/TelemetryTest.cpp +++ b/llvm/unittests/Telemetry/TelemetryTest.cpp @@ -218,7 +218,6 @@ std::shared_ptr getTelemetryConfig(const TestContext &Ctxt) { #define TELEMETRY_TEST(suite, test) TEST(DISABLED_##suite, test) #endif - TELEMETRY_TEST(TelemetryTest, TelemetryDisabled) { TestContext Context; Context.HasVendorPlugin = false; `` https://github.com/llvm/llvm-project/pull/128534 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Build the API unittests with -Wdocumentation (PR #128893)
https://github.com/JDevlieghere created https://github.com/llvm/llvm-project/pull/128893 The LLDB SB API headers should be -Wdocumentation clean as they might get included by projects building with -Wdocumentation. Although I'd love for all of LLDB to be clean, we're pretty far removed from that goal. Until that changes, this PR will detect issues in the SB API headers by including all the headers in the unittests (by including LLDB/API.h) and building that with the warning, if the compiler supports it. rdar://143597614 >From 941fb2b4e928f474dc32c2023ff37952a4034cac Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Wed, 26 Feb 2025 09:17:51 -0600 Subject: [PATCH] [lldb] Build the API unittests with -Wdocumentation The LLDB SB API headers should be -Wdocumentation clean as they might get included by projects building with -Wdocumentation. Although I'd love for all of LLDB to be clean, we're pretty far removed from that goal. Until that changes, this PR will detect issues in the SB API headers by including all the headers in the unittests (by including LLDB/API.h) and building that with the warning, if the compiler supports it. rdar://143597614 --- lldb/include/lldb/API/SBSaveCoreOptions.h | 2 +- lldb/unittests/API/CMakeLists.txt | 10 ++ lldb/unittests/API/SBCommandInterpreterTest.cpp | 5 ++--- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h index 7852858f8ade9..c6d2ab6099b3c 100644 --- a/lldb/include/lldb/API/SBSaveCoreOptions.h +++ b/lldb/include/lldb/API/SBSaveCoreOptions.h @@ -54,7 +54,7 @@ class LLDB_API SBSaveCoreOptions { /// Set the output file path /// /// \param - /// output_file a \class SBFileSpec object that describes the output file. + /// output_file a \ref SBFileSpec object that describes the output file. void SetOutputFile(SBFileSpec output_file); /// Get the output file spec diff --git a/lldb/unittests/API/CMakeLists.txt b/lldb/unittests/API/CMakeLists.txt index 2f066f26d8aaf..52e9a5e991515 100644 --- a/lldb/unittests/API/CMakeLists.txt +++ b/lldb/unittests/API/CMakeLists.txt @@ -5,6 +5,16 @@ add_lldb_unittest(APITests liblldb ) +# Build with -Wdocumentation. This relies on the tests including all the API +# headers through API/LLDB.h. +check_cxx_compiler_flag("-Wdocumentation" +CXX_SUPPORTS_DOCUMENTATION) +if (CXX_SUPPORTS_DOCUMENTATION) + target_compile_options(APITests +PRIVATE -Wdocumentation) +endif() + + if(Python3_RPATH) set_property(TARGET APITests APPEND PROPERTY BUILD_RPATH "${Python3_RPATH}") endif() diff --git a/lldb/unittests/API/SBCommandInterpreterTest.cpp b/lldb/unittests/API/SBCommandInterpreterTest.cpp index d3f6ec639162b..941b738e84ac8 100644 --- a/lldb/unittests/API/SBCommandInterpreterTest.cpp +++ b/lldb/unittests/API/SBCommandInterpreterTest.cpp @@ -8,9 +8,8 @@ #include "gtest/gtest.h" -#include "lldb/API/SBCommandInterpreter.h" -#include "lldb/API/SBCommandReturnObject.h" -#include "lldb/API/SBDebugger.h" +// Use the umbrella header for -Wdocumentation. +#include "lldb/API/LLDB.h" #include #include ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Build the API unittests with -Wdocumentation (PR #128893)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) Changes The LLDB SB API headers should be -Wdocumentation clean as they might get included by projects building with -Wdocumentation. Although I'd love for all of LLDB to be clean, we're pretty far removed from that goal. Until that changes, this PR will detect issues in the SB API headers by including all the headers in the unittests (by including LLDB/API.h) and building that with the warning, if the compiler supports it. rdar://143597614 --- Full diff: https://github.com/llvm/llvm-project/pull/128893.diff 3 Files Affected: - (modified) lldb/include/lldb/API/SBSaveCoreOptions.h (+1-1) - (modified) lldb/unittests/API/CMakeLists.txt (+10) - (modified) lldb/unittests/API/SBCommandInterpreterTest.cpp (+2-3) ``diff diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h index 7852858f8ade9..c6d2ab6099b3c 100644 --- a/lldb/include/lldb/API/SBSaveCoreOptions.h +++ b/lldb/include/lldb/API/SBSaveCoreOptions.h @@ -54,7 +54,7 @@ class LLDB_API SBSaveCoreOptions { /// Set the output file path /// /// \param - /// output_file a \class SBFileSpec object that describes the output file. + /// output_file a \ref SBFileSpec object that describes the output file. void SetOutputFile(SBFileSpec output_file); /// Get the output file spec diff --git a/lldb/unittests/API/CMakeLists.txt b/lldb/unittests/API/CMakeLists.txt index 2f066f26d8aaf..52e9a5e991515 100644 --- a/lldb/unittests/API/CMakeLists.txt +++ b/lldb/unittests/API/CMakeLists.txt @@ -5,6 +5,16 @@ add_lldb_unittest(APITests liblldb ) +# Build with -Wdocumentation. This relies on the tests including all the API +# headers through API/LLDB.h. +check_cxx_compiler_flag("-Wdocumentation" +CXX_SUPPORTS_DOCUMENTATION) +if (CXX_SUPPORTS_DOCUMENTATION) + target_compile_options(APITests +PRIVATE -Wdocumentation) +endif() + + if(Python3_RPATH) set_property(TARGET APITests APPEND PROPERTY BUILD_RPATH "${Python3_RPATH}") endif() diff --git a/lldb/unittests/API/SBCommandInterpreterTest.cpp b/lldb/unittests/API/SBCommandInterpreterTest.cpp index d3f6ec639162b..941b738e84ac8 100644 --- a/lldb/unittests/API/SBCommandInterpreterTest.cpp +++ b/lldb/unittests/API/SBCommandInterpreterTest.cpp @@ -8,9 +8,8 @@ #include "gtest/gtest.h" -#include "lldb/API/SBCommandInterpreter.h" -#include "lldb/API/SBCommandReturnObject.h" -#include "lldb/API/SBDebugger.h" +// Use the umbrella header for -Wdocumentation. +#include "lldb/API/LLDB.h" #include #include `` https://github.com/llvm/llvm-project/pull/128893 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Deindent UnwindAssemblyInstEmulation (PR #128874)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/128874 >From 51e6692e62331b801fe23217c21657e6365179d1 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Wed, 26 Feb 2025 14:42:14 +0100 Subject: [PATCH 1/2] [lldb] Deindent UnwindAssemblyInstEmulation by two levels using early returns. --- .../UnwindAssemblyInstEmulation.cpp | 438 +- 1 file changed, 211 insertions(+), 227 deletions(-) diff --git a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp index 502231002f7f4..3988dcb50f353 100644 --- a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp +++ b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp @@ -56,251 +56,235 @@ bool UnwindAssemblyInstEmulation::GetNonCallSiteUnwindPlanFromAssembly( if (opcode_data == nullptr || opcode_size == 0) return false; - if (range.GetByteSize() > 0 && range.GetBaseAddress().IsValid() && - m_inst_emulator_up.get()) { + if (range.GetByteSize() == 0 || !range.GetBaseAddress().IsValid() || + !m_inst_emulator_up) +return false; -// The instruction emulation subclass setup the unwind plan for the first -// instruction. -m_inst_emulator_up->CreateFunctionEntryUnwind(unwind_plan); + // The instruction emulation subclass setup the unwind plan for the first + // instruction. + m_inst_emulator_up->CreateFunctionEntryUnwind(unwind_plan); -// CreateFunctionEntryUnwind should have created the first row. If it -// doesn't, then we are done. -if (unwind_plan.GetRowCount() == 0) - return false; + // CreateFunctionEntryUnwind should have created the first row. If it doesn't, + // then we are done. + if (unwind_plan.GetRowCount() == 0) +return false; -const bool prefer_file_cache = true; -DisassemblerSP disasm_sp(Disassembler::DisassembleBytes( -m_arch, nullptr, nullptr, nullptr, nullptr, range.GetBaseAddress(), -opcode_data, opcode_size, 9, prefer_file_cache)); + const bool prefer_file_cache = true; + DisassemblerSP disasm_sp(Disassembler::DisassembleBytes( + m_arch, nullptr, nullptr, nullptr, nullptr, range.GetBaseAddress(), + opcode_data, opcode_size, 9, prefer_file_cache)); -Log *log = GetLog(LLDBLog::Unwind); + if (!disasm_sp) +return false; -if (disasm_sp) { + Log *log = GetLog(LLDBLog::Unwind); - m_range_ptr = ⦥ - m_unwind_plan_ptr = &unwind_plan; + m_range_ptr = ⦥ + m_unwind_plan_ptr = &unwind_plan; + + const uint32_t addr_byte_size = m_arch.GetAddressByteSize(); + const bool show_address = true; + const bool show_bytes = true; + const bool show_control_flow_kind = false; + m_cfa_reg_info = *m_inst_emulator_up->GetRegisterInfo( + unwind_plan.GetRegisterKind(), unwind_plan.GetInitialCFARegister()); + m_fp_is_cfa = false; + m_register_values.clear(); + m_pushed_regs.clear(); + + // Initialize the CFA with a known value. In the 32 bit case it will be + // 0x8000, and in the 64 bit case 0x8000. We use the address + // byte size to be safe for any future address sizes + m_initial_sp = (1ull << ((addr_byte_size * 8) - 1)); + RegisterValue cfa_reg_value; + cfa_reg_value.SetUInt(m_initial_sp, m_cfa_reg_info.byte_size); + SetRegisterValue(m_cfa_reg_info, cfa_reg_value); + + const InstructionList &inst_list = disasm_sp->GetInstructionList(); + const size_t num_instructions = inst_list.GetSize(); + + if (num_instructions > 0) { +Instruction *inst = inst_list.GetInstructionAtIndex(0).get(); +const lldb::addr_t base_addr = inst->GetAddress().GetFileAddress(); + +// Map for storing the unwind plan row and the value of the registers at a +// given offset. When we see a forward branch we add a new entry to this map +// with the actual unwind plan row and register context for the target +// address of the branch as the current data have to be valid for the target +// address of the branch too if we are in the same function. +std::map> +saved_unwind_states; + +// Make a copy of the current instruction Row and save it in m_curr_row so +// we can add updates as we process the instructions. +UnwindPlan::RowSP last_row = +std::make_shared(*unwind_plan.GetLastRow()); +m_curr_row = std::make_shared(*last_row); + +// Add the initial state to the save list with offset 0. +saved_unwind_states.insert({0, {last_row, m_register_values}}); + +// cache the stack pointer register number (in whatever register numbering +// this UnwindPlan uses) for quick reference during instruction parsing. +RegisterInfo sp_reg_info = *m_inst_emulator_up->GetRegisterInfo( +eRegisterKindGeneric, LLDB_REGNUM_GENERIC_SP); + +// The architecture dependent condition code of the last processed +// instruction. +Emula
[Lldb-commits] [lldb] [lldb] Deindent UnwindAssemblyInstEmulation (PR #128874)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/128874 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Deindent UnwindAssemblyInstEmulation (PR #128874)
https://github.com/labath created https://github.com/llvm/llvm-project/pull/128874 by two levels using early returns. >From 901fc9d22499bc67d9da944fea138e29c803c44d Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Wed, 26 Feb 2025 14:42:14 +0100 Subject: [PATCH] [lldb] Deindent UnwindAssemblyInstEmulation by two levels using early returns. --- .../UnwindAssemblyInstEmulation.cpp | 438 +- 1 file changed, 211 insertions(+), 227 deletions(-) diff --git a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp index 502231002f7f4..ace76e253dec5 100644 --- a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp +++ b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp @@ -56,251 +56,235 @@ bool UnwindAssemblyInstEmulation::GetNonCallSiteUnwindPlanFromAssembly( if (opcode_data == nullptr || opcode_size == 0) return false; - if (range.GetByteSize() > 0 && range.GetBaseAddress().IsValid() && - m_inst_emulator_up.get()) { + if (range.GetByteSize() == 0 || !range.GetBaseAddress().IsValid() || + !m_inst_emulator_up) +return false; -// The instruction emulation subclass setup the unwind plan for the first -// instruction. -m_inst_emulator_up->CreateFunctionEntryUnwind(unwind_plan); + // The instruction emulation subclass setup the unwind plan for the first + // instruction. + m_inst_emulator_up->CreateFunctionEntryUnwind(unwind_plan); -// CreateFunctionEntryUnwind should have created the first row. If it -// doesn't, then we are done. -if (unwind_plan.GetRowCount() == 0) - return false; + // CreateFunctionEntryUnwind should have created the first row. If it doesn't, + // then we are done. + if (unwind_plan.GetRowCount() == 0) +return false; -const bool prefer_file_cache = true; -DisassemblerSP disasm_sp(Disassembler::DisassembleBytes( -m_arch, nullptr, nullptr, nullptr, nullptr, range.GetBaseAddress(), -opcode_data, opcode_size, 9, prefer_file_cache)); + const bool prefer_file_cache = true; + DisassemblerSP disasm_sp(Disassembler::DisassembleBytes( + m_arch, nullptr, nullptr, nullptr, nullptr, range.GetBaseAddress(), + opcode_data, opcode_size, 9, prefer_file_cache)); -Log *log = GetLog(LLDBLog::Unwind); + if (!disasm_sp) +return false; -if (disasm_sp) { + Log *log = GetLog(LLDBLog::Unwind); - m_range_ptr = ⦥ - m_unwind_plan_ptr = &unwind_plan; + m_range_ptr = ⦥ + m_unwind_plan_ptr = &unwind_plan; + + const uint32_t addr_byte_size = m_arch.GetAddressByteSize(); + const bool show_address = true; + const bool show_bytes = true; + const bool show_control_flow_kind = false; + m_cfa_reg_info = *m_inst_emulator_up->GetRegisterInfo( + unwind_plan.GetRegisterKind(), unwind_plan.GetInitialCFARegister()); + m_fp_is_cfa = false; + m_register_values.clear(); + m_pushed_regs.clear(); + + // Initialize the CFA with a known value. In the 32 bit case it will be + // 0x8000, and in the 64 bit case 0x8000. We use the address + // byte size to be safe for any future address sizes + m_initial_sp = (1ull << ((addr_byte_size * 8) - 1)); + RegisterValue cfa_reg_value; + cfa_reg_value.SetUInt(m_initial_sp, m_cfa_reg_info.byte_size); + SetRegisterValue(m_cfa_reg_info, cfa_reg_value); + + const InstructionList &inst_list = disasm_sp->GetInstructionList(); + const size_t num_instructions = inst_list.GetSize(); + + if (num_instructions > 0) { +Instruction *inst = inst_list.GetInstructionAtIndex(0).get(); +const lldb::addr_t base_addr = inst->GetAddress().GetFileAddress(); + +// Map for storing the unwind plan row and the value of the registers at a +// given offset. When we see a forward branch we add a new entry to this map +// with the actual unwind plan row and register context for the target +// address of the branch as the current data have to be valid for the target +// address of the branch too if we are in the same function. +std::map> +saved_unwind_states; + +// Make a copy of the current instruction Row and save it in m_curr_row so +// we can add updates as we process the instructions. +UnwindPlan::RowSP last_row = +std::make_shared(*unwind_plan.GetLastRow()); +m_curr_row = std::make_shared(*last_row); + +// Add the initial state to the save list with offset 0. +saved_unwind_states.insert({0, {last_row, m_register_values}}); + +// cache the stack pointer register number (in whatever register numbering +// this UnwindPlan uses) for quick reference during instruction parsing. +RegisterInfo sp_reg_info = *m_inst_emulator_up->GetRegisterInfo( +eRegisterKindGeneric, LLDB_REGNUM_GENERIC_SP); + +// The architecture dependent condition code of the last processed
[Lldb-commits] [lldb] [lldb] Deindent UnwindAssemblyInstEmulation (PR #128874)
https://github.com/labath updated https://github.com/llvm/llvm-project/pull/128874 >From 51e6692e62331b801fe23217c21657e6365179d1 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Wed, 26 Feb 2025 14:42:14 +0100 Subject: [PATCH] [lldb] Deindent UnwindAssemblyInstEmulation by two levels using early returns. --- .../UnwindAssemblyInstEmulation.cpp | 438 +- 1 file changed, 211 insertions(+), 227 deletions(-) diff --git a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp index 502231002f7f4..3988dcb50f353 100644 --- a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp +++ b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp @@ -56,251 +56,235 @@ bool UnwindAssemblyInstEmulation::GetNonCallSiteUnwindPlanFromAssembly( if (opcode_data == nullptr || opcode_size == 0) return false; - if (range.GetByteSize() > 0 && range.GetBaseAddress().IsValid() && - m_inst_emulator_up.get()) { + if (range.GetByteSize() == 0 || !range.GetBaseAddress().IsValid() || + !m_inst_emulator_up) +return false; -// The instruction emulation subclass setup the unwind plan for the first -// instruction. -m_inst_emulator_up->CreateFunctionEntryUnwind(unwind_plan); + // The instruction emulation subclass setup the unwind plan for the first + // instruction. + m_inst_emulator_up->CreateFunctionEntryUnwind(unwind_plan); -// CreateFunctionEntryUnwind should have created the first row. If it -// doesn't, then we are done. -if (unwind_plan.GetRowCount() == 0) - return false; + // CreateFunctionEntryUnwind should have created the first row. If it doesn't, + // then we are done. + if (unwind_plan.GetRowCount() == 0) +return false; -const bool prefer_file_cache = true; -DisassemblerSP disasm_sp(Disassembler::DisassembleBytes( -m_arch, nullptr, nullptr, nullptr, nullptr, range.GetBaseAddress(), -opcode_data, opcode_size, 9, prefer_file_cache)); + const bool prefer_file_cache = true; + DisassemblerSP disasm_sp(Disassembler::DisassembleBytes( + m_arch, nullptr, nullptr, nullptr, nullptr, range.GetBaseAddress(), + opcode_data, opcode_size, 9, prefer_file_cache)); -Log *log = GetLog(LLDBLog::Unwind); + if (!disasm_sp) +return false; -if (disasm_sp) { + Log *log = GetLog(LLDBLog::Unwind); - m_range_ptr = ⦥ - m_unwind_plan_ptr = &unwind_plan; + m_range_ptr = ⦥ + m_unwind_plan_ptr = &unwind_plan; + + const uint32_t addr_byte_size = m_arch.GetAddressByteSize(); + const bool show_address = true; + const bool show_bytes = true; + const bool show_control_flow_kind = false; + m_cfa_reg_info = *m_inst_emulator_up->GetRegisterInfo( + unwind_plan.GetRegisterKind(), unwind_plan.GetInitialCFARegister()); + m_fp_is_cfa = false; + m_register_values.clear(); + m_pushed_regs.clear(); + + // Initialize the CFA with a known value. In the 32 bit case it will be + // 0x8000, and in the 64 bit case 0x8000. We use the address + // byte size to be safe for any future address sizes + m_initial_sp = (1ull << ((addr_byte_size * 8) - 1)); + RegisterValue cfa_reg_value; + cfa_reg_value.SetUInt(m_initial_sp, m_cfa_reg_info.byte_size); + SetRegisterValue(m_cfa_reg_info, cfa_reg_value); + + const InstructionList &inst_list = disasm_sp->GetInstructionList(); + const size_t num_instructions = inst_list.GetSize(); + + if (num_instructions > 0) { +Instruction *inst = inst_list.GetInstructionAtIndex(0).get(); +const lldb::addr_t base_addr = inst->GetAddress().GetFileAddress(); + +// Map for storing the unwind plan row and the value of the registers at a +// given offset. When we see a forward branch we add a new entry to this map +// with the actual unwind plan row and register context for the target +// address of the branch as the current data have to be valid for the target +// address of the branch too if we are in the same function. +std::map> +saved_unwind_states; + +// Make a copy of the current instruction Row and save it in m_curr_row so +// we can add updates as we process the instructions. +UnwindPlan::RowSP last_row = +std::make_shared(*unwind_plan.GetLastRow()); +m_curr_row = std::make_shared(*last_row); + +// Add the initial state to the save list with offset 0. +saved_unwind_states.insert({0, {last_row, m_register_values}}); + +// cache the stack pointer register number (in whatever register numbering +// this UnwindPlan uses) for quick reference during instruction parsing. +RegisterInfo sp_reg_info = *m_inst_emulator_up->GetRegisterInfo( +eRegisterKindGeneric, LLDB_REGNUM_GENERIC_SP); + +// The architecture dependent condition code of the last processed +// instruction. +EmulateIn
[Lldb-commits] [lldb] [lldb] Deindent UnwindAssemblyInstEmulation (PR #128874)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Pavel Labath (labath) Changes by two levels using early returns. --- Patch is 22.17 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/128874.diff 1 Files Affected: - (modified) lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp (+211-227) ``diff diff --git a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp index 502231002f7f4..3988dcb50f353 100644 --- a/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp +++ b/lldb/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp @@ -56,251 +56,235 @@ bool UnwindAssemblyInstEmulation::GetNonCallSiteUnwindPlanFromAssembly( if (opcode_data == nullptr || opcode_size == 0) return false; - if (range.GetByteSize() > 0 && range.GetBaseAddress().IsValid() && - m_inst_emulator_up.get()) { + if (range.GetByteSize() == 0 || !range.GetBaseAddress().IsValid() || + !m_inst_emulator_up) +return false; -// The instruction emulation subclass setup the unwind plan for the first -// instruction. -m_inst_emulator_up->CreateFunctionEntryUnwind(unwind_plan); + // The instruction emulation subclass setup the unwind plan for the first + // instruction. + m_inst_emulator_up->CreateFunctionEntryUnwind(unwind_plan); -// CreateFunctionEntryUnwind should have created the first row. If it -// doesn't, then we are done. -if (unwind_plan.GetRowCount() == 0) - return false; + // CreateFunctionEntryUnwind should have created the first row. If it doesn't, + // then we are done. + if (unwind_plan.GetRowCount() == 0) +return false; -const bool prefer_file_cache = true; -DisassemblerSP disasm_sp(Disassembler::DisassembleBytes( -m_arch, nullptr, nullptr, nullptr, nullptr, range.GetBaseAddress(), -opcode_data, opcode_size, 9, prefer_file_cache)); + const bool prefer_file_cache = true; + DisassemblerSP disasm_sp(Disassembler::DisassembleBytes( + m_arch, nullptr, nullptr, nullptr, nullptr, range.GetBaseAddress(), + opcode_data, opcode_size, 9, prefer_file_cache)); -Log *log = GetLog(LLDBLog::Unwind); + if (!disasm_sp) +return false; -if (disasm_sp) { + Log *log = GetLog(LLDBLog::Unwind); - m_range_ptr = ⦥ - m_unwind_plan_ptr = &unwind_plan; + m_range_ptr = ⦥ + m_unwind_plan_ptr = &unwind_plan; + + const uint32_t addr_byte_size = m_arch.GetAddressByteSize(); + const bool show_address = true; + const bool show_bytes = true; + const bool show_control_flow_kind = false; + m_cfa_reg_info = *m_inst_emulator_up->GetRegisterInfo( + unwind_plan.GetRegisterKind(), unwind_plan.GetInitialCFARegister()); + m_fp_is_cfa = false; + m_register_values.clear(); + m_pushed_regs.clear(); + + // Initialize the CFA with a known value. In the 32 bit case it will be + // 0x8000, and in the 64 bit case 0x8000. We use the address + // byte size to be safe for any future address sizes + m_initial_sp = (1ull << ((addr_byte_size * 8) - 1)); + RegisterValue cfa_reg_value; + cfa_reg_value.SetUInt(m_initial_sp, m_cfa_reg_info.byte_size); + SetRegisterValue(m_cfa_reg_info, cfa_reg_value); + + const InstructionList &inst_list = disasm_sp->GetInstructionList(); + const size_t num_instructions = inst_list.GetSize(); + + if (num_instructions > 0) { +Instruction *inst = inst_list.GetInstructionAtIndex(0).get(); +const lldb::addr_t base_addr = inst->GetAddress().GetFileAddress(); + +// Map for storing the unwind plan row and the value of the registers at a +// given offset. When we see a forward branch we add a new entry to this map +// with the actual unwind plan row and register context for the target +// address of the branch as the current data have to be valid for the target +// address of the branch too if we are in the same function. +std::map> +saved_unwind_states; + +// Make a copy of the current instruction Row and save it in m_curr_row so +// we can add updates as we process the instructions. +UnwindPlan::RowSP last_row = +std::make_shared(*unwind_plan.GetLastRow()); +m_curr_row = std::make_shared(*last_row); + +// Add the initial state to the save list with offset 0. +saved_unwind_states.insert({0, {last_row, m_register_values}}); + +// cache the stack pointer register number (in whatever register numbering +// this UnwindPlan uses) for quick reference during instruction parsing. +RegisterInfo sp_reg_info = *m_inst_emulator_up->GetRegisterInfo( +eRegisterKindGeneric, LLDB_REGNUM_GENERIC_SP); + +// The architecture dependent condition code of the last processed +// instruction. +EmulateInstruction::InstructionCondition last_condition =
[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)
https://github.com/oontvoo updated https://github.com/llvm/llvm-project/pull/128534 >From 236ab76b7c9433f8fed6586a4ee18351779cc191 Mon Sep 17 00:00:00 2001 From: Vy Nguyen Date: Mon, 24 Feb 2025 11:32:03 -0500 Subject: [PATCH 01/15] [llvm][telemetr]Change Telemetry-disabling mechanism. Details: - Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any Telemetry code will be built. This has proven to cause more nuisance to both users of the Telemetry and any further extension of it. (Eg., we needed to put #ifdef around caller/user code) - So the new approach is to: + Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by default. + If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would still be built BUT Telemetry cannot be enabled. And no data can be collected. The benefit of this is that it simplifies user (and extension) code since we just need to put the check on Config::EnableTelemetry. Besides, the Telemetry library itself is very small, hence the additional code to be built would not cause any difference in build performance. --- lldb/source/Core/CMakeLists.txt | 4 +--- lldb/source/Core/Telemetry.cpp | 7 ++- lldb/unittests/Core/CMakeLists.txt | 6 +++--- llvm/CMakeLists.txt | 3 ++- llvm/cmake/modules/LLVMConfig.cmake.in | 1 + llvm/include/llvm/Config/llvm-config.h.cmake | 2 ++ llvm/include/llvm/Telemetry/Telemetry.h | 12 ++-- llvm/lib/CMakeLists.txt | 5 ++--- llvm/unittests/CMakeLists.txt| 4 +--- .../gn/secondary/llvm/include/llvm/Config/BUILD.gn | 1 + utils/bazel/llvm_configs/llvm-config.h.cmake | 2 ++ 11 files changed, 27 insertions(+), 20 deletions(-) diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt index 82fb5f42f9f4b..a3c12e4c1bdbc 100644 --- a/lldb/source/Core/CMakeLists.txt +++ b/lldb/source/Core/CMakeLists.txt @@ -16,9 +16,7 @@ if (LLDB_ENABLE_CURSES) endif() endif() -if (LLVM_BUILD_TELEMETRY) - set(TELEMETRY_DEPS Telemetry) -endif() +set(TELEMETRY_DEPS Telemetry) # TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore add_lldb_library(lldbCore diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp index 5222f76704f91..51a860ea5313b 100644 --- a/lldb/source/Core/Telemetry.cpp +++ b/lldb/source/Core/Telemetry.cpp @@ -8,8 +8,6 @@ #include "llvm/Config/llvm-config.h" -#ifdef LLVM_BUILD_TELEMETRY - #include "lldb/Core/Telemetry.h" #include "lldb/Core/Debugger.h" #include "lldb/Utility/LLDBLog.h" @@ -57,7 +55,8 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const { } TelemetryManager::TelemetryManager(std::unique_ptr config) -: m_config(std::move(config)) {} +: m_config(std::move(config)) +} llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) { // Do nothing for now. @@ -75,5 +74,3 @@ void TelemetryManager::setInstance(std::unique_ptr manager) { } // namespace telemetry } // namespace lldb_private - -#endif // LLVM_BUILD_TELEMETRY diff --git a/lldb/unittests/Core/CMakeLists.txt b/lldb/unittests/Core/CMakeLists.txt index d4d3764b67ae3..e8df299631e2e 100644 --- a/lldb/unittests/Core/CMakeLists.txt +++ b/lldb/unittests/Core/CMakeLists.txt @@ -1,6 +1,6 @@ -if (LLVM_BUILD_TELEMETRY) - set(TELEMETRY_DEPS Telemetry) -endif() + +set(TELEMETRY_DEPS Telemetry) + add_lldb_unittest(LLDBCoreTests CommunicationTest.cpp diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt index 88512d0f1dd96..9188fb93b7846 100644 --- a/llvm/CMakeLists.txt +++ b/llvm/CMakeLists.txt @@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm API documentation." OF option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF) option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON) option (LLVM_ENABLE_BINDINGS "Build bindings." ON) -option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not enable telemetry." ON) +option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable the telemetry library. If set to OFF, library cannot be enabled after build (eg., at runtime)" ON) +option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF, library cannot be enabled after build (eg., at runtime)" ON) set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html" CACHE STRING "Doxygen-generated HTML documentation install directory") diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in b/llvm/cmake/modules/LLVMConfig.cmake.in index 28655ee3ab87d..134d107ce79ba 100644 --- a/llvm/cmake/modules/LLVMConfig.cmake.in +++ b/llvm/cmake/modules/LLVMConfig.cmake.in @@ -101,6 +101,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@) set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@) set(LLVM_BUILD_TELEMETRY @LLVM
[Lldb-commits] [lldb] [lldb] Add missing calls to SetThreadHitBreakpointSite/DetectThreadStoppedAtUnexecutedBP (PR #128726)
@@ -462,6 +462,10 @@ void Thread::ResetStopInfo() { } void Thread::SetStopInfo(const lldb::StopInfoSP &stop_info_sp) { + if (stop_info_sp && + stop_info_sp->GetStopReason() == lldb::eStopReasonBreakpoint) +SetThreadHitBreakpointSite(); JDevlieghere wrote: Seems like you could move this after the `m_stop_info_sp` check a few lines below. https://github.com/llvm/llvm-project/pull/128726 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 15ee9d9 - [lldb] Build the API unittests with -Wdocumentation (#128893)
Author: Jonas Devlieghere Date: 2025-02-26T10:23:28-06:00 New Revision: 15ee9d91fbb55a507a8f0bce7d3d66a825c6ec30 URL: https://github.com/llvm/llvm-project/commit/15ee9d91fbb55a507a8f0bce7d3d66a825c6ec30 DIFF: https://github.com/llvm/llvm-project/commit/15ee9d91fbb55a507a8f0bce7d3d66a825c6ec30.diff LOG: [lldb] Build the API unittests with -Wdocumentation (#128893) The LLDB SB API headers should be -Wdocumentation clean as they might get included by projects building with -Wdocumentation. Although I'd love for all of LLDB to be clean, we're pretty far removed from that goal. Until that changes, this PR will detect issues in the SB API headers by including all the headers in the unittests (by including LLDB/API.h) and building that with the warning, if the compiler supports it. rdar://143597614 Added: Modified: lldb/include/lldb/API/SBSaveCoreOptions.h lldb/unittests/API/CMakeLists.txt lldb/unittests/API/SBCommandInterpreterTest.cpp Removed: diff --git a/lldb/include/lldb/API/SBSaveCoreOptions.h b/lldb/include/lldb/API/SBSaveCoreOptions.h index 7852858f8ade9..c6d2ab6099b3c 100644 --- a/lldb/include/lldb/API/SBSaveCoreOptions.h +++ b/lldb/include/lldb/API/SBSaveCoreOptions.h @@ -54,7 +54,7 @@ class LLDB_API SBSaveCoreOptions { /// Set the output file path /// /// \param - /// output_file a \class SBFileSpec object that describes the output file. + /// output_file a \ref SBFileSpec object that describes the output file. void SetOutputFile(SBFileSpec output_file); /// Get the output file spec diff --git a/lldb/unittests/API/CMakeLists.txt b/lldb/unittests/API/CMakeLists.txt index 2f066f26d8aaf..52e9a5e991515 100644 --- a/lldb/unittests/API/CMakeLists.txt +++ b/lldb/unittests/API/CMakeLists.txt @@ -5,6 +5,16 @@ add_lldb_unittest(APITests liblldb ) +# Build with -Wdocumentation. This relies on the tests including all the API +# headers through API/LLDB.h. +check_cxx_compiler_flag("-Wdocumentation" +CXX_SUPPORTS_DOCUMENTATION) +if (CXX_SUPPORTS_DOCUMENTATION) + target_compile_options(APITests +PRIVATE -Wdocumentation) +endif() + + if(Python3_RPATH) set_property(TARGET APITests APPEND PROPERTY BUILD_RPATH "${Python3_RPATH}") endif() diff --git a/lldb/unittests/API/SBCommandInterpreterTest.cpp b/lldb/unittests/API/SBCommandInterpreterTest.cpp index d3f6ec639162b..941b738e84ac8 100644 --- a/lldb/unittests/API/SBCommandInterpreterTest.cpp +++ b/lldb/unittests/API/SBCommandInterpreterTest.cpp @@ -8,9 +8,8 @@ #include "gtest/gtest.h" -#include "lldb/API/SBCommandInterpreter.h" -#include "lldb/API/SBCommandReturnObject.h" -#include "lldb/API/SBDebugger.h" +// Use the umbrella header for -Wdocumentation. +#include "lldb/API/LLDB.h" #include #include ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Build the API unittests with -Wdocumentation (PR #128893)
https://github.com/JDevlieghere closed https://github.com/llvm/llvm-project/pull/128893 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Adding server mode support to lldb-dap VSCode extension. (PR #128957)
https://github.com/ashgti created https://github.com/llvm/llvm-project/pull/128957 This adds support for launching lldb-dap in server mode. The extension will start lldb-dap in server mode on-demand and retain the server until the VSCode window is closed (when the extension context is disposed). While running in server mode, launch performance for binaries is greatly improved by improving caching between debug sessions. For example, on my local M1 Max laptop it takes ~5s to attach for the first attach to an iOS Simulator process and ~0.5s to attach each time after the first. >From e95459a73ad5a1448841ed6c2422f66b23f6b486 Mon Sep 17 00:00:00 2001 From: John Harrison Date: Wed, 26 Feb 2025 14:56:10 -0800 Subject: [PATCH] [lldb-dap] Adding server mode support to lldb-dap VSCode extension. This adds support for launching lldb-dap in server mode. The extension will start lldb-dap in server mode on-demand and retain the server until the VSCode window is closed (when the extension context is disposed). While running in server mode, launch performance for binaries is greatly improved by improving caching between debug sessions. For example, on my local M1 Max laptop it takes ~5s to attach for the first attach to an iOS Simulator process and ~0.5s to attach each time after the first. --- lldb/tools/lldb-dap/package.json | 8 +- .../lldb-dap/src-ts/debug-adapter-factory.ts | 80 +-- lldb/tools/lldb-dap/src-ts/extension.ts | 9 +-- 3 files changed, 68 insertions(+), 29 deletions(-) diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json index 31d808eda4c35..51f392e386b22 100644 --- a/lldb/tools/lldb-dap/package.json +++ b/lldb/tools/lldb-dap/package.json @@ -88,6 +88,12 @@ "additionalProperties": { "type": "string" } +}, +"lldb-dap.serverMode": { + "scope": "resource", + "type": "boolean", + "description": "Run lldb-dap in server mode. When enabled, lldb-dap will start a background server that will be reused between debug sessions. This can improve launch performance and caching of debug symbols between debug sessions.", + "default": false } } }, @@ -543,4 +549,4 @@ } ] } -} +} \ No newline at end of file diff --git a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts index 36107336ebc4d..89a76d7e6f40c 100644 --- a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts +++ b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts @@ -4,6 +4,8 @@ import * as vscode from "vscode"; import * as child_process from "child_process"; import * as fs from "node:fs/promises"; +const exec = util.promisify(child_process.execFile); + export async function isExecutable(path: string): Promise { try { await fs.access(path, fs.constants.X_OK); @@ -16,7 +18,6 @@ export async function isExecutable(path: string): Promise { async function findWithXcrun(executable: string): Promise { if (process.platform === "darwin") { try { - const exec = util.promisify(child_process.execFile); let { stdout, stderr } = await exec("/usr/bin/xcrun", [ "-find", executable, @@ -24,7 +25,7 @@ async function findWithXcrun(executable: string): Promise { if (stdout) { return stdout.toString().trimEnd(); } -} catch (error) {} +} catch (error) { } } return undefined; } @@ -97,8 +98,15 @@ async function getDAPExecutable( * depending on the session configuration. */ export class LLDBDapDescriptorFactory - implements vscode.DebugAdapterDescriptorFactory -{ + implements vscode.DebugAdapterDescriptorFactory, vscode.Disposable { + private server?: Promise<{ process: child_process.ChildProcess, host: string, port: number }>; + + dispose() { +this.server?.then(({ process }) => { + process.kill(); +}); + } + async createDebugAdapterDescriptor( session: vscode.DebugSession, executable: vscode.DebugAdapterExecutable | undefined, @@ -115,7 +123,18 @@ export class LLDBDapDescriptorFactory } const configEnvironment = config.get<{ [key: string]: string }>("environment") || {}; -const dapPath = await getDAPExecutable(session); +const dapPath = (await getDAPExecutable(session)) ?? executable?.command; + +if (!dapPath) { + LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(); + return undefined; +} + +if (!(await isExecutable(dapPath))) { + LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(dapPath); + return; +} + const dbgOptions = { env: { ...executable?.options?.env, @@ -123,30 +142,45 @@ export class LLDBDapDescriptorFactory ...env, }, }; -if (dapPath) { - if (!(await isExecutable(dapPath))) { -LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(dapPath); -return und
[Lldb-commits] [lldb] [lldb-dap] Adding server mode support to lldb-dap VSCode extension. (PR #128957)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: John Harrison (ashgti) Changes This adds support for launching lldb-dap in server mode. The extension will start lldb-dap in server mode on-demand and retain the server until the VSCode window is closed (when the extension context is disposed). While running in server mode, launch performance for binaries is greatly improved by improving caching between debug sessions. For example, on my local M1 Max laptop it takes ~5s to attach for the first attach to an iOS Simulator process and ~0.5s to attach each time after the first. --- Full diff: https://github.com/llvm/llvm-project/pull/128957.diff 3 Files Affected: - (modified) lldb/tools/lldb-dap/package.json (+7-1) - (modified) lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts (+57-23) - (modified) lldb/tools/lldb-dap/src-ts/extension.ts (+4-5) ``diff diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json index 31d808eda4c35..51f392e386b22 100644 --- a/lldb/tools/lldb-dap/package.json +++ b/lldb/tools/lldb-dap/package.json @@ -88,6 +88,12 @@ "additionalProperties": { "type": "string" } +}, +"lldb-dap.serverMode": { + "scope": "resource", + "type": "boolean", + "description": "Run lldb-dap in server mode. When enabled, lldb-dap will start a background server that will be reused between debug sessions. This can improve launch performance and caching of debug symbols between debug sessions.", + "default": false } } }, @@ -543,4 +549,4 @@ } ] } -} +} \ No newline at end of file diff --git a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts index 36107336ebc4d..89a76d7e6f40c 100644 --- a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts +++ b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts @@ -4,6 +4,8 @@ import * as vscode from "vscode"; import * as child_process from "child_process"; import * as fs from "node:fs/promises"; +const exec = util.promisify(child_process.execFile); + export async function isExecutable(path: string): Promise { try { await fs.access(path, fs.constants.X_OK); @@ -16,7 +18,6 @@ export async function isExecutable(path: string): Promise { async function findWithXcrun(executable: string): Promise { if (process.platform === "darwin") { try { - const exec = util.promisify(child_process.execFile); let { stdout, stderr } = await exec("/usr/bin/xcrun", [ "-find", executable, @@ -24,7 +25,7 @@ async function findWithXcrun(executable: string): Promise { if (stdout) { return stdout.toString().trimEnd(); } -} catch (error) {} +} catch (error) { } } return undefined; } @@ -97,8 +98,15 @@ async function getDAPExecutable( * depending on the session configuration. */ export class LLDBDapDescriptorFactory - implements vscode.DebugAdapterDescriptorFactory -{ + implements vscode.DebugAdapterDescriptorFactory, vscode.Disposable { + private server?: Promise<{ process: child_process.ChildProcess, host: string, port: number }>; + + dispose() { +this.server?.then(({ process }) => { + process.kill(); +}); + } + async createDebugAdapterDescriptor( session: vscode.DebugSession, executable: vscode.DebugAdapterExecutable | undefined, @@ -115,7 +123,18 @@ export class LLDBDapDescriptorFactory } const configEnvironment = config.get<{ [key: string]: string }>("environment") || {}; -const dapPath = await getDAPExecutable(session); +const dapPath = (await getDAPExecutable(session)) ?? executable?.command; + +if (!dapPath) { + LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(); + return undefined; +} + +if (!(await isExecutable(dapPath))) { + LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(dapPath); + return; +} + const dbgOptions = { env: { ...executable?.options?.env, @@ -123,30 +142,45 @@ export class LLDBDapDescriptorFactory ...env, }, }; -if (dapPath) { - if (!(await isExecutable(dapPath))) { -LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(dapPath); -return undefined; - } - return new vscode.DebugAdapterExecutable(dapPath, [], dbgOptions); -} else if (executable) { - if (!(await isExecutable(executable.command))) { - LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(executable.command); -return undefined; - } - return new vscode.DebugAdapterExecutable( -executable.command, -executable.args, -dbgOptions, - ); +const dbgArgs = executable?.args ?? []; + +const serverMode = config.get('serverMode', false); +if (serverMode) { + const { host, port } = await this.startServer(dapPath, dbgArgs, dbgOptions); + return new vsc
[Lldb-commits] [lldb] [lldb-dap] Adding server mode support to lldb-dap VSCode extension. (PR #128957)
https://github.com/ashgti updated https://github.com/llvm/llvm-project/pull/128957 >From e95459a73ad5a1448841ed6c2422f66b23f6b486 Mon Sep 17 00:00:00 2001 From: John Harrison Date: Wed, 26 Feb 2025 14:56:10 -0800 Subject: [PATCH 1/2] [lldb-dap] Adding server mode support to lldb-dap VSCode extension. This adds support for launching lldb-dap in server mode. The extension will start lldb-dap in server mode on-demand and retain the server until the VSCode window is closed (when the extension context is disposed). While running in server mode, launch performance for binaries is greatly improved by improving caching between debug sessions. For example, on my local M1 Max laptop it takes ~5s to attach for the first attach to an iOS Simulator process and ~0.5s to attach each time after the first. --- lldb/tools/lldb-dap/package.json | 8 +- .../lldb-dap/src-ts/debug-adapter-factory.ts | 80 +-- lldb/tools/lldb-dap/src-ts/extension.ts | 9 +-- 3 files changed, 68 insertions(+), 29 deletions(-) diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json index 31d808eda4c35..51f392e386b22 100644 --- a/lldb/tools/lldb-dap/package.json +++ b/lldb/tools/lldb-dap/package.json @@ -88,6 +88,12 @@ "additionalProperties": { "type": "string" } +}, +"lldb-dap.serverMode": { + "scope": "resource", + "type": "boolean", + "description": "Run lldb-dap in server mode. When enabled, lldb-dap will start a background server that will be reused between debug sessions. This can improve launch performance and caching of debug symbols between debug sessions.", + "default": false } } }, @@ -543,4 +549,4 @@ } ] } -} +} \ No newline at end of file diff --git a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts index 36107336ebc4d..89a76d7e6f40c 100644 --- a/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts +++ b/lldb/tools/lldb-dap/src-ts/debug-adapter-factory.ts @@ -4,6 +4,8 @@ import * as vscode from "vscode"; import * as child_process from "child_process"; import * as fs from "node:fs/promises"; +const exec = util.promisify(child_process.execFile); + export async function isExecutable(path: string): Promise { try { await fs.access(path, fs.constants.X_OK); @@ -16,7 +18,6 @@ export async function isExecutable(path: string): Promise { async function findWithXcrun(executable: string): Promise { if (process.platform === "darwin") { try { - const exec = util.promisify(child_process.execFile); let { stdout, stderr } = await exec("/usr/bin/xcrun", [ "-find", executable, @@ -24,7 +25,7 @@ async function findWithXcrun(executable: string): Promise { if (stdout) { return stdout.toString().trimEnd(); } -} catch (error) {} +} catch (error) { } } return undefined; } @@ -97,8 +98,15 @@ async function getDAPExecutable( * depending on the session configuration. */ export class LLDBDapDescriptorFactory - implements vscode.DebugAdapterDescriptorFactory -{ + implements vscode.DebugAdapterDescriptorFactory, vscode.Disposable { + private server?: Promise<{ process: child_process.ChildProcess, host: string, port: number }>; + + dispose() { +this.server?.then(({ process }) => { + process.kill(); +}); + } + async createDebugAdapterDescriptor( session: vscode.DebugSession, executable: vscode.DebugAdapterExecutable | undefined, @@ -115,7 +123,18 @@ export class LLDBDapDescriptorFactory } const configEnvironment = config.get<{ [key: string]: string }>("environment") || {}; -const dapPath = await getDAPExecutable(session); +const dapPath = (await getDAPExecutable(session)) ?? executable?.command; + +if (!dapPath) { + LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(); + return undefined; +} + +if (!(await isExecutable(dapPath))) { + LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(dapPath); + return; +} + const dbgOptions = { env: { ...executable?.options?.env, @@ -123,30 +142,45 @@ export class LLDBDapDescriptorFactory ...env, }, }; -if (dapPath) { - if (!(await isExecutable(dapPath))) { -LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(dapPath); -return undefined; - } - return new vscode.DebugAdapterExecutable(dapPath, [], dbgOptions); -} else if (executable) { - if (!(await isExecutable(executable.command))) { - LLDBDapDescriptorFactory.showLLDBDapNotFoundMessage(executable.command); -return undefined; - } - return new vscode.DebugAdapterExecutable( -executable.command, -executable.args, -dbgOptions, - ); +const dbgArgs = executable?.args ?? []; + +const serverMode = c
[Lldb-commits] [lldb] [lldb-dap] Gardening in lldb-dap.cpp (NFC) (PR #128949)
https://github.com/ashgti approved this pull request. https://github.com/llvm/llvm-project/pull/128949 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Fix bad optional access in sbprogress (PR #128971)
https://github.com/Jlalond created https://github.com/llvm/llvm-project/pull/128971 This fixes the obvious, but untested case of sending None/Null to SBProgress. This was an oversight on my part. >From ac90ec73ccfb02923ff0343189d2efaeb6108fa3 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 26 Feb 2025 15:48:14 -0800 Subject: [PATCH] Fix bad optional access in sbprogress --- lldb/source/API/SBProgress.cpp | 5 - .../test/API/python_api/sbprogress/TestSBProgress.py | 12 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/lldb/source/API/SBProgress.cpp b/lldb/source/API/SBProgress.cpp index e67e289a60eff..d40e11da973d4 100644 --- a/lldb/source/API/SBProgress.cpp +++ b/lldb/source/API/SBProgress.cpp @@ -40,7 +40,10 @@ SBProgress::~SBProgress() = default; void SBProgress::Increment(uint64_t amount, const char *description) { LLDB_INSTRUMENT_VA(amount, description); - m_opaque_up->Increment(amount, description); + std::optional description_opt; + if (description && description[0]) +description_opt = description; + m_opaque_up->Increment(amount, description_opt); } lldb_private::Progress &SBProgress::ref() const { return *m_opaque_up; } diff --git a/lldb/test/API/python_api/sbprogress/TestSBProgress.py b/lldb/test/API/python_api/sbprogress/TestSBProgress.py index c456247da80c6..5f7820a5bd81e 100644 --- a/lldb/test/API/python_api/sbprogress/TestSBProgress.py +++ b/lldb/test/API/python_api/sbprogress/TestSBProgress.py @@ -33,3 +33,15 @@ def test_without_external_bit_set(self): expected_string = "Test progress first increment" progress.Increment(1, expected_string) self.assertFalse(listener.PeekAtNextEvent(event)) + +def test_with_external_bit_set(self): +"""Test SBProgress can handle null events.""" + +progress = lldb.SBProgress("Test SBProgress", "Test progress", self.dbg) +listener = lldb.SBListener("Test listener") +broadcaster = self.dbg.GetBroadcaster() +broadcaster.AddListener(listener, lldb.eBroadcastBitExternalProgress) +event = lldb.SBEvent() + +progress.Increment(1, None) +self.assertTrue(listener.PeekAtNextEvent(event)) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Add a finalize method (PR #128966)
https://github.com/Jlalond edited https://github.com/llvm/llvm-project/pull/128966 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Fix bad optional access in sbprogress (PR #128971)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Jacob Lalonde (Jlalond) Changes This fixes the obvious, but untested case of sending None/Null to SBProgress. This was an oversight on my part. --- Full diff: https://github.com/llvm/llvm-project/pull/128971.diff 2 Files Affected: - (modified) lldb/source/API/SBProgress.cpp (+4-1) - (modified) lldb/test/API/python_api/sbprogress/TestSBProgress.py (+12) ``diff diff --git a/lldb/source/API/SBProgress.cpp b/lldb/source/API/SBProgress.cpp index e67e289a60eff..d40e11da973d4 100644 --- a/lldb/source/API/SBProgress.cpp +++ b/lldb/source/API/SBProgress.cpp @@ -40,7 +40,10 @@ SBProgress::~SBProgress() = default; void SBProgress::Increment(uint64_t amount, const char *description) { LLDB_INSTRUMENT_VA(amount, description); - m_opaque_up->Increment(amount, description); + std::optional description_opt; + if (description && description[0]) +description_opt = description; + m_opaque_up->Increment(amount, description_opt); } lldb_private::Progress &SBProgress::ref() const { return *m_opaque_up; } diff --git a/lldb/test/API/python_api/sbprogress/TestSBProgress.py b/lldb/test/API/python_api/sbprogress/TestSBProgress.py index c456247da80c6..5f7820a5bd81e 100644 --- a/lldb/test/API/python_api/sbprogress/TestSBProgress.py +++ b/lldb/test/API/python_api/sbprogress/TestSBProgress.py @@ -33,3 +33,15 @@ def test_without_external_bit_set(self): expected_string = "Test progress first increment" progress.Increment(1, expected_string) self.assertFalse(listener.PeekAtNextEvent(event)) + +def test_with_external_bit_set(self): +"""Test SBProgress can handle null events.""" + +progress = lldb.SBProgress("Test SBProgress", "Test progress", self.dbg) +listener = lldb.SBListener("Test listener") +broadcaster = self.dbg.GetBroadcaster() +broadcaster.AddListener(listener, lldb.eBroadcastBitExternalProgress) +event = lldb.SBEvent() + +progress.Increment(1, None) +self.assertTrue(listener.PeekAtNextEvent(event)) `` https://github.com/llvm/llvm-project/pull/128971 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Add a finalize method (PR #128966)
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/128966 >From 3c4b333869c17005deb8d4e9307babc94d0dc9b5 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 26 Feb 2025 15:38:24 -0800 Subject: [PATCH 1/2] Add a finalize method to the sbprogress, with an associated doc string and a test --- lldb/bindings/interface/SBProgressDocstrings.i| 7 +++ lldb/include/lldb/API/SBProgress.h| 5 + lldb/source/API/SBProgress.cpp| 15 ++- .../API/python_api/sbprogress/TestSBProgress.py | 14 ++ 4 files changed, 40 insertions(+), 1 deletion(-) diff --git a/lldb/bindings/interface/SBProgressDocstrings.i b/lldb/bindings/interface/SBProgressDocstrings.i index 2997fe619fcc7..50648b62411f8 100644 --- a/lldb/bindings/interface/SBProgressDocstrings.i +++ b/lldb/bindings/interface/SBProgressDocstrings.i @@ -12,3 +12,10 @@ and will always send an initial progress update, updates when Progress::Increment() is called, and also will make sure that a progress completed update is reported even if the user doesn't explicitly cause one to be sent.") lldb::SBProgress; + +%feature("docstring", +"Explicitly end an SBProgress, this is a utility to send the progressEnd event +without waiting for your language or language implementations non-deterministic destruction +of the SBProgress object. + +Note once finalized, no further increments will be processed.") lldb::SBProgress::Finalize; diff --git a/lldb/include/lldb/API/SBProgress.h b/lldb/include/lldb/API/SBProgress.h index d574d1d2982b9..1558cdc08e080 100644 --- a/lldb/include/lldb/API/SBProgress.h +++ b/lldb/include/lldb/API/SBProgress.h @@ -59,6 +59,11 @@ class LLDB_API SBProgress { void Increment(uint64_t amount, const char *description = nullptr); + /// Explicitly finalize an SBProgress, this can be used to terminate a + /// progress on command instead of waiting for a garbage collection or other + /// finalizer. + void Finalize(); + protected: lldb_private::Progress &ref() const; diff --git a/lldb/source/API/SBProgress.cpp b/lldb/source/API/SBProgress.cpp index e67e289a60eff..4991d55ad0248 100644 --- a/lldb/source/API/SBProgress.cpp +++ b/lldb/source/API/SBProgress.cpp @@ -40,7 +40,20 @@ SBProgress::~SBProgress() = default; void SBProgress::Increment(uint64_t amount, const char *description) { LLDB_INSTRUMENT_VA(amount, description); - m_opaque_up->Increment(amount, description); + if (!m_opaque_up) +return; + + std::optional description_opt; + if (description && description[0]) +description_opt = description; + m_opaque_up->Increment(amount, description_opt); +} + +void SBProgress::Finalize() { + if (!m_opaque_up) +return; + + m_opaque_up.reset(); } lldb_private::Progress &SBProgress::ref() const { return *m_opaque_up; } diff --git a/lldb/test/API/python_api/sbprogress/TestSBProgress.py b/lldb/test/API/python_api/sbprogress/TestSBProgress.py index c456247da80c6..dd7e4b6f2ac5c 100644 --- a/lldb/test/API/python_api/sbprogress/TestSBProgress.py +++ b/lldb/test/API/python_api/sbprogress/TestSBProgress.py @@ -33,3 +33,17 @@ def test_without_external_bit_set(self): expected_string = "Test progress first increment" progress.Increment(1, expected_string) self.assertFalse(listener.PeekAtNextEvent(event)) + +def test_progress_finalize(self): +"""Test SBProgress finalize sends the progressEnd event""" + +progress = lldb.SBProgress("Test SBProgress", "Test finalize", self.dbg) +listener = lldb.SBListener("Test listener") +broadcaster = self.dbg.GetBroadcaster() +broadcaster.AddListener(listener, lldb.eBroadcastBitExternalProgressCategory) +event = lldb.SBEvent() +progress.Finalize() +self.assertTrue(listener.WaitForEvent(5, event)) +stream = lldb.SBStream() +event.GetDescription(stream) +self.assertIn("type = end", stream.GetData()) >From c461be577af8d13b889bab5a54b429610d9c969c Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 26 Feb 2025 15:54:52 -0800 Subject: [PATCH 2/2] Drop the bug fix now that I opened 128971 --- lldb/source/API/SBProgress.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/lldb/source/API/SBProgress.cpp b/lldb/source/API/SBProgress.cpp index 4991d55ad0248..656c86ac953dc 100644 --- a/lldb/source/API/SBProgress.cpp +++ b/lldb/source/API/SBProgress.cpp @@ -43,9 +43,6 @@ void SBProgress::Increment(uint64_t amount, const char *description) { if (!m_opaque_up) return; - std::optional description_opt; - if (description && description[0]) -description_opt = description; m_opaque_up->Increment(amount, description_opt); } ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Add a finalize method (PR #128966)
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/128966 >From 3c4b333869c17005deb8d4e9307babc94d0dc9b5 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 26 Feb 2025 15:38:24 -0800 Subject: [PATCH 1/2] Add a finalize method to the sbprogress, with an associated doc string and a test --- lldb/bindings/interface/SBProgressDocstrings.i| 7 +++ lldb/include/lldb/API/SBProgress.h| 5 + lldb/source/API/SBProgress.cpp| 15 ++- .../API/python_api/sbprogress/TestSBProgress.py | 14 ++ 4 files changed, 40 insertions(+), 1 deletion(-) diff --git a/lldb/bindings/interface/SBProgressDocstrings.i b/lldb/bindings/interface/SBProgressDocstrings.i index 2997fe619fcc7..50648b62411f8 100644 --- a/lldb/bindings/interface/SBProgressDocstrings.i +++ b/lldb/bindings/interface/SBProgressDocstrings.i @@ -12,3 +12,10 @@ and will always send an initial progress update, updates when Progress::Increment() is called, and also will make sure that a progress completed update is reported even if the user doesn't explicitly cause one to be sent.") lldb::SBProgress; + +%feature("docstring", +"Explicitly end an SBProgress, this is a utility to send the progressEnd event +without waiting for your language or language implementations non-deterministic destruction +of the SBProgress object. + +Note once finalized, no further increments will be processed.") lldb::SBProgress::Finalize; diff --git a/lldb/include/lldb/API/SBProgress.h b/lldb/include/lldb/API/SBProgress.h index d574d1d2982b9..1558cdc08e080 100644 --- a/lldb/include/lldb/API/SBProgress.h +++ b/lldb/include/lldb/API/SBProgress.h @@ -59,6 +59,11 @@ class LLDB_API SBProgress { void Increment(uint64_t amount, const char *description = nullptr); + /// Explicitly finalize an SBProgress, this can be used to terminate a + /// progress on command instead of waiting for a garbage collection or other + /// finalizer. + void Finalize(); + protected: lldb_private::Progress &ref() const; diff --git a/lldb/source/API/SBProgress.cpp b/lldb/source/API/SBProgress.cpp index e67e289a60eff..4991d55ad0248 100644 --- a/lldb/source/API/SBProgress.cpp +++ b/lldb/source/API/SBProgress.cpp @@ -40,7 +40,20 @@ SBProgress::~SBProgress() = default; void SBProgress::Increment(uint64_t amount, const char *description) { LLDB_INSTRUMENT_VA(amount, description); - m_opaque_up->Increment(amount, description); + if (!m_opaque_up) +return; + + std::optional description_opt; + if (description && description[0]) +description_opt = description; + m_opaque_up->Increment(amount, description_opt); +} + +void SBProgress::Finalize() { + if (!m_opaque_up) +return; + + m_opaque_up.reset(); } lldb_private::Progress &SBProgress::ref() const { return *m_opaque_up; } diff --git a/lldb/test/API/python_api/sbprogress/TestSBProgress.py b/lldb/test/API/python_api/sbprogress/TestSBProgress.py index c456247da80c6..dd7e4b6f2ac5c 100644 --- a/lldb/test/API/python_api/sbprogress/TestSBProgress.py +++ b/lldb/test/API/python_api/sbprogress/TestSBProgress.py @@ -33,3 +33,17 @@ def test_without_external_bit_set(self): expected_string = "Test progress first increment" progress.Increment(1, expected_string) self.assertFalse(listener.PeekAtNextEvent(event)) + +def test_progress_finalize(self): +"""Test SBProgress finalize sends the progressEnd event""" + +progress = lldb.SBProgress("Test SBProgress", "Test finalize", self.dbg) +listener = lldb.SBListener("Test listener") +broadcaster = self.dbg.GetBroadcaster() +broadcaster.AddListener(listener, lldb.eBroadcastBitExternalProgressCategory) +event = lldb.SBEvent() +progress.Finalize() +self.assertTrue(listener.WaitForEvent(5, event)) +stream = lldb.SBStream() +event.GetDescription(stream) +self.assertIn("type = end", stream.GetData()) >From eb5d36f37b3a990e52350243b45e3907e1362ff5 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 26 Feb 2025 15:54:52 -0800 Subject: [PATCH 2/2] Drop the bug fix now that I opened 128971 --- lldb/source/API/SBProgress.cpp | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lldb/source/API/SBProgress.cpp b/lldb/source/API/SBProgress.cpp index 4991d55ad0248..240b6e51eb9b9 100644 --- a/lldb/source/API/SBProgress.cpp +++ b/lldb/source/API/SBProgress.cpp @@ -43,10 +43,7 @@ void SBProgress::Increment(uint64_t amount, const char *description) { if (!m_opaque_up) return; - std::optional description_opt; - if (description && description[0]) -description_opt = description; - m_opaque_up->Increment(amount, description_opt); + m_opaque_up->Increment(amount, description); } void SBProgress::Finalize() { ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/lis
[Lldb-commits] [lldb] [lldb-dap] Creating a dedicated Transport class for DAP communication. (PR #128972)
https://github.com/ashgti created https://github.com/llvm/llvm-project/pull/128972 This removes the previous 'OutputStream' and 'InputStream' objects and the new 'Transport' class takes on the responsibility of serializing and encoding messages. The new Protocol.h file has a dedicated namespace for POD style structures to represent the Debug Adapter Protocol messages. >From cc70fea42c936aec2bf43e91539f509fb22c194d Mon Sep 17 00:00:00 2001 From: John Harrison Date: Wed, 26 Feb 2025 11:29:46 -0800 Subject: [PATCH] [lldb-dap] Creating a dedicated Transport mechnaism for handling protocol communication. This removes the previous 'OutputStream' and 'InputStream' objects and the new 'Transport' class takes on the responsibility of seriliaizing and encoding messages. The new Protocol.h file has a dedicated namespace for POD style structures to represent the Debug Adapter Protocol messages. --- lldb/tools/lldb-dap/CMakeLists.txt | 4 +- lldb/tools/lldb-dap/DAP.cpp| 135 +--- lldb/tools/lldb-dap/DAP.h | 17 ++- lldb/tools/lldb-dap/DAPLog.cpp | 18 +++ lldb/tools/lldb-dap/DAPLog.h | 33 + lldb/tools/lldb-dap/IOStream.cpp | 65 -- lldb/tools/lldb-dap/IOStream.h | 42 --- lldb/tools/lldb-dap/Protocol.cpp | 191 lldb/tools/lldb-dap/Protocol.h | 192 + lldb/tools/lldb-dap/Transport.cpp | 110 + lldb/tools/lldb-dap/Transport.h| 48 lldb/tools/lldb-dap/lldb-dap.cpp | 38 -- 12 files changed, 665 insertions(+), 228 deletions(-) create mode 100644 lldb/tools/lldb-dap/DAPLog.cpp create mode 100644 lldb/tools/lldb-dap/DAPLog.h delete mode 100644 lldb/tools/lldb-dap/IOStream.cpp delete mode 100644 lldb/tools/lldb-dap/IOStream.h create mode 100644 lldb/tools/lldb-dap/Protocol.cpp create mode 100644 lldb/tools/lldb-dap/Protocol.h create mode 100644 lldb/tools/lldb-dap/Transport.cpp create mode 100644 lldb/tools/lldb-dap/Transport.h diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt index 8b3c520ec4360..cba20a53edcb3 100644 --- a/lldb/tools/lldb-dap/CMakeLists.txt +++ b/lldb/tools/lldb-dap/CMakeLists.txt @@ -23,18 +23,20 @@ add_lldb_tool(lldb-dap Breakpoint.cpp BreakpointBase.cpp DAP.cpp + DAPLog.cpp EventHelper.cpp ExceptionBreakpoint.cpp FifoFiles.cpp FunctionBreakpoint.cpp InstructionBreakpoint.cpp - IOStream.cpp JSONUtils.cpp LLDBUtils.cpp OutputRedirector.cpp ProgressEvent.cpp RunInTerminal.cpp SourceBreakpoint.cpp + Transport.cpp + Protocol.cpp Watchpoint.cpp Handler/ResponseHandler.cpp diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index cd53e2aca3fb6..e23d9f4e910cc 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -7,10 +7,12 @@ //===--===// #include "DAP.h" +#include "DAPLog.h" #include "Handler/ResponseHandler.h" #include "JSONUtils.h" #include "LLDBUtils.h" #include "OutputRedirector.h" +#include "Protocol.h" #include "lldb/API/SBBreakpoint.h" #include "lldb/API/SBCommandInterpreter.h" #include "lldb/API/SBCommandReturnObject.h" @@ -29,6 +31,7 @@ #include "llvm/Support/Error.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FormatVariadic.h" +#include "llvm/Support/JSON.h" #include "llvm/Support/raw_ostream.h" #include #include @@ -50,6 +53,7 @@ #endif using namespace lldb_dap; +using namespace lldb_private; namespace { #ifdef _WIN32 @@ -61,11 +65,11 @@ const char DEV_NULL[] = "/dev/null"; namespace lldb_dap { -DAP::DAP(std::string name, llvm::StringRef path, std::ofstream *log, +DAP::DAP(llvm::StringRef name, llvm::StringRef path, std::ofstream *log, lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode, std::vector pre_init_commands) -: name(std::move(name)), debug_adaptor_path(path), log(log), - input(std::move(input)), output(std::move(output)), +: name(name), debug_adaptor_path(path), log(log), + transport(name, std::move(input), std::move(output)), broadcaster("lldb-dap"), exception_breakpoints(), pre_init_commands(std::move(pre_init_commands)), focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false), is_attach(false), @@ -237,65 +241,22 @@ void DAP::StopEventHandlers() { } } -// Send the JSON in "json_str" to the "out" stream. Correctly send the -// "Content-Length:" field followed by the length, followed by the raw -// JSON bytes. -void DAP::SendJSON(const std::string &json_str) { - output.write_full("Content-Length: "); - output.write_full(llvm::utostr(json_str.size())); - output.write_full("\r\n\r\n"); - output.write_full(json_str); -} - // Serialize the JSON value into a string and send the JSON packet to // the "out" stream. void DAP::SendJSON(const llvm::json::Value &json)
[Lldb-commits] [lldb] [lldb-dap] Creating a dedicated Transport class for DAP communication. (PR #128972)
https://github.com/ashgti updated https://github.com/llvm/llvm-project/pull/128972 >From 397c479a8238a06e81c53c2735aa361ae12a5f42 Mon Sep 17 00:00:00 2001 From: John Harrison Date: Wed, 26 Feb 2025 11:29:46 -0800 Subject: [PATCH] [lldb-dap] Creating a dedicated Transport mechnaism for handling protocol communication. This removes the previous 'OutputStream' and 'InputStream' objects and the new 'Transport' class takes on the responsibility of seriliaizing and encoding messages. The new Protocol.h file has a dedicated namespace for POD style structures to represent the Debug Adapter Protocol messages. --- lldb/tools/lldb-dap/CMakeLists.txt | 4 +- lldb/tools/lldb-dap/DAP.cpp| 135 +--- lldb/tools/lldb-dap/DAP.h | 17 ++- lldb/tools/lldb-dap/DAPLog.cpp | 20 +++ lldb/tools/lldb-dap/DAPLog.h | 33 + lldb/tools/lldb-dap/IOStream.cpp | 65 -- lldb/tools/lldb-dap/IOStream.h | 42 --- lldb/tools/lldb-dap/Protocol.cpp | 191 lldb/tools/lldb-dap/Protocol.h | 192 + lldb/tools/lldb-dap/Transport.cpp | 110 + lldb/tools/lldb-dap/Transport.h| 48 lldb/tools/lldb-dap/lldb-dap.cpp | 38 -- 12 files changed, 667 insertions(+), 228 deletions(-) create mode 100644 lldb/tools/lldb-dap/DAPLog.cpp create mode 100644 lldb/tools/lldb-dap/DAPLog.h delete mode 100644 lldb/tools/lldb-dap/IOStream.cpp delete mode 100644 lldb/tools/lldb-dap/IOStream.h create mode 100644 lldb/tools/lldb-dap/Protocol.cpp create mode 100644 lldb/tools/lldb-dap/Protocol.h create mode 100644 lldb/tools/lldb-dap/Transport.cpp create mode 100644 lldb/tools/lldb-dap/Transport.h diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt index 8b3c520ec4360..cba20a53edcb3 100644 --- a/lldb/tools/lldb-dap/CMakeLists.txt +++ b/lldb/tools/lldb-dap/CMakeLists.txt @@ -23,18 +23,20 @@ add_lldb_tool(lldb-dap Breakpoint.cpp BreakpointBase.cpp DAP.cpp + DAPLog.cpp EventHelper.cpp ExceptionBreakpoint.cpp FifoFiles.cpp FunctionBreakpoint.cpp InstructionBreakpoint.cpp - IOStream.cpp JSONUtils.cpp LLDBUtils.cpp OutputRedirector.cpp ProgressEvent.cpp RunInTerminal.cpp SourceBreakpoint.cpp + Transport.cpp + Protocol.cpp Watchpoint.cpp Handler/ResponseHandler.cpp diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index cd53e2aca3fb6..e23d9f4e910cc 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -7,10 +7,12 @@ //===--===// #include "DAP.h" +#include "DAPLog.h" #include "Handler/ResponseHandler.h" #include "JSONUtils.h" #include "LLDBUtils.h" #include "OutputRedirector.h" +#include "Protocol.h" #include "lldb/API/SBBreakpoint.h" #include "lldb/API/SBCommandInterpreter.h" #include "lldb/API/SBCommandReturnObject.h" @@ -29,6 +31,7 @@ #include "llvm/Support/Error.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/FormatVariadic.h" +#include "llvm/Support/JSON.h" #include "llvm/Support/raw_ostream.h" #include #include @@ -50,6 +53,7 @@ #endif using namespace lldb_dap; +using namespace lldb_private; namespace { #ifdef _WIN32 @@ -61,11 +65,11 @@ const char DEV_NULL[] = "/dev/null"; namespace lldb_dap { -DAP::DAP(std::string name, llvm::StringRef path, std::ofstream *log, +DAP::DAP(llvm::StringRef name, llvm::StringRef path, std::ofstream *log, lldb::IOObjectSP input, lldb::IOObjectSP output, ReplMode repl_mode, std::vector pre_init_commands) -: name(std::move(name)), debug_adaptor_path(path), log(log), - input(std::move(input)), output(std::move(output)), +: name(name), debug_adaptor_path(path), log(log), + transport(name, std::move(input), std::move(output)), broadcaster("lldb-dap"), exception_breakpoints(), pre_init_commands(std::move(pre_init_commands)), focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false), is_attach(false), @@ -237,65 +241,22 @@ void DAP::StopEventHandlers() { } } -// Send the JSON in "json_str" to the "out" stream. Correctly send the -// "Content-Length:" field followed by the length, followed by the raw -// JSON bytes. -void DAP::SendJSON(const std::string &json_str) { - output.write_full("Content-Length: "); - output.write_full(llvm::utostr(json_str.size())); - output.write_full("\r\n\r\n"); - output.write_full(json_str); -} - // Serialize the JSON value into a string and send the JSON packet to // the "out" stream. void DAP::SendJSON(const llvm::json::Value &json) { - std::string json_str; - llvm::raw_string_ostream strm(json_str); - strm << json; - static std::mutex mutex; - std::lock_guard locker(mutex); - SendJSON(json_str); - - if (log) { -auto now = std::chrono::duration( -std::chrono::system_clock::now().time_since_epoch()); -
[Lldb-commits] [lldb] [LLDB][SBProgress] Fix bad optional access in sbprogress (PR #128971)
@@ -40,7 +40,10 @@ SBProgress::~SBProgress() = default; void SBProgress::Increment(uint64_t amount, const char *description) { LLDB_INSTRUMENT_VA(amount, description); - m_opaque_up->Increment(amount, description); + std::optional description_opt; + if (description && description[0]) dmpots wrote: So this only calls increment if there is a non-null and non-empty description. Is that the correct behavior? Or should we still be calling increment with a `nullopt` in that case? https://github.com/llvm/llvm-project/pull/128971 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Fix bad optional access in sbprogress (PR #128971)
@@ -40,7 +40,10 @@ SBProgress::~SBProgress() = default; void SBProgress::Increment(uint64_t amount, const char *description) { LLDB_INSTRUMENT_VA(amount, description); - m_opaque_up->Increment(amount, description); + std::optional description_opt; + if (description && description[0]) Jlalond wrote: No we only set the value of the description optional if non-null, non-empty description. So `Increment(1, NULL/None in Python)` will bump the count by one but send no message. https://github.com/llvm/llvm-project/pull/128971 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Fix bad optional access in sbprogress (PR #128971)
@@ -33,3 +33,15 @@ def test_without_external_bit_set(self): expected_string = "Test progress first increment" progress.Increment(1, expected_string) self.assertFalse(listener.PeekAtNextEvent(event)) + +def test_with_external_bit_set(self): +"""Test SBProgress can handle null events.""" + +progress = lldb.SBProgress("Test SBProgress", "Test progress", self.dbg) +listener = lldb.SBListener("Test listener") +broadcaster = self.dbg.GetBroadcaster() +broadcaster.AddListener(listener, lldb.eBroadcastBitExternalProgress) +event = lldb.SBEvent() + +progress.Increment(1, None) dmpots wrote: Would `progress.Increment(1, "")` map to the case where `description[0] == '\0'`? If so seems like a cheap test to add. https://github.com/llvm/llvm-project/pull/128971 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Fix bad optional in sbprogress (PR #128971)
https://github.com/Jlalond edited https://github.com/llvm/llvm-project/pull/128971 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Fix bad optional in sbprogress (PR #128971)
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/128971 >From ac90ec73ccfb02923ff0343189d2efaeb6108fa3 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 26 Feb 2025 15:48:14 -0800 Subject: [PATCH 1/2] Fix bad optional access in sbprogress --- lldb/source/API/SBProgress.cpp | 5 - .../test/API/python_api/sbprogress/TestSBProgress.py | 12 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/lldb/source/API/SBProgress.cpp b/lldb/source/API/SBProgress.cpp index e67e289a60eff..d40e11da973d4 100644 --- a/lldb/source/API/SBProgress.cpp +++ b/lldb/source/API/SBProgress.cpp @@ -40,7 +40,10 @@ SBProgress::~SBProgress() = default; void SBProgress::Increment(uint64_t amount, const char *description) { LLDB_INSTRUMENT_VA(amount, description); - m_opaque_up->Increment(amount, description); + std::optional description_opt; + if (description && description[0]) +description_opt = description; + m_opaque_up->Increment(amount, description_opt); } lldb_private::Progress &SBProgress::ref() const { return *m_opaque_up; } diff --git a/lldb/test/API/python_api/sbprogress/TestSBProgress.py b/lldb/test/API/python_api/sbprogress/TestSBProgress.py index c456247da80c6..5f7820a5bd81e 100644 --- a/lldb/test/API/python_api/sbprogress/TestSBProgress.py +++ b/lldb/test/API/python_api/sbprogress/TestSBProgress.py @@ -33,3 +33,15 @@ def test_without_external_bit_set(self): expected_string = "Test progress first increment" progress.Increment(1, expected_string) self.assertFalse(listener.PeekAtNextEvent(event)) + +def test_with_external_bit_set(self): +"""Test SBProgress can handle null events.""" + +progress = lldb.SBProgress("Test SBProgress", "Test progress", self.dbg) +listener = lldb.SBListener("Test listener") +broadcaster = self.dbg.GetBroadcaster() +broadcaster.AddListener(listener, lldb.eBroadcastBitExternalProgress) +event = lldb.SBEvent() + +progress.Increment(1, None) +self.assertTrue(listener.PeekAtNextEvent(event)) >From 4f1aaa6a2cf81da53db634a69155ef1e8aa29c6c Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 26 Feb 2025 16:18:11 -0800 Subject: [PATCH 2/2] Expand test case based on Dave's feedback --- lldb/test/API/python_api/sbprogress/TestSBProgress.py | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/python_api/sbprogress/TestSBProgress.py b/lldb/test/API/python_api/sbprogress/TestSBProgress.py index 5f7820a5bd81e..eab5e5bb18cf0 100644 --- a/lldb/test/API/python_api/sbprogress/TestSBProgress.py +++ b/lldb/test/API/python_api/sbprogress/TestSBProgress.py @@ -37,11 +37,18 @@ def test_without_external_bit_set(self): def test_with_external_bit_set(self): """Test SBProgress can handle null events.""" -progress = lldb.SBProgress("Test SBProgress", "Test progress", self.dbg) +progress = lldb.SBProgress("Test SBProgress", "Test progress", 3, self.dbg) listener = lldb.SBListener("Test listener") broadcaster = self.dbg.GetBroadcaster() broadcaster.AddListener(listener, lldb.eBroadcastBitExternalProgress) event = lldb.SBEvent() progress.Increment(1, None) -self.assertTrue(listener.PeekAtNextEvent(event)) +self.assertTrue(listener.GetNextEvent(event)) +progress.Increment(1, "") +self.assertTrue(listener.GetNextEvent(event)) +progress.Increment(1, "Step 3") +self.assertTrue(listener.GetNextEvent(event)) +stream = lldb.SBStream() +event.GetDescription(stream) +self.assertIn("Step 3", stream.GetData()) ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Fix bad optional in sbprogress (PR #128971)
@@ -33,3 +33,15 @@ def test_without_external_bit_set(self): expected_string = "Test progress first increment" progress.Increment(1, expected_string) self.assertFalse(listener.PeekAtNextEvent(event)) + +def test_with_external_bit_set(self): +"""Test SBProgress can handle null events.""" + +progress = lldb.SBProgress("Test SBProgress", "Test progress", self.dbg) +listener = lldb.SBListener("Test listener") +broadcaster = self.dbg.GetBroadcaster() +broadcaster.AddListener(listener, lldb.eBroadcastBitExternalProgress) +event = lldb.SBEvent() + +progress.Increment(1, None) Jlalond wrote: Good callout, I expanded the test case. https://github.com/llvm/llvm-project/pull/128971 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Add a finalize method (PR #128966)
https://github.com/clayborg requested changes to this pull request. https://github.com/llvm/llvm-project/pull/128966 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Add a finalize method (PR #128966)
@@ -33,3 +33,17 @@ def test_without_external_bit_set(self): expected_string = "Test progress first increment" progress.Increment(1, expected_string) self.assertFalse(listener.PeekAtNextEvent(event)) + +def test_progress_finalize(self): +"""Test SBProgress finalize sends the progressEnd event""" + +progress = lldb.SBProgress("Test SBProgress", "Test finalize", self.dbg) +listener = lldb.SBListener("Test listener") +broadcaster = self.dbg.GetBroadcaster() +broadcaster.AddListener(listener, lldb.eBroadcastBitExternalProgressCategory) +event = lldb.SBEvent() +progress.Finalize() +self.assertTrue(listener.WaitForEvent(5, event)) clayborg wrote: Don't you have to wait for the first progress start event and also the end event here? https://github.com/llvm/llvm-project/pull/128966 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Add a finalize method (PR #128966)
@@ -33,3 +33,17 @@ def test_without_external_bit_set(self): expected_string = "Test progress first increment" progress.Increment(1, expected_string) self.assertFalse(listener.PeekAtNextEvent(event)) + +def test_progress_finalize(self): clayborg wrote: We should test for deterministic and non determinisitic progress objects here. https://github.com/llvm/llvm-project/pull/128966 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Add a finalize method (PR #128966)
@@ -59,6 +59,11 @@ class LLDB_API SBProgress { void Increment(uint64_t amount, const char *description = nullptr); + /// Explicitly finalize an SBProgress, this can be used to terminate a + /// progress on command instead of waiting for a garbage collection or other + /// finalizer. clayborg wrote: ``` /// progress on command instead of waiting for a garbage collection or other /// RAII to destroy the contained progress object. https://github.com/llvm/llvm-project/pull/128966 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Add a finalize method (PR #128966)
@@ -12,3 +12,10 @@ and will always send an initial progress update, updates when Progress::Increment() is called, and also will make sure that a progress completed update is reported even if the user doesn't explicitly cause one to be sent.") lldb::SBProgress; clayborg wrote: Lets add some usage documentation here for both non-determinisitic and deterministic progress class usage in python here. That way people can see this and use the class as intended. https://github.com/llvm/llvm-project/pull/128966 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Fix bad optional in sbprogress (PR #128971)
@@ -40,7 +40,10 @@ SBProgress::~SBProgress() = default; void SBProgress::Increment(uint64_t amount, const char *description) { LLDB_INSTRUMENT_VA(amount, description); - m_opaque_up->Increment(amount, description); + std::optional description_opt; + if (description && description[0]) dmpots wrote: Ah yeah, I read that wrong. Thanks for confirming. https://github.com/llvm/llvm-project/pull/128971 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Fix bad optional in sbprogress (PR #128971)
https://github.com/dmpots edited https://github.com/llvm/llvm-project/pull/128971 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Fix bad optional in sbprogress (PR #128971)
https://github.com/dmpots approved this pull request. LGTM! https://github.com/llvm/llvm-project/pull/128971 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] f409340 - [lldb-dap] Return an llvm::Error instead of calling exit directly (NFC) (#128951)
Author: Jonas Devlieghere Date: 2025-02-26T18:34:21-06:00 New Revision: f409340cc217c55c3960a375054a17b2bc927e53 URL: https://github.com/llvm/llvm-project/commit/f409340cc217c55c3960a375054a17b2bc927e53 DIFF: https://github.com/llvm/llvm-project/commit/f409340cc217c55c3960a375054a17b2bc927e53.diff LOG: [lldb-dap] Return an llvm::Error instead of calling exit directly (NFC) (#128951) Return an `llvm::Error` from `LaunchRunInTerminalTarget` instead of calling `exit()` directly. Added: Modified: lldb/tools/lldb-dap/lldb-dap.cpp Removed: diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index d8b92831d5540..e6a28919694bc 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -202,12 +202,13 @@ static void printHelp(LLDBDAPOptTable &table, llvm::StringRef tool_name) { // // In case of errors launching the target, a suitable error message will be // emitted to the debug adaptor. -static void LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, - llvm::StringRef comm_file, - lldb::pid_t debugger_pid, char *argv[]) { +static llvm::Error LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, + llvm::StringRef comm_file, + lldb::pid_t debugger_pid, + char *argv[]) { #if defined(_WIN32) - llvm::errs() << "runInTerminal is only supported on POSIX systems\n"; - exit(EXIT_FAILURE); + return llvm::createStringError( + "runInTerminal is only supported on POSIX systems"); #else // On Linux with the Yama security module enabled, a process can only attach @@ -219,10 +220,8 @@ static void LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, #endif RunInTerminalLauncherCommChannel comm_channel(comm_file); - if (llvm::Error err = comm_channel.NotifyPid()) { -llvm::errs() << llvm::toString(std::move(err)) << "\n"; -exit(EXIT_FAILURE); - } + if (llvm::Error err = comm_channel.NotifyPid()) +return err; // We will wait to be attached with a timeout. We don't wait indefinitely // using a signal to prevent being paused forever. @@ -233,8 +232,7 @@ static void LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, timeout_env_var != nullptr ? atoi(timeout_env_var) : 2; if (llvm::Error err = comm_channel.WaitUntilDebugAdaptorAttaches( std::chrono::milliseconds(timeout_in_ms))) { -llvm::errs() << llvm::toString(std::move(err)) << "\n"; -exit(EXIT_FAILURE); +return err; } const char *target = target_arg.getValue(); @@ -242,8 +240,8 @@ static void LaunchRunInTerminalTarget(llvm::opt::Arg &target_arg, std::string error = std::strerror(errno); comm_channel.NotifyError(error); - llvm::errs() << error << "\n"; - exit(EXIT_FAILURE); + return llvm::createStringError(llvm::inconvertibleErrorCode(), + std::move(error)); #endif } @@ -471,13 +469,18 @@ int main(int argc, char *argv[]) { } } int target_args_pos = argc; - for (int i = 0; i < argc; i++) + for (int i = 0; i < argc; i++) { if (strcmp(argv[i], "--launch-target") == 0) { target_args_pos = i + 1; break; } - LaunchRunInTerminalTarget(*target_arg, comm_file->getValue(), pid, -argv + target_args_pos); + } + if (llvm::Error err = + LaunchRunInTerminalTarget(*target_arg, comm_file->getValue(), pid, +argv + target_args_pos)) { +llvm::errs() << llvm::toString(std::move(err)) << '\n'; +return EXIT_FAILURE; + } } else { llvm::errs() << "\"--launch-target\" requires \"--comm-file\" to be " "specified\n"; ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Fix bad optional in sbprogress (PR #128971)
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/128971 >From ac90ec73ccfb02923ff0343189d2efaeb6108fa3 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 26 Feb 2025 15:48:14 -0800 Subject: [PATCH 1/3] Fix bad optional access in sbprogress --- lldb/source/API/SBProgress.cpp | 5 - .../test/API/python_api/sbprogress/TestSBProgress.py | 12 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/lldb/source/API/SBProgress.cpp b/lldb/source/API/SBProgress.cpp index e67e289a60eff..d40e11da973d4 100644 --- a/lldb/source/API/SBProgress.cpp +++ b/lldb/source/API/SBProgress.cpp @@ -40,7 +40,10 @@ SBProgress::~SBProgress() = default; void SBProgress::Increment(uint64_t amount, const char *description) { LLDB_INSTRUMENT_VA(amount, description); - m_opaque_up->Increment(amount, description); + std::optional description_opt; + if (description && description[0]) +description_opt = description; + m_opaque_up->Increment(amount, description_opt); } lldb_private::Progress &SBProgress::ref() const { return *m_opaque_up; } diff --git a/lldb/test/API/python_api/sbprogress/TestSBProgress.py b/lldb/test/API/python_api/sbprogress/TestSBProgress.py index c456247da80c6..5f7820a5bd81e 100644 --- a/lldb/test/API/python_api/sbprogress/TestSBProgress.py +++ b/lldb/test/API/python_api/sbprogress/TestSBProgress.py @@ -33,3 +33,15 @@ def test_without_external_bit_set(self): expected_string = "Test progress first increment" progress.Increment(1, expected_string) self.assertFalse(listener.PeekAtNextEvent(event)) + +def test_with_external_bit_set(self): +"""Test SBProgress can handle null events.""" + +progress = lldb.SBProgress("Test SBProgress", "Test progress", self.dbg) +listener = lldb.SBListener("Test listener") +broadcaster = self.dbg.GetBroadcaster() +broadcaster.AddListener(listener, lldb.eBroadcastBitExternalProgress) +event = lldb.SBEvent() + +progress.Increment(1, None) +self.assertTrue(listener.PeekAtNextEvent(event)) >From 4f1aaa6a2cf81da53db634a69155ef1e8aa29c6c Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 26 Feb 2025 16:18:11 -0800 Subject: [PATCH 2/3] Expand test case based on Dave's feedback --- lldb/test/API/python_api/sbprogress/TestSBProgress.py | 11 +-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/python_api/sbprogress/TestSBProgress.py b/lldb/test/API/python_api/sbprogress/TestSBProgress.py index 5f7820a5bd81e..eab5e5bb18cf0 100644 --- a/lldb/test/API/python_api/sbprogress/TestSBProgress.py +++ b/lldb/test/API/python_api/sbprogress/TestSBProgress.py @@ -37,11 +37,18 @@ def test_without_external_bit_set(self): def test_with_external_bit_set(self): """Test SBProgress can handle null events.""" -progress = lldb.SBProgress("Test SBProgress", "Test progress", self.dbg) +progress = lldb.SBProgress("Test SBProgress", "Test progress", 3, self.dbg) listener = lldb.SBListener("Test listener") broadcaster = self.dbg.GetBroadcaster() broadcaster.AddListener(listener, lldb.eBroadcastBitExternalProgress) event = lldb.SBEvent() progress.Increment(1, None) -self.assertTrue(listener.PeekAtNextEvent(event)) +self.assertTrue(listener.GetNextEvent(event)) +progress.Increment(1, "") +self.assertTrue(listener.GetNextEvent(event)) +progress.Increment(1, "Step 3") +self.assertTrue(listener.GetNextEvent(event)) +stream = lldb.SBStream() +event.GetDescription(stream) +self.assertIn("Step 3", stream.GetData()) >From 6e9ce7e7e55924e93231799e03c4ea3cc45c1810 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 26 Feb 2025 16:35:55 -0800 Subject: [PATCH 3/3] Further asserts based on Greg's feedback --- .../API/python_api/sbprogress/TestSBProgress.py | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/lldb/test/API/python_api/sbprogress/TestSBProgress.py b/lldb/test/API/python_api/sbprogress/TestSBProgress.py index eab5e5bb18cf0..1b8f01d3e8bb1 100644 --- a/lldb/test/API/python_api/sbprogress/TestSBProgress.py +++ b/lldb/test/API/python_api/sbprogress/TestSBProgress.py @@ -42,11 +42,24 @@ def test_with_external_bit_set(self): broadcaster = self.dbg.GetBroadcaster() broadcaster.AddListener(listener, lldb.eBroadcastBitExternalProgress) event = lldb.SBEvent() - +# Sample JSON we're expecting: +# { id = 2, title = "Test SBProgress", details = "Test progress", type = update, progress = 1 of 3} +# details remains the same as specified in the constructor of the progress +# until we update it in the increment function, so we check for the Null and empty string case +# that details hasn't changed, but progress x of 3 has. progress.Increm
[Lldb-commits] [lldb] [LLDB][SBProgress] Fix bad optional in sbprogress (PR #128971)
@@ -33,3 +33,22 @@ def test_without_external_bit_set(self): expected_string = "Test progress first increment" progress.Increment(1, expected_string) self.assertFalse(listener.PeekAtNextEvent(event)) + +def test_with_external_bit_set(self): +"""Test SBProgress can handle null events.""" + +progress = lldb.SBProgress("Test SBProgress", "Test progress", 3, self.dbg) +listener = lldb.SBListener("Test listener") +broadcaster = self.dbg.GetBroadcaster() +broadcaster.AddListener(listener, lldb.eBroadcastBitExternalProgress) +event = lldb.SBEvent() + +progress.Increment(1, None) +self.assertTrue(listener.GetNextEvent(event)) +progress.Increment(1, "") +self.assertTrue(listener.GetNextEvent(event)) +progress.Increment(1, "Step 3") +self.assertTrue(listener.GetNextEvent(event)) +stream = lldb.SBStream() +event.GetDescription(stream) +self.assertIn("Step 3", stream.GetData()) clayborg wrote: can we test that we receive these events with the correct data here? (get the start event, the 3 increment events with no detail for the first two and then "Step 3" for the 3rd increment, though right now that might come through as an end event? https://github.com/llvm/llvm-project/pull/128971 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Add a finalize method (PR #128966)
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/128966 >From 3c4b333869c17005deb8d4e9307babc94d0dc9b5 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 26 Feb 2025 15:38:24 -0800 Subject: [PATCH 1/3] Add a finalize method to the sbprogress, with an associated doc string and a test --- lldb/bindings/interface/SBProgressDocstrings.i| 7 +++ lldb/include/lldb/API/SBProgress.h| 5 + lldb/source/API/SBProgress.cpp| 15 ++- .../API/python_api/sbprogress/TestSBProgress.py | 14 ++ 4 files changed, 40 insertions(+), 1 deletion(-) diff --git a/lldb/bindings/interface/SBProgressDocstrings.i b/lldb/bindings/interface/SBProgressDocstrings.i index 2997fe619fcc7..50648b62411f8 100644 --- a/lldb/bindings/interface/SBProgressDocstrings.i +++ b/lldb/bindings/interface/SBProgressDocstrings.i @@ -12,3 +12,10 @@ and will always send an initial progress update, updates when Progress::Increment() is called, and also will make sure that a progress completed update is reported even if the user doesn't explicitly cause one to be sent.") lldb::SBProgress; + +%feature("docstring", +"Explicitly end an SBProgress, this is a utility to send the progressEnd event +without waiting for your language or language implementations non-deterministic destruction +of the SBProgress object. + +Note once finalized, no further increments will be processed.") lldb::SBProgress::Finalize; diff --git a/lldb/include/lldb/API/SBProgress.h b/lldb/include/lldb/API/SBProgress.h index d574d1d2982b9..1558cdc08e080 100644 --- a/lldb/include/lldb/API/SBProgress.h +++ b/lldb/include/lldb/API/SBProgress.h @@ -59,6 +59,11 @@ class LLDB_API SBProgress { void Increment(uint64_t amount, const char *description = nullptr); + /// Explicitly finalize an SBProgress, this can be used to terminate a + /// progress on command instead of waiting for a garbage collection or other + /// finalizer. + void Finalize(); + protected: lldb_private::Progress &ref() const; diff --git a/lldb/source/API/SBProgress.cpp b/lldb/source/API/SBProgress.cpp index e67e289a60eff..4991d55ad0248 100644 --- a/lldb/source/API/SBProgress.cpp +++ b/lldb/source/API/SBProgress.cpp @@ -40,7 +40,20 @@ SBProgress::~SBProgress() = default; void SBProgress::Increment(uint64_t amount, const char *description) { LLDB_INSTRUMENT_VA(amount, description); - m_opaque_up->Increment(amount, description); + if (!m_opaque_up) +return; + + std::optional description_opt; + if (description && description[0]) +description_opt = description; + m_opaque_up->Increment(amount, description_opt); +} + +void SBProgress::Finalize() { + if (!m_opaque_up) +return; + + m_opaque_up.reset(); } lldb_private::Progress &SBProgress::ref() const { return *m_opaque_up; } diff --git a/lldb/test/API/python_api/sbprogress/TestSBProgress.py b/lldb/test/API/python_api/sbprogress/TestSBProgress.py index c456247da80c6..dd7e4b6f2ac5c 100644 --- a/lldb/test/API/python_api/sbprogress/TestSBProgress.py +++ b/lldb/test/API/python_api/sbprogress/TestSBProgress.py @@ -33,3 +33,17 @@ def test_without_external_bit_set(self): expected_string = "Test progress first increment" progress.Increment(1, expected_string) self.assertFalse(listener.PeekAtNextEvent(event)) + +def test_progress_finalize(self): +"""Test SBProgress finalize sends the progressEnd event""" + +progress = lldb.SBProgress("Test SBProgress", "Test finalize", self.dbg) +listener = lldb.SBListener("Test listener") +broadcaster = self.dbg.GetBroadcaster() +broadcaster.AddListener(listener, lldb.eBroadcastBitExternalProgressCategory) +event = lldb.SBEvent() +progress.Finalize() +self.assertTrue(listener.WaitForEvent(5, event)) +stream = lldb.SBStream() +event.GetDescription(stream) +self.assertIn("type = end", stream.GetData()) >From eb5d36f37b3a990e52350243b45e3907e1362ff5 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 26 Feb 2025 15:54:52 -0800 Subject: [PATCH 2/3] Drop the bug fix now that I opened 128971 --- lldb/source/API/SBProgress.cpp | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lldb/source/API/SBProgress.cpp b/lldb/source/API/SBProgress.cpp index 4991d55ad0248..240b6e51eb9b9 100644 --- a/lldb/source/API/SBProgress.cpp +++ b/lldb/source/API/SBProgress.cpp @@ -43,10 +43,7 @@ void SBProgress::Increment(uint64_t amount, const char *description) { if (!m_opaque_up) return; - std::optional description_opt; - if (description && description[0]) -description_opt = description; - m_opaque_up->Increment(amount, description_opt); + m_opaque_up->Increment(amount, description); } void SBProgress::Finalize() { >From d43f4710a4ebdfe01335603c2850aeed325221dd Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Wed, 26 Feb 2025 16:44:29 -0800 Subject: [PAT
[Lldb-commits] [lldb] [LLDB][SBProgress] Add a finalize method (PR #128966)
@@ -33,3 +33,17 @@ def test_without_external_bit_set(self): expected_string = "Test progress first increment" progress.Increment(1, expected_string) self.assertFalse(listener.PeekAtNextEvent(event)) + +def test_progress_finalize(self): +"""Test SBProgress finalize sends the progressEnd event""" + +progress = lldb.SBProgress("Test SBProgress", "Test finalize", self.dbg) +listener = lldb.SBListener("Test listener") +broadcaster = self.dbg.GetBroadcaster() +broadcaster.AddListener(listener, lldb.eBroadcastBitExternalProgressCategory) +event = lldb.SBEvent() +progress.Finalize() +self.assertTrue(listener.WaitForEvent(5, event)) Jlalond wrote: Nope! Because we create the progress event before wiring up the listener we only listen for the end message. https://github.com/llvm/llvm-project/pull/128966 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Add a finalize method (PR #128966)
@@ -12,3 +12,10 @@ and will always send an initial progress update, updates when Progress::Increment() is called, and also will make sure that a progress completed update is reported even if the user doesn't explicitly cause one to be sent.") lldb::SBProgress; Jlalond wrote: I remember this being 1:1 with Progress and not being changed so I would think we should add a new doc string to Increment. Is there anything in particular you want to call out? Many of the deterministic/non-deterministic are predominantly relevant to DAP. https://github.com/llvm/llvm-project/pull/128966 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Gardening in lldb-dap.cpp (NFC) (PR #128949)
https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/128949 >From 679e038a2322fdf884eb22baafe9167904e27dd7 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Wed, 26 Feb 2025 16:29:26 -0600 Subject: [PATCH 1/2] [lldb-dap] Gardening in lldb-dap.cpp (NFC) - Remove more unused includes - Limit anonymous namespace to llvm::opt - Fix code style --- lldb/tools/lldb-dap/lldb-dap.cpp | 22 +- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index 1c6bd7e903409..d4ebd6430d0aa 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -1,4 +1,4 @@ -//===-- lldb-dap.cpp -*- C++ -*-===// +//===-- lldb-dap.cpp --===// // // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. // See https://llvm.org/LICENSE.txt for license information. @@ -8,10 +8,7 @@ #include "DAP.h" #include "EventHelper.h" -#include "FifoFiles.h" #include "Handler/RequestHandler.h" -#include "JSONUtils.h" -#include "LLDBUtils.h" #include "RunInTerminal.h" #include "lldb/API/SBStream.h" #include "lldb/Host/Config.h" @@ -23,7 +20,6 @@ #include "lldb/lldb-forward.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/ScopeExit.h" -#include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Option/Arg.h" #include "llvm/Option/ArgList.h" @@ -37,22 +33,15 @@ #include "llvm/Support/Signals.h" #include "llvm/Support/Threading.h" #include "llvm/Support/raw_ostream.h" -#include #include -#include #include #include -#include #include #include #include #include #include -#include -#include #include -#include -#include #include #include #include @@ -113,8 +102,9 @@ class LLDBDAPOptTable : public llvm::opt::GenericOptTable { : llvm::opt::GenericOptTable(OptionStrTable, OptionPrefixesTable, InfoTable, true) {} }; +} // anonymous namespace -void RegisterRequestCallbacks(DAP &dap) { +static void RegisterRequestCallbacks(DAP &dap) { dap.RegisterRequest(); dap.RegisterRequest(); dap.RegisterRequest(); @@ -155,9 +145,7 @@ void RegisterRequestCallbacks(DAP &dap) { dap.RegisterRequest(); } -} // anonymous namespace - -static void printHelp(LLDBDAPOptTable &table, llvm::StringRef tool_name) { +static void PrintHelp(LLDBDAPOptTable &table, llvm::StringRef tool_name) { std::string usage_str = tool_name.str() + " options"; table.printHelp(llvm::outs(), usage_str.c_str(), "LLDB DAP", false); @@ -433,7 +421,7 @@ int main(int argc, char *argv[]) { llvm::opt::InputArgList input_args = T.ParseArgs(ArgsArr, MAI, MAC); if (input_args.hasArg(OPT_help)) { -printHelp(T, llvm::sys::path::filename(argv[0])); +PrintHelp(T, llvm::sys::path::filename(argv[0])); return EXIT_SUCCESS; } >From cf1ab243c0231b2825a972025a5cc11c51560dc7 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Wed, 26 Feb 2025 19:06:40 -0600 Subject: [PATCH 2/2] Don't remove llvm/ADT/StringExtras.h --- lldb/tools/lldb-dap/lldb-dap.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp index d4ebd6430d0aa..b692754969757 100644 --- a/lldb/tools/lldb-dap/lldb-dap.cpp +++ b/lldb/tools/lldb-dap/lldb-dap.cpp @@ -20,6 +20,7 @@ #include "lldb/lldb-forward.h" #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/ScopeExit.h" +#include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Option/Arg.h" #include "llvm/Option/ArgList.h" ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][SBProgress] Add a finalize method (PR #128966)
@@ -40,7 +40,17 @@ SBProgress::~SBProgress() = default; void SBProgress::Increment(uint64_t amount, const char *description) { LLDB_INSTRUMENT_VA(amount, description); + if (!m_opaque_up) +return; + m_opaque_up->Increment(amount, description); } +void SBProgress::Finalize() { + if (!m_opaque_up) +return; + + m_opaque_up.reset(); clayborg wrote: no need to check m_opaque_up here, just call `m_opaque_up.reset();`. If it is empty already, it will do nothing. Also add a comment here to explain what is going on: ``` // The lldb_private::Progress object is designed to be RAII and send the end progress // event when it gets destroyed. So force our contained object to be destroyed and // send the progress end event. Clearing this object also allows all other methods to // quickly return without doing any work if they are called after this method. m_opaque_up.reset(); ``` https://github.com/llvm/llvm-project/pull/128966 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Return an llvm::Error instead of calling exit directly (NFC) (PR #128951)
https://github.com/JDevlieghere closed https://github.com/llvm/llvm-project/pull/128951 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Also show session history in fzf_history (PR #128986)
https://github.com/JDevlieghere approved this pull request. https://github.com/llvm/llvm-project/pull/128986 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 159b872 - [llvm][telemetry]Change Telemetry-disabling mechanism. (#128534)
Author: Vy Nguyen Date: 2025-02-26T13:01:53-05:00 New Revision: 159b872b37363511a359c800bcc9230bb09f2457 URL: https://github.com/llvm/llvm-project/commit/159b872b37363511a359c800bcc9230bb09f2457 DIFF: https://github.com/llvm/llvm-project/commit/159b872b37363511a359c800bcc9230bb09f2457.diff LOG: [llvm][telemetry]Change Telemetry-disabling mechanism. (#128534) Details: - Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any Telemetry code will be built. This has proven to cause more nuisance to both users of the Telemetry and any further extension of it. (Eg., we needed to put #ifdef around caller/user code) - So the new approach is to: + Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by default. + If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would still be built BUT Telemetry cannot be enabled. And no data can be collected. The benefit of this is that it simplifies user (and extension) code since we just need to put the check on Config::EnableTelemetry. Besides, the Telemetry library itself is very small, hence the additional code to be built would not cause any difference in build performance. - Co-authored-by: Pavel Labath Added: Modified: lldb/source/Core/CMakeLists.txt lldb/source/Core/Telemetry.cpp lldb/unittests/Core/CMakeLists.txt lldb/unittests/Core/TelemetryTest.cpp llvm/CMakeLists.txt llvm/cmake/modules/LLVMConfig.cmake.in llvm/include/llvm/Config/llvm-config.h.cmake llvm/include/llvm/Telemetry/Telemetry.h llvm/lib/CMakeLists.txt llvm/lib/Telemetry/Telemetry.cpp llvm/unittests/CMakeLists.txt llvm/unittests/Telemetry/TelemetryTest.cpp llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn utils/bazel/llvm_configs/llvm-config.h.cmake Removed: diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt index 82fb5f42f9f4b..d5d8a9d5088fc 100644 --- a/lldb/source/Core/CMakeLists.txt +++ b/lldb/source/Core/CMakeLists.txt @@ -16,10 +16,6 @@ if (LLDB_ENABLE_CURSES) endif() endif() -if (LLVM_BUILD_TELEMETRY) - set(TELEMETRY_DEPS Telemetry) -endif() - # TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore add_lldb_library(lldbCore Address.cpp @@ -84,7 +80,7 @@ add_lldb_library(lldbCore Support Demangle TargetParser -${TELEMETRY_DEPS} +Telemetry ) add_dependencies(lldbCore diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp index a0c9fb83e3a6b..fef63e3696e70 100644 --- a/lldb/source/Core/Telemetry.cpp +++ b/lldb/source/Core/Telemetry.cpp @@ -5,11 +5,6 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // //===--===// - -#include "llvm/Config/llvm-config.h" - -#ifdef LLVM_BUILD_TELEMETRY - #include "lldb/Core/Telemetry.h" #include "lldb/Core/Debugger.h" #include "lldb/Utility/LLDBLog.h" @@ -67,7 +62,11 @@ llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) { } std::unique_ptr TelemetryManager::g_instance = nullptr; -TelemetryManager *TelemetryManager::GetInstance() { return g_instance.get(); } +TelemetryManager *TelemetryManager::GetInstance() { + if (!Config::BuildTimeEnableTelemetry) +return nullptr; + return g_instance.get(); +} void TelemetryManager::SetInstance(std::unique_ptr manager) { g_instance = std::move(manager); @@ -75,5 +74,3 @@ void TelemetryManager::SetInstance(std::unique_ptr manager) { } // namespace telemetry } // namespace lldb_private - -#endif // LLVM_BUILD_TELEMETRY diff --git a/lldb/unittests/Core/CMakeLists.txt b/lldb/unittests/Core/CMakeLists.txt index d4d3764b67ae3..60265f794b5e8 100644 --- a/lldb/unittests/Core/CMakeLists.txt +++ b/lldb/unittests/Core/CMakeLists.txt @@ -1,6 +1,3 @@ -if (LLVM_BUILD_TELEMETRY) - set(TELEMETRY_DEPS Telemetry) -endif() add_lldb_unittest(LLDBCoreTests CommunicationTest.cpp @@ -31,5 +28,5 @@ add_lldb_unittest(LLDBCoreTests LLVMTestingSupport LINK_COMPONENTS Support -${TELEMETRY_DEPS} +Telemetry ) diff --git a/lldb/unittests/Core/TelemetryTest.cpp b/lldb/unittests/Core/TelemetryTest.cpp index 3ee6451429619..22fade58660b5 100644 --- a/lldb/unittests/Core/TelemetryTest.cpp +++ b/lldb/unittests/Core/TelemetryTest.cpp @@ -5,11 +5,6 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception // //===--===// - -#include "llvm/Config/llvm-config.h" - -#ifdef LLVM_BUILD_TELEMETRY - #include "lldb/Core/PluginInterface.h" #include "lldb/Core/PluginManager.h" #include "lldb/Core/Telemetry.h" @@ -71,7 +66,13 @@ class FakePlugin : public telemetry::TelemetryManager { } // namespace lldb_private -TEST(TelemetryTest, PluginTest) { +#if LLVM_ENABLE_TELEMETRY +#define TELEMETRY_TEST(suite, test) TEST(suite
[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)
https://github.com/oontvoo closed https://github.com/llvm/llvm-project/pull/128534 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Telemetry]Define DebuggerTelemetryInfo and related methods (PR #127696)
https://github.com/oontvoo updated https://github.com/llvm/llvm-project/pull/127696 >From 24e9f78744f98ecf3ac01f1f719f1eac9b3479f0 Mon Sep 17 00:00:00 2001 From: Vy Nguyen Date: Tue, 18 Feb 2025 15:58:08 -0500 Subject: [PATCH 1/5] [LLDB][Telemetry]Define DebuggerTelemetryInfo and related methods - This type of entry is used to collect data about the debugger startup/exit - Tests will be added (They may need to be shell test with a "test-only" TelemetryManager plugin defined. I'm trying to figure out how to get that linked only when tests are running and not to the LLDB binary all the time. --- lldb/include/lldb/Core/Telemetry.h | 78 +++ lldb/source/Core/Debugger.cpp | 40 ++ lldb/source/Core/Telemetry.cpp | 115 + 3 files changed, 220 insertions(+), 13 deletions(-) diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h index b72556ecaf3c9..d6eec5dc687be 100644 --- a/lldb/include/lldb/Core/Telemetry.h +++ b/lldb/include/lldb/Core/Telemetry.h @@ -13,6 +13,7 @@ #include "lldb/Interpreter/CommandReturnObject.h" #include "lldb/Utility/StructuredData.h" #include "lldb/lldb-forward.h" +#include "llvm/ADT/DenseMap.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/JSON.h" @@ -29,6 +30,9 @@ namespace telemetry { struct LLDBEntryKind : public ::llvm::telemetry::EntryKind { static const llvm::telemetry::KindType BaseInfo = 0b11000; + static const llvm::telemetry::KindType DebuggerInfo = 0b11001; + // There are other entries in between (added in separate PRs) + static const llvm::telemetry::KindType MiscInfo = 0b0; }; /// Defines a convenient type for timestamp of various events. @@ -56,6 +60,71 @@ struct LLDBBaseTelemetryInfo : public llvm::telemetry::TelemetryInfo { void serialize(llvm::telemetry::Serializer &serializer) const override; }; +/// Describes the exit status of a debugger. +struct ExitDescription { + int exit_code; + std::string description; +}; + +struct DebuggerTelemetryInfo : public LLDBBaseTelemetryInfo { + std::string username; + std::string lldb_git_sha; + std::string lldb_path; + std::string cwd; + std::optional exit_desc; + + DebuggerTelemetryInfo() = default; + + // Provide a copy ctor because we may need to make a copy before + // sanitizing the data. + // (The sanitization might differ between different Destination classes). + DebuggerTelemetryInfo(const DebuggerTelemetryInfo &other) { +username = other.username; +lldb_git_sha = other.lldb_git_sha; +lldb_path = other.lldb_path; +cwd = other.cwd; + }; + + llvm::telemetry::KindType getKind() const override { +return LLDBEntryKind::DebuggerInfo; + } + + static bool classof(const llvm::telemetry::TelemetryInfo *T) { +return T->getKind() == LLDBEntryKind::DebuggerInfo; + } + + void serialize(llvm::telemetry::Serializer &serializer) const override; +}; + +/// The "catch-all" entry to store a set of non-standard data, such as +/// error-messages, etc. +struct MiscTelemetryInfo : public LLDBBaseTelemetryInfo { + /// If the event is/can be associated with a target entry, + /// this field contains that target's UUID. + /// otherwise. + std::string target_uuid; + + /// Set of key-value pairs for any optional (or impl-specific) data + std::map meta_data; + + MiscTelemetryInfo() = default; + + MiscTelemetryInfo(const MiscTelemetryInfo &other) { +target_uuid = other.target_uuid; +meta_data = other.meta_data; + } + + llvm::telemetry::KindType getKind() const override { +return LLDBEntryKind::MiscInfo; + } + + static bool classof(const llvm::telemetry::TelemetryInfo *T) { +return T->getKind() == LLDBEntryKind::MiscInfo; + } + + void serialize(llvm::telemetry::Serializer &serializer) const override; +}; + /// The base Telemetry manager instance in LLDB. /// This class declares additional instrumentation points /// applicable to LLDB. @@ -63,6 +132,11 @@ class TelemetryManager : public llvm::telemetry::Manager { public: llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override; + const llvm::telemetry::Config *getConfig(); + + void atDebuggerStartup(DebuggerTelemetryInfo *entry); + void atDebuggerExit(DebuggerTelemetryInfo *entry); + virtual llvm::StringRef GetInstanceName() const = 0; static TelemetryManager *getInstance(); @@ -73,6 +147,10 @@ class TelemetryManager : public llvm::telemetry::Manager { private: std::unique_ptr m_config; + // Each debugger is assigned a unique ID (session_id). + // All TelemetryInfo entries emitted for the same debugger instance + // will get the same session_id. + llvm::DenseMap session_ids; static std::unique_ptr g_instance; }; diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index 18569e155b517..b458abc798a9e 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -62,6 +62,7 @@
[Lldb-commits] [lldb] [llvm] [llvm][telemetry]Change Telemetry-disabling mechanism. (PR #128534)
kazutakahirata wrote: @oontvoo I've landed 5a5a9e79369ae6cf320fc7b79a48d3e8b60f19a9 to fix a warning. Thanks! https://github.com/llvm/llvm-project/pull/128534 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Use existing lldb::IOObjectSP for DAP IO (NFC). (PR #128750)
https://github.com/JDevlieghere approved this pull request. https://github.com/llvm/llvm-project/pull/128750 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Reimplement LineTable::FindLineEntryByAddress on top of lower_bound (PR #127799)
https://github.com/JDevlieghere approved this pull request. LGTM. Since you're here... how would you feel about a little renaissance and returning a `std::optional` to eliminate the `index_ptr`? https://github.com/llvm/llvm-project/pull/127799 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Telemetry]Define DebuggerTelemetryInfo and related methods (PR #127696)
https://github.com/oontvoo updated https://github.com/llvm/llvm-project/pull/127696 >From 24e9f78744f98ecf3ac01f1f719f1eac9b3479f0 Mon Sep 17 00:00:00 2001 From: Vy Nguyen Date: Tue, 18 Feb 2025 15:58:08 -0500 Subject: [PATCH 1/8] [LLDB][Telemetry]Define DebuggerTelemetryInfo and related methods - This type of entry is used to collect data about the debugger startup/exit - Tests will be added (They may need to be shell test with a "test-only" TelemetryManager plugin defined. I'm trying to figure out how to get that linked only when tests are running and not to the LLDB binary all the time. --- lldb/include/lldb/Core/Telemetry.h | 78 +++ lldb/source/Core/Debugger.cpp | 40 ++ lldb/source/Core/Telemetry.cpp | 115 + 3 files changed, 220 insertions(+), 13 deletions(-) diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h index b72556ecaf3c9..d6eec5dc687be 100644 --- a/lldb/include/lldb/Core/Telemetry.h +++ b/lldb/include/lldb/Core/Telemetry.h @@ -13,6 +13,7 @@ #include "lldb/Interpreter/CommandReturnObject.h" #include "lldb/Utility/StructuredData.h" #include "lldb/lldb-forward.h" +#include "llvm/ADT/DenseMap.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/JSON.h" @@ -29,6 +30,9 @@ namespace telemetry { struct LLDBEntryKind : public ::llvm::telemetry::EntryKind { static const llvm::telemetry::KindType BaseInfo = 0b11000; + static const llvm::telemetry::KindType DebuggerInfo = 0b11001; + // There are other entries in between (added in separate PRs) + static const llvm::telemetry::KindType MiscInfo = 0b0; }; /// Defines a convenient type for timestamp of various events. @@ -56,6 +60,71 @@ struct LLDBBaseTelemetryInfo : public llvm::telemetry::TelemetryInfo { void serialize(llvm::telemetry::Serializer &serializer) const override; }; +/// Describes the exit status of a debugger. +struct ExitDescription { + int exit_code; + std::string description; +}; + +struct DebuggerTelemetryInfo : public LLDBBaseTelemetryInfo { + std::string username; + std::string lldb_git_sha; + std::string lldb_path; + std::string cwd; + std::optional exit_desc; + + DebuggerTelemetryInfo() = default; + + // Provide a copy ctor because we may need to make a copy before + // sanitizing the data. + // (The sanitization might differ between different Destination classes). + DebuggerTelemetryInfo(const DebuggerTelemetryInfo &other) { +username = other.username; +lldb_git_sha = other.lldb_git_sha; +lldb_path = other.lldb_path; +cwd = other.cwd; + }; + + llvm::telemetry::KindType getKind() const override { +return LLDBEntryKind::DebuggerInfo; + } + + static bool classof(const llvm::telemetry::TelemetryInfo *T) { +return T->getKind() == LLDBEntryKind::DebuggerInfo; + } + + void serialize(llvm::telemetry::Serializer &serializer) const override; +}; + +/// The "catch-all" entry to store a set of non-standard data, such as +/// error-messages, etc. +struct MiscTelemetryInfo : public LLDBBaseTelemetryInfo { + /// If the event is/can be associated with a target entry, + /// this field contains that target's UUID. + /// otherwise. + std::string target_uuid; + + /// Set of key-value pairs for any optional (or impl-specific) data + std::map meta_data; + + MiscTelemetryInfo() = default; + + MiscTelemetryInfo(const MiscTelemetryInfo &other) { +target_uuid = other.target_uuid; +meta_data = other.meta_data; + } + + llvm::telemetry::KindType getKind() const override { +return LLDBEntryKind::MiscInfo; + } + + static bool classof(const llvm::telemetry::TelemetryInfo *T) { +return T->getKind() == LLDBEntryKind::MiscInfo; + } + + void serialize(llvm::telemetry::Serializer &serializer) const override; +}; + /// The base Telemetry manager instance in LLDB. /// This class declares additional instrumentation points /// applicable to LLDB. @@ -63,6 +132,11 @@ class TelemetryManager : public llvm::telemetry::Manager { public: llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override; + const llvm::telemetry::Config *getConfig(); + + void atDebuggerStartup(DebuggerTelemetryInfo *entry); + void atDebuggerExit(DebuggerTelemetryInfo *entry); + virtual llvm::StringRef GetInstanceName() const = 0; static TelemetryManager *getInstance(); @@ -73,6 +147,10 @@ class TelemetryManager : public llvm::telemetry::Manager { private: std::unique_ptr m_config; + // Each debugger is assigned a unique ID (session_id). + // All TelemetryInfo entries emitted for the same debugger instance + // will get the same session_id. + llvm::DenseMap session_ids; static std::unique_ptr g_instance; }; diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index 18569e155b517..b458abc798a9e 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -62,6 +62,7 @@
[Lldb-commits] [lldb] [LLDB-DAP] SBDebugger don't prefix title on progress updates (PR #124648)
https://github.com/Jlalond updated https://github.com/llvm/llvm-project/pull/124648 >From 0587ba8239dbbd22aa40bde23d61f9504975817d Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Mon, 27 Jan 2025 13:41:58 -0800 Subject: [PATCH 1/9] Only include title on the first message --- lldb/include/lldb/Core/DebuggerEvents.h | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h index 49a4ecf8e537e..52e4f77d7637d 100644 --- a/lldb/include/lldb/Core/DebuggerEvents.h +++ b/lldb/include/lldb/Core/DebuggerEvents.h @@ -44,12 +44,15 @@ class ProgressEventData : public EventData { uint64_t GetCompleted() const { return m_completed; } uint64_t GetTotal() const { return m_total; } std::string GetMessage() const { -std::string message = m_title; -if (!m_details.empty()) { - message.append(": "); - message.append(m_details); -} -return message; +if (m_completed == 0) { + std::string message = m_title; + if (!m_details.empty()) { +message.append(": "); +message.append(m_details); + } + return message; +} else + return !m_details.empty() ? m_details : std::string(); } const std::string &GetTitle() const { return m_title; } const std::string &GetDetails() const { return m_details; } >From b64fe53741486ea9b6cde2e658b766aa68bf9389 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Mon, 27 Jan 2025 14:48:01 -0800 Subject: [PATCH 2/9] Add comment explaining if and add a test --- lldb/include/lldb/Core/DebuggerEvents.h | 1 + lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py | 5 + 2 files changed, 6 insertions(+) diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h index 52e4f77d7637d..ca06ee835fde3 100644 --- a/lldb/include/lldb/Core/DebuggerEvents.h +++ b/lldb/include/lldb/Core/DebuggerEvents.h @@ -44,6 +44,7 @@ class ProgressEventData : public EventData { uint64_t GetCompleted() const { return m_completed; } uint64_t GetTotal() const { return m_total; } std::string GetMessage() const { +// Only put the title in the message of the progress create event. if (m_completed == 0) { std::string message = m_title; if (!m_details.empty()) { diff --git a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py index 36c0cef9c4714..945c3f7633364 100755 --- a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py +++ b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py @@ -41,8 +41,13 @@ def test_output(self): for event in self.dap_server.progress_events: event_type = event["event"] if "progressStart" in event_type: +title = event["body"]["title"] +self.assertIn("Progress tester", title) start_found = True if "progressUpdate" in event_type: +message = event["body"]["message"] +print(f"Progress update: {message}") +self.assertNotIn("Progres tester", message) update_found = True self.assertTrue(start_found) >From 96f0ebb43f83cce80c76915f5412b9bb31dd3f12 Mon Sep 17 00:00:00 2001 From: Jacob Lalonde Date: Thu, 30 Jan 2025 10:04:04 -0800 Subject: [PATCH 3/9] Migrate to structured data Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D68927453 --- lldb/include/lldb/Core/DebuggerEvents.h | 16 ++-- .../Handler/InitializeRequestHandler.cpp | 77 --- 2 files changed, 73 insertions(+), 20 deletions(-) diff --git a/lldb/include/lldb/Core/DebuggerEvents.h b/lldb/include/lldb/Core/DebuggerEvents.h index ca06ee835fde3..49a4ecf8e537e 100644 --- a/lldb/include/lldb/Core/DebuggerEvents.h +++ b/lldb/include/lldb/Core/DebuggerEvents.h @@ -44,16 +44,12 @@ class ProgressEventData : public EventData { uint64_t GetCompleted() const { return m_completed; } uint64_t GetTotal() const { return m_total; } std::string GetMessage() const { -// Only put the title in the message of the progress create event. -if (m_completed == 0) { - std::string message = m_title; - if (!m_details.empty()) { -message.append(": "); -message.append(m_details); - } - return message; -} else - return !m_details.empty() ? m_details : std::string(); +std::string message = m_title; +if (!m_details.empty()) { + message.append(": "); + message.append(m_details); +} +return message; } const std::string &GetTitle() const { return m_title; } const std::string &GetDetails() const { return m_details; } diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp index e9041f3
[Lldb-commits] [lldb] [lldb] Add missing calls to SetThreadHitBreakpointSite/DetectThreadStoppedAtUnexecutedBP (PR #128726)
jasonmolenda wrote: Felipe and I have been discussing these mechanisms all this week, and one edge case would be a thread that hits a breakpoint instruction, and the bp has a thread ID match requirement which this thread doesn't satisfy. In that scenario, the thread will not have an eStopReasonBreakpoint, but we still need to `SetThreadHitBreakpointSite()` here. That information -- we hit a breakpoint but it's not applicable to this thread -- isn't explicitly recorded in the Thread object today. https://github.com/llvm/llvm-project/pull/128726 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add missing calls to SetThreadHitBreakpointSite/DetectThreadStoppedAtUnexecutedBP (PR #128726)
jasonmolenda wrote: I'm not sure if this edge case can come up with this MemoryThread use cases. https://github.com/llvm/llvm-project/pull/128726 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][Telemetry]Define DebuggerTelemetryInfo and related methods (PR #127696)
@@ -226,13 +227,45 @@ lldb::SBError SBDebugger::InitializeWithErrorHandling() { return error; } +#if LLVM_ENABLE_TELEMETRY +#if ENABLE_BACKTRACES +static void TelemetryHandler(void *) { + // TODO: get the bt into the msg? + // Also need to pre-alloc the memory for this entry? + lldb_private::telemetry::DebuggerInfo entry; + entry.exit_desc = {-1, ""}; + if (auto* instance = lldb_private::telemetry::TelemeryManager::getInstance()) { +if (instance->GetConfig()->EnableTelemetry()) { + instance->AtDebuggerExit(&entry); +} + } +} + +static bool RegisterTelemetryHander() { + sys::AddSignalHandler(TelemetryHandler, nullptr); + return false; +} +#endif +#endif + +static void InstallCrashTelemetryReporter() { +#if LLVM_ENABLE_TELEMETRY + oontvoo wrote: @labath Is this the right place to add the crash-handler? https://github.com/llvm/llvm-project/pull/127696 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add process picker command to VS Code extension (PR #128943)
https://github.com/matthewbastien created https://github.com/llvm/llvm-project/pull/128943 Adds a process picker command to the LLDB DAP extension that will prompt the user to select a process running on their machine. It is hidden from the command palette, but can be used in an `"attach"` debug configuration to select a process at the start of a debug session. I've also added a debug configuration snippet for this called `"LLDB: Attach to Process"` that will fill in the appropriate variable substitution. e.g: ```json { "type": "lldb-dap", "request": "attach", "name": "Attach to Process", "pid": "${command:PickProcess}" } ``` The logic is largely the same as the process picker in the `vscode-js-debug` extension created by Microsoft. It will use available executables based on the current platform to find the list of available processes: - **Linux**: uses the `ps` executable to list processes. - **macOS**: nearly identical to Linux except that the command line options passed to `ps` are different - **Windows**: uses `WMIC.exe` to query WMI for processes I manually tested this on a MacBook Pro running macOS Sequoia, a Windows 11 VM, and an Ubuntu 22.04 VM. Fixes #96279 >From b9083ea16c7b1dba70cc04acf78f5001f0fb86c6 Mon Sep 17 00:00:00 2001 From: Matthew Bastien Date: Wed, 26 Feb 2025 11:18:21 -0500 Subject: [PATCH 1/2] add a process picker for attaching by PID --- lldb/tools/lldb-dap/package.json | 30 +- .../lldb-dap/src-ts/commands/pick-process.ts | 37 +++ lldb/tools/lldb-dap/src-ts/extension.ts | 7 +- .../src-ts/process-tree/base-process-tree.ts | 102 ++ .../lldb-dap/src-ts/process-tree/index.ts | 36 +++ .../platforms/darwin-process-tree.ts | 16 +++ .../platforms/linux-process-tree.ts | 38 +++ .../platforms/windows-process-tree.ts | 52 + 8 files changed, 315 insertions(+), 3 deletions(-) create mode 100644 lldb/tools/lldb-dap/src-ts/commands/pick-process.ts create mode 100644 lldb/tools/lldb-dap/src-ts/process-tree/base-process-tree.ts create mode 100644 lldb/tools/lldb-dap/src-ts/process-tree/index.ts create mode 100644 lldb/tools/lldb-dap/src-ts/process-tree/platforms/darwin-process-tree.ts create mode 100644 lldb/tools/lldb-dap/src-ts/process-tree/platforms/linux-process-tree.ts create mode 100644 lldb/tools/lldb-dap/src-ts/process-tree/platforms/windows-process-tree.ts diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json index 31d808eda4c35..1bbdbf045dd1b 100644 --- a/lldb/tools/lldb-dap/package.json +++ b/lldb/tools/lldb-dap/package.json @@ -146,6 +146,9 @@ "windows": { "program": "./bin/lldb-dap.exe" }, +"variables": { + "PickProcess": "lldb-dap.pickProcess" +}, "configurationAttributes": { "launch": { "required": [ @@ -517,6 +520,16 @@ "cwd": "^\"\\${workspaceRoot}\"" } }, + { +"label": "LLDB: Attach to Process", +"description": "", +"body": { + "type": "lldb-dap", + "request": "attach", + "name": "${1:Attach}", + "pid": "^\"\\${command:PickProcess}\"" +} + }, { "label": "LLDB: Attach", "description": "", @@ -541,6 +554,21 @@ } ] } -] +], +"commands": [ + { +"command": "lldb-dap.pickProcess", +"title": "Pick Process", +"category": "LLDB DAP" + } +], +"menus": { + "commandPalette": [ +{ + "command": "lldb-dap.pickProcess", + "when": "false" +} + ] +} } } diff --git a/lldb/tools/lldb-dap/src-ts/commands/pick-process.ts b/lldb/tools/lldb-dap/src-ts/commands/pick-process.ts new file mode 100644 index 0..b83e749e7da7b --- /dev/null +++ b/lldb/tools/lldb-dap/src-ts/commands/pick-process.ts @@ -0,0 +1,37 @@ +import * as path from "path"; +import * as vscode from "vscode"; +import { createProcessTree } from "../process-tree"; + +interface ProcessQuickPick extends vscode.QuickPickItem { + processId: number; +} + +/** + * Prompts the user to select a running process. + * + * @returns The pid of the process as a string or undefined if cancelled. + */ +export async function pickProcess(): Promise { + const processTree = createProcessTree(); + const selectedProcess = await vscode.window.showQuickPick( +processTree.listAllProcesses().then((processes): ProcessQuickPick[] => { + return processes +.sort((a, b) => b.start - a.start) // Sort by start date in descending order +.map((proc) => { + return { +processId: proc.id, +label: path.basename(proc.command), +description: proc.id.toString(), +detail: proc.arguments, + } satisf
[Lldb-commits] [lldb] [lldb-dap] Add process picker command to VS Code extension (PR #128943)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Matthew Bastien (matthewbastien) Changes Adds a process picker command to the LLDB DAP extension that will prompt the user to select a process running on their machine. It is hidden from the command palette, but can be used in an `"attach"` debug configuration to select a process at the start of a debug session. I've also added a debug configuration snippet for this called `"LLDB: Attach to Process"` that will fill in the appropriate variable substitution. e.g: ```json { "type": "lldb-dap", "request": "attach", "name": "Attach to Process", "pid": "${command:PickProcess}" } ``` The logic is largely the same as the process picker in the `vscode-js-debug` extension created by Microsoft. It will use available executables based on the current platform to find the list of available processes: - **Linux**: uses the `ps` executable to list processes. - **macOS**: nearly identical to Linux except that the command line options passed to `ps` are different - **Windows**: uses `WMIC.exe` to query WMI for processes I manually tested this on a MacBook Pro running macOS Sequoia, a Windows 11 VM, and an Ubuntu 22.04 VM. Fixes #96279 --- Full diff: https://github.com/llvm/llvm-project/pull/128943.diff 9 Files Affected: - (modified) lldb/tools/lldb-dap/package.json (+29-1) - (added) lldb/tools/lldb-dap/src-ts/commands/pick-process.ts (+41) - (added) lldb/tools/lldb-dap/src-ts/debug-configuration-provider.ts (+54) - (modified) lldb/tools/lldb-dap/src-ts/extension.ts (+12-2) - (added) lldb/tools/lldb-dap/src-ts/process-tree/base-process-tree.ts (+102) - (added) lldb/tools/lldb-dap/src-ts/process-tree/index.ts (+36) - (added) lldb/tools/lldb-dap/src-ts/process-tree/platforms/darwin-process-tree.ts (+16) - (added) lldb/tools/lldb-dap/src-ts/process-tree/platforms/linux-process-tree.ts (+38) - (added) lldb/tools/lldb-dap/src-ts/process-tree/platforms/windows-process-tree.ts (+52) ``diff diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json index 31d808eda4c35..1bbdbf045dd1b 100644 --- a/lldb/tools/lldb-dap/package.json +++ b/lldb/tools/lldb-dap/package.json @@ -146,6 +146,9 @@ "windows": { "program": "./bin/lldb-dap.exe" }, +"variables": { + "PickProcess": "lldb-dap.pickProcess" +}, "configurationAttributes": { "launch": { "required": [ @@ -517,6 +520,16 @@ "cwd": "^\"\\${workspaceRoot}\"" } }, + { +"label": "LLDB: Attach to Process", +"description": "", +"body": { + "type": "lldb-dap", + "request": "attach", + "name": "${1:Attach}", + "pid": "^\"\\${command:PickProcess}\"" +} + }, { "label": "LLDB: Attach", "description": "", @@ -541,6 +554,21 @@ } ] } -] +], +"commands": [ + { +"command": "lldb-dap.pickProcess", +"title": "Pick Process", +"category": "LLDB DAP" + } +], +"menus": { + "commandPalette": [ +{ + "command": "lldb-dap.pickProcess", + "when": "false" +} + ] +} } } diff --git a/lldb/tools/lldb-dap/src-ts/commands/pick-process.ts b/lldb/tools/lldb-dap/src-ts/commands/pick-process.ts new file mode 100644 index 0..355d508075080 --- /dev/null +++ b/lldb/tools/lldb-dap/src-ts/commands/pick-process.ts @@ -0,0 +1,41 @@ +import * as path from "path"; +import * as vscode from "vscode"; +import { createProcessTree } from "../process-tree"; + +interface ProcessQuickPick extends vscode.QuickPickItem { + processId: number; +} + +/** + * Prompts the user to select a running process. + * + * The return value must be a string so that it is compatible with VS Code's + * string substitution infrastructure. The value will eventually be converted + * to a number by the debug configuration provider. + * + * @returns The pid of the process as a string or undefined if cancelled. + */ +export async function pickProcess(): Promise { + const processTree = createProcessTree(); + const selectedProcess = await vscode.window.showQuickPick( +processTree.listAllProcesses().then((processes): ProcessQuickPick[] => { + return processes +.sort((a, b) => b.start - a.start) // Sort by start date in descending order +.map((proc) => { + return { +processId: proc.id, +label: path.basename(proc.command), +description: proc.id.toString(), +detail: proc.arguments, + } satisfies ProcessQuickPick; +}); +}), +{ + placeHolder: "Select a process to attach the debugger to", +}, + ); + if (!selectedProcess) { +return; + } + return selectedProcess.processId.toString(); +} diff --git a/lld
[Lldb-commits] [lldb] [lldb-dap] Add process picker command to VS Code extension (PR #128943)
https://github.com/matthewbastien updated https://github.com/llvm/llvm-project/pull/128943 >From b9083ea16c7b1dba70cc04acf78f5001f0fb86c6 Mon Sep 17 00:00:00 2001 From: Matthew Bastien Date: Wed, 26 Feb 2025 11:18:21 -0500 Subject: [PATCH 1/3] add a process picker for attaching by PID --- lldb/tools/lldb-dap/package.json | 30 +- .../lldb-dap/src-ts/commands/pick-process.ts | 37 +++ lldb/tools/lldb-dap/src-ts/extension.ts | 7 +- .../src-ts/process-tree/base-process-tree.ts | 102 ++ .../lldb-dap/src-ts/process-tree/index.ts | 36 +++ .../platforms/darwin-process-tree.ts | 16 +++ .../platforms/linux-process-tree.ts | 38 +++ .../platforms/windows-process-tree.ts | 52 + 8 files changed, 315 insertions(+), 3 deletions(-) create mode 100644 lldb/tools/lldb-dap/src-ts/commands/pick-process.ts create mode 100644 lldb/tools/lldb-dap/src-ts/process-tree/base-process-tree.ts create mode 100644 lldb/tools/lldb-dap/src-ts/process-tree/index.ts create mode 100644 lldb/tools/lldb-dap/src-ts/process-tree/platforms/darwin-process-tree.ts create mode 100644 lldb/tools/lldb-dap/src-ts/process-tree/platforms/linux-process-tree.ts create mode 100644 lldb/tools/lldb-dap/src-ts/process-tree/platforms/windows-process-tree.ts diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json index 31d808eda4c35..1bbdbf045dd1b 100644 --- a/lldb/tools/lldb-dap/package.json +++ b/lldb/tools/lldb-dap/package.json @@ -146,6 +146,9 @@ "windows": { "program": "./bin/lldb-dap.exe" }, +"variables": { + "PickProcess": "lldb-dap.pickProcess" +}, "configurationAttributes": { "launch": { "required": [ @@ -517,6 +520,16 @@ "cwd": "^\"\\${workspaceRoot}\"" } }, + { +"label": "LLDB: Attach to Process", +"description": "", +"body": { + "type": "lldb-dap", + "request": "attach", + "name": "${1:Attach}", + "pid": "^\"\\${command:PickProcess}\"" +} + }, { "label": "LLDB: Attach", "description": "", @@ -541,6 +554,21 @@ } ] } -] +], +"commands": [ + { +"command": "lldb-dap.pickProcess", +"title": "Pick Process", +"category": "LLDB DAP" + } +], +"menus": { + "commandPalette": [ +{ + "command": "lldb-dap.pickProcess", + "when": "false" +} + ] +} } } diff --git a/lldb/tools/lldb-dap/src-ts/commands/pick-process.ts b/lldb/tools/lldb-dap/src-ts/commands/pick-process.ts new file mode 100644 index 0..b83e749e7da7b --- /dev/null +++ b/lldb/tools/lldb-dap/src-ts/commands/pick-process.ts @@ -0,0 +1,37 @@ +import * as path from "path"; +import * as vscode from "vscode"; +import { createProcessTree } from "../process-tree"; + +interface ProcessQuickPick extends vscode.QuickPickItem { + processId: number; +} + +/** + * Prompts the user to select a running process. + * + * @returns The pid of the process as a string or undefined if cancelled. + */ +export async function pickProcess(): Promise { + const processTree = createProcessTree(); + const selectedProcess = await vscode.window.showQuickPick( +processTree.listAllProcesses().then((processes): ProcessQuickPick[] => { + return processes +.sort((a, b) => b.start - a.start) // Sort by start date in descending order +.map((proc) => { + return { +processId: proc.id, +label: path.basename(proc.command), +description: proc.id.toString(), +detail: proc.arguments, + } satisfies ProcessQuickPick; +}); +}), +{ + placeHolder: "Select a process to attach the debugger to", +}, + ); + if (!selectedProcess) { +return; + } + return selectedProcess.processId.toString(); +} diff --git a/lldb/tools/lldb-dap/src-ts/extension.ts b/lldb/tools/lldb-dap/src-ts/extension.ts index 71fd48298f8f5..3532a2143155b 100644 --- a/lldb/tools/lldb-dap/src-ts/extension.ts +++ b/lldb/tools/lldb-dap/src-ts/extension.ts @@ -1,7 +1,6 @@ -import * as path from "path"; -import * as util from "util"; import * as vscode from "vscode"; +import { pickProcess } from "./commands/pick-process"; import { LLDBDapDescriptorFactory, isExecutable, @@ -38,6 +37,10 @@ export class LLDBDapExtension extends DisposableContext { } }), ); + +this.pushSubscription( + vscode.commands.registerCommand("lldb-dap.pickProcess", pickProcess), +); } } diff --git a/lldb/tools/lldb-dap/src-ts/process-tree/base-process-tree.ts b/lldb/tools/lldb-dap/src-ts/process-tree/base-process-tree.ts new file mode 100644 index 0..3c08f49035b35 --- /dev/null
[Lldb-commits] [lldb] [lldb] Add missing calls to SetThreadHitBreakpointSite/DetectThreadStoppedAtUnexecutedBP (PR #128726)
https://github.com/felipepiovezan closed https://github.com/llvm/llvm-project/pull/128726 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add missing calls to SetThreadHitBreakpointSite/DetectThreadStoppedAtUnexecutedBP (PR #128726)
felipepiovezan wrote: Yup, as Jason said, we have been thinking about this and we believe there may be a better way of doing this! I'm going to close this PR for now https://github.com/llvm/llvm-project/pull/128726 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add process picker command to VS Code extension (PR #128943)
https://github.com/matthewbastien updated https://github.com/llvm/llvm-project/pull/128943 >From b9083ea16c7b1dba70cc04acf78f5001f0fb86c6 Mon Sep 17 00:00:00 2001 From: Matthew Bastien Date: Wed, 26 Feb 2025 11:18:21 -0500 Subject: [PATCH 1/4] add a process picker for attaching by PID --- lldb/tools/lldb-dap/package.json | 30 +- .../lldb-dap/src-ts/commands/pick-process.ts | 37 +++ lldb/tools/lldb-dap/src-ts/extension.ts | 7 +- .../src-ts/process-tree/base-process-tree.ts | 102 ++ .../lldb-dap/src-ts/process-tree/index.ts | 36 +++ .../platforms/darwin-process-tree.ts | 16 +++ .../platforms/linux-process-tree.ts | 38 +++ .../platforms/windows-process-tree.ts | 52 + 8 files changed, 315 insertions(+), 3 deletions(-) create mode 100644 lldb/tools/lldb-dap/src-ts/commands/pick-process.ts create mode 100644 lldb/tools/lldb-dap/src-ts/process-tree/base-process-tree.ts create mode 100644 lldb/tools/lldb-dap/src-ts/process-tree/index.ts create mode 100644 lldb/tools/lldb-dap/src-ts/process-tree/platforms/darwin-process-tree.ts create mode 100644 lldb/tools/lldb-dap/src-ts/process-tree/platforms/linux-process-tree.ts create mode 100644 lldb/tools/lldb-dap/src-ts/process-tree/platforms/windows-process-tree.ts diff --git a/lldb/tools/lldb-dap/package.json b/lldb/tools/lldb-dap/package.json index 31d808eda4c35..1bbdbf045dd1b 100644 --- a/lldb/tools/lldb-dap/package.json +++ b/lldb/tools/lldb-dap/package.json @@ -146,6 +146,9 @@ "windows": { "program": "./bin/lldb-dap.exe" }, +"variables": { + "PickProcess": "lldb-dap.pickProcess" +}, "configurationAttributes": { "launch": { "required": [ @@ -517,6 +520,16 @@ "cwd": "^\"\\${workspaceRoot}\"" } }, + { +"label": "LLDB: Attach to Process", +"description": "", +"body": { + "type": "lldb-dap", + "request": "attach", + "name": "${1:Attach}", + "pid": "^\"\\${command:PickProcess}\"" +} + }, { "label": "LLDB: Attach", "description": "", @@ -541,6 +554,21 @@ } ] } -] +], +"commands": [ + { +"command": "lldb-dap.pickProcess", +"title": "Pick Process", +"category": "LLDB DAP" + } +], +"menus": { + "commandPalette": [ +{ + "command": "lldb-dap.pickProcess", + "when": "false" +} + ] +} } } diff --git a/lldb/tools/lldb-dap/src-ts/commands/pick-process.ts b/lldb/tools/lldb-dap/src-ts/commands/pick-process.ts new file mode 100644 index 0..b83e749e7da7b --- /dev/null +++ b/lldb/tools/lldb-dap/src-ts/commands/pick-process.ts @@ -0,0 +1,37 @@ +import * as path from "path"; +import * as vscode from "vscode"; +import { createProcessTree } from "../process-tree"; + +interface ProcessQuickPick extends vscode.QuickPickItem { + processId: number; +} + +/** + * Prompts the user to select a running process. + * + * @returns The pid of the process as a string or undefined if cancelled. + */ +export async function pickProcess(): Promise { + const processTree = createProcessTree(); + const selectedProcess = await vscode.window.showQuickPick( +processTree.listAllProcesses().then((processes): ProcessQuickPick[] => { + return processes +.sort((a, b) => b.start - a.start) // Sort by start date in descending order +.map((proc) => { + return { +processId: proc.id, +label: path.basename(proc.command), +description: proc.id.toString(), +detail: proc.arguments, + } satisfies ProcessQuickPick; +}); +}), +{ + placeHolder: "Select a process to attach the debugger to", +}, + ); + if (!selectedProcess) { +return; + } + return selectedProcess.processId.toString(); +} diff --git a/lldb/tools/lldb-dap/src-ts/extension.ts b/lldb/tools/lldb-dap/src-ts/extension.ts index 71fd48298f8f5..3532a2143155b 100644 --- a/lldb/tools/lldb-dap/src-ts/extension.ts +++ b/lldb/tools/lldb-dap/src-ts/extension.ts @@ -1,7 +1,6 @@ -import * as path from "path"; -import * as util from "util"; import * as vscode from "vscode"; +import { pickProcess } from "./commands/pick-process"; import { LLDBDapDescriptorFactory, isExecutable, @@ -38,6 +37,10 @@ export class LLDBDapExtension extends DisposableContext { } }), ); + +this.pushSubscription( + vscode.commands.registerCommand("lldb-dap.pickProcess", pickProcess), +); } } diff --git a/lldb/tools/lldb-dap/src-ts/process-tree/base-process-tree.ts b/lldb/tools/lldb-dap/src-ts/process-tree/base-process-tree.ts new file mode 100644 index 0..3c08f49035b35 --- /dev/null
[Lldb-commits] [lldb] [LLDB-DAP] SBDebugger don't prefix title on progress updates (PR #124648)
@@ -12,6 +12,41 @@ class TestDAP_progress(lldbdap_testcase.DAPTestCaseBase): +def verify_progress_events( +self, +expected_title, +expected_message=None, +expected_not_in_message=None, +only_verify_first_update=False, +): +self.dap_server.wait_for_event("progressEnd", 15) +self.assertTrue(len(self.dap_server.progress_events) > 0) +start_found = False +update_found = False +end_found = False +for event in self.dap_server.progress_events: +event_type = event["event"] +if "progressStart" in event_type: +title = event["body"]["title"] +self.assertIn(expected_title, title) +start_found = True +if "progressUpdate" in event_type: +message = event["body"]["message"] +print(f"Progress update: {message}") clayborg wrote: remove print statements https://github.com/llvm/llvm-project/pull/124648 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB-DAP] SBDebugger don't prefix title on progress updates (PR #124648)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/124648 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB-DAP] SBDebugger don't prefix title on progress updates (PR #124648)
@@ -37,7 +37,11 @@ def create_options(cls): ) parser.add_option( -"--total", dest="total", help="Total to count up.", type="int" +"--total", +dest="total", +help="Total to count up, use None to identify as indeterminate", clayborg wrote: "Total items in this progress object. When this option is not specified, this will be an indeterminate progress." https://github.com/llvm/llvm-project/pull/124648 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB-DAP] SBDebugger don't prefix title on progress updates (PR #124648)
https://github.com/clayborg requested changes to this pull request. Reading Progress.h/.cpp, I see that Increment will do nothing for non deterministic progress objects. We really should add a dedicated `void SBProgress::Finished()` to allow objects to say when they are done, there are no guarantees of when the object will be destroyed. So we should do that here to clean this issue up and guarantee we don't cause these tests to run longer than needed. https://github.com/llvm/llvm-project/pull/124648 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB-DAP] SBDebugger don't prefix title on progress updates (PR #124648)
@@ -35,14 +59,47 @@ static void ProgressEventThreadFunction(DAP &dap) { done = true; } } else { -uint64_t progress_id = 0; -uint64_t completed = 0; -uint64_t total = 0; -bool is_debugger_specific = false; -const char *message = lldb::SBDebugger::GetProgressFromEvent( -event, progress_id, completed, total, is_debugger_specific); -if (message) - dap.SendProgressEvent(progress_id, message, completed, total); +lldb::SBStructuredData data = +lldb::SBDebugger::GetProgressDataFromEvent(event); + +const uint64_t progress_id = +GetUintFromStructuredData(data, "progress_id"); +const uint64_t completed = GetUintFromStructuredData(data, "completed"); +const uint64_t total = GetUintFromStructuredData(data, "total"); +const std::string details = +GetStringFromStructuredData(data, "details"); + +if (completed == 0) { + if (total == 1) { clayborg wrote: Reading the internal Progress.h and Progress.cpp, this value should be reported as ``` static constexpr uint64_t kNonDeterministicTotal = UINT64_MAX; ``` This test must not be working then or this code is never executed? We really should add a function to `SBProgress`: ``` bool SBProgress::IsDeterministic(); ``` Or "total" should not be in the structured data if it isn't deterministic. https://github.com/llvm/llvm-project/pull/124648 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add process picker command to VS Code extension (PR #128943)
JDevlieghere wrote: It's a little unfortunate that listing processes is not part of the Debug Adapter Protocol. LLDB already knows how to do this through the host platform. I guess even more unfortunate is that neither node nor VSCode provides a cross platform API to do this. I was expecting the implementation to be more convoluted but @matthewbastien did a great job abstracting the different platforms. This is definitely better than relying on an external package that's outside of our control. One possible alternative would be to add a flag to the `lldb-dap` executable to print the processes (e.g. as JSON) and then use that. The upside is that we don't need to invoke different executables depending on the platform. The downside is that this now depends on a certain version of the `lldb-dap` binary and that we need to resolve the binary before we do this operation. I'm just floating the idea, I'm not sure if that's actually better or not. https://github.com/llvm/llvm-project/pull/128943 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Add process picker command to VS Code extension (PR #128943)
matthewbastien wrote: Yeah, the CodeLLDB extension uses `lldb` to print the processes for the current platform (which could be a possibility as well, though then you'd also have to have an option to specify the `lldb` executable alongside `lldb-dap`). I'd rather have a NodeJS approach if only because relying on a specific version of `lldb-dap` won't really work for Swift until it updates its version of `lldb-dap` meaning that the functionality probably won't be available until Swift 6.2. I'd prefer to have it available sooner since we'd like to switch away from CodeLLDB as soon as possible. https://github.com/llvm/llvm-project/pull/128943 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Also show session history in fzf_history (PR #128986)
https://github.com/kastiglione closed https://github.com/llvm/llvm-project/pull/128986 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 354eb88 - [lldb] Also show session history in fzf_history (#128986)
Author: Dave Lee Date: 2025-02-26T19:14:49-08:00 New Revision: 354eb88285c0d803b0674a3b2961b4109905383a URL: https://github.com/llvm/llvm-project/commit/354eb88285c0d803b0674a3b2961b4109905383a DIFF: https://github.com/llvm/llvm-project/commit/354eb88285c0d803b0674a3b2961b4109905383a.diff LOG: [lldb] Also show session history in fzf_history (#128986) lldb's history log file is written to at the end of a debugging session. As a result, the log does not contain commands run during the current session. This extends the `fzf_history` to include the output of `session history`. Added: Modified: lldb/examples/python/fzf_history.py Removed: diff --git a/lldb/examples/python/fzf_history.py b/lldb/examples/python/fzf_history.py index 546edb2ed2712..d70789c8c0259 100644 --- a/lldb/examples/python/fzf_history.py +++ b/lldb/examples/python/fzf_history.py @@ -14,7 +14,7 @@ def fzf_history(debugger, cmdstr, ctx, result, _): if not os.path.exists(history_file): result.SetError("history file does not exist") return -history = _load_history(history_file) +history = _load_history(debugger, history_file) if sys.platform != "darwin": # The ability to integrate fzf's result into lldb uses copy and paste. @@ -79,16 +79,49 @@ def _handle_command(debugger, command): debugger.HandleCommand(command) -def _load_history(history_file): -"""Load, decode, parse, and prepare an lldb history file for fzf.""" +# `session history` example formatting: +#1: first command +#2: penultimate command +#3: latest command +_HISTORY_PREFIX = re.compile(r"^\s+\d+:\s+") + + +def _load_session_history(debugger): +"""Load and parse lldb session history.""" +result = lldb.SBCommandReturnObject() +interp = debugger.GetCommandInterpreter() +interp.HandleCommand("session history", result) +history = result.GetOutput() +commands = [] +for line in history.splitlines(): +# Strip the prefix. +command = _HISTORY_PREFIX.sub("", line) +commands.append(command) +return commands + + +def _load_persisted_history(history_file): +"""Load and decode lldb persisted history.""" with open(history_file) as f: history_contents = f.read() +# Some characters (ex spaces and newlines) are encoded as octal values, but +# as _characters_ (not bytes). Space is the string r"\\040". history_decoded = re.sub(r"\\0([0-7][0-7])", _decode_char, history_contents) history_lines = history_decoded.splitlines() # Skip the header line (_HiStOrY_V2_) del history_lines[0] +return history_lines + + +def _load_history(debugger, history_file): +"""Load, decode, parse, and prepare lldb history for fzf.""" +# Persisted history is older (earlier). +history_lines = _load_persisted_history(history_file) +# Session history is newer (later). +history_lines.extend(_load_session_history(debugger)) + # Reverse to show latest first. history_lines.reverse() ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits