[Lldb-commits] [lldb] 58279d1 - [lldb] Synchronize the debuggers output & error streams
Author: Jonas Devlieghere
Date: 2025-02-19T20:32:00-08:00
New Revision: 58279d1ee1b567e8ca793d6d1eb6e0f1d5e7279e
URL:
https://github.com/llvm/llvm-project/commit/58279d1ee1b567e8ca793d6d1eb6e0f1d5e7279e
DIFF:
https://github.com/llvm/llvm-project/commit/58279d1ee1b567e8ca793d6d1eb6e0f1d5e7279e.diff
LOG: [lldb] Synchronize the debuggers output & error streams
This patch improves the synchronization of the debugger's output and error
streams using two new abstractions: `LockableStreamFile` and
`LockedStreamFile`.
- `LockableStreamFile` is a wrapper around a `StreamFile` and a mutex. Client
cannot use the `StreamFile` without calling `Lock`, which returns a
`LockedStreamFile`.
- `LockedStreamFile` is an RAII object that locks the stream for the duration
of its existence. As long as you hold on to the returned object you are
permitted to write to the stream. The destruction of the object
automatically flush the output stream.
Added:
Modified:
lldb/include/lldb/Core/Debugger.h
lldb/include/lldb/Core/IOHandler.h
lldb/include/lldb/Host/Editline.h
lldb/include/lldb/Host/File.h
lldb/include/lldb/Host/StreamFile.h
lldb/include/lldb/Interpreter/ScriptInterpreter.h
lldb/include/lldb/lldb-forward.h
lldb/source/Commands/CommandObjectBreakpointCommand.cpp
lldb/source/Commands/CommandObjectCommands.cpp
lldb/source/Commands/CommandObjectExpression.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Commands/CommandObjectType.cpp
lldb/source/Commands/CommandObjectWatchpointCommand.cpp
lldb/source/Core/Debugger.cpp
lldb/source/Core/IOHandler.cpp
lldb/source/Core/IOHandlerCursesGUI.cpp
lldb/source/Expression/REPL.cpp
lldb/source/Host/common/Editline.cpp
lldb/source/Interpreter/CommandInterpreter.cpp
lldb/source/Interpreter/ScriptInterpreter.cpp
lldb/source/Plugins/ScriptInterpreter/Lua/ScriptInterpreterLua.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/unittests/Editline/EditlineTest.cpp
Removed:
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 7f08f3dd26106..9c8a9623fe689 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -131,13 +131,16 @@ class Debugger : public
std::enable_shared_from_this,
void SetAsyncExecution(bool async);
- File &GetInputFile() { return *m_input_file_sp; }
-
lldb::FileSP GetInputFileSP() { return m_input_file_sp; }
+ File &GetInputFile() { return *m_input_file_sp; }
- lldb::FileSP GetOutputFileSP() { return m_output_stream_sp->GetFileSP(); }
+ lldb::FileSP GetOutputFileSP() {
+return m_output_stream_sp->GetUnlockedFileSP();
+ }
- lldb::FileSP GetErrorFileSP() { return m_error_stream_sp->GetFileSP(); }
+ lldb::FileSP GetErrorFileSP() {
+return m_error_stream_sp->GetUnlockedFileSP();
+ }
repro::DataRecorder *GetInputRecorder();
@@ -198,8 +201,8 @@ class Debugger : public
std::enable_shared_from_this,
// If any of the streams are not set, set them to the in/out/err stream of
// the top most input reader to ensure they at least have something
void AdoptTopIOHandlerFilesIfInvalid(lldb::FileSP &in,
- lldb::StreamFileSP &out,
- lldb::StreamFileSP &err);
+ lldb::LockableStreamFileSP &out,
+ lldb::LockableStreamFileSP &err);
/// Run the given IO handler and return immediately.
void RunIOHandlerAsync(const lldb::IOHandlerSP &reader_sp,
@@ -649,8 +652,8 @@ class Debugger : public
std::enable_shared_from_this,
/// should not be used directly. Use GetAsyncOutputStream and
/// GetAsyncErrorStream instead.
/// @{
- lldb::StreamFileSP GetOutputStreamSP() { return m_output_stream_sp; }
- lldb::StreamFileSP GetErrorStreamSP() { return m_error_stream_sp; }
+ lldb::LockableStreamFileSP GetOutputStreamSP() { return m_output_stream_sp; }
+ lldb::LockableStreamFileSP GetErrorStreamSP() { return m_error_stream_sp; }
/// @}
void PushIOHandler(const lldb::IOHandlerSP &reader_sp,
@@ -693,8 +696,9 @@ class Debugger : public
std::enable_shared_from_this,
// these should never be NULL
lldb::FileSP m_input_file_sp;
- lldb::StreamFileSP m_output_stream_sp;
- lldb::StreamFileSP m_error_stream_sp;
+ lldb::LockableStreamFileSP m_output_stream_sp;
+ lldb::LockableStreamFileSP m_error_stream_sp;
+ LockableStreamFile::Mutex m_output_mutex;
/// Used for shadowing the input file when capturing a reproducer.
repro::DataRecorder *m_input_recorder;
diff --git a/lldb/include/lldb/Core/IOHandler.h
b/lldb/include/lldb/Core/IOHandler.h
index d6ac1cc8b5a14..fc0c676883b4a 100644
--- a/lldb/include/lldb/Core/IOHandler.h
+++ b/lldb/include/lldb/Core/IOHandler.h
@@ -5
[Lldb-commits] [lldb] [lldb] Synchronize the debugger's stdout and stderr streams (PR #126630)
https://github.com/JDevlieghere closed https://github.com/llvm/llvm-project/pull/126630 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 57bac14 - [lldb] Fix header include order in ScriptInterpreterPython.cpp
Author: Jonas Devlieghere Date: 2025-02-19T21:09:32-08:00 New Revision: 57bac14f4bcc8b43edc3e27be3d93609f7f4037b URL: https://github.com/llvm/llvm-project/commit/57bac14f4bcc8b43edc3e27be3d93609f7f4037b DIFF: https://github.com/llvm/llvm-project/commit/57bac14f4bcc8b43edc3e27be3d93609f7f4037b.diff LOG: [lldb] Fix header include order in ScriptInterpreterPython.cpp Should fix the following compile error on Windows: C:\Python312\include\pyconfig.h(225): error C2371: 'pid_t': redefinition; different basic types C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\include\lldb/Host/windows/PosixApi.h(80): note: see declaration of 'pid_t' Added: Modified: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp Removed: diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp index 2b92beaf6c5ef..00d01981c64ff 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -7,10 +7,6 @@ //===--===// #include "lldb/Host/Config.h" -#include "lldb/Host/StreamFile.h" -#include "lldb/lldb-enumerations.h" -#include "lldb/lldb-forward.h" -#include #if LLDB_ENABLE_PYTHON @@ -36,6 +32,7 @@ #include "lldb/Host/FileSystem.h" #include "lldb/Host/HostInfo.h" #include "lldb/Host/Pipe.h" +#include "lldb/Host/StreamFile.h" #include "lldb/Interpreter/CommandInterpreter.h" #include "lldb/Interpreter/CommandReturnObject.h" #include "lldb/Target/Thread.h" @@ -44,6 +41,8 @@ #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/Timer.h" #include "lldb/ValueObject/ValueObject.h" +#include "lldb/lldb-enumerations.h" +#include "lldb/lldb-forward.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Store StreamAsynchronousIO in a unique_ptr (NFC) (PR #127961)
https://github.com/JDevlieghere created
https://github.com/llvm/llvm-project/pull/127961
Make StreamAsynchronousIO an unique_ptr instead of a shared_ptr. I tried
passing the class by value, but the llvm::raw_ostream forwarded stored in the
Stream parent class isn't movable and I don't think it's worth changing that.
Additionally, there's a few places that expect a StreamSP, which are easily
created from a StreamUP.
>From d8c91ab0d401384f2724a8c2c67c12e9c9446af7 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Wed, 19 Feb 2025 21:24:29 -0800
Subject: [PATCH] [lldb] Store StreamAsynchronousIO in a unique_ptr (NFC)
Make StreamAsynchronousIO an unique_ptr instead of a shared_ptr. I tried
passing the class by value, but the llvm::raw_ostream forwarded stored
in the Stream parent class isn't movable and I don't think it's worth
changing that. Additionally, there's a few places that expect a
StreamSP, which are easily created from a StreamUP.
---
lldb/include/lldb/Core/Debugger.h | 4 +-
lldb/include/lldb/lldb-forward.h | 1 +
lldb/source/Breakpoint/BreakpointOptions.cpp | 6 +-
.../source/Commands/CommandObjectCommands.cpp | 7 +-
.../CommandObjectWatchpointCommand.cpp| 6 +-
lldb/source/Core/Debugger.cpp | 85 +--
lldb/source/Core/DynamicLoader.cpp| 2 +-
.../DynamicLoaderDarwinKernel.cpp | 6 +-
.../DynamicLoaderFreeBSDKernel.cpp| 10 +--
.../Process/MacOSX-Kernel/ProcessKDP.cpp | 18 +---
.../Process/gdb-remote/ProcessGDBRemote.cpp | 3 +-
lldb/source/Target/Process.cpp| 14 ++-
lldb/source/Target/StopInfo.cpp | 8 +-
lldb/source/Target/Target.cpp | 2 +-
14 files changed, 72 insertions(+), 100 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 9c8a9623fe689..6ebc6147800e1 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -156,9 +156,9 @@ class Debugger : public
std::enable_shared_from_this,
void RestoreInputTerminalState();
- lldb::StreamSP GetAsyncOutputStream();
+ lldb::StreamUP GetAsyncOutputStream();
- lldb::StreamSP GetAsyncErrorStream();
+ lldb::StreamUP GetAsyncErrorStream();
CommandInterpreter &GetCommandInterpreter() {
assert(m_command_interpreter_up.get());
diff --git a/lldb/include/lldb/lldb-forward.h b/lldb/include/lldb/lldb-forward.h
index cda55ef06e549..c664d1398f74d 100644
--- a/lldb/include/lldb/lldb-forward.h
+++ b/lldb/include/lldb/lldb-forward.h
@@ -432,6 +432,7 @@ typedef
std::unique_ptr
StackFrameRecognizerManagerUP;
typedef std::shared_ptr StopInfoSP;
typedef std::shared_ptr StreamSP;
+typedef std::unique_ptr StreamUP;
typedef std::shared_ptr StreamFileSP;
typedef std::shared_ptr LockableStreamFileSP;
typedef std::shared_ptr
diff --git a/lldb/source/Breakpoint/BreakpointOptions.cpp
b/lldb/source/Breakpoint/BreakpointOptions.cpp
index 09abcf5e081d2..242b5b30168c5 100644
--- a/lldb/source/Breakpoint/BreakpointOptions.cpp
+++ b/lldb/source/Breakpoint/BreakpointOptions.cpp
@@ -620,10 +620,8 @@ bool BreakpointOptions::BreakpointOptionsCallbackFunction(
// Rig up the results secondary output stream to the debugger's, so the
// output will come out synchronously if the debugger is set up that way.
- StreamSP output_stream(debugger.GetAsyncOutputStream());
- StreamSP error_stream(debugger.GetAsyncErrorStream());
- result.SetImmediateOutputStream(output_stream);
- result.SetImmediateErrorStream(error_stream);
+ result.SetImmediateOutputStream(debugger.GetAsyncOutputStream());
+ result.SetImmediateErrorStream(debugger.GetAsyncErrorStream());
CommandInterpreterRunOptions options;
options.SetStopOnContinue(true);
diff --git a/lldb/source/Commands/CommandObjectCommands.cpp
b/lldb/source/Commands/CommandObjectCommands.cpp
index dd841cb5cb4cc..9510cf4d14467 100644
--- a/lldb/source/Commands/CommandObjectCommands.cpp
+++ b/lldb/source/Commands/CommandObjectCommands.cpp
@@ -815,10 +815,9 @@ a number follows 'f':"
for (const std::string &line : lines) {
Status error = AppendRegexSubstitution(line, check_only);
if (error.Fail()) {
-if (!GetDebugger().GetCommandInterpreter().GetBatchCommandMode()) {
- StreamSP out_stream = GetDebugger().GetAsyncOutputStream();
- out_stream->Printf("error: %s\n", error.AsCString());
-}
+if (!GetDebugger().GetCommandInterpreter().GetBatchCommandMode())
+ GetDebugger().GetAsyncOutputStream()->Printf("error: %s\n",
+ error.AsCString());
}
}
}
diff --git a/lldb/source/Commands/CommandObjectWatchpointCommand.cpp
b/lldb/source/Commands/CommandObjectWatchpointCommand.cpp
index 507ef3fbe4759..32cb80b421fd6 100644
--- a/lldb/so
[Lldb-commits] [lldb] [lldb] Store StreamAsynchronousIO in a unique_ptr (NFC) (PR #127961)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Jonas Devlieghere (JDevlieghere)
Changes
Make StreamAsynchronousIO an unique_ptr instead of a shared_ptr. I tried
passing the class by value, but the llvm::raw_ostream forwarder stored in the
Stream parent class isn't movable and I don't think it's worth changing that.
Additionally, there's a few places that expect a StreamSP, which are easily
created from a StreamUP.
---
Patch is 20.81 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/127961.diff
14 Files Affected:
- (modified) lldb/include/lldb/Core/Debugger.h (+2-2)
- (modified) lldb/include/lldb/lldb-forward.h (+1)
- (modified) lldb/source/Breakpoint/BreakpointOptions.cpp (+2-4)
- (modified) lldb/source/Commands/CommandObjectCommands.cpp (+3-4)
- (modified) lldb/source/Commands/CommandObjectWatchpointCommand.cpp (+2-4)
- (modified) lldb/source/Core/Debugger.cpp (+38-47)
- (modified) lldb/source/Core/DynamicLoader.cpp (+1-1)
- (modified)
lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
(+3-3)
- (modified)
lldb/source/Plugins/DynamicLoader/FreeBSD-Kernel/DynamicLoaderFreeBSDKernel.cpp
(+5-5)
- (modified) lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp (+4-14)
- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+1-2)
- (modified) lldb/source/Target/Process.cpp (+6-8)
- (modified) lldb/source/Target/StopInfo.cpp (+3-5)
- (modified) lldb/source/Target/Target.cpp (+1-1)
``diff
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 9c8a9623fe689..6ebc6147800e1 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -156,9 +156,9 @@ class Debugger : public
std::enable_shared_from_this,
void RestoreInputTerminalState();
- lldb::StreamSP GetAsyncOutputStream();
+ lldb::StreamUP GetAsyncOutputStream();
- lldb::StreamSP GetAsyncErrorStream();
+ lldb::StreamUP GetAsyncErrorStream();
CommandInterpreter &GetCommandInterpreter() {
assert(m_command_interpreter_up.get());
diff --git a/lldb/include/lldb/lldb-forward.h b/lldb/include/lldb/lldb-forward.h
index cda55ef06e549..c664d1398f74d 100644
--- a/lldb/include/lldb/lldb-forward.h
+++ b/lldb/include/lldb/lldb-forward.h
@@ -432,6 +432,7 @@ typedef
std::unique_ptr
StackFrameRecognizerManagerUP;
typedef std::shared_ptr StopInfoSP;
typedef std::shared_ptr StreamSP;
+typedef std::unique_ptr StreamUP;
typedef std::shared_ptr StreamFileSP;
typedef std::shared_ptr LockableStreamFileSP;
typedef std::shared_ptr
diff --git a/lldb/source/Breakpoint/BreakpointOptions.cpp
b/lldb/source/Breakpoint/BreakpointOptions.cpp
index 09abcf5e081d2..242b5b30168c5 100644
--- a/lldb/source/Breakpoint/BreakpointOptions.cpp
+++ b/lldb/source/Breakpoint/BreakpointOptions.cpp
@@ -620,10 +620,8 @@ bool BreakpointOptions::BreakpointOptionsCallbackFunction(
// Rig up the results secondary output stream to the debugger's, so the
// output will come out synchronously if the debugger is set up that way.
- StreamSP output_stream(debugger.GetAsyncOutputStream());
- StreamSP error_stream(debugger.GetAsyncErrorStream());
- result.SetImmediateOutputStream(output_stream);
- result.SetImmediateErrorStream(error_stream);
+ result.SetImmediateOutputStream(debugger.GetAsyncOutputStream());
+ result.SetImmediateErrorStream(debugger.GetAsyncErrorStream());
CommandInterpreterRunOptions options;
options.SetStopOnContinue(true);
diff --git a/lldb/source/Commands/CommandObjectCommands.cpp
b/lldb/source/Commands/CommandObjectCommands.cpp
index dd841cb5cb4cc..9510cf4d14467 100644
--- a/lldb/source/Commands/CommandObjectCommands.cpp
+++ b/lldb/source/Commands/CommandObjectCommands.cpp
@@ -815,10 +815,9 @@ a number follows 'f':"
for (const std::string &line : lines) {
Status error = AppendRegexSubstitution(line, check_only);
if (error.Fail()) {
-if (!GetDebugger().GetCommandInterpreter().GetBatchCommandMode()) {
- StreamSP out_stream = GetDebugger().GetAsyncOutputStream();
- out_stream->Printf("error: %s\n", error.AsCString());
-}
+if (!GetDebugger().GetCommandInterpreter().GetBatchCommandMode())
+ GetDebugger().GetAsyncOutputStream()->Printf("error: %s\n",
+ error.AsCString());
}
}
}
diff --git a/lldb/source/Commands/CommandObjectWatchpointCommand.cpp
b/lldb/source/Commands/CommandObjectWatchpointCommand.cpp
index 507ef3fbe4759..32cb80b421fd6 100644
--- a/lldb/source/Commands/CommandObjectWatchpointCommand.cpp
+++ b/lldb/source/Commands/CommandObjectWatchpointCommand.cpp
@@ -252,10 +252,8 @@ are no syntax errors may indicate that a function was
declared but never called.
// Rig up the results secondary output stream to
[Lldb-commits] [lldb] [lldb] Store StreamAsynchronousIO in a unique_ptr (NFC) (PR #127961)
https://github.com/JDevlieghere edited https://github.com/llvm/llvm-project/pull/127961 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb] Extended if conditions to support alias names for registers (PR #124475)
@@ -652,6 +652,153 @@ def haltReason(self):
)
self.match("register read s31", ["s31 = 128"])
+@skipIfXmlSupportMissing
+@skipIfRemote
+@skipIfLLVMTargetMissing("RISCV")
+def test_riscv64_regs(self):
+"""Test grabbing various riscv64 registers from gdbserver."""
+
+class MyResponder(MockGDBServerResponder):
+reg_data = (
+"0102030405060708" # zero
+"0102030405060708" # ra
+"0102030405060708" # sp
+"0102030405060708" # gp
+"0102030405060708" # tp
+"0102030405060708" # t0
+"0102030405060708" # t1
+"0102030405060708" # t2
+"0102030405060708" # fp
+"0102030405060708" # s1
+"0102030405060708" # a0
+"0102030405060708" # a1
+"0102030405060708" # a2
+"0102030405060708" # a3
+"0102030405060708" # a4
+"0102030405060708" # a5
+"0102030405060708" # a6
+"0102030405060708" # a7
+"0102030405060708" # s2
+"0102030405060708" # s3
+"0102030405060708" # s4
+"0102030405060708" # s5
+"0102030405060708" # s6
+"0102030405060708" # s7
+"0102030405060708" # s8
+"0102030405060708" # s9
+"0102030405060708" # s10
+"0102030405060708" # s11
+"0102030405060708" # t3
+"0102030405060708" # t4
+"0102030405060708" # t5
+"0102030405060708" # t6
+)
+
+def qXferRead(self, obj, annex, offset, length):
+if annex == "target.xml":
+# Note that this XML does not include any aliases, LLDB
must generate them itself.
+return (
+"""
+
+
+riscv
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+""",
+False,
+)
+else:
+return None, False
+
+def readRegister(self, regnum):
+return ""
+
+def readRegisters(self):
+return self.reg_data
+
+def writeRegisters(self, reg_hex):
+self.reg_data = reg_hex
+return "OK"
+
+def haltReason(self):
+return
"T02thread:1ff0d;threads:1ff0d;thread-pcs:00010001bc00;07:0102030405060708;10:1112131415161718;"
+
+self.server.responder = MyResponder()
+
+target = self.createTarget("basic_eh_frame-riscv64.yaml")
+process = self.connect(target)
+lldbutil.expect_state_changes(
+self, self.dbg.GetListener(), process, [lldb.eStateStopped]
+)
+
+# test generic aliases
+self.match("register read x0", ["zero = 0x0807060504030201"])
+self.match("register read x1", ["ra = 0x0807060504030201"])
+self.match("register read x2", ["sp = 0x0807060504030201"])
+self.match("register read x3", ["gp = 0x0807060504030201"])
+self.match("register read x4", ["tp = 0x0807060504030201"])
+self.match("register read x5", ["t0 = 0x0807060504030201"])
+self.match("register read x6", ["t1 = 0x0807060504030201"])
+self.match("register read x7", ["t2 = 0x0807060504030201"])
+# Register x8 is probably not working because it has two aliases fp, s0
+# self.match("register read x8", ["fp = 0x0807060504030201"])
kper wrote:
https://github.com/llvm/llvm-project/issues/127900 :)
ht
[Lldb-commits] [lldb] [LLDB][Telemetry]Define DebuggerTelemetryInfo and related methods (PR #127696)
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 5ecce45ea2980aff35d1283d4dd3feb8f74de16c
11efc09e1ab9ffcca8e965906530af995f5c8c83 --extensions cpp,h --
lldb/include/lldb/Core/Telemetry.h lldb/source/Core/Debugger.cpp
lldb/source/Core/Telemetry.cpp
``
View the diff from clang-format here.
``diff
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 2e86d793c5..6c74ac2965 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -125,7 +125,7 @@ void
TelemetryManager::atDebuggerStartup(DebuggerTelemetryInfo *entry) {
misc_info.meta_data["internal_errors"] = "Cannot determine CWD";
if (llvm::Error er = dispatch(&misc_info)) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er),
- "Failed to dispatch misc-info at startup: {0}");
+ "Failed to dispatch misc-info at startup: {0}");
}
}
``
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] 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] [lldb] Synchronize the debugger's stdout and stderr streams (PR #126630)
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/126630
>From 8471301ad9dd694e1e6f3116d4e4ee0422c9b1dd Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Wed, 19 Feb 2025 14:30:29 -0800
Subject: [PATCH] [lldb] Synchronize the debugger's output & error stream
This patch improves the synchronization of the debugger's output and
error streams using two new abstractions: LockableStreamFile and
LockedStreamFile.
- LockableStreamFile is a wrapper around a StreamFile and a mutex.
Client cannot use the StreamFile without calling Lock, which returns
a LockedStreamFile.
- LockedStreamFile is an RAII object that locks the stream for the
duration of its existence. As long as you hold on to the returned
object you are permitted to write to the stream. The destruction of
the object automatically flush the output stream.
---
lldb/include/lldb/Core/Debugger.h | 24 ++--
lldb/include/lldb/Core/IOHandler.h| 33 ++---
lldb/include/lldb/Host/Editline.h | 17 ++-
lldb/include/lldb/Host/File.h | 5 +
lldb/include/lldb/Host/StreamFile.h | 52 +++
.../lldb/Interpreter/ScriptInterpreter.h | 19 ++-
lldb/include/lldb/lldb-forward.h | 2 +
.../CommandObjectBreakpointCommand.cpp| 10 +-
.../source/Commands/CommandObjectCommands.cpp | 62 +
.../Commands/CommandObjectExpression.cpp | 9 +-
lldb/source/Commands/CommandObjectTarget.cpp | 35 ++---
lldb/source/Commands/CommandObjectType.cpp| 122
.../CommandObjectWatchpointCommand.cpp| 14 +-
lldb/source/Core/Debugger.cpp | 36 +++--
lldb/source/Core/IOHandler.cpp| 91 +---
lldb/source/Core/IOHandlerCursesGUI.cpp | 4 +-
lldb/source/Expression/REPL.cpp | 10 +-
lldb/source/Host/common/Editline.cpp | 130 +++---
.../source/Interpreter/CommandInterpreter.cpp | 26 ++--
lldb/source/Interpreter/ScriptInterpreter.cpp | 13 +-
.../Lua/ScriptInterpreterLua.cpp | 24 +++-
.../Python/ScriptInterpreterPython.cpp| 35 ++---
lldb/unittests/Editline/EditlineTest.cpp | 13 +-
23 files changed, 472 insertions(+), 314 deletions(-)
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 7f08f3dd26106..9c8a9623fe689 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -131,13 +131,16 @@ class Debugger : public
std::enable_shared_from_this,
void SetAsyncExecution(bool async);
- File &GetInputFile() { return *m_input_file_sp; }
-
lldb::FileSP GetInputFileSP() { return m_input_file_sp; }
+ File &GetInputFile() { return *m_input_file_sp; }
- lldb::FileSP GetOutputFileSP() { return m_output_stream_sp->GetFileSP(); }
+ lldb::FileSP GetOutputFileSP() {
+return m_output_stream_sp->GetUnlockedFileSP();
+ }
- lldb::FileSP GetErrorFileSP() { return m_error_stream_sp->GetFileSP(); }
+ lldb::FileSP GetErrorFileSP() {
+return m_error_stream_sp->GetUnlockedFileSP();
+ }
repro::DataRecorder *GetInputRecorder();
@@ -198,8 +201,8 @@ class Debugger : public
std::enable_shared_from_this,
// If any of the streams are not set, set them to the in/out/err stream of
// the top most input reader to ensure they at least have something
void AdoptTopIOHandlerFilesIfInvalid(lldb::FileSP &in,
- lldb::StreamFileSP &out,
- lldb::StreamFileSP &err);
+ lldb::LockableStreamFileSP &out,
+ lldb::LockableStreamFileSP &err);
/// Run the given IO handler and return immediately.
void RunIOHandlerAsync(const lldb::IOHandlerSP &reader_sp,
@@ -649,8 +652,8 @@ class Debugger : public
std::enable_shared_from_this,
/// should not be used directly. Use GetAsyncOutputStream and
/// GetAsyncErrorStream instead.
/// @{
- lldb::StreamFileSP GetOutputStreamSP() { return m_output_stream_sp; }
- lldb::StreamFileSP GetErrorStreamSP() { return m_error_stream_sp; }
+ lldb::LockableStreamFileSP GetOutputStreamSP() { return m_output_stream_sp; }
+ lldb::LockableStreamFileSP GetErrorStreamSP() { return m_error_stream_sp; }
/// @}
void PushIOHandler(const lldb::IOHandlerSP &reader_sp,
@@ -693,8 +696,9 @@ class Debugger : public
std::enable_shared_from_this,
// these should never be NULL
lldb::FileSP m_input_file_sp;
- lldb::StreamFileSP m_output_stream_sp;
- lldb::StreamFileSP m_error_stream_sp;
+ lldb::LockableStreamFileSP m_output_stream_sp;
+ lldb::LockableStreamFileSP m_error_stream_sp;
+ LockableStreamFile::Mutex m_output_mutex;
/// Used for shadowing the input file when capturing a reproducer.
repro::DataRecorder *m_input_recorder;
diff --git a/lldb/include/lldb/Core/IOHandler.h
b/lldb/include/lldb/Core/IOHandler.h
index
[Lldb-commits] [lldb] [LLDB-DAP] SBDebugger don't prefix title on progress updates (PR #124648)
@@ -428,14 +453,35 @@ 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 we're only creating and sending on progress event, we send
+// the title and the detail as a single message.
+if (completed == 0 && total == 1) {
+ const std::string message =
+ GetStringFromStructuredData(data, "message");
+ dap.SendProgressEvent(progress_id, message.c_str(), completed,
total);
+} else if (completed == 0) {
+ // On the first event, send just the title which will be captured by
+ // DAP Then if details is not empty, send a second event with the
+ // detail.
+ const std::string title = GetStringFromStructuredData(data, "title");
+ dap.SendProgressEvent(progress_id, title.c_str(), completed, total);
+ if (!details.empty())
+dap.SendProgressEvent(progress_id, details.c_str(), completed,
+ total);
+} else
+ // We don't check if message has any contents, because we still want
+ // to broadcast a status update.
clayborg wrote:
Lets document what VS code expects a bit more to make it clear. Something like:
```
// This progress event is either the end of the progress dialog, or an update
// with possible detail. The "detail" string we send to VS Code will be
appended to
// the progress dialog's initial text from when it was created.
```
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)
@@ -68,10 +78,23 @@ def __call__(self, debugger, command, exe_ctx, result):
return
total = cmd_options.total
-progress = lldb.SBProgress("Progress tester", "Detail", total,
debugger)
+if total == -1:
clayborg wrote:
Default will be None if not set now, so this should be:
```
if total is None:
```
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,10 @@ 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 -1 to identify as indeterminate", +type="int", clayborg wrote: Set the default value to `None`: ``` default=None ``` 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,10 @@ 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 -1 to identify as indeterminate", clayborg wrote: Don't specify to use -1 as indeterminate, if not set, this will default to `None`, see comment below. You can say "If this option isn't specified with a valid value of 1 or higher, the progress will be indeterminate." 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)
@@ -428,14 +453,35 @@ 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 we're only creating and sending on progress event, we send
+// the title and the detail as a single message.
+if (completed == 0 && total == 1) {
+ const std::string message =
+ GetStringFromStructuredData(data, "message");
+ dap.SendProgressEvent(progress_id, message.c_str(), completed,
total);
+} else if (completed == 0) {
+ // On the first event, send just the title which will be captured by
+ // DAP Then if details is not empty, send a second event with the
+ // detail.
clayborg wrote:
Lets explain this better so any others making modifications will know what is
going on. Make it clear that the first string we send to VS code when creating
a new progress will be copied into the progress window title, and any
subsequent detail strings will get appended to the initial title we provide to
VS code.
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)
@@ -428,14 +453,35 @@ 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 we're only creating and sending on progress event, we send
+// the title and the detail as a single message.
+if (completed == 0 && total == 1) {
clayborg wrote:
restructure as:
```
if (completed == 0) {
if (total == 1) {
// This progress is non deterministic and won't get updated until it is
completed.
// Send the "message" which will be the combined title and detail. The only
other
// progress event for thus non deterministic progress will be the completed
event
// So there will be no need to update the detail.
const std::string message = ...
} else {
// This progress is deterministic and will receive updates. On the first
event...
}
}
```
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)
@@ -428,14 +453,35 @@ 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 we're only creating and sending on progress event, we send
+// the title and the detail as a single message.
+if (completed == 0 && total == 1) {
+ const std::string message =
+ GetStringFromStructuredData(data, "message");
clayborg wrote:
is the "message" value a combined string if title + detail?
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][Telemetry]Define DebuggerTelemetryInfo and related methods (PR #127696)
@@ -56,13 +60,83 @@ 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 {
JDevlieghere wrote:
My arguments for separate events is that it's much easier to spot what's being
collected: my backend is explicitly collecting events A, B and C and ignoring
everything else. If I need to filter out fields that's a lot harder to audit.
If you want to emit that as a single event in your backend, that's totally
fine, but that's the responsibility of the back-end. I think it's much more
important that the events are meaningful semantically.
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][Telemetry]Define DebuggerTelemetryInfo and related methods (PR #127696)
https://github.com/JDevlieghere edited 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] SBDebugger don't prefix title on progress updates (PR #124648)
https://github.com/Jlalond updated
https://github.com/llvm/llvm-project/pull/124648
>From 12ff645735c1dbf51e58b9f80d4e3e13a0babdf5 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Mon, 27 Jan 2025 13:41:58 -0800
Subject: [PATCH 1/6] 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 82ed76ae3b6bef176bf54dd1031f0cb9c95081c1 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Mon, 27 Jan 2025 14:48:01 -0800
Subject: [PATCH 2/6] 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 e15090782a93e07e8a260a0594ca02430e68e09f Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Thu, 30 Jan 2025 10:04:04 -0800
Subject: [PATCH 3/6] 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 +++-
lldb/tools/lldb-dap/lldb-dap.cpp| 54 +
2 files changed, 52 insertions(+), 18 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/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index e323990d8b6ed..4b3460190a7c9 100644
--- a/lldb/tools/lldb
[Lldb-commits] [lldb] [llvm] [lldb] Extended if conditions to support alias names for registers (PR #124475)
https://github.com/kper updated https://github.com/llvm/llvm-project/pull/124475
>From d44dd60bc7ae5deeed91fd9389b164e441164edc Mon Sep 17 00:00:00 2001
From: Kevin Per
Date: Sun, 26 Jan 2025 17:34:17 +
Subject: [PATCH] [lldb] Extended if conditions to support alias names for
registers
---
.../Plugins/ABI/RISCV/ABISysV_riscv.cpp | 54 ++
.../TestGDBServerTargetXML.py | 147
.../basic_eh_frame-riscv64.yaml | 20 +++
.../postmortem/elf-core/TestLinuxCore.py | 158 ++
.../Shell/Register/Inputs/riscv64-gp-read.cpp | 36
lldb/test/Shell/Register/riscv64-gp-read.test | 37
llvm/utils/lit/lit/llvm/config.py | 8 +-
7 files changed, 386 insertions(+), 74 deletions(-)
create mode 100644
lldb/test/API/functionalities/gdb_remote_client/basic_eh_frame-riscv64.yaml
create mode 100644 lldb/test/Shell/Register/Inputs/riscv64-gp-read.cpp
create mode 100644 lldb/test/Shell/Register/riscv64-gp-read.test
diff --git a/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
b/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
index 8412991933d27..c463bd006b3db 100644
--- a/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
+++ b/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
@@ -850,8 +850,62 @@ void ABISysV_riscv::AugmentRegisterInfo(
it.value().alt_name.SetCString("x3");
else if (it.value().name == "fp")
it.value().alt_name.SetCString("s0");
+else if (it.value().name == "tp")
+ it.value().alt_name.SetCString("x4");
else if (it.value().name == "s0")
it.value().alt_name.SetCString("x8");
+else if (it.value().name == "s1")
+ it.value().alt_name.SetCString("x9");
+else if (it.value().name == "t0")
+ it.value().alt_name.SetCString("x5");
+else if (it.value().name == "t1")
+ it.value().alt_name.SetCString("x6");
+else if (it.value().name == "t2")
+ it.value().alt_name.SetCString("x7");
+else if (it.value().name == "a0")
+ it.value().alt_name.SetCString("x10");
+else if (it.value().name == "a1")
+ it.value().alt_name.SetCString("x11");
+else if (it.value().name == "a2")
+ it.value().alt_name.SetCString("x12");
+else if (it.value().name == "a3")
+ it.value().alt_name.SetCString("x13");
+else if (it.value().name == "a4")
+ it.value().alt_name.SetCString("x14");
+else if (it.value().name == "a5")
+ it.value().alt_name.SetCString("x15");
+else if (it.value().name == "a6")
+ it.value().alt_name.SetCString("x16");
+else if (it.value().name == "a7")
+ it.value().alt_name.SetCString("x17");
+else if (it.value().name == "s2")
+ it.value().alt_name.SetCString("x18");
+else if (it.value().name == "s3")
+ it.value().alt_name.SetCString("x19");
+else if (it.value().name == "s4")
+ it.value().alt_name.SetCString("x20");
+else if (it.value().name == "s5")
+ it.value().alt_name.SetCString("x21");
+else if (it.value().name == "s6")
+ it.value().alt_name.SetCString("x22");
+else if (it.value().name == "s7")
+ it.value().alt_name.SetCString("x23");
+else if (it.value().name == "s8")
+ it.value().alt_name.SetCString("x24");
+else if (it.value().name == "s9")
+ it.value().alt_name.SetCString("x25");
+else if (it.value().name == "s10")
+ it.value().alt_name.SetCString("x26");
+else if (it.value().name == "s11")
+ it.value().alt_name.SetCString("x27");
+else if (it.value().name == "t3")
+ it.value().alt_name.SetCString("x28");
+else if (it.value().name == "t4")
+ it.value().alt_name.SetCString("x29");
+else if (it.value().name == "t5")
+ it.value().alt_name.SetCString("x30");
+else if (it.value().name == "t6")
+ it.value().alt_name.SetCString("x31");
// Set generic regnum so lldb knows what the PC, etc is
it.value().regnum_generic = GetGenericNum(it.value().name.GetStringRef());
diff --git
a/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
b/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
index 22f5553e40802..cf82f7f168c20 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -652,6 +652,153 @@ def haltReason(self):
)
self.match("register read s31", ["s31 = 128"])
+@skipIfXmlSupportMissing
+@skipIfRemote
+@skipIfLLVMTargetMissing("RISCV")
+def test_riscv64_regs(self):
+"""Test grabbing various riscv64 registers from gdbserver."""
+
+class MyResponder(MockGDBServerResponder):
+reg_data = (
+"0102030405060708" # zero
+"0102030405060708" # ra
+"0102030405060708" # sp
+"0102030405060708" # gp
+"0102030405060708" # tp
+"0102030405060708" # t0
[Lldb-commits] [lldb] [LLDB-DAP] SBDebugger don't prefix title on progress updates (PR #124648)
Jlalond wrote: @clayborg DAP with a detail and a total  Indeterminate with an increment detail  Then with an indeterminate progress, but no increment detail  For number 2 I think I found another bug for indeterminate messages not updating the detail correctly, but I will address this in a future patch. 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][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/3] [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] [lldb] Extended if conditions to support alias names for registers (PR #124475)
https://github.com/kper updated https://github.com/llvm/llvm-project/pull/124475
>From d44dd60bc7ae5deeed91fd9389b164e441164edc Mon Sep 17 00:00:00 2001
From: Kevin Per
Date: Sun, 26 Jan 2025 17:34:17 +
Subject: [PATCH] [lldb] Extended if conditions to support alias names for
registers
---
.../Plugins/ABI/RISCV/ABISysV_riscv.cpp | 54 ++
.../TestGDBServerTargetXML.py | 147
.../basic_eh_frame-riscv64.yaml | 20 +++
.../postmortem/elf-core/TestLinuxCore.py | 158 ++
.../Shell/Register/Inputs/riscv64-gp-read.cpp | 36
lldb/test/Shell/Register/riscv64-gp-read.test | 37
llvm/utils/lit/lit/llvm/config.py | 8 +-
7 files changed, 386 insertions(+), 74 deletions(-)
create mode 100644
lldb/test/API/functionalities/gdb_remote_client/basic_eh_frame-riscv64.yaml
create mode 100644 lldb/test/Shell/Register/Inputs/riscv64-gp-read.cpp
create mode 100644 lldb/test/Shell/Register/riscv64-gp-read.test
diff --git a/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
b/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
index 8412991933d27..c463bd006b3db 100644
--- a/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
+++ b/lldb/source/Plugins/ABI/RISCV/ABISysV_riscv.cpp
@@ -850,8 +850,62 @@ void ABISysV_riscv::AugmentRegisterInfo(
it.value().alt_name.SetCString("x3");
else if (it.value().name == "fp")
it.value().alt_name.SetCString("s0");
+else if (it.value().name == "tp")
+ it.value().alt_name.SetCString("x4");
else if (it.value().name == "s0")
it.value().alt_name.SetCString("x8");
+else if (it.value().name == "s1")
+ it.value().alt_name.SetCString("x9");
+else if (it.value().name == "t0")
+ it.value().alt_name.SetCString("x5");
+else if (it.value().name == "t1")
+ it.value().alt_name.SetCString("x6");
+else if (it.value().name == "t2")
+ it.value().alt_name.SetCString("x7");
+else if (it.value().name == "a0")
+ it.value().alt_name.SetCString("x10");
+else if (it.value().name == "a1")
+ it.value().alt_name.SetCString("x11");
+else if (it.value().name == "a2")
+ it.value().alt_name.SetCString("x12");
+else if (it.value().name == "a3")
+ it.value().alt_name.SetCString("x13");
+else if (it.value().name == "a4")
+ it.value().alt_name.SetCString("x14");
+else if (it.value().name == "a5")
+ it.value().alt_name.SetCString("x15");
+else if (it.value().name == "a6")
+ it.value().alt_name.SetCString("x16");
+else if (it.value().name == "a7")
+ it.value().alt_name.SetCString("x17");
+else if (it.value().name == "s2")
+ it.value().alt_name.SetCString("x18");
+else if (it.value().name == "s3")
+ it.value().alt_name.SetCString("x19");
+else if (it.value().name == "s4")
+ it.value().alt_name.SetCString("x20");
+else if (it.value().name == "s5")
+ it.value().alt_name.SetCString("x21");
+else if (it.value().name == "s6")
+ it.value().alt_name.SetCString("x22");
+else if (it.value().name == "s7")
+ it.value().alt_name.SetCString("x23");
+else if (it.value().name == "s8")
+ it.value().alt_name.SetCString("x24");
+else if (it.value().name == "s9")
+ it.value().alt_name.SetCString("x25");
+else if (it.value().name == "s10")
+ it.value().alt_name.SetCString("x26");
+else if (it.value().name == "s11")
+ it.value().alt_name.SetCString("x27");
+else if (it.value().name == "t3")
+ it.value().alt_name.SetCString("x28");
+else if (it.value().name == "t4")
+ it.value().alt_name.SetCString("x29");
+else if (it.value().name == "t5")
+ it.value().alt_name.SetCString("x30");
+else if (it.value().name == "t6")
+ it.value().alt_name.SetCString("x31");
// Set generic regnum so lldb knows what the PC, etc is
it.value().regnum_generic = GetGenericNum(it.value().name.GetStringRef());
diff --git
a/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
b/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
index 22f5553e40802..cf82f7f168c20 100644
--- a/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
+++ b/lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py
@@ -652,6 +652,153 @@ def haltReason(self):
)
self.match("register read s31", ["s31 = 128"])
+@skipIfXmlSupportMissing
+@skipIfRemote
+@skipIfLLVMTargetMissing("RISCV")
+def test_riscv64_regs(self):
+"""Test grabbing various riscv64 registers from gdbserver."""
+
+class MyResponder(MockGDBServerResponder):
+reg_data = (
+"0102030405060708" # zero
+"0102030405060708" # ra
+"0102030405060708" # sp
+"0102030405060708" # gp
+"0102030405060708" # tp
+"0102030405060708" # t0
[Lldb-commits] [lldb] [LLDB-DAP] SBDebugger don't prefix title on progress updates (PR #124648)
@@ -68,10 +78,23 @@ def __call__(self, debugger, command, exe_ctx, result):
return
total = cmd_options.total
-progress = lldb.SBProgress("Progress tester", "Detail", total,
debugger)
+if total == -1:
+progress = lldb.SBProgress(
+"Progress tester", "Indeterminate Detail", debugger
+)
+else:
+progress = lldb.SBProgress("Progress tester", "Detail", total,
debugger)
+
+# Check to see if total is set to -1 to indicate an indeterminate
progress
+# then default to 10 steps.
+if total == -1:
clayborg wrote:
```
if total is None:
```
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][Telemetry]Define DebuggerTelemetryInfo and related methods (PR #127696)
@@ -761,12 +767,29 @@ void Debugger::InstanceInitialize() {
DebuggerSP Debugger::CreateInstance(lldb::LogOutputCallback log_callback,
void *baton) {
+#ifdef LLVM_BUILD_TELEMETRY
+ lldb_private::telemetry::SteadyTimePoint start_time =
+ std::chrono::steady_clock::now();
+#endif
DebuggerSP debugger_sp(new Debugger(log_callback, baton));
if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
std::lock_guard guard(*g_debugger_list_mutex_ptr);
g_debugger_list_ptr->push_back(debugger_sp);
}
debugger_sp->InstanceInitialize();
+
+#ifdef LLVM_BUILD_TELEMETRY
+ if (auto *telemetry_manager = telemetry::TelemetryManager::getInstance()) {
+if (telemetry_manager->getConfig()->EnableTelemetry) {
+ lldb_private::telemetry::DebuggerTelemetryInfo entry;
+ entry.start_time = start_time;
+ entry.end_time = std::chrono::steady_clock::now();
+ entry.debugger = debugger_sp.get();
+ telemetry_manager->atDebuggerStartup(&entry);
+}
+ }
+#endif
JDevlieghere wrote:
When I introduced the CMake variable, I had something in mind like what we do
for signposts on macOS (`LLVM_SUPPORT_XCODE_SIGNPOSTS`) where we have a wrapper
and then compile out support when it's not available. Admittedly, that's not
what I did by conditionalizing the library, so that's on me and I'm happy to
rectify that.
That said, as I'm seeing how this is actually used in practice, I'm not so sure
of the benefit of compiling things out, so maybe keeping the CMake variable and
making it so that it's impossible to turn on at runtime if the flag is set, is
good enough.
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] SBDebugger don't prefix title on progress updates (PR #124648)
https://github.com/Jlalond updated
https://github.com/llvm/llvm-project/pull/124648
>From 12ff645735c1dbf51e58b9f80d4e3e13a0babdf5 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Mon, 27 Jan 2025 13:41:58 -0800
Subject: [PATCH 1/6] 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 82ed76ae3b6bef176bf54dd1031f0cb9c95081c1 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Mon, 27 Jan 2025 14:48:01 -0800
Subject: [PATCH 2/6] 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 e15090782a93e07e8a260a0594ca02430e68e09f Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Thu, 30 Jan 2025 10:04:04 -0800
Subject: [PATCH 3/6] 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 +++-
lldb/tools/lldb-dap/lldb-dap.cpp| 54 +
2 files changed, 52 insertions(+), 18 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/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index e323990d8b6ed..4b3460190a7c9 100644
--- a/lldb/tools/lldb
[Lldb-commits] [lldb] [lldb] Store the return SBValueList in the CommandReturnObject (PR #127566)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/127566 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Prepare UnwindPlans for discontinuous functions (PR #127661)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/127661 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Prepare UnwindPlans for discontinuous functions (PR #127661)
@@ -564,9 +557,8 @@ class UnwindPlan {
}
private:
- typedef std::vector collection;
- collection m_row_list;
- AddressRange m_plan_valid_address_range;
+ std::map m_rows;
labath wrote:
It won't work, since the order of entries is important. What I can imagine is
constructing an unwind plan (in lower case, i.e. not `UnwindPlan`) in a map
object, and then passing that to an UnwindPlan constructor which will flatten
it in. It would be a slightly larger change, but I think the result could be
nice as it would essentially make an UnwindPlan immutable after construction.
(I like immutable data structures, and unwind plans really must be immutable as
they are cached)
https://github.com/llvm/llvm-project/pull/127661
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Allow option to ignore module load errors in ScriptedProcess (PR #127153)
labath wrote: > It's never the case on Darwin that you know nothing about a loaded shared > library. You will always know at least its name, UUID, and load addresses. It > doesn't matter whether the binary exists on disk or not, we will always be > able to reconstruct at least a skeleton form from the loader's information. > > So thinking just about Darwin, I'd opine that you should never totally omit > information about a loaded library. If the loader tells you about it, you > should at least make its presence available even if you can't reconstruct > much information from it. > > The ELF behavior surprises me, I would have definitely though it was a bug > that the absence of the library on disk would cause the library to totally > disappear from a debug session that was clearly using it. That seems to me > like information we really should try to surface the user. > > I really haven't played around with ELF much so I'm not up on how lossy it > can be. In the case where you've deleted the library on disk is there really > NOTHING that lldb can tell you about that loaded library? Jim Reading an ELF file from memory is slightly tricky, but not impossible (I think Greg is working on making that work). I agree that showing /some/ information about the module would be better than pretending it doesn't exist, but that's not the point I was trying to make. My point was that now showing information the missing module is (MUCH) better not showing information about ANY module (even those that were found successfully). Even in the darwin case, I can reproduce the missing module by deleting the file *and* overwriting the mach header in the memory of the process. Here I am doing it on purpose, but it could happen, however unlikely, due to a bug in the debugged binary. I'm assuming that in this case, the loader information will still contain information about the library being loaded at this address, but give up on trying to load it because the magic value doesn't match. The other modules still load fine, though. https://github.com/llvm/llvm-project/pull/127153 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Store the return SBValueList in the CommandReturnObject (PR #127566)
labath wrote: > > * what about commands (I don't know if we have any) whose output consists > > of more than formatting a single value (which, I guess would be lost if the > > IDE decides to ignore the output). Are we fine with not supporting that? > > (we could say that if a command wants to print some extra output, it should > > use the error stream for that) > > In the longer term, we can also add an optional SBStructuredData for commands > that return structured data, like `image lists` or `image lookup` so the UI > can print these as tables or something else nice. So if we end up having > fancier commands that have both a ValueObject return plus extra annotation, > we could add an SBValue type to what an SBStructured data can hold, and have > it emit `before` and `after` entries for the extra text. > > I think that would be better than trying to dump some of the output to stderr. That sort of makes sense, but it doesn't sound like you're planning to do that soon, so I'm asking about what should we do until (and if) that happens. The reason I suggested stderr is because that made sense for the commands I'm thinking of. For example, the expression command can produce a result and some warnings and fixit-applied notes. I don't know if these currently go to stderr, but I think it would be okay if they did... https://github.com/llvm/llvm-project/pull/127566 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Gardening in StreamAsynchronousIO (NFC) (PR #127717)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/127717 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb] Extended if conditions to support alias names for registers (PR #124475)
@@ -148,6 +148,7 @@ def __init__(self, lit_config, config):
features.add("long_tests")
if target_triple:
+print(target_triple)
DavidSpickett wrote:
Remove this
https://github.com/llvm/llvm-project/pull/124475
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb] Extended if conditions to support alias names for registers (PR #124475)
https://github.com/DavidSpickett commented: I'm somewhat professionally restricted from getting too involved in RISC-V ,as much as I would like to see it actually work normally, just to save others hassle. But I can see this pretty obviously fixes the bug, so I don't have a problem letting this land. Someone else can get the `ninja check-lldb` experience into shape. https://github.com/llvm/llvm-project/pull/124475 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb] Extended if conditions to support alias names for registers (PR #124475)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/124475 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb] Extended if conditions to support alias names for registers (PR #124475)
@@ -652,6 +652,153 @@ def haltReason(self):
)
self.match("register read s31", ["s31 = 128"])
+@skipIfXmlSupportMissing
+@skipIfRemote
+@skipIfLLVMTargetMissing("RISCV")
+def test_riscv64_regs(self):
+"""Test grabbing various riscv64 registers from gdbserver."""
+
+class MyResponder(MockGDBServerResponder):
+reg_data = (
+"0102030405060708" # zero
+"0102030405060708" # ra
+"0102030405060708" # sp
+"0102030405060708" # gp
+"0102030405060708" # tp
+"0102030405060708" # t0
+"0102030405060708" # t1
+"0102030405060708" # t2
+"0102030405060708" # fp
+"0102030405060708" # s1
+"0102030405060708" # a0
+"0102030405060708" # a1
+"0102030405060708" # a2
+"0102030405060708" # a3
+"0102030405060708" # a4
+"0102030405060708" # a5
+"0102030405060708" # a6
+"0102030405060708" # a7
+"0102030405060708" # s2
+"0102030405060708" # s3
+"0102030405060708" # s4
+"0102030405060708" # s5
+"0102030405060708" # s6
+"0102030405060708" # s7
+"0102030405060708" # s8
+"0102030405060708" # s9
+"0102030405060708" # s10
+"0102030405060708" # s11
+"0102030405060708" # t3
+"0102030405060708" # t4
+"0102030405060708" # t5
+"0102030405060708" # t6
+)
+
+def qXferRead(self, obj, annex, offset, length):
+if annex == "target.xml":
+# Note that this XML does not include any aliases, LLDB
must generate them itself.
+return (
+"""
+
+
+riscv
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+""",
+False,
+)
+else:
+return None, False
+
+def readRegister(self, regnum):
+return ""
+
+def readRegisters(self):
+return self.reg_data
+
+def writeRegisters(self, reg_hex):
+self.reg_data = reg_hex
+return "OK"
+
+def haltReason(self):
+return
"T02thread:1ff0d;threads:1ff0d;thread-pcs:00010001bc00;07:0102030405060708;10:1112131415161718;"
+
+self.server.responder = MyResponder()
+
+target = self.createTarget("basic_eh_frame-riscv64.yaml")
+process = self.connect(target)
+lldbutil.expect_state_changes(
+self, self.dbg.GetListener(), process, [lldb.eStateStopped]
+)
+
+# test generic aliases
+self.match("register read x0", ["zero = 0x0807060504030201"])
+self.match("register read x1", ["ra = 0x0807060504030201"])
+self.match("register read x2", ["sp = 0x0807060504030201"])
+self.match("register read x3", ["gp = 0x0807060504030201"])
+self.match("register read x4", ["tp = 0x0807060504030201"])
+self.match("register read x5", ["t0 = 0x0807060504030201"])
+self.match("register read x6", ["t1 = 0x0807060504030201"])
+self.match("register read x7", ["t2 = 0x0807060504030201"])
+# Register x8 is probably not working because it has two aliases fp, s0
+# self.match("register read x8", ["fp = 0x0807060504030201"])
DavidSpickett wrote:
Please open a separate issue for this and add a
[Lldb-commits] [lldb] [llvm] [lldb] Extended if conditions to support alias names for registers (PR #124475)
@@ -0,0 +1,37 @@ +# REQUIRES: native && target-riscv64 +# RUN: %clangxx_host %p/Inputs/riscv64-gp-read.cpp -o %t +# RUN: %lldb -b -s %s %t | FileCheck %s +process launch + +register read --all +# CHECK-DAG: x0 = 0x0 +# CHECK-DAG: x1 = 0x1 +# CHECK-DAG: x2 = 0x2 +# CHECK-DAG: x3 = 0x3 +# CHECK-DAG: x4 = 0x4 +# CHECK-DAG: x5 = 0x5 +# CHECK-DAG: x6 = 0x6 +# CHECK-DAG: x7 = 0x7 +# CHECK-DAG: x9 = 0x9 +# CHECK-DAG: x10 = 0xa +# CHECK-DAG: x11 = 0xb +# CHECK-DAG: x12 = 0xc +# CHECK-DAG: x13 = 0xd +# CHECK-DAG: x14 = 0xe +# CHECK-DAG: x15 = 0xf +# CHECK-DAG: x16 = 0x10 +# CHECK-DAG: x17 = 0x11 +# CHECK-DAG: x18 = 0x12 +# CHECK-DAG: x19 = 0x13 +# CHECK-DAG: x20 = 0x14 +# CHECK-DAG: x21 = 0x15 +# CHECK-DAG: x22 = 0x16 +# CHECK-DAG: x23 = 0x17 +# CHECK-DAG: x24 = 0x18 +# CHECK-DAG: x25 = 0x19 +# CHECK-DAG: x26 = 0x1a +# CHECK-DAG: x27 = 0x1b +# CHECK-DAG: x28 = 0x1c +# CHECK-DAG: x29 = 0x1d +# CHECK-DAG: x30 = 0x1e +# CHECK-DAG: x31 = 0x1f DavidSpickett wrote: This does not check the alias names, it should. https://github.com/llvm/llvm-project/pull/124475 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb] Extended if conditions to support alias names for registers (PR #124475)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/124475 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][LoongArch] Complete register alias name in `AugmentRegisterInfo` (PR #124059)
https://github.com/DavidSpickett requested changes to this pull request. Please add a test like `lldb/test/Shell/Register/aarch64-gp-read.test` which tests the normal and alias names using a live process. Note that the main "trick" of that test is to manually insert a breakpoint instruction: ``` asm volatile( "ldp x0, x1, [%0]\n\t" <...> "brk #0\n\t" <...> ); ``` Then lldb will stop there, before the compiler restores anything. The usual way to do these tests is set each register to its number plus some offset. x0 = 1, x1 = 2, etc. One is being added for RISC-V in https://github.com/llvm/llvm-project/pull/124475, so you can use that as an example. https://github.com/llvm/llvm-project/pull/124059 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][LoongArch] Complete register alias name in `AugmentRegisterInfo` (PR #124059)
DavidSpickett wrote: Everything in the PR at the moment is fine, just need the extra test case. https://github.com/llvm/llvm-project/pull/124059 ___ 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)
@@ -56,13 +60,83 @@ 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).
labath wrote:
That makes sense to me, but it doesn't explain why we need to spell out the
copying by hand. It's very easy to forget to update it (and it looks like even
this implementation does not copy entries from the base class).
I thought this had something to do with this being being a polymorphic class
(which usually have their copy constructors deleted -- for a reason), but
AFAICT, all of the bases actually have their copy constructors (implicitly)
defined. The possibility of object slicing is another complaint I have about
this design (and I'm surprised it doesn't trigger any static analysis warnings).
In order to prevent object slicing, and avoid spelling out copy operations, how
about we do something like [this](https://godbolt.org/z/GT591beeY). Basically,
divide the two classes in the hierarchy into two kinds:
- abstract classes: these cannot be instantiated, so there's no danger of
slicing. They still have a copy constructor, but they cannot be copied on their
own.
- concrete final classes: these can be copied (using the default copy
constructor implementation). However, they cannot be inherited from, which
means they cannot slice anything.
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] [llvm] [lldb] Extended if conditions to support alias names for registers (PR #124475)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/124475 ___ 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)
@@ -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;
labath wrote:
I think it could work, but I do believe its error-prone -- and maybe also
unnecessary. I think we could avoid all this is we make the unique identifier
of a debugger a pair. The first component would be the identifier of the
TelemetryManager instance, generated when it's constructed (using random data,
current time stamp, or whatever). The second component could the debugger ID
(as retrieved by `debugger->GetID()`, which is a sequentially increasing
integer). Then we don't need to store any per-debugger data,
The second component guarantees uniqueness of the identifier within a single
process, the second does it "globally". If a particular backend doesn't like
the fact that this is a pair, it can just concatenate them.
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] [clang] [lldb] [lldb][TypeSystemClang] Initialize ClassTemplateSpecializationDecl's StrictPackMatch field (PR #126215)
mizvekov wrote: Cherry picking this onto the 20.x release branch requires https://github.com/llvm/llvm-project/pull/125791 to be cherry-picked first. https://github.com/llvm/llvm-project/pull/126215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] Reland: [clang] fix P3310 overload resolution flag propagation (PR #125791)
mizvekov wrote: /cherry-pick 08bda1cc6b0d2f1d31a89a76b5c154a11086c420 https://github.com/llvm/llvm-project/pull/125791 ___ 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)
@@ -56,13 +60,83 @@ 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 {
labath wrote:
I have many thoughts here. In no particular order, they are:
- I don't find the I/O argument particularly convincing, as I expect that any
reasonable backend will want to cache/batch sending of these entries anyway
- I can believe that for analysis purposes, it's nicer to have a single "this
is everything I know about the environment of the session" entry. I can also
understand the desire for easy filtering of unwanted data. I don't know if
there's a reasonable compromise here. Group this data into two sub-structs?
- the collect-then-filter approach makes sense for things which cannot be
collected in any other way. However, pretty much everything being collected
here is static data. There's no reason why it has to be collected at a
particular location. (Like, even if we just stopped collecting the username
here, a backend could just refetch it if it wanted to). Maybe we could not
include things that recomputed in the backend? Or at least things that are
likely to be problematic?
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] [clang] [lldb] Reland: [clang] fix P3310 overload resolution flag propagation (PR #125791)
llvmbot wrote: /pull-request llvm/llvm-project#127779 https://github.com/llvm/llvm-project/pull/125791 ___ 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)
@@ -761,12 +767,29 @@ void Debugger::InstanceInitialize() {
DebuggerSP Debugger::CreateInstance(lldb::LogOutputCallback log_callback,
void *baton) {
+#ifdef LLVM_BUILD_TELEMETRY
+ lldb_private::telemetry::SteadyTimePoint start_time =
+ std::chrono::steady_clock::now();
+#endif
DebuggerSP debugger_sp(new Debugger(log_callback, baton));
if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
std::lock_guard guard(*g_debugger_list_mutex_ptr);
g_debugger_list_ptr->push_back(debugger_sp);
}
debugger_sp->InstanceInitialize();
+
+#ifdef LLVM_BUILD_TELEMETRY
+ if (auto *telemetry_manager = telemetry::TelemetryManager::getInstance()) {
+if (telemetry_manager->getConfig()->EnableTelemetry) {
+ lldb_private::telemetry::DebuggerTelemetryInfo entry;
+ entry.start_time = start_time;
+ entry.end_time = std::chrono::steady_clock::now();
+ entry.debugger = debugger_sp.get();
+ telemetry_manager->atDebuggerStartup(&entry);
+}
+ }
+#endif
labath wrote:
I'd very much like that, but I don't know how feasible is that. However, if
that doesn't work out, I think we should still figure out a way to reduce the
number of ifdefs. The RAII object might be way to do that (by defining a noop
raii object if telemetry is disabled).
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][Telemetry]Define DebuggerTelemetryInfo and related methods (PR #127696)
@@ -56,16 +54,107 @@ void LLDBBaseTelemetryInfo::serialize(Serializer
&serializer) const {
return UUID(random_bytes).GetAsString();
}
+void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const {
+ serializer.write("entry_kind", getKind());
+ serializer.write("session_id", SessionId);
+ serializer.write("start_time", ToNanosec(start_time));
+ if (end_time.has_value())
+serializer.write("end_time", ToNanosec(end_time.value()));
+}
+
+void DebuggerTelemetryInfo::serialize(Serializer &serializer) const {
+ LLDBBaseTelemetryInfo::serialize(serializer);
+
+ serializer.write("username", username);
+ serializer.write("lldb_git_sha", lldb_git_sha);
+ serializer.write("lldb_path", lldb_path);
+ serializer.write("cwd", cwd);
+ if (exit_desc.has_value()) {
+serializer.write("exit_code", exit_desc->exit_code);
+serializer.write("exit_desc", exit_desc->description);
+ }
+}
+
+void MiscTelemetryInfo::serialize(Serializer &serializer) const {
+ LLDBBaseTelemetryInfo::serialize(serializer);
+ serializer.write("target_uuid", target_uuid);
+ serializer.beginObject("meta_data");
+ for (const auto &kv : meta_data)
+serializer.write(kv.first, kv.second);
+ serializer.endObject();
+}
+
TelemetryManager::TelemetryManager(std::unique_ptr config)
: m_config(std::move(config)) {}
llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
- // Do nothing for now.
- // In up-coming patch, this would be where the manager
- // attach the session_uuid to the entry.
+ LLDBBaseTelemetryInfo *lldb_entry =
+ llvm::dyn_cast(entry);
+ std::string session_id = "";
+ if (Debugger *debugger = lldb_entry->debugger) {
+auto session_id_pos = session_ids.find(debugger);
+if (session_id_pos != session_ids.end())
+ session_id = session_id_pos->second;
+else
+ session_id_pos->second = session_id = MakeUUID(debugger);
+ }
+ lldb_entry->SessionId = session_id;
+
return llvm::Error::success();
}
+const Config *getConfig() { return m_config.get(); }
+
+void TelemetryManager::atDebuggerStartup(DebuggerTelemetryInfo *entry) {
+ UserIDResolver &resolver = lldb_private::HostInfo::GetUserIDResolver();
+ std::optional opt_username =
+ resolver.GetUserName(lldb_private::HostInfo::GetUserID());
+ if (opt_username)
+entry->username = *opt_username;
+
+ entry->lldb_git_sha =
+ lldb_private::GetVersion(); // TODO: find the real git sha?
+
+ entry->lldb_path = HostInfo::GetProgramFileSpec().GetPath();
+
+ llvm::SmallString<64> cwd;
+ if (!llvm::sys::fs::current_path(cwd)) {
+entry->cwd = cwd.c_str();
+ } else {
+MiscTelemetryInfo misc_info;
+misc_info.meta_data["internal_errors"] = "Cannot determine CWD";
+if (auto er = dispatch(&misc_info)) {
+ LLDB_LOG(GetLog(LLDBLog::Object),
+ "Failed to dispatch misc-info at startup");
labath wrote:
llvm::Error needs to be consumed (and it's better to make it's spell out its
type, so that one can see that).
```suggestion
if (llvm::Error er = dispatch(&misc_info)) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er),
"Failed to dispatch misc-info at startup: {0}");
```
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][Telemetry]Define DebuggerTelemetryInfo and related methods (PR #127696)
@@ -56,16 +54,107 @@ void LLDBBaseTelemetryInfo::serialize(Serializer
&serializer) const {
return UUID(random_bytes).GetAsString();
}
+void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const {
+ serializer.write("entry_kind", getKind());
+ serializer.write("session_id", SessionId);
+ serializer.write("start_time", ToNanosec(start_time));
+ if (end_time.has_value())
+serializer.write("end_time", ToNanosec(end_time.value()));
+}
+
+void DebuggerTelemetryInfo::serialize(Serializer &serializer) const {
+ LLDBBaseTelemetryInfo::serialize(serializer);
+
+ serializer.write("username", username);
+ serializer.write("lldb_git_sha", lldb_git_sha);
+ serializer.write("lldb_path", lldb_path);
+ serializer.write("cwd", cwd);
+ if (exit_desc.has_value()) {
+serializer.write("exit_code", exit_desc->exit_code);
+serializer.write("exit_desc", exit_desc->description);
+ }
+}
+
+void MiscTelemetryInfo::serialize(Serializer &serializer) const {
+ LLDBBaseTelemetryInfo::serialize(serializer);
+ serializer.write("target_uuid", target_uuid);
+ serializer.beginObject("meta_data");
+ for (const auto &kv : meta_data)
+serializer.write(kv.first, kv.second);
+ serializer.endObject();
+}
+
TelemetryManager::TelemetryManager(std::unique_ptr config)
: m_config(std::move(config)) {}
llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
- // Do nothing for now.
- // In up-coming patch, this would be where the manager
- // attach the session_uuid to the entry.
+ LLDBBaseTelemetryInfo *lldb_entry =
+ llvm::dyn_cast(entry);
+ std::string session_id = "";
+ if (Debugger *debugger = lldb_entry->debugger) {
+auto session_id_pos = session_ids.find(debugger);
+if (session_id_pos != session_ids.end())
+ session_id = session_id_pos->second;
+else
+ session_id_pos->second = session_id = MakeUUID(debugger);
+ }
+ lldb_entry->SessionId = session_id;
+
return llvm::Error::success();
}
+const Config *getConfig() { return m_config.get(); }
+
+void TelemetryManager::atDebuggerStartup(DebuggerTelemetryInfo *entry) {
+ UserIDResolver &resolver = lldb_private::HostInfo::GetUserIDResolver();
+ std::optional opt_username =
+ resolver.GetUserName(lldb_private::HostInfo::GetUserID());
+ if (opt_username)
+entry->username = *opt_username;
+
+ entry->lldb_git_sha =
+ lldb_private::GetVersion(); // TODO: find the real git sha?
+
+ entry->lldb_path = HostInfo::GetProgramFileSpec().GetPath();
+
+ llvm::SmallString<64> cwd;
+ if (!llvm::sys::fs::current_path(cwd)) {
+entry->cwd = cwd.c_str();
+ } else {
+MiscTelemetryInfo misc_info;
+misc_info.meta_data["internal_errors"] = "Cannot determine CWD";
+if (auto er = dispatch(&misc_info)) {
+ LLDB_LOG(GetLog(LLDBLog::Object),
+ "Failed to dispatch misc-info at startup");
+}
+ }
+
+ if (auto er = dispatch(entry)) {
labath wrote:
same here
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][Telemetry]Define DebuggerTelemetryInfo and related methods (PR #127696)
@@ -56,16 +54,107 @@ void LLDBBaseTelemetryInfo::serialize(Serializer
&serializer) const {
return UUID(random_bytes).GetAsString();
}
+void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const {
+ serializer.write("entry_kind", getKind());
+ serializer.write("session_id", SessionId);
+ serializer.write("start_time", ToNanosec(start_time));
+ if (end_time.has_value())
+serializer.write("end_time", ToNanosec(end_time.value()));
+}
+
+void DebuggerTelemetryInfo::serialize(Serializer &serializer) const {
+ LLDBBaseTelemetryInfo::serialize(serializer);
+
+ serializer.write("username", username);
+ serializer.write("lldb_git_sha", lldb_git_sha);
+ serializer.write("lldb_path", lldb_path);
+ serializer.write("cwd", cwd);
+ if (exit_desc.has_value()) {
+serializer.write("exit_code", exit_desc->exit_code);
+serializer.write("exit_desc", exit_desc->description);
+ }
+}
+
+void MiscTelemetryInfo::serialize(Serializer &serializer) const {
+ LLDBBaseTelemetryInfo::serialize(serializer);
+ serializer.write("target_uuid", target_uuid);
+ serializer.beginObject("meta_data");
+ for (const auto &kv : meta_data)
+serializer.write(kv.first, kv.second);
+ serializer.endObject();
+}
+
TelemetryManager::TelemetryManager(std::unique_ptr config)
: m_config(std::move(config)) {}
llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
- // Do nothing for now.
- // In up-coming patch, this would be where the manager
- // attach the session_uuid to the entry.
+ LLDBBaseTelemetryInfo *lldb_entry =
+ llvm::dyn_cast(entry);
+ std::string session_id = "";
+ if (Debugger *debugger = lldb_entry->debugger) {
+auto session_id_pos = session_ids.find(debugger);
+if (session_id_pos != session_ids.end())
+ session_id = session_id_pos->second;
+else
+ session_id_pos->second = session_id = MakeUUID(debugger);
+ }
+ lldb_entry->SessionId = session_id;
+
return llvm::Error::success();
}
+const Config *getConfig() { return m_config.get(); }
+
+void TelemetryManager::atDebuggerStartup(DebuggerTelemetryInfo *entry) {
+ UserIDResolver &resolver = lldb_private::HostInfo::GetUserIDResolver();
+ std::optional opt_username =
+ resolver.GetUserName(lldb_private::HostInfo::GetUserID());
+ if (opt_username)
+entry->username = *opt_username;
+
+ entry->lldb_git_sha =
+ lldb_private::GetVersion(); // TODO: find the real git sha?
+
+ entry->lldb_path = HostInfo::GetProgramFileSpec().GetPath();
+
+ llvm::SmallString<64> cwd;
+ if (!llvm::sys::fs::current_path(cwd)) {
+entry->cwd = cwd.c_str();
+ } else {
+MiscTelemetryInfo misc_info;
+misc_info.meta_data["internal_errors"] = "Cannot determine CWD";
+if (auto er = dispatch(&misc_info)) {
+ LLDB_LOG(GetLog(LLDBLog::Object),
+ "Failed to dispatch misc-info at startup");
+}
+ }
+
+ if (auto er = dispatch(entry)) {
+LLDB_LOG(GetLog(LLDBLog::Object), "Failed to dispatch entry at startup");
+ }
+}
+
+void TelemetryManager::atDebuggerExit(DebuggerTelemetryInfo *entry) {
+ // There must be a reference to the debugger at this point.
+ assert(entry->debugger != nullptr);
+
+ if (auto *selected_target =
+ entry->debugger->GetSelectedExecutionContext().GetTargetPtr()) {
+if (!selected_target->IsDummyTarget()) {
+ const lldb::ProcessSP proc = selected_target->GetProcessSP();
+ if (proc == nullptr) {
+// no process has been launched yet.
+entry->exit_desc = {-1, "no process launched."};
+ } else {
+entry->exit_desc = {proc->GetExitStatus(), ""};
+if (const char *description = proc->GetExitDescription())
+ entry->exit_desc->description = std::string(description);
+ }
+}
+ }
labath wrote:
This is a fairly strange place to collect this kind of information. There's no
guarantee that Process object will exist at the time the debugger terminates,
or that it will have ran exactly once, etc.
If you're interesting in collecting the exit status, why not hook into a place
that deals with that directly (somewhere around Process::SetExitStatus)?
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][Telemetry]Define DebuggerTelemetryInfo and related methods (PR #127696)
@@ -56,16 +54,107 @@ void LLDBBaseTelemetryInfo::serialize(Serializer
&serializer) const {
return UUID(random_bytes).GetAsString();
}
+void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const {
+ serializer.write("entry_kind", getKind());
+ serializer.write("session_id", SessionId);
+ serializer.write("start_time", ToNanosec(start_time));
+ if (end_time.has_value())
+serializer.write("end_time", ToNanosec(end_time.value()));
+}
+
+void DebuggerTelemetryInfo::serialize(Serializer &serializer) const {
+ LLDBBaseTelemetryInfo::serialize(serializer);
+
+ serializer.write("username", username);
+ serializer.write("lldb_git_sha", lldb_git_sha);
+ serializer.write("lldb_path", lldb_path);
+ serializer.write("cwd", cwd);
+ if (exit_desc.has_value()) {
+serializer.write("exit_code", exit_desc->exit_code);
+serializer.write("exit_desc", exit_desc->description);
+ }
+}
+
+void MiscTelemetryInfo::serialize(Serializer &serializer) const {
+ LLDBBaseTelemetryInfo::serialize(serializer);
+ serializer.write("target_uuid", target_uuid);
+ serializer.beginObject("meta_data");
+ for (const auto &kv : meta_data)
+serializer.write(kv.first, kv.second);
+ serializer.endObject();
+}
+
TelemetryManager::TelemetryManager(std::unique_ptr config)
: m_config(std::move(config)) {}
llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
- // Do nothing for now.
- // In up-coming patch, this would be where the manager
- // attach the session_uuid to the entry.
+ LLDBBaseTelemetryInfo *lldb_entry =
+ llvm::dyn_cast(entry);
+ std::string session_id = "";
+ if (Debugger *debugger = lldb_entry->debugger) {
+auto session_id_pos = session_ids.find(debugger);
+if (session_id_pos != session_ids.end())
+ session_id = session_id_pos->second;
+else
+ session_id_pos->second = session_id = MakeUUID(debugger);
+ }
+ lldb_entry->SessionId = session_id;
+
return llvm::Error::success();
}
+const Config *getConfig() { return m_config.get(); }
+
+void TelemetryManager::atDebuggerStartup(DebuggerTelemetryInfo *entry) {
+ UserIDResolver &resolver = lldb_private::HostInfo::GetUserIDResolver();
+ std::optional opt_username =
+ resolver.GetUserName(lldb_private::HostInfo::GetUserID());
+ if (opt_username)
+entry->username = *opt_username;
+
+ entry->lldb_git_sha =
+ lldb_private::GetVersion(); // TODO: find the real git sha?
+
+ entry->lldb_path = HostInfo::GetProgramFileSpec().GetPath();
+
+ llvm::SmallString<64> cwd;
+ if (!llvm::sys::fs::current_path(cwd)) {
+entry->cwd = cwd.c_str();
+ } else {
+MiscTelemetryInfo misc_info;
+misc_info.meta_data["internal_errors"] = "Cannot determine CWD";
+if (auto er = dispatch(&misc_info)) {
labath wrote:
Is this actually useful? All it really does is tell you that determining CWD
failed, but you could also find that out by looking at whether the `cwd` field
is empty.
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][Telemetry]Define DebuggerTelemetryInfo and related methods (PR #127696)
@@ -56,16 +54,107 @@ void LLDBBaseTelemetryInfo::serialize(Serializer
&serializer) const {
return UUID(random_bytes).GetAsString();
}
+void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const {
+ serializer.write("entry_kind", getKind());
+ serializer.write("session_id", SessionId);
+ serializer.write("start_time", ToNanosec(start_time));
+ if (end_time.has_value())
+serializer.write("end_time", ToNanosec(end_time.value()));
+}
+
+void DebuggerTelemetryInfo::serialize(Serializer &serializer) const {
+ LLDBBaseTelemetryInfo::serialize(serializer);
+
+ serializer.write("username", username);
+ serializer.write("lldb_git_sha", lldb_git_sha);
+ serializer.write("lldb_path", lldb_path);
+ serializer.write("cwd", cwd);
+ if (exit_desc.has_value()) {
+serializer.write("exit_code", exit_desc->exit_code);
+serializer.write("exit_desc", exit_desc->description);
+ }
+}
+
+void MiscTelemetryInfo::serialize(Serializer &serializer) const {
+ LLDBBaseTelemetryInfo::serialize(serializer);
+ serializer.write("target_uuid", target_uuid);
+ serializer.beginObject("meta_data");
+ for (const auto &kv : meta_data)
+serializer.write(kv.first, kv.second);
+ serializer.endObject();
+}
+
TelemetryManager::TelemetryManager(std::unique_ptr config)
: m_config(std::move(config)) {}
llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
- // Do nothing for now.
- // In up-coming patch, this would be where the manager
- // attach the session_uuid to the entry.
+ LLDBBaseTelemetryInfo *lldb_entry =
+ llvm::dyn_cast(entry);
+ std::string session_id = "";
+ if (Debugger *debugger = lldb_entry->debugger) {
+auto session_id_pos = session_ids.find(debugger);
+if (session_id_pos != session_ids.end())
+ session_id = session_id_pos->second;
+else
+ session_id_pos->second = session_id = MakeUUID(debugger);
+ }
+ lldb_entry->SessionId = session_id;
+
return llvm::Error::success();
}
+const Config *getConfig() { return m_config.get(); }
+
+void TelemetryManager::atDebuggerStartup(DebuggerTelemetryInfo *entry) {
+ UserIDResolver &resolver = lldb_private::HostInfo::GetUserIDResolver();
labath wrote:
(This is somewhat related to
)
None of the information collected here is actually specific to the debugger
object being created. If some tool embedding lldb creates 10 debugger objects,
they will all send an entry with the same data.
It might be that's fine. I'm just saying that another way to organize this
would be to have something like a "process startup" entry which reports this
data, and then another one which gets sent when the debugger is created (and
which only reports the time that took?)
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] [clang] [lldb] [lldb][TypeSystemClang] Initialize ClassTemplateSpecializationDecl's StrictPackMatch field (PR #126215)
labath wrote: > Cherry picking this onto the 20.x release branch requires #125791 to be > cherry-picked first. But this is just a bugfix for that PR, right? (IOW, if there's no reason to cherry-pick that PR, then there's also no reason cherry-pick this fix) https://github.com/llvm/llvm-project/pull/126215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix RangeDataVector::CombineConsecutiveEntriesWithEqualData (PR #127059)
@@ -493,36 +493,27 @@ class RangeDataVector {
#ifdef ASSERT_RANGEMAP_ARE_SORTED
assert(IsSorted());
#endif
-typename Collection::iterator pos;
-typename Collection::iterator end;
-typename Collection::iterator prev;
-bool can_combine = false;
-// First we determine if we can combine any of the Entry objects so we
-// don't end up allocating and making a new collection for no reason
-for (pos = m_entries.begin(), end = m_entries.end(), prev = end; pos !=
end;
- prev = pos++) {
- if (prev != end && prev->data == pos->data) {
-can_combine = true;
-break;
- }
-}
+auto first_intersect = std::adjacent_find(
+m_entries.begin(), m_entries.end(), [](const Entry &a, const Entry &b)
{
+ return a.DoesAdjoinOrIntersect(b) && a.data == b.data;
labath wrote:
Both of them are integer comparisons (in practice -- technically, this is a
template so it could be whatever), so it really comes down to "which one is
more likely to be false". I'm not sure about that, but I doubt this code is hot
enough for it to matter. If we wanted to optimize this we could change the
`DoesAdjoinOrIntersect` call to `b.GetRangeBase() <= a.GetRangeEnd()` (since
the other check inside that function is guaranteed to be true due to sorting).
https://github.com/llvm/llvm-project/pull/127059
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix RangeDataVector::CombineConsecutiveEntriesWithEqualData (PR #127059)
@@ -493,36 +493,27 @@ class RangeDataVector {
#ifdef ASSERT_RANGEMAP_ARE_SORTED
assert(IsSorted());
#endif
-typename Collection::iterator pos;
-typename Collection::iterator end;
-typename Collection::iterator prev;
-bool can_combine = false;
-// First we determine if we can combine any of the Entry objects so we
-// don't end up allocating and making a new collection for no reason
-for (pos = m_entries.begin(), end = m_entries.end(), prev = end; pos !=
end;
- prev = pos++) {
- if (prev != end && prev->data == pos->data) {
-can_combine = true;
-break;
- }
-}
+auto first_intersect = std::adjacent_find(
+m_entries.begin(), m_entries.end(), [](const Entry &a, const Entry &b)
{
+ return a.DoesAdjoinOrIntersect(b) && a.data == b.data;
+});
labath wrote:
works for me.
https://github.com/llvm/llvm-project/pull/127059
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix RangeDataVector::CombineConsecutiveEntriesWithEqualData (PR #127059)
https://github.com/labath updated
https://github.com/llvm/llvm-project/pull/127059
>From 450371c5fd3ecce3ec54237379fef751fba45751 Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Thu, 13 Feb 2025 14:08:25 +0100
Subject: [PATCH 1/2] [lldb] Fix
RangeDataVector::CombineConsecutiveEntriesWithEqualData
Function was merging equal data even if they weren't adjecant. This
caused a problem in command-disassemble.s test because the two ranges
describing the function would be merged and "swallow" the function
between them.
This PR copies/adapts the algorithm from
RangeVector::CombineConsecutiveEntries (which does not have the same
problem) and also adds a call to ComputeUpperBounds as moving entries
around invalidates the binary tree. (The lack of this call wasn't
noticed until now either because we were not calling methods which rely
on upper bounds (right now, it's only the ill-named FindEntryIndexes
method), or because we weren't merging anything.
---
lldb/include/lldb/Utility/RangeMap.h | 47 ---
.../test/Shell/Commands/command-disassemble.s | 3 +-
lldb/unittests/Utility/RangeMapTest.cpp | 21 +
3 files changed, 41 insertions(+), 30 deletions(-)
diff --git a/lldb/include/lldb/Utility/RangeMap.h
b/lldb/include/lldb/Utility/RangeMap.h
index 433466eebced8..2bebe74cc4cfe 100644
--- a/lldb/include/lldb/Utility/RangeMap.h
+++ b/lldb/include/lldb/Utility/RangeMap.h
@@ -493,36 +493,27 @@ class RangeDataVector {
#ifdef ASSERT_RANGEMAP_ARE_SORTED
assert(IsSorted());
#endif
-typename Collection::iterator pos;
-typename Collection::iterator end;
-typename Collection::iterator prev;
-bool can_combine = false;
-// First we determine if we can combine any of the Entry objects so we
-// don't end up allocating and making a new collection for no reason
-for (pos = m_entries.begin(), end = m_entries.end(), prev = end; pos !=
end;
- prev = pos++) {
- if (prev != end && prev->data == pos->data) {
-can_combine = true;
-break;
- }
-}
+auto first_intersect = std::adjacent_find(
+m_entries.begin(), m_entries.end(), [](const Entry &a, const Entry &b)
{
+ return a.DoesAdjoinOrIntersect(b) && a.data == b.data;
+});
+if (first_intersect == m_entries.end())
+ return;
-// We can combine at least one entry, then we make a new collection and
-// populate it accordingly, and then swap it into place.
-if (can_combine) {
- Collection minimal_ranges;
- for (pos = m_entries.begin(), end = m_entries.end(), prev = end;
- pos != end; prev = pos++) {
-if (prev != end && prev->data == pos->data)
- minimal_ranges.back().SetRangeEnd(pos->GetRangeEnd());
-else
- minimal_ranges.push_back(*pos);
- }
- // Use the swap technique in case our new vector is much smaller. We must
- // swap when using the STL because std::vector objects never release or
- // reduce the memory once it has been allocated/reserved.
- m_entries.swap(minimal_ranges);
+// We can combine at least one entry. Make a new collection and populate it
+// accordingly, and then swap it into place.
+auto pos = std::next(first_intersect);
+Collection minimal_ranges(m_entries.begin(), pos);
+for (; pos != m_entries.end(); ++pos) {
+ Entry &back = minimal_ranges.back();
+ if (back.DoesAdjoinOrIntersect(*pos) && back.data == pos->data)
+back.SetRangeEnd(std::max(back.GetRangeEnd(), pos->GetRangeEnd()));
+ else
+minimal_ranges.push_back(*pos);
}
+m_entries.swap(minimal_ranges);
+if (!m_entries.empty())
+ ComputeUpperBounds(0, m_entries.size());
}
void Clear() { m_entries.clear(); }
diff --git a/lldb/test/Shell/Commands/command-disassemble.s
b/lldb/test/Shell/Commands/command-disassemble.s
index eb84a9ce39d4a..14f416d221231 100644
--- a/lldb/test/Shell/Commands/command-disassemble.s
+++ b/lldb/test/Shell/Commands/command-disassemble.s
@@ -94,8 +94,7 @@
# CHECK-EMPTY:
# CHECK-NEXT: command-disassemble.s.tmp`n2::case3:
# CHECK-NEXT: command-disassemble.s.tmp[0x9046] <+0>: jmp 0x6046 ; <-12288>
-## FIXME: This should resolve to `middle_of_case3`
-# CHECK-NEXT: command-disassemble.s.tmp[0x904b] <+5>: jmp 0x7046 ; n2::case3 -
8192
+# CHECK-NEXT: command-disassemble.s.tmp[0x904b] <+5>: jmp 0x7046 ;
middle_of_case3
# CHECK-NEXT: command-disassemble.s.tmp[0x9050] <+10>: int$0x2a
# CHECK-EMPTY:
# CHECK-NEXT: command-disassemble.s.tmp`n1::case3:
diff --git a/lldb/unittests/Utility/RangeMapTest.cpp
b/lldb/unittests/Utility/RangeMapTest.cpp
index 981fa2a7d1c34..2022a2374fb8d 100644
--- a/lldb/unittests/Utility/RangeMapTest.cpp
+++ b/lldb/unittests/Utility/RangeMapTest.cpp
@@ -238,3 +238,24 @@ TEST(RangeDataVector, FindEntryIndexesThatContain_Overlap)
{
EXPECT_THAT(FindEntryIndexes(39, Map), testing::ElementsAre(10));
EXPECT_THAT(FindEntryIndexes(40, Map), testing::El
[Lldb-commits] [lldb] [lldb] Fix RangeDataVector::CombineConsecutiveEntriesWithEqualData (PR #127059)
@@ -493,36 +493,27 @@ class RangeDataVector {
#ifdef ASSERT_RANGEMAP_ARE_SORTED
assert(IsSorted());
#endif
-typename Collection::iterator pos;
-typename Collection::iterator end;
-typename Collection::iterator prev;
-bool can_combine = false;
-// First we determine if we can combine any of the Entry objects so we
-// don't end up allocating and making a new collection for no reason
-for (pos = m_entries.begin(), end = m_entries.end(), prev = end; pos !=
end;
- prev = pos++) {
- if (prev != end && prev->data == pos->data) {
-can_combine = true;
-break;
- }
-}
+auto first_intersect = std::adjacent_find(
+m_entries.begin(), m_entries.end(), [](const Entry &a, const Entry &b)
{
+ return a.DoesAdjoinOrIntersect(b) && a.data == b.data;
+});
+if (first_intersect == m_entries.end())
+ return;
-// We can combine at least one entry, then we make a new collection and
-// populate it accordingly, and then swap it into place.
-if (can_combine) {
- Collection minimal_ranges;
- for (pos = m_entries.begin(), end = m_entries.end(), prev = end;
- pos != end; prev = pos++) {
-if (prev != end && prev->data == pos->data)
- minimal_ranges.back().SetRangeEnd(pos->GetRangeEnd());
-else
- minimal_ranges.push_back(*pos);
- }
- // Use the swap technique in case our new vector is much smaller. We must
- // swap when using the STL because std::vector objects never release or
- // reduce the memory once it has been allocated/reserved.
- m_entries.swap(minimal_ranges);
+// We can combine at least one entry. Make a new collection and populate it
+// accordingly, and then swap it into place.
+auto pos = std::next(first_intersect);
+Collection minimal_ranges(m_entries.begin(), pos);
+for (; pos != m_entries.end(); ++pos) {
+ Entry &back = minimal_ranges.back();
+ if (back.DoesAdjoinOrIntersect(*pos) && back.data == pos->data)
+back.SetRangeEnd(std::max(back.GetRangeEnd(), pos->GetRangeEnd()));
+ else
+minimal_ranges.push_back(*pos);
}
+m_entries.swap(minimal_ranges);
+if (!m_entries.empty())
labath wrote:
Good catch. I am usually all about deleting these.
I was actually thinking about follow this up with a patch to move the check
inside the function, as it's currently repeated at all call sites (including
the recursive ones).
https://github.com/llvm/llvm-project/pull/127059
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Renaissance LineTable sequences (PR #127800)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
Changes
LineSeqeunce is basically a vector, except that users aren't supposed to know
that. This implements the same concept in a slightly simpler fashion.
---
Patch is 21.79 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/127800.diff
7 Files Affected:
- (modified) lldb/include/lldb/Symbol/LineTable.h (+38-51)
- (modified) lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
(+5-7)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+3-4)
- (modified) lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
(+5-6)
- (modified) lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp (+12-14)
- (modified) lldb/source/Symbol/LineTable.cpp (+25-49)
- (modified) lldb/unittests/Symbol/LineTableTest.cpp (+7-12)
``diff
diff --git a/lldb/include/lldb/Symbol/LineTable.h
b/lldb/include/lldb/Symbol/LineTable.h
index f66081b6ee110..2ce5d8cee6e95 100644
--- a/lldb/include/lldb/Symbol/LineTable.h
+++ b/lldb/include/lldb/Symbol/LineTable.h
@@ -20,25 +20,11 @@
namespace lldb_private {
-/// \class LineSequence LineTable.h "lldb/Symbol/LineTable.h" An abstract base
-/// class used during symbol table creation.
-class LineSequence {
-public:
- LineSequence();
-
- virtual ~LineSequence() = default;
-
- virtual void Clear() = 0;
-
-private:
- LineSequence(const LineSequence &) = delete;
- const LineSequence &operator=(const LineSequence &) = delete;
-};
-
/// \class LineTable LineTable.h "lldb/Symbol/LineTable.h"
/// A line table class.
class LineTable {
public:
+ class Sequence;
/// Construct with compile unit.
///
/// \param[in] comp_unit
@@ -49,8 +35,7 @@ class LineTable {
///
/// \param[in] sequences
/// Unsorted list of line sequences.
- LineTable(CompileUnit *comp_unit,
-std::vector> &&sequences);
+ LineTable(CompileUnit *comp_unit, std::vector &&sequences);
/// Destructor.
~LineTable();
@@ -73,20 +58,17 @@ class LineTable {
bool is_start_of_basic_block, bool is_prologue_end,
bool is_epilogue_begin, bool is_terminal_entry);
- // Used to instantiate the LineSequence helper class
- static std::unique_ptr CreateLineSequenceContainer();
-
// Append an entry to a caller-provided collection that will later be
// inserted in this line table.
- static void AppendLineEntryToSequence(LineSequence *sequence, lldb::addr_t
file_addr,
- uint32_t line, uint16_t column,
- uint16_t file_idx, bool is_start_of_statement,
- bool is_start_of_basic_block,
- bool is_prologue_end, bool is_epilogue_begin,
- bool is_terminal_entry);
+ static void
+ AppendLineEntryToSequence(Sequence &sequence, lldb::addr_t file_addr,
+uint32_t line, uint16_t column, uint16_t file_idx,
+bool is_start_of_statement,
+bool is_start_of_basic_block, bool is_prologue_end,
+bool is_epilogue_begin, bool is_terminal_entry);
// Insert a sequence of entries into this line table.
- void InsertSequence(LineSequence *sequence);
+ void InsertSequence(Sequence sequence);
/// Dump all line entries in this line table to the stream \a s.
///
@@ -273,17 +255,6 @@ class LineTable {
return 0;
}
-class LessThanBinaryPredicate {
-public:
- LessThanBinaryPredicate(LineTable *line_table);
- bool operator()(const LineTable::Entry &, const LineTable::Entry &)
const;
- bool operator()(const std::unique_ptr &,
- const std::unique_ptr &) const;
-
-protected:
- LineTable *m_line_table;
-};
-
static bool EntryAddressLessThan(const Entry &lhs, const Entry &rhs) {
return lhs.file_addr < rhs.file_addr;
}
@@ -315,6 +286,35 @@ class LineTable {
uint16_t file_idx = 0;
};
+ class Sequence {
+ public:
+Sequence() = default;
+// Moving clears moved-from object so it can be used anew. Copying is
+// generally an error. C++ doesn't guarantee that a moved-from vector is
+// empty(), so we clear it explicitly.
+Sequence(Sequence &&rhs) : m_entries(std::exchange(rhs.m_entries, {})) {}
+Sequence &operator=(Sequence &&rhs) {
+ m_entries = std::exchange(rhs.m_entries, {});
+ return *this;
+}
+Sequence(const Sequence &) = delete;
+Sequence &operator=(const Sequence &) = delete;
+
+ private:
+std::vector m_entries;
+friend class LineTable;
+ };
+
+ class LessThanBinaryPredicate {
+ public:
+LessThanBinaryPredicate(LineTable *line_table) : m_line_table(line_table)
{}
+bool operator()(const LineTable::Entry &, const LineTable::Entry &) const;
+bool operator()(const Seq
[Lldb-commits] [clang] [lldb] [lldb][TypeSystemClang] Initialize ClassTemplateSpecializationDecl's StrictPackMatch field (PR #126215)
labath wrote: Okay, nevermind me then. :) Thanks for the explanation. https://github.com/llvm/llvm-project/pull/126215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Synchronize the debugger's stdout and stderr streams (PR #126630)
https://github.com/labath approved this pull request. LGTM. Thanks for bearing with me. https://github.com/llvm/llvm-project/pull/126630 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [lldb][TypeSystemClang] Initialize ClassTemplateSpecializationDecl's StrictPackMatch field (PR #126215)
mizvekov wrote: But there is reason to cherry-pick https://github.com/llvm/llvm-project/pull/125791, as it fixes a regression which would otherwise be introduced in 20.x. https://github.com/llvm/llvm-project/pull/126215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Renaissance LineTable sequences (PR #127800)
https://github.com/labath created
https://github.com/llvm/llvm-project/pull/127800
LineSeqeunce is basically a vector, except that users aren't supposed to know
that. This implements the same concept in a slightly simpler fashion.
>From b82a233736efc391aaa408513cbea257871ab7b5 Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Wed, 19 Feb 2025 15:03:40 +0100
Subject: [PATCH] [lldb] Renaissance LineTable sequences
LineSeqeunce is basically a vector, except that users aren't supposed to
know that. This implements the same concept in a slightly simpler
fashion.
---
lldb/include/lldb/Symbol/LineTable.h | 89 ---
.../Breakpad/SymbolFileBreakpad.cpp | 12 ++-
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 7 +-
.../NativePDB/SymbolFileNativePDB.cpp | 11 ++-
.../Plugins/SymbolFile/PDB/SymbolFilePDB.cpp | 26 +++---
lldb/source/Symbol/LineTable.cpp | 74 ++-
lldb/unittests/Symbol/LineTableTest.cpp | 19 ++--
7 files changed, 95 insertions(+), 143 deletions(-)
diff --git a/lldb/include/lldb/Symbol/LineTable.h
b/lldb/include/lldb/Symbol/LineTable.h
index f66081b6ee110..2ce5d8cee6e95 100644
--- a/lldb/include/lldb/Symbol/LineTable.h
+++ b/lldb/include/lldb/Symbol/LineTable.h
@@ -20,25 +20,11 @@
namespace lldb_private {
-/// \class LineSequence LineTable.h "lldb/Symbol/LineTable.h" An abstract base
-/// class used during symbol table creation.
-class LineSequence {
-public:
- LineSequence();
-
- virtual ~LineSequence() = default;
-
- virtual void Clear() = 0;
-
-private:
- LineSequence(const LineSequence &) = delete;
- const LineSequence &operator=(const LineSequence &) = delete;
-};
-
/// \class LineTable LineTable.h "lldb/Symbol/LineTable.h"
/// A line table class.
class LineTable {
public:
+ class Sequence;
/// Construct with compile unit.
///
/// \param[in] comp_unit
@@ -49,8 +35,7 @@ class LineTable {
///
/// \param[in] sequences
/// Unsorted list of line sequences.
- LineTable(CompileUnit *comp_unit,
-std::vector> &&sequences);
+ LineTable(CompileUnit *comp_unit, std::vector &&sequences);
/// Destructor.
~LineTable();
@@ -73,20 +58,17 @@ class LineTable {
bool is_start_of_basic_block, bool is_prologue_end,
bool is_epilogue_begin, bool is_terminal_entry);
- // Used to instantiate the LineSequence helper class
- static std::unique_ptr CreateLineSequenceContainer();
-
// Append an entry to a caller-provided collection that will later be
// inserted in this line table.
- static void AppendLineEntryToSequence(LineSequence *sequence, lldb::addr_t
file_addr,
- uint32_t line, uint16_t column,
- uint16_t file_idx, bool is_start_of_statement,
- bool is_start_of_basic_block,
- bool is_prologue_end, bool is_epilogue_begin,
- bool is_terminal_entry);
+ static void
+ AppendLineEntryToSequence(Sequence &sequence, lldb::addr_t file_addr,
+uint32_t line, uint16_t column, uint16_t file_idx,
+bool is_start_of_statement,
+bool is_start_of_basic_block, bool is_prologue_end,
+bool is_epilogue_begin, bool is_terminal_entry);
// Insert a sequence of entries into this line table.
- void InsertSequence(LineSequence *sequence);
+ void InsertSequence(Sequence sequence);
/// Dump all line entries in this line table to the stream \a s.
///
@@ -273,17 +255,6 @@ class LineTable {
return 0;
}
-class LessThanBinaryPredicate {
-public:
- LessThanBinaryPredicate(LineTable *line_table);
- bool operator()(const LineTable::Entry &, const LineTable::Entry &)
const;
- bool operator()(const std::unique_ptr &,
- const std::unique_ptr &) const;
-
-protected:
- LineTable *m_line_table;
-};
-
static bool EntryAddressLessThan(const Entry &lhs, const Entry &rhs) {
return lhs.file_addr < rhs.file_addr;
}
@@ -315,6 +286,35 @@ class LineTable {
uint16_t file_idx = 0;
};
+ class Sequence {
+ public:
+Sequence() = default;
+// Moving clears moved-from object so it can be used anew. Copying is
+// generally an error. C++ doesn't guarantee that a moved-from vector is
+// empty(), so we clear it explicitly.
+Sequence(Sequence &&rhs) : m_entries(std::exchange(rhs.m_entries, {})) {}
+Sequence &operator=(Sequence &&rhs) {
+ m_entries = std::exchange(rhs.m_entries, {});
+ return *this;
+}
+Sequence(const Sequence &) = delete;
+Sequence &operator=(const Sequence &) = delete;
+
+ private:
+std::vector m_entries;
+friend class LineTable;
+ };
+
+ class LessThanBinaryPredicate {
+ public:
+LessThanBinaryPredicate(LineTable *line_
[Lldb-commits] [lldb] [lldb] Replace LineTable::upper_bound with GetLineEntryIndexRange (PR #127806)
https://github.com/labath created
https://github.com/llvm/llvm-project/pull/127806
After (too) much deliberation, I came to the conclusion that this isn't the
right abstraction, as it doesn't behave completely like the standard
upper_bound method. What I really wanted was to get the range of line entries
for a given address range -- so I implement just that.
lower_bound is still useful as a primitive for building other kinds of lookups.
>From d1da63666eeb50b688d3df66bec54772f22ad9d4 Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Wed, 19 Feb 2025 15:59:44 +0100
Subject: [PATCH] [lldb] Replace LineTable::upper_bound with
GetLineEntryIndexRange
After (too) much deliberation, I came to the conclusion that this isn't
the right abstraction, as it doesn't behave completely like the standard
upper_bound method. What I really wanted was to get the range of line
entries for a given address range -- so I implement just that.
lower_bound is still useful as a primitive for building other kinds of
lookups.
---
lldb/include/lldb/Symbol/LineTable.h| 23
lldb/source/Symbol/LineTable.cpp| 24
lldb/unittests/Symbol/LineTableTest.cpp | 74 -
3 files changed, 73 insertions(+), 48 deletions(-)
diff --git a/lldb/include/lldb/Symbol/LineTable.h
b/lldb/include/lldb/Symbol/LineTable.h
index f66081b6ee110..c1a973635cdd4 100644
--- a/lldb/include/lldb/Symbol/LineTable.h
+++ b/lldb/include/lldb/Symbol/LineTable.h
@@ -102,18 +102,19 @@ class LineTable {
void GetDescription(Stream *s, Target *target, lldb::DescriptionLevel level);
- /// Helper function for line table iteration. \c lower_bound returns the
index
- /// of the first line entry which ends after the given address (i.e., the
- /// first entry which contains the given address or it comes after it).
- /// \c upper_bound returns the index of the first line entry which begins on
- /// or after the given address (i.e., the entry which would come after the
- /// entry containing the given address, if such an entry exists). Functions
- /// return GetSize() if there is no such entry. The functions are
- /// most useful in combination: iterating from lower_bound(a) to
- /// upper_bound(b) returns all line tables which intersect the half-open
- /// range [a,b).
+ /// Returns the index of the first line entry which ends after the given
+ /// address (i.e., the first entry which contains the given address or it
+ /// comes after it). Returns GetSize() if there is no such entry.
uint32_t lower_bound(const Address &so_addr) const;
- uint32_t upper_bound(const Address &so_addr) const;
+
+ /// Returns the (half-open) range of line entry indexes which overlap the
+ /// given address range. Line entries partially overlapping the range (on
+ /// either side) are included as well. Returns an empty range
+ /// (first==second) pointing to the "right" place in the list if
+ /// there are no such line entries. Empty input ranges always result in an
+ /// empty output range.
+ std::pair
+ GetLineEntryIndexRange(const AddressRange &range) const;
/// Find a line entry that contains the section offset address \a so_addr.
///
diff --git a/lldb/source/Symbol/LineTable.cpp b/lldb/source/Symbol/LineTable.cpp
index aae4ab59ff156..c5914a2719cc9 100644
--- a/lldb/source/Symbol/LineTable.cpp
+++ b/lldb/source/Symbol/LineTable.cpp
@@ -206,25 +206,27 @@ uint32_t LineTable::lower_bound(const Address &so_addr)
const {
return std::distance(m_entries.begin(), pos);
}
-uint32_t LineTable::upper_bound(const Address &so_addr) const {
- if (so_addr.GetModule() != m_comp_unit->GetModule())
-return GetSize();
+std::pair
+LineTable::GetLineEntryIndexRange(const AddressRange &range) const {
+ uint32_t first = lower_bound(range.GetBaseAddress());
+ if (first >= GetSize() || range.GetByteSize() == 0)
+return {first, first};
Entry search_entry;
- search_entry.file_addr = so_addr.GetFileAddress();
- if (search_entry.file_addr == LLDB_INVALID_ADDRESS)
-return GetSize();
+ search_entry.file_addr =
+ range.GetBaseAddress().GetFileAddress() + range.GetByteSize();
- // This is not a typo. lower_bound returns the first entry which starts on or
- // after the given address, which is exactly what we want -- *except* if the
- // entry is a termination entry (in that case, we want the one after it).
+ // lower_bound returns the first entry which starts on or after the given
+ // address, which is exactly what we want -- *except* if the entry is a
+ // termination entry (in that case, we want the one after it).
auto pos =
- llvm::lower_bound(m_entries, search_entry, Entry::EntryAddressLessThan);
+ std::lower_bound(std::next(m_entries.begin(), first), m_entries.end(),
+ search_entry, Entry::EntryAddressLessThan);
if (pos != m_entries.end() && pos->file_addr == search_entry.file_addr &&
pos->is_terminal_entry)
++pos;
- return std::distan
[Lldb-commits] [lldb] [lldb] Reimplement LineTable::FindLineEntryByAddress on top of lower_bound (PR #127799)
https://github.com/labath created
https://github.com/llvm/llvm-project/pull/127799
I *think* this should be equivalent to the original implementation for all line
tables occurring in practice. One difference I'm aware of is that the original
implementation tried to return the first line entry out of multiple ones for
the same address. However, this is not possible (anymore?) because of the check
in LineTable::AppendLineEntryToSequence.
>From 1c24ddb7f3ec385c01bb056551e080ea4b38800d Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Wed, 19 Feb 2025 13:57:56 +0100
Subject: [PATCH] [lldb] Reimplement LineTable::FindLineEntryByAddress on top
of lower_bound
I *think* this should be equivalent to the original implementation for
all line tables occuring in practice. One difference I'm aware of is
that the original implementation tried to return the first line entry
out of multiple ones for the same address. However, this is not possible
(anymore?) because of the check in LineTable::AppendLineEntryToSequence.
---
lldb/source/Symbol/LineTable.cpp | 72 +---
1 file changed, 10 insertions(+), 62 deletions(-)
diff --git a/lldb/source/Symbol/LineTable.cpp b/lldb/source/Symbol/LineTable.cpp
index aae4ab59ff156..c199398f31a4c 100644
--- a/lldb/source/Symbol/LineTable.cpp
+++ b/lldb/source/Symbol/LineTable.cpp
@@ -233,69 +233,17 @@ bool LineTable::FindLineEntryByAddress(const Address
&so_addr,
if (index_ptr != nullptr)
*index_ptr = UINT32_MAX;
- bool success = false;
-
- if (so_addr.GetModule().get() == m_comp_unit->GetModule().get()) {
-Entry search_entry;
-search_entry.file_addr = so_addr.GetFileAddress();
-if (search_entry.file_addr != LLDB_INVALID_ADDRESS) {
- entry_collection::const_iterator begin_pos = m_entries.begin();
- entry_collection::const_iterator end_pos = m_entries.end();
- entry_collection::const_iterator pos = std::lower_bound(
- begin_pos, end_pos, search_entry, Entry::EntryAddressLessThan);
- if (pos != end_pos) {
-if (pos != begin_pos) {
- if (pos->file_addr != search_entry.file_addr)
---pos;
- else if (pos->file_addr == search_entry.file_addr) {
-// If this is a termination entry, it shouldn't match since entries
-// with the "is_terminal_entry" member set to true are termination
-// entries that define the range for the previous entry.
-if (pos->is_terminal_entry) {
- // The matching entry is a terminal entry, so we skip ahead to
- // the next entry to see if there is another entry following this
- // one whose section/offset matches.
- ++pos;
- if (pos != end_pos) {
-if (pos->file_addr != search_entry.file_addr)
- pos = end_pos;
- }
-}
-
-if (pos != end_pos) {
- // While in the same section/offset backup to find the first line
- // entry that matches the address in case there are multiple
- while (pos != begin_pos) {
-entry_collection::const_iterator prev_pos = pos - 1;
-if (prev_pos->file_addr == search_entry.file_addr &&
-prev_pos->is_terminal_entry == false)
- --pos;
-else
- break;
- }
-}
- }
-}
-else
-{
- // There might be code in the containing objfile before the first
- // line table entry. Make sure that does not get considered part of
- // the first line table entry.
- if (pos->file_addr > so_addr.GetFileAddress())
-return false;
-}
+ uint32_t idx = lower_bound(so_addr);
+ if (idx >= GetSize())
+return false;
-// Make sure we have a valid match and that the match isn't a
-// terminating entry for a previous line...
-if (pos != end_pos && pos->is_terminal_entry == false) {
- uint32_t match_idx = std::distance(begin_pos, pos);
- success = ConvertEntryAtIndexToLineEntry(match_idx, line_entry);
- if (index_ptr != nullptr && success)
-*index_ptr = match_idx;
-}
- }
-}
- }
+ addr_t file_addr = so_addr.GetFileAddress();
+ if (m_entries[idx].file_addr > file_addr)
+return false;
+
+ bool success = ConvertEntryAtIndexToLineEntry(idx, line_entry);
+ if (index_ptr != nullptr && success)
+*index_ptr = idx;
return success;
}
___
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)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
Changes
I *think* this should be equivalent to the original implementation for all line
tables occurring in practice. One difference I'm aware of is that the original
implementation tried to return the first line entry out of multiple ones for
the same address. However, this is not possible (anymore?) because of the check
in LineTable::AppendLineEntryToSequence.
---
Full diff: https://github.com/llvm/llvm-project/pull/127799.diff
1 Files Affected:
- (modified) lldb/source/Symbol/LineTable.cpp (+10-62)
``diff
diff --git a/lldb/source/Symbol/LineTable.cpp b/lldb/source/Symbol/LineTable.cpp
index aae4ab59ff156..c199398f31a4c 100644
--- a/lldb/source/Symbol/LineTable.cpp
+++ b/lldb/source/Symbol/LineTable.cpp
@@ -233,69 +233,17 @@ bool LineTable::FindLineEntryByAddress(const Address
&so_addr,
if (index_ptr != nullptr)
*index_ptr = UINT32_MAX;
- bool success = false;
-
- if (so_addr.GetModule().get() == m_comp_unit->GetModule().get()) {
-Entry search_entry;
-search_entry.file_addr = so_addr.GetFileAddress();
-if (search_entry.file_addr != LLDB_INVALID_ADDRESS) {
- entry_collection::const_iterator begin_pos = m_entries.begin();
- entry_collection::const_iterator end_pos = m_entries.end();
- entry_collection::const_iterator pos = std::lower_bound(
- begin_pos, end_pos, search_entry, Entry::EntryAddressLessThan);
- if (pos != end_pos) {
-if (pos != begin_pos) {
- if (pos->file_addr != search_entry.file_addr)
---pos;
- else if (pos->file_addr == search_entry.file_addr) {
-// If this is a termination entry, it shouldn't match since entries
-// with the "is_terminal_entry" member set to true are termination
-// entries that define the range for the previous entry.
-if (pos->is_terminal_entry) {
- // The matching entry is a terminal entry, so we skip ahead to
- // the next entry to see if there is another entry following this
- // one whose section/offset matches.
- ++pos;
- if (pos != end_pos) {
-if (pos->file_addr != search_entry.file_addr)
- pos = end_pos;
- }
-}
-
-if (pos != end_pos) {
- // While in the same section/offset backup to find the first line
- // entry that matches the address in case there are multiple
- while (pos != begin_pos) {
-entry_collection::const_iterator prev_pos = pos - 1;
-if (prev_pos->file_addr == search_entry.file_addr &&
-prev_pos->is_terminal_entry == false)
- --pos;
-else
- break;
- }
-}
- }
-}
-else
-{
- // There might be code in the containing objfile before the first
- // line table entry. Make sure that does not get considered part of
- // the first line table entry.
- if (pos->file_addr > so_addr.GetFileAddress())
-return false;
-}
+ uint32_t idx = lower_bound(so_addr);
+ if (idx >= GetSize())
+return false;
-// Make sure we have a valid match and that the match isn't a
-// terminating entry for a previous line...
-if (pos != end_pos && pos->is_terminal_entry == false) {
- uint32_t match_idx = std::distance(begin_pos, pos);
- success = ConvertEntryAtIndexToLineEntry(match_idx, line_entry);
- if (index_ptr != nullptr && success)
-*index_ptr = match_idx;
-}
- }
-}
- }
+ addr_t file_addr = so_addr.GetFileAddress();
+ if (m_entries[idx].file_addr > file_addr)
+return false;
+
+ bool success = ConvertEntryAtIndexToLineEntry(idx, line_entry);
+ if (index_ptr != nullptr && success)
+*index_ptr = idx;
return success;
}
``
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] fadbc33 - [lldb] Add LineTable::{upper, lower}_bound (#127519)
Author: Pavel Labath
Date: 2025-02-19T12:58:55+01:00
New Revision: fadbc33b01d6815bf05d802d1323322262b54d42
URL:
https://github.com/llvm/llvm-project/commit/fadbc33b01d6815bf05d802d1323322262b54d42
DIFF:
https://github.com/llvm/llvm-project/commit/fadbc33b01d6815bf05d802d1323322262b54d42.diff
LOG: [lldb] Add LineTable::{upper,lower}_bound (#127519)
The motivation is #123622 and the fact that is hard to fine the last
line entry in a given range. `FindLineEntryByAddress(range_end-1)` is
the best we have, but it's not ideal because it has a magic -1 and that
it relies on there existing a line entry at that address (generally, it
should be there, but if for some case it isn't, we might end up ignoring
the entries that are there (or -- like my incorrect fix in #123622 did
-- iterating through the entire line table).
What we really want is to get the last entry that exists in the given
range. Or, equivalently (and more STL-like) the first entry after that
range. This is what these functions do. I've used the STL names since
they do pretty much exactly what the standard functions do (the main
head-scratcher comes from the fact that our entries represent ranges
rather than single values).
The functions can also be used to simplify the maze of `if` statements
in `FindLineEntryByAddress`, but I'm keeping that as a separate patch.
For now, I'm just adding some unit testing for that function to gain
more confidence that the patch does not change the function behavior.
-
Co-authored-by: Jonas Devlieghere
Added:
lldb/unittests/Symbol/LineTableTest.cpp
Modified:
lldb/include/lldb/Symbol/LineTable.h
lldb/source/Symbol/LineTable.cpp
lldb/unittests/Symbol/CMakeLists.txt
Removed:
diff --git a/lldb/include/lldb/Symbol/LineTable.h
b/lldb/include/lldb/Symbol/LineTable.h
index 6d158ab518879..f66081b6ee110 100644
--- a/lldb/include/lldb/Symbol/LineTable.h
+++ b/lldb/include/lldb/Symbol/LineTable.h
@@ -102,6 +102,19 @@ class LineTable {
void GetDescription(Stream *s, Target *target, lldb::DescriptionLevel level);
+ /// Helper function for line table iteration. \c lower_bound returns the
index
+ /// of the first line entry which ends after the given address (i.e., the
+ /// first entry which contains the given address or it comes after it).
+ /// \c upper_bound returns the index of the first line entry which begins on
+ /// or after the given address (i.e., the entry which would come after the
+ /// entry containing the given address, if such an entry exists). Functions
+ /// return GetSize() if there is no such entry. The functions are
+ /// most useful in combination: iterating from lower_bound(a) to
+ /// upper_bound(b) returns all line tables which intersect the half-open
+ /// range [a,b).
+ uint32_t lower_bound(const Address &so_addr) const;
+ uint32_t upper_bound(const Address &so_addr) const;
+
/// Find a line entry that contains the section offset address \a so_addr.
///
/// \param[in] so_addr
diff --git a/lldb/source/Symbol/LineTable.cpp
b/lldb/source/Symbol/LineTable.cpp
index 3d2afcdd11997..aae4ab59ff156 100644
--- a/lldb/source/Symbol/LineTable.cpp
+++ b/lldb/source/Symbol/LineTable.cpp
@@ -123,7 +123,7 @@ void LineTable::InsertSequence(LineSequence *sequence) {
entry_collection::iterator end_pos = m_entries.end();
LineTable::Entry::LessThanBinaryPredicate less_than_bp(this);
entry_collection::iterator pos =
- upper_bound(begin_pos, end_pos, entry, less_than_bp);
+ std::upper_bound(begin_pos, end_pos, entry, less_than_bp);
// We should never insert a sequence in the middle of another sequence
if (pos != begin_pos) {
@@ -185,6 +185,48 @@ bool LineTable::GetLineEntryAtIndex(uint32_t idx,
LineEntry &line_entry) {
return false;
}
+uint32_t LineTable::lower_bound(const Address &so_addr) const {
+ if (so_addr.GetModule() != m_comp_unit->GetModule())
+return GetSize();
+
+ Entry search_entry;
+ search_entry.file_addr = so_addr.GetFileAddress();
+ if (search_entry.file_addr == LLDB_INVALID_ADDRESS)
+return GetSize();
+
+ // This is not a typo. upper_bound returns the first entry which definitely
+ // does not contain this address, which means the entry before it *might*
+ // contain it -- if it is not a termination entry.
+ auto pos =
+ llvm::upper_bound(m_entries, search_entry, Entry::EntryAddressLessThan);
+
+ if (pos != m_entries.begin() && !std::prev(pos)->is_terminal_entry)
+--pos;
+
+ return std::distance(m_entries.begin(), pos);
+}
+
+uint32_t LineTable::upper_bound(const Address &so_addr) const {
+ if (so_addr.GetModule() != m_comp_unit->GetModule())
+return GetSize();
+
+ Entry search_entry;
+ search_entry.file_addr = so_addr.GetFileAddress();
+ if (search_entry.file_addr == LLDB_INVALID_ADDRESS)
+return GetSize();
+
+ // This is not a typo. lower_bound returns the first entry which st
[Lldb-commits] [lldb] [lldb] Add LineTable::{upper, lower}_bound (PR #127519)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/127519 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make GetOutputStreamSP and GetErrorStreamSP protected (PR #127682)
@@ -2837,8 +2837,8 @@ void CommandInterpreter::HandleCommandsFromFile(
}
if (flags & eHandleCommandFlagPrintResult) {
-debugger.GetOutputFile().Printf("Executing commands in '%s'.\n",
-cmd_file_path.c_str());
+debugger.GetOutputFileSP()->Printf("Executing commands in '%s'.\n",
labath wrote:
This is one of those suspicious calls.
https://github.com/llvm/llvm-project/pull/127682
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make GetOutputStreamSP and GetErrorStreamSP protected (PR #127682)
https://github.com/labath approved this pull request. LGTM. I am not saying that you have to do anything about this, but there are two things that caught my eye: - Debugger::GetOutputFileSP is now the biggest backdoor into our locking mechanism. In a way, it's even worse than GetOutputStreamSP, since it bypasses locking completely. And since it supports `Printf` I can totally see someone reaching for that without realizing the problem. - It would sure be nice if StreamAsync was more of an RAII thing. Like, at least stored in a unique pointer, and maybe even passed by value (if we can make it movable). https://github.com/llvm/llvm-project/pull/127682 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Make GetOutputStreamSP and GetErrorStreamSP protected (PR #127682)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/127682 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Replace LineTable::upper_bound with GetLineEntryIndexRange (PR #127806)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Pavel Labath (labath)
Changes
After (too) much deliberation, I came to the conclusion that this isn't the
right abstraction, as it doesn't behave completely like the standard
upper_bound method. What I really wanted was to get the range of line entries
for a given address range -- so I implement just that.
lower_bound is still useful as a primitive for building other kinds of lookups.
---
Full diff: https://github.com/llvm/llvm-project/pull/127806.diff
3 Files Affected:
- (modified) lldb/include/lldb/Symbol/LineTable.h (+12-11)
- (modified) lldb/source/Symbol/LineTable.cpp (+13-11)
- (modified) lldb/unittests/Symbol/LineTableTest.cpp (+48-26)
``diff
diff --git a/lldb/include/lldb/Symbol/LineTable.h
b/lldb/include/lldb/Symbol/LineTable.h
index f66081b6ee110..c1a973635cdd4 100644
--- a/lldb/include/lldb/Symbol/LineTable.h
+++ b/lldb/include/lldb/Symbol/LineTable.h
@@ -102,18 +102,19 @@ class LineTable {
void GetDescription(Stream *s, Target *target, lldb::DescriptionLevel level);
- /// Helper function for line table iteration. \c lower_bound returns the
index
- /// of the first line entry which ends after the given address (i.e., the
- /// first entry which contains the given address or it comes after it).
- /// \c upper_bound returns the index of the first line entry which begins on
- /// or after the given address (i.e., the entry which would come after the
- /// entry containing the given address, if such an entry exists). Functions
- /// return GetSize() if there is no such entry. The functions are
- /// most useful in combination: iterating from lower_bound(a) to
- /// upper_bound(b) returns all line tables which intersect the half-open
- /// range [a,b).
+ /// Returns the index of the first line entry which ends after the given
+ /// address (i.e., the first entry which contains the given address or it
+ /// comes after it). Returns GetSize() if there is no such entry.
uint32_t lower_bound(const Address &so_addr) const;
- uint32_t upper_bound(const Address &so_addr) const;
+
+ /// Returns the (half-open) range of line entry indexes which overlap the
+ /// given address range. Line entries partially overlapping the range (on
+ /// either side) are included as well. Returns an empty range
+ /// (first==second) pointing to the "right" place in the list if
+ /// there are no such line entries. Empty input ranges always result in an
+ /// empty output range.
+ std::pair
+ GetLineEntryIndexRange(const AddressRange &range) const;
/// Find a line entry that contains the section offset address \a so_addr.
///
diff --git a/lldb/source/Symbol/LineTable.cpp b/lldb/source/Symbol/LineTable.cpp
index aae4ab59ff156..c5914a2719cc9 100644
--- a/lldb/source/Symbol/LineTable.cpp
+++ b/lldb/source/Symbol/LineTable.cpp
@@ -206,25 +206,27 @@ uint32_t LineTable::lower_bound(const Address &so_addr)
const {
return std::distance(m_entries.begin(), pos);
}
-uint32_t LineTable::upper_bound(const Address &so_addr) const {
- if (so_addr.GetModule() != m_comp_unit->GetModule())
-return GetSize();
+std::pair
+LineTable::GetLineEntryIndexRange(const AddressRange &range) const {
+ uint32_t first = lower_bound(range.GetBaseAddress());
+ if (first >= GetSize() || range.GetByteSize() == 0)
+return {first, first};
Entry search_entry;
- search_entry.file_addr = so_addr.GetFileAddress();
- if (search_entry.file_addr == LLDB_INVALID_ADDRESS)
-return GetSize();
+ search_entry.file_addr =
+ range.GetBaseAddress().GetFileAddress() + range.GetByteSize();
- // This is not a typo. lower_bound returns the first entry which starts on or
- // after the given address, which is exactly what we want -- *except* if the
- // entry is a termination entry (in that case, we want the one after it).
+ // lower_bound returns the first entry which starts on or after the given
+ // address, which is exactly what we want -- *except* if the entry is a
+ // termination entry (in that case, we want the one after it).
auto pos =
- llvm::lower_bound(m_entries, search_entry, Entry::EntryAddressLessThan);
+ std::lower_bound(std::next(m_entries.begin(), first), m_entries.end(),
+ search_entry, Entry::EntryAddressLessThan);
if (pos != m_entries.end() && pos->file_addr == search_entry.file_addr &&
pos->is_terminal_entry)
++pos;
- return std::distance(m_entries.begin(), pos);
+ return {first, std::distance(m_entries.begin(), pos)};
}
bool LineTable::FindLineEntryByAddress(const Address &so_addr,
diff --git a/lldb/unittests/Symbol/LineTableTest.cpp
b/lldb/unittests/Symbol/LineTableTest.cpp
index 2fa2913f67f9e..ef5493138f318 100644
--- a/lldb/unittests/Symbol/LineTableTest.cpp
+++ b/lldb/unittests/Symbol/LineTableTest.cpp
@@ -194,7 +194,7 @@ CreateFakeModule(std::vector>
line_sequences) {
std::move(text_sp), line_table};
}
-TEST
[Lldb-commits] [lldb] [WIP] [lldb][TypeSystemClang] Create clang::SourceLocation from DWARF and attach to AST (PR #127829)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/127829
This patch revives https://reviews.llvm.org/D78095 with some changes.
This patch uses the `DW_AT_decl_XXX` DWARF attributes to create
`clang::SourceLocation`s and attach them to the Clang decls that LLDB creates.
The primary motivation for this is better expression evaluator diagnostics.
Instead of:
```
(lldb) expr func()
˄
╰─ error: no matching function for call to 'func'
note: note: candidate function not viable: requires 1 argument, but 0 were
provided
note: note: candidate function not viable: requires 1 argument, but 0 were
provided
note: note: candidate function not viable: requires single argument 'x', but no
arguments were provided
note: note: candidate function not viable: requires single argument 'y', but no
arguments were provided
note: note: candidate function not viable: requires 2 arguments, but 0 were
provided
```
We would see:
```
(lldb) expr func()
˄
╰─ error: no matching function for call to 'func'
note: /Users/michaelbuch/source-locs/lib.h:4:1: candidate function not viable:
requires 1 argument, but 0 were provided
4 | void func(char const* c, char) {}; void func(bool) {}
| ^
note: /Users/michaelbuch/source-locs/lib.h:4:1: candidate function not viable:
requires 2 arguments, but 0 were provided
4 | void func(char const* c, char) {}; void func(bool) {}
| ^
note: /Users/michaelbuch/source-locs/main.cpp:5:1: candidate function not
viable: requires single argument 'x', but no arguments were provided
5 | void func(int x) { var = 6; }
| ^
note: /Users/michaelbuch/source-locs/main.cpp:6:1: candidate function not
viable: requires single argument 'y', but no arguments were provided
6 | void func(float y) { var = 6; }
| ^
```
There's a couple of quirks in the output still, mainly because Clang we doesn't
emit `DW_AT_decl_column`, so Clang has to place the caret on column `1`. But
this is something we can probably configure (either as a pre-requisite to this
patch or as a follow-up).
This is currently WIP. It's lacking tests and we probably also want to make
sure we only use the file `DW_AT_decl_file` hasn't changed since the debug-info
was generated (i think there's some checksum we can use for this, like we do in
the SupportFiles?). Just looking for early concerns/feedback.
I split up the patch into 3 separate commits:
1. Commit 1: makes sure each ASTContext that LLDB creates (apart from the
scratch AST), has a `MainFileID`. LLDB will pretend that this dummy file is the
main source file, and all other FileIDs (those that we create in commit 3) are
included into it
2. Commit 2: makes sure we call `setLocation` in all the appropriate places.
3. Implements `GetLocForDecl` which turns a `lldb_private::Declaration` into
`clang::SourceLocation`
The three main differences to https://reviews.llvm.org/D78095 are:
1. we no longer hide this feature behind a flag
2. the original patch didn't yet support using `DW_AT_decl_file` for displaying
source snippets
3. we no longer have a separate `clang::TextDiagnosticPrinter` for printing
these source snippets. In theory it would be nice to do this, because we could
configure it separately (e.g., to hide the column number). I didn't carry that
over because (1) setting `ShowCarets` to `false` causes the source snippet to
not be printed (probably something we could change in Clang), and (2) we
somehow need to differentiate whether a `FileID` was created by LLDB versus
Clang, which got tricky because we need to have cross-`DWARFASTParserClang`
state, which also meant that we have to copy over this metadata to the
expression context. All of that didn't seem worth it, but we could definitely
add this back
>From 1aabc8a93ffac06755cbaf5e6c1fa8913cd63729 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 19 Feb 2025 12:47:30 +
Subject: [PATCH 1/3] [lldb][TypeSystemClang] Create MainFileID for
TypeSystemClang ASTContexts
---
.../TypeSystem/Clang/TypeSystemClang.cpp | 39 +++
1 file changed, 39 insertions(+)
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 1e0c7f0514941..563961b9a4971 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -13,6 +13,7 @@
#include "llvm/Support/Casting.h"
#include "llvm/Support/FormatAdapters.h"
#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/SmallVectorMemoryBuffer.h"
#include
#include
@@ -361,6 +362,39 @@ static void SetMemberOwningModule(clang::Decl *member,
}
}
+/// Creates a dummy main file for the given SourceManager.
+/// This file only serves as a container for include locations to other
+/// FileIDs that are put into this type system (either by the ASTImporter
+/// or when TypeSystemClang ge
[Lldb-commits] [lldb] [WIP] [lldb][TypeSystemClang] Create clang::SourceLocation from DWARF and attach to AST (PR #127829)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Michael Buch (Michael137)
Changes
This patch revives https://reviews.llvm.org/D78095 with some changes.
This patch uses the `DW_AT_decl_XXX` DWARF attributes to create
`clang::SourceLocation`s and attach them to the Clang decls that LLDB creates.
The primary motivation for this is better expression evaluator diagnostics.
Instead of:
```
(lldb) expr func()
˄
╰─ error: no matching function for call to 'func'
note: note: candidate function not viable: requires 1 argument, but 0 were
provided
note: note: candidate function not viable: requires 1 argument, but 0 were
provided
note: note: candidate function not viable: requires single argument 'x', but no
arguments were provided
note: note: candidate function not viable: requires single argument 'y', but no
arguments were provided
note: note: candidate function not viable: requires 2 arguments, but 0 were
provided
```
We would see:
```
(lldb) expr func()
˄
╰─ error: no matching function for call to 'func'
note: /Users/michaelbuch/source-locs/lib.h:4:1: candidate function not viable:
requires 1 argument, but 0 were provided
4 | void func(char const* c, char) {}; void func(bool) {}
| ^
note: /Users/michaelbuch/source-locs/lib.h:4:1: candidate function not viable:
requires 2 arguments, but 0 were provided
4 | void func(char const* c, char) {}; void func(bool) {}
| ^
note: /Users/michaelbuch/source-locs/main.cpp:5:1: candidate function not
viable: requires single argument 'x', but no arguments were provided
5 | void func(int x) { var = 6; }
| ^
note: /Users/michaelbuch/source-locs/main.cpp:6:1: candidate function not
viable: requires single argument 'y', but no arguments were provided
6 | void func(float y) { var = 6; }
| ^
```
There's a couple of quirks in the output still, mainly because Clang we doesn't
emit `DW_AT_decl_column`, so Clang has to place the caret on column `1`. But
this is something we can probably configure (either as a pre-requisite to this
patch or as a follow-up).
This is currently WIP. It's lacking tests and we probably also want to make
sure we only use the file `DW_AT_decl_file` hasn't changed since the debug-info
was generated (i think there's some checksum we can use for this, like we do in
the SupportFiles?). Just looking for early concerns/feedback.
I split up the patch into 3 separate commits:
1. Commit 1: makes sure each ASTContext that LLDB creates (apart from the
scratch AST), has a `MainFileID`. LLDB will pretend that this dummy file is the
main source file, and all other FileIDs (those that we create in commit 3) are
included into it
2. Commit 2: makes sure we call `setLocation` in all the appropriate places.
3. Implements `GetLocForDecl` which turns a `lldb_private::Declaration` into
`clang::SourceLocation`
The three main differences to https://reviews.llvm.org/D78095 are:
1. we no longer hide this feature behind a flag
2. the original patch didn't yet support using `DW_AT_decl_file` for displaying
source snippets
3. we no longer have a separate `clang::TextDiagnosticPrinter` for printing
these source snippets. In theory it would be nice to do this, because we could
configure it separately (e.g., to hide the column number). I didn't carry that
over because (1) setting `ShowCarets` to `false` causes the source snippet to
not be printed (probably something we could change in Clang), and (2) we
somehow need to differentiate whether a `FileID` was created by LLDB versus
Clang, which got tricky because we need to have cross-`DWARFASTParserClang`
state, which also meant that we have to copy over this metadata to the
expression context. All of that didn't seem worth it, but we could definitely
add this back
---
Patch is 20.05 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/127829.diff
5 Files Affected:
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
(+5-5)
- (modified) lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp (+1-1)
- (modified) lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp (+2-1)
- (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+104-12)
- (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h (+26-16)
``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 2d4d22559963f..abf3b22b0ae15 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1372,7 +1372,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
ignore_containing_context ? m_ast.GetTranslationUnitDecl()
: containing_decl_ctx,
GetOwningClangModule(die), name, clang_type, attrs.storage,
-at
[Lldb-commits] [lldb] [WIP] [lldb][TypeSystemClang] Create clang::SourceLocation from DWARF and attach to AST (PR #127829)
https://github.com/Michael137 edited 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] [WIP] [lldb][TypeSystemClang] Create clang::SourceLocation from DWARF and attach to AST (PR #127829)
https://github.com/Michael137 edited 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] [WIP] [lldb][TypeSystemClang] Create clang::SourceLocation from DWARF and attach to AST (PR #127829)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/127829
>From 1aabc8a93ffac06755cbaf5e6c1fa8913cd63729 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 19 Feb 2025 12:47:30 +
Subject: [PATCH 1/3] [lldb][TypeSystemClang] Create MainFileID for
TypeSystemClang ASTContexts
---
.../TypeSystem/Clang/TypeSystemClang.cpp | 39 +++
1 file changed, 39 insertions(+)
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 1e0c7f0514941..563961b9a4971 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -13,6 +13,7 @@
#include "llvm/Support/Casting.h"
#include "llvm/Support/FormatAdapters.h"
#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/SmallVectorMemoryBuffer.h"
#include
#include
@@ -361,6 +362,39 @@ static void SetMemberOwningModule(clang::Decl *member,
}
}
+/// Creates a dummy main file for the given SourceManager.
+/// This file only serves as a container for include locations to other
+/// FileIDs that are put into this type system (either by the ASTImporter
+/// or when TypeSystemClang generates source locations for declarations).
+/// This file is not reflected to disk.
+static clang::FileID CreateDummyMainFile(clang::SourceManager &sm,
+ clang::FileManager &fm) {
+ llvm::StringRef main_file_path = "";
+ // The file contents are empty and should never be seen by the user. The new
+ // line is just there to not throw off any line counting logic that might
+ // expect files to end with a newline.
+ llvm::StringRef main_file_contents = "\n";
+ const time_t mod_time = 0;
+ const off_t file_size = static_cast(main_file_contents.size());
+
+ // Create a virtual FileEntry for our dummy file.
+ auto fe = fm.getVirtualFileRef(main_file_path, file_size, mod_time);
+
+ // Overwrite the file buffer with our empty file contents.
+ llvm::SmallVector buffer;
+ buffer.append(main_file_contents.begin(), main_file_contents.end());
+ auto file_contents = std::make_unique(
+ std::move(buffer), main_file_path);
+ sm.overrideFileContents(fe, std::move(file_contents));
+
+ // Create the actual file id for the FileEntry and set it as the main file.
+ clang::FileID fid =
+ sm.createFileID(fe, SourceLocation(), clang::SrcMgr::C_User);
+ sm.setMainFileID(fid);
+
+ return fid;
+}
+
char TypeSystemClang::ID;
bool TypeSystemClang::IsOperator(llvm::StringRef name,
@@ -692,6 +726,11 @@ void TypeSystemClang::CreateASTContext() {
m_diagnostic_consumer_up = std::make_unique();
m_ast_up->getDiagnostics().setClient(m_diagnostic_consumer_up.get(), false);
+ // Set up the MainFileID of this ASTContext. All other FileIDs created
+ // by this TypeSystem will act as-if included into this dummy main file.
+ auto fid = CreateDummyMainFile(*m_source_manager_up, *m_file_manager_up);
+ assert(m_ast_up->getSourceManager().getMainFileID() == fid);
+
// This can be NULL if we don't know anything about the architecture or if
// the target for an architecture isn't enabled in the llvm/clang that we
// built
>From 68b389059b1bee5401f6173351c16c59b7286f69 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 19 Feb 2025 13:37:01 +
Subject: [PATCH 2/3] [lldb][TypeSystemClang] Set location on functions,
parameters, enums and structures
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 10 ++--
.../SymbolFile/NativePDB/PdbAstBuilder.cpp| 2 +-
.../Plugins/SymbolFile/PDB/PDBASTParser.cpp | 3 +-
.../TypeSystem/Clang/TypeSystemClang.cpp | 48 ++-
.../TypeSystem/Clang/TypeSystemClang.h| 46 +++---
5 files changed, 74 insertions(+), 35 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 2d4d22559963f..abf3b22b0ae15 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1372,7 +1372,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
ignore_containing_context ? m_ast.GetTranslationUnitDecl()
: containing_decl_ctx,
GetOwningClangModule(die), name, clang_type, attrs.storage,
-attrs.is_inline);
+attrs.is_inline, attrs.decl);
std::free(name_buf);
if (has_template_params) {
@@ -1382,11 +1382,11 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE
&die,
ignore_containing_context ? m_ast.GetTranslationUnitDecl()
: containing_decl_ctx,
GetOwningClangModule(die), attrs.name.GetStringRef(), clang_type,
- attrs.storage, attrs.is_inline);
+ attrs.storage, attrs.
[Lldb-commits] [lldb] [lldb] Store the return SBValueList in the CommandReturnObject (PR #127566)
jimingham wrote: > > > * what about commands (I don't know if we have any) whose output consists > > > of more than formatting a single value (which, I guess would be lost if > > > the IDE decides to ignore the output). Are we fine with not supporting > > > that? (we could say that if a command wants to print some extra output, > > > it should use the error stream for that) > > > > > > In the longer term, we can also add an optional SBStructuredData for > > commands that return structured data, like `image lists` or `image lookup` > > so the UI can print these as tables or something else nice. So if we end up > > having fancier commands that have both a ValueObject return plus extra > > annotation, we could add an SBValue type to what an SBStructured data can > > hold, and have it emit `before` and `after` entries for the extra text. > > I think that would be better than trying to dump some of the output to > > stderr. > > That sort of makes sense, but it doesn't sound like you're planning to do > that soon, so I'm asking about what should we do until (and if) that happens. > The reason I suggested stderr is because that made sense for the commands I'm > thinking of. For example, the expression command can produce a result and > some warnings and fixit-applied notes. I don't know if these currently go to > stderr, but I think it would be okay if they did... I don't think it's a good idea for commands to put any of their output anywhere but the command result. Anything that goes to stderr is lost, and particularly if is trying to run their own command loop, so that they are by hand printing the command result, there will be no way to associate that output with the command that produced it. We don't dump warnings on successful expression evaluation, though we do print fixits. I would suggest for the near term that if a command that produces a ValueObject returns successfully but with "notes" we should put those notes in the Error object - but still have the state be Success. That would make sense for the warnings and fixits since Adrian has been working on getting the error output into a structured form to better hold compiler warnings and errors. https://github.com/llvm/llvm-project/pull/127566 ___ 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 TargetInfo for collecting data about a target (PR #127834)
https://github.com/oontvoo created
https://github.com/llvm/llvm-project/pull/127834
None
>From 0d6a36d84df50ccb9eef9ef3dd6f59d4299edeac Mon Sep 17 00:00:00 2001
From: Vy Nguyen
Date: Wed, 19 Feb 2025 12:47:57 -0500
Subject: [PATCH] [LLDB][Telemetry]Define TargetInfo for collecting data about
a target
---
lldb/include/lldb/Core/Telemetry.h | 86 +-
lldb/source/Core/Telemetry.cpp | 99 ++
2 files changed, 170 insertions(+), 15 deletions(-)
diff --git a/lldb/include/lldb/Core/Telemetry.h
b/lldb/include/lldb/Core/Telemetry.h
index b72556ecaf3c9..4be81951254de 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 KindType TargetInfo = 0b11010;
+ // 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,14 +60,88 @@ struct LLDBBaseTelemetryInfo : public
llvm::telemetry::TelemetryInfo {
void serialize(llvm::telemetry::Serializer &serializer) const override;
};
+/// Describes an exit status.
+struct ExitDescription {
+ int exit_code;
+ std::string description;
+};
+
+struct TargetTelemetryInfo : public LldbBaseTelemetryInfo {
+ lldb::ModuleSP exec_mod;
+ Target *target_ptr;
+
+ // The same as the executable-module's UUID.
+ std::string target_uuid;
+ std::string file_format;
+
+ std::string binary_path;
+ size_t binary_size;
+
+ std::optional exit_desc;
+ TargetTelemetryInfo() = default;
+
+ TargetTelemetryInfo(const TargetTelemetryInfo &other) {
+exec_mod = other.exec_mod;
+target_uuid = other.target_uuid;
+file_format = other.file_format;
+binary_path = other.binary_path;
+binary_size = other.binary_size;
+exit_desc = other.exit_desc;
+ }
+
+ KindType getKind() const override { return LldbEntryKind::TargetInfo; }
+
+ static bool classof(const TelemetryInfo *T) {
+if (T == nullptr)
+ return false;
+return T->getKind() == LldbEntryKind::TargetInfo;
+ }
+
+ void serialize(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.
-class TelemetryManager : public llvm::telemetry::Manager {
+class TelemetryMager : public llvm::telemetry::Manager {
public:
llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override;
- virtual llvm::StringRef GetInstanceName() const = 0;
+ const llvm::telemetry::Config *getConfig();
+
+ virtual void AtMainExecutableLoadStart(TargetInfo * entry);
+ virtual void AtMainExecutableLoadEnd(TargetInfo *entry);
+
+ virtual llvm::StringRef GetInstanceName() const = 0;
static TelemetryManager *getInstance();
protected:
@@ -73,6 +151,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/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 5222f76704f91..da7aee01680fc 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -10,14 +10,20 @@
#ifdef LLVM_BUILD_TELEMETRY
-#include "lldb/Core/Telemetry.h"
#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Telemetry.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Process.h"
+#include "l
[Lldb-commits] [lldb] [LLDB][Telemetry]Define TargetInfo for collecting data about a target (PR #127834)
https://github.com/oontvoo converted_to_draft https://github.com/llvm/llvm-project/pull/127834 ___ 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 TargetInfo for collecting data about a target (PR #127834)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Vy Nguyen (oontvoo)
Changes
---
Full diff: https://github.com/llvm/llvm-project/pull/127834.diff
2 Files Affected:
- (modified) lldb/include/lldb/Core/Telemetry.h (+84-2)
- (modified) lldb/source/Core/Telemetry.cpp (+86-13)
``diff
diff --git a/lldb/include/lldb/Core/Telemetry.h
b/lldb/include/lldb/Core/Telemetry.h
index b72556ecaf3c9..4be81951254de 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 KindType TargetInfo = 0b11010;
+ // 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,14 +60,88 @@ struct LLDBBaseTelemetryInfo : public
llvm::telemetry::TelemetryInfo {
void serialize(llvm::telemetry::Serializer &serializer) const override;
};
+/// Describes an exit status.
+struct ExitDescription {
+ int exit_code;
+ std::string description;
+};
+
+struct TargetTelemetryInfo : public LldbBaseTelemetryInfo {
+ lldb::ModuleSP exec_mod;
+ Target *target_ptr;
+
+ // The same as the executable-module's UUID.
+ std::string target_uuid;
+ std::string file_format;
+
+ std::string binary_path;
+ size_t binary_size;
+
+ std::optional exit_desc;
+ TargetTelemetryInfo() = default;
+
+ TargetTelemetryInfo(const TargetTelemetryInfo &other) {
+exec_mod = other.exec_mod;
+target_uuid = other.target_uuid;
+file_format = other.file_format;
+binary_path = other.binary_path;
+binary_size = other.binary_size;
+exit_desc = other.exit_desc;
+ }
+
+ KindType getKind() const override { return LldbEntryKind::TargetInfo; }
+
+ static bool classof(const TelemetryInfo *T) {
+if (T == nullptr)
+ return false;
+return T->getKind() == LldbEntryKind::TargetInfo;
+ }
+
+ void serialize(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.
-class TelemetryManager : public llvm::telemetry::Manager {
+class TelemetryMager : public llvm::telemetry::Manager {
public:
llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override;
- virtual llvm::StringRef GetInstanceName() const = 0;
+ const llvm::telemetry::Config *getConfig();
+
+ virtual void AtMainExecutableLoadStart(TargetInfo * entry);
+ virtual void AtMainExecutableLoadEnd(TargetInfo *entry);
+
+ virtual llvm::StringRef GetInstanceName() const = 0;
static TelemetryManager *getInstance();
protected:
@@ -73,6 +151,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/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 5222f76704f91..da7aee01680fc 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -10,14 +10,20 @@
#ifdef LLVM_BUILD_TELEMETRY
-#include "lldb/Core/Telemetry.h"
#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Telemetry.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Statistics.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/UUID.h"
+#include "lldb/Version/Version.h"
#include "lldb/lldb-enumerations.h"
#include "lldb/lldb-forward.h"
#inc
[Lldb-commits] [lldb] [LLDB][Telemetry]Define TargetInfo for collecting data about a target (PR #127834)
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 5ecce45ea2980aff35d1283d4dd3feb8f74de16c 0d6a36d84df50ccb9eef9ef3dd6f59d4299edeac --extensions h,cpp -- lldb/include/lldb/Core/Telemetry.h lldb/source/Core/Telemetry.cpp `` View the diff from clang-format here. ``diff diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h index 4be8195125..9e24bbfa58 100644 --- a/lldb/include/lldb/Core/Telemetry.h +++ b/lldb/include/lldb/Core/Telemetry.h @@ -138,10 +138,10 @@ public: const llvm::telemetry::Config *getConfig(); - virtual void AtMainExecutableLoadStart(TargetInfo * entry); + virtual void AtMainExecutableLoadStart(TargetInfo *entry); virtual void AtMainExecutableLoadEnd(TargetInfo *entry); - virtual llvm::StringRef GetInstanceName() const = 0; + virtual llvm::StringRef GetInstanceName() const = 0; static TelemetryManager *getInstance(); protected: `` https://github.com/llvm/llvm-project/pull/127834 ___ 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)
@@ -56,13 +60,83 @@ 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 {
oontvoo wrote:
I still don't understand how it would be much simpler filter out some structs.
Then we would still have to make the decision which fields go into the the
so-called "sensitive data struct".
I imagine the vendor-specific code would look something like this
```
static DebuggerInfo* sanitizeFields(DebuggerInfo* entry) {
// null out any fields you dont want to store
//
}
Error VendorDestination::receiveEntry(DebuggerInfo* entry) {
auto* newEntry = sanitizeFields(entry);
storeToInternalStorage(entry);
}
```
And yes, it's more convenient to group all the context together (otherwise in
downstream data structure, we'd need to do a bunch of joining to build a
complete "picture").
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][Telemetry]Define DebuggerTelemetryInfo and related methods (PR #127696)
@@ -761,12 +767,29 @@ void Debugger::InstanceInitialize() {
DebuggerSP Debugger::CreateInstance(lldb::LogOutputCallback log_callback,
void *baton) {
+#ifdef LLVM_BUILD_TELEMETRY
+ lldb_private::telemetry::SteadyTimePoint start_time =
+ std::chrono::steady_clock::now();
+#endif
DebuggerSP debugger_sp(new Debugger(log_callback, baton));
if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) {
std::lock_guard guard(*g_debugger_list_mutex_ptr);
g_debugger_list_ptr->push_back(debugger_sp);
}
debugger_sp->InstanceInitialize();
+
+#ifdef LLVM_BUILD_TELEMETRY
+ if (auto *telemetry_manager = telemetry::TelemetryManager::getInstance()) {
+if (telemetry_manager->getConfig()->EnableTelemetry) {
+ lldb_private::telemetry::DebuggerTelemetryInfo entry;
+ entry.start_time = start_time;
+ entry.end_time = std::chrono::steady_clock::now();
+ entry.debugger = debugger_sp.get();
+ telemetry_manager->atDebuggerStartup(&entry);
+}
+ }
+#endif
oontvoo wrote:
> but I don't know how feasible is that.
I would really like to understand the objection. I recall the reasoning was
something like it's faster to check for the non-existence of "Telemetry"
symbol. If that was the case, we can offer to define some statically defined
symbol like DISABLE_TELEMETRY that would set the
telemetry::Config::EnableTelemetry=false at build time and cannot be overriden.
I believe that would simplify a lot of things
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] Add terminfo dependency for ncurses support (PR #126810)
ajordanr-google wrote: > Okay, I think I understand what's going on with fuchsia: they manually set > CURSES_INCLUDE_DIRS, CURSES_LIBRARIES, and PANEL_LIBRARIES. On line 17 we go > into the else-case because TINFO_LIBRARIES is not set. That's correct! > So next we check if the provided CURSES_LIBRARIES has our symbol... I believe they don't even get this far. They apparently have an issue with find_package/find_library lookup mechanism, and that fails. So it breaks then without even trying to check for the symbol. > (As an aside, while I'm reading this: why is this checking TINFO_LIBRARIES > and not HAS_TERMINFO_SYMBOLS, that's what this package sets. I don't think we > should be mentioning TINFO_LIBRARIES at all, unless we know someone is > setting that manually?) Mostly this variable was derived from the spack commit which uses it. That's the only place I know it exists. Technically now fuchsia is using it too, after they implemented the fix (which also logically makes some sense, `TINFO_LIBRARIES` in their case IS `CURSES_LIBRARIES`. --- If we were to hide TINFO_LIBRARIES entirely in the conditional, we must check `CURSES_LIBRARIES` to see if what's passed has the necessary terminfo symbols (or just tell people it needs to be there). I'm fine with that. We shouldn't _need_ to check `HAS_TERMINFO_SYMBOLS` if they're providing this all directly. --- Cool cool, will do about the `CURSES_LIBRARIES` update. https://github.com/llvm/llvm-project/pull/126810 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [WIP] [lldb][TypeSystemClang] Create clang::SourceLocation from DWARF and attach to AST (PR #127829)
https://github.com/Michael137 edited 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
