[Lldb-commits] [lldb] [lldb][DWARFASTParserClang][NFCI] Factor out CV-qualifier/is_static parsing from ParseChildParameters (PR #123951)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/123951
This patch continues simplifying `ParseChildParameters` by moving out the logic
that parses the first parameter of a function DIE into a helper function. Since
with GCC (and lately Clang) function declarations have `DW_AT_object_pointer`s,
we should be able to check for the attribute's existence to determine if a
function is static (and also deduce CV-qualifiers from it). This will be useful
for cases where the object parameter is explicit (which is possible since
C++23).
This should be NFC. I added a FIXME to places where we assume an implicit
object parameter (which will be addressed in a follow-up patch).
We used to guard parsing of the CV-qualifiers of the "this" parameter with a
`encoding_mask & Type::eEncodingIsPointerUID`, which is incorrect, because
`eEncodingIsPointerUID` cannot be used as a bitmask directly (see
https://github.com/llvm/llvm-project/issues/120856). This patch corrects this,
but it should still be NFC because any parameter in C++ called "this" *is* an
implicit object parameter.
>From 99398439a91ee5130c4907e8ea08f83dc93ee699 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 22 Jan 2025 13:01:44 +
Subject: [PATCH] [lldb][DWARFASTParserClang][NFCI] Factor out
CV-qualifier/is_static parsing from ParseChildParameters
This patch continues simplifying `ParseChildParameters` by moving the logic
that parses the first parameter of a function DIE in case it's an implicit
object parameter. Since with GCC and now Clang function declarations have
`DW_AT_object_pointer`s, we should be able to check for its existence to
determine if a function is static (and also deduce CV-qualifiers from it). This
will be useful for cases where the object parameter is explicit (which is
possible since C++23).
This should be NFC. I added a FIXME to places where we assume an implicit
object parameter (which will be addressed in a follow-up patch).
We used to guard parsing of the CV-qualifiers of the "this" parameter
with a `encoding_mask & Type::eEncodingIsPointerUID`, which is
incorrect, because `eEncodingIsPointerUID` cannot be used as a bitmask
directly (see https://github.com/llvm/llvm-project/issues/120856). This
patch corrects this, but it should still be NFC because any parameter in
C++ called "this" *is* an implicit object parameter.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 169 +++---
.../SymbolFile/DWARF/DWARFASTParserClang.h| 8 +-
2 files changed, 112 insertions(+), 65 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 81a1375c037182..1170ad32b64999 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -159,6 +159,79 @@ static bool TagIsRecordType(dw_tag_t tag) {
}
}
+/// Get the object parameter DIE if one exists, otherwise returns
+/// a default DWARFDIE. If \c containing_decl_ctx is not a valid
+/// C++ declaration context for class methods, assume no object
+/// parameter exists for the given \c subprogram.
+static DWARFDIE
+GetCXXObjectParameter(const DWARFDIE &subprogram,
+ clang::DeclContext const &containing_decl_ctx) {
+ assert(subprogram.Tag() == DW_TAG_subprogram ||
+ subprogram.Tag() == DW_TAG_inlined_subroutine ||
+ subprogram.Tag() == DW_TAG_subroutine_type);
+
+ if (!DeclKindIsCXXClass(containing_decl_ctx.getDeclKind()))
+return {};
+
+ if (!subprogram.HasChildren())
+return {};
+
+ // FIXME: if subprogram has a explicit DW_AT_object_pointer, use it.
+
+ // If no DW_AT_object_pointer was specified, assume the implicit object
+ // parameter is the first parameter to the function, is called "this" and is
+ // artificial (which is what most compilers would generate).
+ auto children = subprogram.children();
+ auto it = llvm::find_if(children, [](const DWARFDIE &child) {
+return child.Tag() == DW_TAG_formal_parameter;
+ });
+
+ if (it == children.end())
+return {};
+
+ DWARFDIE object_pointer = *it;
+
+ if (!object_pointer.GetAttributeValueAsUnsigned(DW_AT_artificial, 0))
+return {};
+
+ // Often times compilers omit the "this" name for the
+ // specification DIEs, so we can't rely upon the name being in
+ // the formal parameter DIE...
+ if (char const *name = object_pointer.GetName();
+ name && ::strcmp(name, "this") == 0)
+return {};
+
+ return object_pointer;
+}
+
+/// In order to determine the CV-qualifiers for a C++ class
+/// method in DWARF, we have to look at the CV-qualifiers of
+/// the object parameter's type.
+static unsigned GetCXXMethodCVQuals(DWARFDIE const &subprogram,
+DWARFDIE const &object_parameter) {
+ if (!subprogram || !object_parameter)
+return 0;
+
+ Type *this_type = subprogram.ResolveTypeU
[Lldb-commits] [lldb] [lldb/windows] Make "anonymous" pipe names more unique (PR #123905)
labath wrote: Yes, the issue is two instances of the atomic counter in two libraries (well, one of them is an executable). I'm not sure why `GetTickCount` would be a better solution though. It returns time in milliseconds, which definitely too coarse-grained. Even if we used an API with greater resolution (I don't know which, but I assume Windows has one), I don't think we can completely rule out collisions. I've recently been porting some code to aarch64, and one of the things I ran into was that (some) arm cpus have a (relatively) low frequency of the hardware clock, which broke code which assumes that functions like `absl::Now()` return monotonically increasing values (usually code which tried to measure the throughput of something, which then crashed due to division by zero). So even if we went with something time-based, I would would still want to add some sort of a backup counter for an additional source of entropy. However, if we have that, I'm not sure if the time is useful. The new pipe name is `lldb.pipe...` (the old one was without the middle member), which seems unique enough (first value identifies a process, second one an address within that process and the last one a value at that address). https://github.com/llvm/llvm-project/pull/123905 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #112079)
@@ -0,0 +1,492 @@
+import os
+import os.path
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbproxy import *
+import lldbgdbserverutils
+import re
+
+
+class ThreadSnapshot:
+def __init__(self, thread_id, registers):
+self.thread_id = thread_id
+self.registers = registers
+
+
+class MemoryBlockSnapshot:
+def __init__(self, address, data):
+self.address = address
+self.data = data
+
+
+class StateSnapshot:
+def __init__(self, thread_snapshots, memory):
+self.thread_snapshots = thread_snapshots
+self.memory = memory
+self.thread_id = None
+
+
+class RegisterInfo:
+def __init__(self, lldb_index, bitsize, little_endian):
+self.lldb_index = lldb_index
+self.bitsize = bitsize
+self.little_endian = little_endian
+
+
+BELOW_STACK_POINTER = 16384
+ABOVE_STACK_POINTER = 4096
+
+BLOCK_SIZE = 1024
+
+SOFTWARE_BREAKPOINTS = 0
+HARDWARE_BREAKPOINTS = 1
+WRITE_WATCHPOINTS = 2
+
+
+class ReverseTestBase(GDBProxyTestBase):
+"""
+Base class for tests that need reverse execution.
+
+This class uses a gdbserver proxy to add very limited reverse-
+execution capability to lldb-server/debugserver for testing
+purposes only.
+
+To use this class, run the inferior forward until some stopping point.
+Then call `start_recording()` and execute forward again until reaching
+a software breakpoint; this class records the state before each execution
executes.
+At that point, the server will accept "bc" and "bs" packets to step
+backwards through the state.
+When executing during recording, we only allow single-step and continue
without
+delivering a signal, and only software breakpoint stops are allowed.
+
+We assume that while recording is enabled, the only effects of instructions
+are on general-purpose registers (read/written by the 'g' and 'G' packets)
+and on memory bytes between [SP - BELOW_STACK_POINTER, SP +
ABOVE_STACK_POINTER).
+"""
+
+NO_DEBUG_INFO_TESTCASE = True
+
+"""
+A list of StateSnapshots in time order.
+
+There is one snapshot per single-stepped instruction,
+representing the state before that instruction was
+executed. The last snapshot in the list is the
+snapshot before the last instruction was executed.
+This is an undo log; we snapshot a superset of the state that may have
+been changed by the instruction's execution.
+"""
+snapshots = None
+recording_enabled = False
+
+breakpoints = None
+
+pc_register_info = None
+sp_register_info = None
+general_purpose_register_info = None
+
+def __init__(self, *args, **kwargs):
+GDBProxyTestBase.__init__(self, *args, **kwargs)
+self.breakpoints = [set(), set(), set(), set(), set()]
+
+def respond(self, packet):
+if not packet:
+raise ValueError("Invalid empty packet")
+if packet == self.server.PACKET_INTERRUPT:
+# Don't send a response. We'll just run to completion.
+return []
+if self.is_command(packet, "qSupported", ":"):
+# Disable multiprocess support in the server and in LLDB
+# since Mac debugserver doesn't support it and we want lldb-server
to
+# be consistent with that
+reply = self.pass_through(packet.replace(";multiprocess", ""))
+return reply.replace(";multiprocess", "") +
";ReverseStep+;ReverseContinue+"
+if packet == "c" or packet == "s":
+packet = "vCont;" + packet
+elif (
+packet[0] == "c" or packet[0] == "s" or packet[0] == "C" or
packet[0] == "S"
+):
+raise ValueError(
+"Old-style continuation packets with address or signal not
supported yet"
+)
+if self.is_command(packet, "vCont", ";"):
+if self.recording_enabled:
+return self.continue_with_recording(packet)
+snapshots = []
+if packet == "bc":
+return self.reverse_continue()
+if packet == "bs":
+return self.reverse_step()
+if packet == "jThreadsInfo":
+# Suppress this because it contains thread stop reasons which we
might
+# need to modify, and we don't want to have to implement that.
+return ""
+if packet[0] == "z" or packet[0] == "Z":
+reply = self.pass_through(packet)
+if reply == "OK":
+self.update_breakpoints(packet)
+return reply
+return GDBProxyTestBase.respond(self, packet)
+
+def start_recording(self):
+self.recording_enabled = True
+self.snapshots = []
+
+def stop_recording(self):
+"""
+Don't record when executing foward.
+
+Reverse execution is still supported until the next forward continue.
+"""
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #112079)
labath wrote: The failure was on the arm(32), not aarch64. (but yes, ptrace-read-only memory sounds like a plausible explanation) https://github.com/llvm/llvm-project/pull/112079 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement a statusline in LLDB (PR #121860)
@@ -0,0 +1,58 @@ +//===-- Statusline.h -===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +#include "lldb/Core/Debugger.h" +#include "llvm/ADT/SmallVector.h" +#include +#ifndef LLDB_CORE_STATUSBAR_H +#define LLDB_CORE_STATUSBAR_H labath wrote: ```suggestion #ifndef LLDB_CORE_STATUSBAR_H #define LLDB_CORE_STATUSBAR_H #include "lldb/Core/Debugger.h" #include "llvm/ADT/SmallVector.h" #include ``` https://github.com/llvm/llvm-project/pull/121860 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/windows] Make "anonymous" pipe names more unique (PR #123905)
https://github.com/slydiman approved this pull request. Ah, I missed `address_of_atomic`. Sorry. LGTM then. https://github.com/llvm/llvm-project/pull/123905 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/windows] Make "anonymous" pipe names more unique (PR #123905)
https://github.com/slydiman edited https://github.com/llvm/llvm-project/pull/123905 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement a statusline in LLDB (PR #121860)
@@ -1958,6 +1984,12 @@ lldb::thread_result_t Debugger::DefaultEventHandler() {
// are now listening to all required events so no events get missed
m_sync_broadcaster.BroadcastEvent(eBroadcastBitEventThreadIsListening);
+ if (!m_statusline && GetShowStatusline())
+m_statusline.emplace(*this);
labath wrote:
So, once it's enabled it can no longer be disabled? Could we send some sort of
an event when the setting changes?
https://github.com/llvm/llvm-project/pull/121860
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement a statusline in LLDB (PR #121860)
@@ -0,0 +1,161 @@
+//===-- Statusline.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/Statusline.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/FormatEntity.h"
+#include "lldb/Host/ThreadLauncher.h"
+#include "lldb/Interpreter/CommandInterpreter.h"
+#include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Target/StackFrame.h"
+#include "lldb/Utility/AnsiTerminal.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/StreamString.h"
+#include "llvm/Support/Locale.h"
+
+#include
+#include
+
+#define ESCAPE "\x1b"
+#define ANSI_SAVE_CURSOR ESCAPE "7"
+#define ANSI_RESTORE_CURSOR ESCAPE "8"
+#define ANSI_CLEAR_BELOW ESCAPE "[J"
+#define ANSI_CLEAR_LINE "\r\x1B[2K"
+#define ANSI_SET_SCROLL_ROWS ESCAPE "[0;%ur"
+#define ANSI_TO_START_OF_ROW ESCAPE "[%u;0f"
+#define ANSI_UP_ROWS ESCAPE "[%dA"
+#define ANSI_DOWN_ROWS ESCAPE "[%dB"
+#define ANSI_FORWARD_COLS ESCAPE "\033[%dC"
+#define ANSI_BACKWARD_COLS ESCAPE "\033[%dD"
+
+using namespace lldb;
+using namespace lldb_private;
+
+static size_t ColumnWidth(llvm::StringRef str) {
+ std::string stripped = ansi::StripAnsiTerminalCodes(str);
+ return llvm::sys::locale::columnWidth(stripped);
+}
+
+Statusline::Statusline(Debugger &debugger) : m_debugger(debugger) {}
+
+Statusline::~Statusline() { Disable(); }
+
+bool Statusline::IsSupported() const {
+ File &file = m_debugger.GetOutputFile();
+ return file.GetIsInteractive() && file.GetIsTerminalWithColors();
+}
+
+void Statusline::Enable() {
+ if (!IsSupported())
+return;
+
+ UpdateTerminalProperties();
+
+ // Reduce the scroll window to make space for the status bar below.
+ SetScrollWindow(m_terminal_height - 1);
+
+ // Draw the statusline.
+ Update();
+}
+
+void Statusline::Disable() {
+ UpdateTerminalProperties();
+ // Clear the previous status bar if any.
+ Clear();
+ // Extend the scroll window to cover the status bar.
+ SetScrollWindow(m_terminal_height);
+}
+
+void Statusline::Draw(llvm::StringRef str) {
+ UpdateTerminalProperties();
+
+ const size_t ellipsis = 3;
+ const size_t column_width = ColumnWidth(str);
+
+ if (column_width + ellipsis >= m_terminal_width)
+str = str.substr(0, m_terminal_width - ellipsis);
+
+ StreamFile &out = m_debugger.GetOutputStream();
+ out << ANSI_SAVE_CURSOR;
+ out.Printf(ANSI_TO_START_OF_ROW, static_cast(m_terminal_height));
+ out << ANSI_CLEAR_LINE;
+ out << str;
+ out << std::string(m_terminal_width - column_width, ' ');
+ out << ansi::FormatAnsiTerminalCodes(k_ansi_suffix);
+ out << ANSI_RESTORE_CURSOR;
+}
+
+void Statusline::Reset() {
+ StreamFile &out = m_debugger.GetOutputStream();
+ out << ANSI_SAVE_CURSOR;
+ out.Printf(ANSI_TO_START_OF_ROW, static_cast(m_terminal_height));
+ out << ANSI_CLEAR_LINE;
+ out << ANSI_RESTORE_CURSOR;
+}
+
+void Statusline::Clear() { Draw(""); }
+
+void Statusline::UpdateTerminalProperties() {
+ if (m_terminal_size_has_changed == 0)
+return;
+
+ // Clear the previous statusline.
+ Reset();
+
+ // Purposely ignore the terminal settings. If the setting doesn't match
+ // reality and we draw the status bar over existing text, we have no way to
+ // recover.
+ struct winsize window_size;
+ if ((isatty(STDIN_FILENO) != 0) &&
+ ::ioctl(STDIN_FILENO, TIOCGWINSZ, &window_size) == 0) {
labath wrote:
- this won't build on windows (it only works in the lldb driver because of a
very ugly hack in `driver/Platform.cpp`)
- Shouldn't this be getting file/fd from the debuggers stdin?
- actually maybe stdout, since stdin might be redirected?
- I'm not convinced this is the right thing to do anyway. What's this trying to
fix/work around?
https://github.com/llvm/llvm-project/pull/121860
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)
https://github.com/DavidSpickett created
https://github.com/llvm/llvm-project/pull/123918
When the Guarded Control Stack (GCS) is enabled, returns cause the
processor to validate that the address at the location pointed to
by gcspr_el0 matches the one in the link register.
```
ret (lr=A) << pc
| GCS |
+=+
| A |
| B | << gcspr_el0
Fault: tried to return to A when you should have returned to B.
```
Therefore when an expression wraper function tries to return to
the expression return address (usually `_start` if there is a libc),
it would fault.
```
ret (lr=_start) << pc
| GCS|
++
| user_func1 |
| user_func2 | << gcspr_el0
Fault: tried to return to _start when you should have return to user_func2.
```
To fix this we must push that return address to the GCS in PrepareTrivialCall.
This value is then consumed by the final return and the expression
completes as expected.
```
ret (lr=_start) << pc
| GCS|
++
| user_func1 |
| user_func2 |
| _start | << gcspr_el0
No fault, we return to _start as normal.
```
The gcspr_el0 register will be restored after expression evaluation
so that the program can continue correctly.
However, due to restrictions in the Linux GCS ABI, we will not
restore the enable bit of gcs_features_enabled. Re-enabling GCS
via ptrace is not supported because it requires memory to be
allocated.
We could disable GCS if the expression enabled GCS, however this
would use up that state transition that the program might later
rely on. And generally it is cleaner to ignore the whole bit
rather than one state transition of it.
We will also not restore the GCS entry that was overwritten with
the expression's return address. On the grounds that:
* This entry will never be used by the program. If the program branches,
the entry will be overwritten. If the program returns, gcspr_el0
will point to the entry before the expression return address
and that entry will instead be validated.
* Any expression that calls functions will overwrite even more
entries, so the user needs to be aware of that anyway if they
want to preserve the contents of the GCS for inspection.
* An expression could leave the program in a state where
restoring the value makes the situation worse. Especially
if we ever support this in bare metal debugging.
I will later document all this on https://lldb.llvm.org/use/aarch64-linux.html
as well.
Tests have been added for:
* A function call that does not interact with GCS.
* A call that does, and disables it (we do not re-enable it).
* A call that does, and enables it (we do not disable it again).
>From 951a38910e49f3e93dc6c9a084e955ab01ad4c49 Mon Sep 17 00:00:00 2001
From: David Spickett
Date: Fri, 9 Aug 2024 11:56:29 +0100
Subject: [PATCH 1/2] [lldb][AArch64] Add Guarded Control Stack registers
The Guarded Control Stack extension implements a shadow stack and
the Linux kernel provides access to 3 registers for it via ptrace.
struct user_gcs {
__u64 features_enabled;
__u64 features_locked;
__u64 gcspr_el0;
};
This commit adds support for reading those from a live process.
The first 2 are pseudo registers based on the real control register
and the 3rd is a real register. This is the stack pointer for the
guarded stack.
I have added a "gcs_" prefix to the "features" registers so that
they have a clear name when shown individually. Also this means
they will tab complete from "gcs", and be next to gcspr_el0 in
any sorted lists of registers.
Guarded Control Stack Registers:
gcs_features_enabled = 0x
gcs_features_locked = 0x
gcspr_el0 = 0x
Testing is more of the usual, where possible I'm writing a register
then doing something in the program to confirm the value was actually
sent to ptrace.
---
.../NativeRegisterContextLinux_arm64.cpp | 86 +++
.../Linux/NativeRegisterContextLinux_arm64.h | 16 +++
.../Utility/RegisterContextPOSIX_arm64.cpp| 4 +
.../Utility/RegisterContextPOSIX_arm64.h | 1 +
.../Utility/RegisterInfoPOSIX_arm64.cpp | 39 -
.../Process/Utility/RegisterInfoPOSIX_arm64.h | 7 +
.../linux/aarch64/gcs/TestAArch64LinuxGCS.py | 134 ++
lldb/test/API/linux/aarch64/gcs/main.c| 23 ++-
8 files changed, 305 insertions(+), 5 deletions(-)
diff --git
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 6056f3001fed6e..efd3385c46e92f 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -64,8 +64,14 @@
#define NT_ARM_FPMR 0x40e /* Floating point mode register */
#endif
+#ifndef NT_ARM_GCS
+#define NT_ARM_GCS 0x410 /* Guarded Control Stack control registers */
+#endif
+
#define HWCAP_PACA (1 << 30)
+#define HWCAP_GCS (1UL << 32)
+
#define HWCAP2
[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: David Spickett (DavidSpickett)
Changes
When the Guarded Control Stack (GCS) is enabled, returns cause the
processor to validate that the address at the location pointed to
by gcspr_el0 matches the one in the link register.
```
ret (lr=A) << pc
| GCS |
+=+
| A |
| B | << gcspr_el0
Fault: tried to return to A when you should have returned to B.
```
Therefore when an expression wraper function tries to return to
the expression return address (usually `_start` if there is a libc),
it would fault.
```
ret (lr=_start) << pc
| GCS|
++
| user_func1 |
| user_func2 | << gcspr_el0
Fault: tried to return to _start when you should have return to user_func2.
```
To fix this we must push that return address to the GCS in PrepareTrivialCall.
This value is then consumed by the final return and the expression
completes as expected.
```
ret (lr=_start) << pc
| GCS|
++
| user_func1 |
| user_func2 |
| _start | << gcspr_el0
No fault, we return to _start as normal.
```
The gcspr_el0 register will be restored after expression evaluation
so that the program can continue correctly.
However, due to restrictions in the Linux GCS ABI, we will not
restore the enable bit of gcs_features_enabled. Re-enabling GCS
via ptrace is not supported because it requires memory to be
allocated.
We could disable GCS if the expression enabled GCS, however this
would use up that state transition that the program might later
rely on. And generally it is cleaner to ignore the whole bit
rather than one state transition of it.
We will also not restore the GCS entry that was overwritten with
the expression's return address. On the grounds that:
* This entry will never be used by the program. If the program branches,
the entry will be overwritten. If the program returns, gcspr_el0
will point to the entry before the expression return address
and that entry will instead be validated.
* Any expression that calls functions will overwrite even more
entries, so the user needs to be aware of that anyway if they
want to preserve the contents of the GCS for inspection.
* An expression could leave the program in a state where
restoring the value makes the situation worse. Especially
if we ever support this in bare metal debugging.
I will later document all this on https://lldb.llvm.org/use/aarch64-linux.html
as well.
Tests have been added for:
* A function call that does not interact with GCS.
* A call that does, and disables it (we do not re-enable it).
* A call that does, and enables it (we do not disable it again).
---
Patch is 32.32 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/123918.diff
9 Files Affected:
- (modified) lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp (+56)
- (modified)
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp (+104)
- (modified)
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h (+16)
- (modified) lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
(+4)
- (modified) lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
(+1)
- (modified) lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
(+38-1)
- (modified) lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h (+7)
- (modified) lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py (+260)
- (modified) lldb/test/API/linux/aarch64/gcs/main.c (+57-5)
``diff
diff --git a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
index 93b8141e97ef86..d843e718b6875e 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
+++ b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
@@ -60,6 +60,59 @@ ABISysV_arm64::CreateInstance(lldb::ProcessSP process_sp,
const ArchSpec &arch)
return ABISP();
}
+static bool PushToLinuxGuardedControlStack(addr_t return_addr,
+ RegisterContext *reg_ctx,
+ Thread &thread) {
+ // If the Guarded Control Stack extension is enabled we need to put the
return
+ // address onto that stack.
+ const RegisterInfo *gcs_features_enabled_info =
+ reg_ctx->GetRegisterInfoByName("gcs_features_enabled");
+ if (!gcs_features_enabled_info)
+return false;
+
+ uint64_t gcs_features_enabled = reg_ctx->ReadRegisterAsUnsigned(
+ gcs_features_enabled_info, LLDB_INVALID_ADDRESS);
+ if (gcs_features_enabled == LLDB_INVALID_ADDRESS)
+return false;
+
+ // Only attempt this if GCS is enabled. If it's not enabled then gcspr_el0
+ // may point to unmapped memory.
+ if ((gcs_features_enabled & 1) == 0)
+return false;
+
+ const RegisterInfo *gcspr_el0_info =
+ reg_ctx->GetRegisterInfoByName("gcspr_el0");
+ if (!gcspr_el0_info)
+return false;
+
+ uint64_t gcspr_el0 =
+ reg_ctx->ReadRegisterAsUnsigned(gcspr_el0_in
[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)
DavidSpickett wrote: The first commit of this is https://github.com/llvm/llvm-project/pull/123720. I'm putting this up now because that previous PR is pretty straightforward, it's just adding registers. But this is the one that needs the most thought. So if you like, you can tackle them in one session while it's fresh in your mind. What I have implemented here should match what I have described in https://github.com/llvm/llvm-project/pull/117860. https://github.com/llvm/llvm-project/pull/123918 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #112079)
@@ -0,0 +1,492 @@
+import os
+import os.path
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbproxy import *
+import lldbgdbserverutils
+import re
+
+
+class ThreadSnapshot:
+def __init__(self, thread_id, registers):
+self.thread_id = thread_id
+self.registers = registers
+
+
+class MemoryBlockSnapshot:
+def __init__(self, address, data):
+self.address = address
+self.data = data
+
+
+class StateSnapshot:
+def __init__(self, thread_snapshots, memory):
+self.thread_snapshots = thread_snapshots
+self.memory = memory
+self.thread_id = None
+
+
+class RegisterInfo:
+def __init__(self, lldb_index, bitsize, little_endian):
+self.lldb_index = lldb_index
+self.bitsize = bitsize
+self.little_endian = little_endian
+
+
+BELOW_STACK_POINTER = 16384
+ABOVE_STACK_POINTER = 4096
+
+BLOCK_SIZE = 1024
+
+SOFTWARE_BREAKPOINTS = 0
+HARDWARE_BREAKPOINTS = 1
+WRITE_WATCHPOINTS = 2
+
+
+class ReverseTestBase(GDBProxyTestBase):
+"""
+Base class for tests that need reverse execution.
+
+This class uses a gdbserver proxy to add very limited reverse-
+execution capability to lldb-server/debugserver for testing
+purposes only.
+
+To use this class, run the inferior forward until some stopping point.
+Then call `start_recording()` and execute forward again until reaching
+a software breakpoint; this class records the state before each execution
executes.
+At that point, the server will accept "bc" and "bs" packets to step
+backwards through the state.
+When executing during recording, we only allow single-step and continue
without
+delivering a signal, and only software breakpoint stops are allowed.
+
+We assume that while recording is enabled, the only effects of instructions
+are on general-purpose registers (read/written by the 'g' and 'G' packets)
+and on memory bytes between [SP - BELOW_STACK_POINTER, SP +
ABOVE_STACK_POINTER).
+"""
+
+NO_DEBUG_INFO_TESTCASE = True
+
+"""
+A list of StateSnapshots in time order.
+
+There is one snapshot per single-stepped instruction,
+representing the state before that instruction was
+executed. The last snapshot in the list is the
+snapshot before the last instruction was executed.
+This is an undo log; we snapshot a superset of the state that may have
+been changed by the instruction's execution.
+"""
+snapshots = None
+recording_enabled = False
+
+breakpoints = None
+
+pc_register_info = None
+sp_register_info = None
+general_purpose_register_info = None
+
+def __init__(self, *args, **kwargs):
+GDBProxyTestBase.__init__(self, *args, **kwargs)
+self.breakpoints = [set(), set(), set(), set(), set()]
+
+def respond(self, packet):
+if not packet:
+raise ValueError("Invalid empty packet")
+if packet == self.server.PACKET_INTERRUPT:
+# Don't send a response. We'll just run to completion.
+return []
+if self.is_command(packet, "qSupported", ":"):
+# Disable multiprocess support in the server and in LLDB
+# since Mac debugserver doesn't support it and we want lldb-server
to
+# be consistent with that
+reply = self.pass_through(packet.replace(";multiprocess", ""))
+return reply.replace(";multiprocess", "") +
";ReverseStep+;ReverseContinue+"
+if packet == "c" or packet == "s":
+packet = "vCont;" + packet
+elif (
+packet[0] == "c" or packet[0] == "s" or packet[0] == "C" or
packet[0] == "S"
+):
+raise ValueError(
+"Old-style continuation packets with address or signal not
supported yet"
+)
+if self.is_command(packet, "vCont", ";"):
+if self.recording_enabled:
+return self.continue_with_recording(packet)
+snapshots = []
+if packet == "bc":
+return self.reverse_continue()
+if packet == "bs":
+return self.reverse_step()
+if packet == "jThreadsInfo":
+# Suppress this because it contains thread stop reasons which we
might
+# need to modify, and we don't want to have to implement that.
+return ""
+if packet[0] == "z" or packet[0] == "Z":
+reply = self.pass_through(packet)
+if reply == "OK":
+self.update_breakpoints(packet)
+return reply
+return GDBProxyTestBase.respond(self, packet)
+
+def start_recording(self):
+self.recording_enabled = True
+self.snapshots = []
+
+def stop_recording(self):
+"""
+Don't record when executing foward.
+
+Reverse execution is still supported until the next forward continue.
+"""
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #112079)
DavidSpickett wrote: Yes, I presented AArch64 as the counter example to show where it does work, but didn't make that clear. I've got a solution for this bit, but the watchpoint test is still failing one part on Arm 32 bit. https://github.com/llvm/llvm-project/pull/112079 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reland "[lldb] Implement basic support for reverse-continue" (#123906)" (PR #123945)
labath wrote: @rocallahan The fix makes sense, but I think we still need to figure out what's the deal with green dragon. I can check out [the arm64 watchpoint failure](https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake/18969/testReport/junit/lldb-api/functionalities_reverse-execution/TestReverseContinueWatchpoints_py/) (eventually), but we might need help with the x86_64 failures. Judging by the error message, I'd say that the x86 debugserver reports some extra registers, which then turn out to be unreadable (@jasonmolenda does that sound plausible?). Maybe we should skip unreadable registers? https://github.com/llvm/llvm-project/pull/123945 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang][NFCI] Factor out CV-qualifier/is_static parsing from ParseChildParameters (PR #123951)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/123951 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang][NFCI] Factor out CV-qualifier/is_static parsing from ParseChildParameters (PR #123951)
@@ -159,6 +159,79 @@ static bool TagIsRecordType(dw_tag_t tag) {
}
}
+/// Get the object parameter DIE if one exists, otherwise returns
+/// a default DWARFDIE. If \c containing_decl_ctx is not a valid
+/// C++ declaration context for class methods, assume no object
+/// parameter exists for the given \c subprogram.
+static DWARFDIE
+GetCXXObjectParameter(const DWARFDIE &subprogram,
+ clang::DeclContext const &containing_decl_ctx) {
labath wrote:
```suggestion
const clang::DeclContext &containing_decl_ctx) {
```
Inconsistent placement of const (through the patch). I think this is the usual
order we use.
https://github.com/llvm/llvm-project/pull/123951
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang][NFCI] Factor out CV-qualifier/is_static parsing from ParseChildParameters (PR #123951)
@@ -159,6 +159,79 @@ static bool TagIsRecordType(dw_tag_t tag) {
}
}
+/// Get the object parameter DIE if one exists, otherwise returns
+/// a default DWARFDIE. If \c containing_decl_ctx is not a valid
+/// C++ declaration context for class methods, assume no object
+/// parameter exists for the given \c subprogram.
+static DWARFDIE
+GetCXXObjectParameter(const DWARFDIE &subprogram,
+ clang::DeclContext const &containing_decl_ctx) {
+ assert(subprogram.Tag() == DW_TAG_subprogram ||
+ subprogram.Tag() == DW_TAG_inlined_subroutine ||
+ subprogram.Tag() == DW_TAG_subroutine_type);
+
+ if (!DeclKindIsCXXClass(containing_decl_ctx.getDeclKind()))
+return {};
+
+ if (!subprogram.HasChildren())
+return {};
+
labath wrote:
```suggestion
```
The code should work even without this.
https://github.com/llvm/llvm-project/pull/123951
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 23fd8f6 - [lldb][DWARFASTParserClang][NFCI] Simplify ParseChildParameters (#123790)
Author: Michael Buch
Date: 2025-01-22T09:21:16Z
New Revision: 23fd8f6f263d14d43fd4b2a599652beaadc9920e
URL:
https://github.com/llvm/llvm-project/commit/23fd8f6f263d14d43fd4b2a599652beaadc9920e
DIFF:
https://github.com/llvm/llvm-project/commit/23fd8f6f263d14d43fd4b2a599652beaadc9920e.diff
LOG: [lldb][DWARFASTParserClang][NFCI] Simplify ParseChildParameters (#123790)
This patch refactors `ParseChildParameters` in a way which makes it (in
my opinion) more readable, removing some redundant local variables in
the process and reduces the scope of some variables.
**Motivation**
Since `DW_AT_object_pointer`s are now attached to declarations, we can
test for their existence to check whether a C++ method is static or not
(whereas currently we're deducing this from `ParseChildParameters` based
on some heuristics we know are true for most compilers). So my plan is
to move the code for determining `type_quals` and `is_static` out of
`ParseChildParameters`. The refactoring in this PR will make this
follow-up patch hopefully easier to review.
**Testing**
* This should be NFC. The main change is that we now no longer iterate
over `GetAttributes()` but instead retrieve the name, type and
is_artificial attributes of the parameters individually.
Added:
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 68fa1d13943a16..81a1375c037182 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -3084,50 +3084,10 @@ size_t DWARFASTParserClang::ParseChildParameters(
const dw_tag_t tag = die.Tag();
switch (tag) {
case DW_TAG_formal_parameter: {
- DWARFAttributes attributes = die.GetAttributes();
- if (attributes.Size() == 0) {
-arg_idx++;
-break;
- }
-
- const char *name = nullptr;
- DWARFFormValue param_type_die_form;
- bool is_artificial = false;
- // one of None, Auto, Register, Extern, Static, PrivateExtern
-
- clang::StorageClass storage = clang::SC_None;
- uint32_t i;
- for (i = 0; i < attributes.Size(); ++i) {
-const dw_attr_t attr = attributes.AttributeAtIndex(i);
-DWARFFormValue form_value;
-if (attributes.ExtractFormValueAtIndex(i, form_value)) {
- switch (attr) {
- case DW_AT_name:
-name = form_value.AsCString();
-break;
- case DW_AT_type:
-param_type_die_form = form_value;
-break;
- case DW_AT_artificial:
-is_artificial = form_value.Boolean();
-break;
- case DW_AT_location:
- case DW_AT_const_value:
- case DW_AT_default_value:
- case DW_AT_description:
- case DW_AT_endianity:
- case DW_AT_is_optional:
- case DW_AT_segment:
- case DW_AT_variable_parameter:
- default:
- case DW_AT_abstract_origin:
- case DW_AT_sibling:
-break;
- }
-}
- }
+ const char *name = die.GetName();
+ DWARFDIE param_type_die =
die.GetAttributeValueAsReferenceDIE(DW_AT_type);
- if (is_artificial) {
+ if (die.GetAttributeValueAsUnsigned(DW_AT_artificial, 0)) {
// In order to determine if a C++ member function is "const" we
// have to look at the const-ness of "this"...
if (arg_idx == 0 &&
@@ -3136,8 +3096,7 @@ size_t DWARFASTParserClang::ParseChildParameters(
// specification DIEs, so we can't rely upon the name being in
// the formal parameter DIE...
(name == nullptr || ::strcmp(name, "this") == 0)) {
- Type *this_type =
die.ResolveTypeUID(param_type_die_form.Reference());
- if (this_type) {
+ if (Type *this_type = die.ResolveTypeUID(param_type_die)) {
uint32_t encoding_mask = this_type->GetEncodingMask();
if (encoding_mask & Type::eEncodingIsPointerUID) {
is_static = false;
@@ -3149,20 +3108,18 @@ size_t DWARFASTParserClang::ParseChildParameters(
}
}
}
- } else {
-Type *type = die.ResolveTypeUID(param_type_die_form.Reference());
-if (type) {
- function_param_types.push_back(type->GetForwardCompilerType());
+ } else if (Type *type = die.ResolveTypeUID(param_type_die)) {
+function_param_types.push_back(type->GetForwardCompilerType());
- clang::ParmVarDecl *param_var_decl =
m_ast.CreateParameterDeclaration(
- containing_decl_ctx, GetOwningClangModule(die), name,
- type->GetForwardCompilerType(), storage);
- assert(param_var_decl);
- function_param_decls.push_back
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang][NFCI] Simplify ParseChildParameters (PR #123790)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/123790 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/windows] Make "anonymous" pipe names more unique (PR #123905)
slydiman wrote: If `two copies of the pipe code` means 2 instances of g_pipe_serial in different dlls, probably it is better to use GetTickCount() or such. https://github.com/llvm/llvm-project/pull/123905 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #112079)
@@ -0,0 +1,492 @@
+import os
+import os.path
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbproxy import *
+import lldbgdbserverutils
+import re
+
+
+class ThreadSnapshot:
+def __init__(self, thread_id, registers):
+self.thread_id = thread_id
+self.registers = registers
+
+
+class MemoryBlockSnapshot:
+def __init__(self, address, data):
+self.address = address
+self.data = data
+
+
+class StateSnapshot:
+def __init__(self, thread_snapshots, memory):
+self.thread_snapshots = thread_snapshots
+self.memory = memory
+self.thread_id = None
+
+
+class RegisterInfo:
+def __init__(self, lldb_index, bitsize, little_endian):
+self.lldb_index = lldb_index
+self.bitsize = bitsize
+self.little_endian = little_endian
+
+
+BELOW_STACK_POINTER = 16384
+ABOVE_STACK_POINTER = 4096
+
+BLOCK_SIZE = 1024
+
+SOFTWARE_BREAKPOINTS = 0
+HARDWARE_BREAKPOINTS = 1
+WRITE_WATCHPOINTS = 2
+
+
+class ReverseTestBase(GDBProxyTestBase):
+"""
+Base class for tests that need reverse execution.
+
+This class uses a gdbserver proxy to add very limited reverse-
+execution capability to lldb-server/debugserver for testing
+purposes only.
+
+To use this class, run the inferior forward until some stopping point.
+Then call `start_recording()` and execute forward again until reaching
+a software breakpoint; this class records the state before each execution
executes.
+At that point, the server will accept "bc" and "bs" packets to step
+backwards through the state.
+When executing during recording, we only allow single-step and continue
without
+delivering a signal, and only software breakpoint stops are allowed.
+
+We assume that while recording is enabled, the only effects of instructions
+are on general-purpose registers (read/written by the 'g' and 'G' packets)
+and on memory bytes between [SP - BELOW_STACK_POINTER, SP +
ABOVE_STACK_POINTER).
+"""
+
+NO_DEBUG_INFO_TESTCASE = True
+
+"""
+A list of StateSnapshots in time order.
+
+There is one snapshot per single-stepped instruction,
+representing the state before that instruction was
+executed. The last snapshot in the list is the
+snapshot before the last instruction was executed.
+This is an undo log; we snapshot a superset of the state that may have
+been changed by the instruction's execution.
+"""
+snapshots = None
+recording_enabled = False
+
+breakpoints = None
+
+pc_register_info = None
+sp_register_info = None
+general_purpose_register_info = None
+
+def __init__(self, *args, **kwargs):
+GDBProxyTestBase.__init__(self, *args, **kwargs)
+self.breakpoints = [set(), set(), set(), set(), set()]
+
+def respond(self, packet):
+if not packet:
+raise ValueError("Invalid empty packet")
+if packet == self.server.PACKET_INTERRUPT:
+# Don't send a response. We'll just run to completion.
+return []
+if self.is_command(packet, "qSupported", ":"):
+# Disable multiprocess support in the server and in LLDB
+# since Mac debugserver doesn't support it and we want lldb-server
to
+# be consistent with that
+reply = self.pass_through(packet.replace(";multiprocess", ""))
+return reply.replace(";multiprocess", "") +
";ReverseStep+;ReverseContinue+"
+if packet == "c" or packet == "s":
+packet = "vCont;" + packet
+elif (
+packet[0] == "c" or packet[0] == "s" or packet[0] == "C" or
packet[0] == "S"
+):
+raise ValueError(
+"Old-style continuation packets with address or signal not
supported yet"
+)
+if self.is_command(packet, "vCont", ";"):
+if self.recording_enabled:
+return self.continue_with_recording(packet)
+snapshots = []
+if packet == "bc":
+return self.reverse_continue()
+if packet == "bs":
+return self.reverse_step()
+if packet == "jThreadsInfo":
+# Suppress this because it contains thread stop reasons which we
might
+# need to modify, and we don't want to have to implement that.
+return ""
+if packet[0] == "z" or packet[0] == "Z":
+reply = self.pass_through(packet)
+if reply == "OK":
+self.update_breakpoints(packet)
+return reply
+return GDBProxyTestBase.respond(self, packet)
+
+def start_recording(self):
+self.recording_enabled = True
+self.snapshots = []
+
+def stop_recording(self):
+"""
+Don't record when executing foward.
+
+Reverse execution is still supported until the next forward continue.
+"""
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang][NFCI] Factor out CV-qualifier/is_static parsing from ParseChildParameters (PR #123951)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/123951
>From 99398439a91ee5130c4907e8ea08f83dc93ee699 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 22 Jan 2025 13:01:44 +
Subject: [PATCH 1/2] [lldb][DWARFASTParserClang][NFCI] Factor out
CV-qualifier/is_static parsing from ParseChildParameters
This patch continues simplifying `ParseChildParameters` by moving the logic
that parses the first parameter of a function DIE in case it's an implicit
object parameter. Since with GCC and now Clang function declarations have
`DW_AT_object_pointer`s, we should be able to check for its existence to
determine if a function is static (and also deduce CV-qualifiers from it). This
will be useful for cases where the object parameter is explicit (which is
possible since C++23).
This should be NFC. I added a FIXME to places where we assume an implicit
object parameter (which will be addressed in a follow-up patch).
We used to guard parsing of the CV-qualifiers of the "this" parameter
with a `encoding_mask & Type::eEncodingIsPointerUID`, which is
incorrect, because `eEncodingIsPointerUID` cannot be used as a bitmask
directly (see https://github.com/llvm/llvm-project/issues/120856). This
patch corrects this, but it should still be NFC because any parameter in
C++ called "this" *is* an implicit object parameter.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 169 +++---
.../SymbolFile/DWARF/DWARFASTParserClang.h| 8 +-
2 files changed, 112 insertions(+), 65 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 81a1375c037182..1170ad32b64999 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -159,6 +159,79 @@ static bool TagIsRecordType(dw_tag_t tag) {
}
}
+/// Get the object parameter DIE if one exists, otherwise returns
+/// a default DWARFDIE. If \c containing_decl_ctx is not a valid
+/// C++ declaration context for class methods, assume no object
+/// parameter exists for the given \c subprogram.
+static DWARFDIE
+GetCXXObjectParameter(const DWARFDIE &subprogram,
+ clang::DeclContext const &containing_decl_ctx) {
+ assert(subprogram.Tag() == DW_TAG_subprogram ||
+ subprogram.Tag() == DW_TAG_inlined_subroutine ||
+ subprogram.Tag() == DW_TAG_subroutine_type);
+
+ if (!DeclKindIsCXXClass(containing_decl_ctx.getDeclKind()))
+return {};
+
+ if (!subprogram.HasChildren())
+return {};
+
+ // FIXME: if subprogram has a explicit DW_AT_object_pointer, use it.
+
+ // If no DW_AT_object_pointer was specified, assume the implicit object
+ // parameter is the first parameter to the function, is called "this" and is
+ // artificial (which is what most compilers would generate).
+ auto children = subprogram.children();
+ auto it = llvm::find_if(children, [](const DWARFDIE &child) {
+return child.Tag() == DW_TAG_formal_parameter;
+ });
+
+ if (it == children.end())
+return {};
+
+ DWARFDIE object_pointer = *it;
+
+ if (!object_pointer.GetAttributeValueAsUnsigned(DW_AT_artificial, 0))
+return {};
+
+ // Often times compilers omit the "this" name for the
+ // specification DIEs, so we can't rely upon the name being in
+ // the formal parameter DIE...
+ if (char const *name = object_pointer.GetName();
+ name && ::strcmp(name, "this") == 0)
+return {};
+
+ return object_pointer;
+}
+
+/// In order to determine the CV-qualifiers for a C++ class
+/// method in DWARF, we have to look at the CV-qualifiers of
+/// the object parameter's type.
+static unsigned GetCXXMethodCVQuals(DWARFDIE const &subprogram,
+DWARFDIE const &object_parameter) {
+ if (!subprogram || !object_parameter)
+return 0;
+
+ Type *this_type = subprogram.ResolveTypeUID(
+ object_parameter.GetAttributeValueAsReferenceDIE(DW_AT_type));
+ if (!this_type)
+return 0;
+
+ uint32_t encoding_mask = this_type->GetEncodingMask();
+
+ // FIXME: explicit object parameters need not to be pointers
+ if (!(encoding_mask & (1u << Type::eEncodingIsPointerUID)))
+return 0;
+
+ unsigned cv_quals = 0;
+ if (encoding_mask & (1u << Type::eEncodingIsConstUID))
+cv_quals |= clang::Qualifiers::Const;
+ if (encoding_mask & (1u << Type::eEncodingIsVolatileUID))
+cv_quals |= clang::Qualifiers::Volatile;
+
+ return cv_quals;
+}
+
TypeSP DWARFASTParserClang::ParseTypeFromClangModule(const SymbolContext &sc,
const DWARFDIE &die,
Log *log) {
@@ -1188,11 +1261,8 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
const dw_tag_t tag = die.Tag();
bool is_variadic = false;
- bool is_static = false;
bool has_template_params = false;
- unsigned type_qual
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang][NFCI] Factor out CV-qualifier/is_static parsing from ParseChildParameters (PR #123951)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Michael Buch (Michael137)
Changes
This patch continues simplifying `ParseChildParameters` by moving out the logic
that parses the first parameter of a function DIE into a helper function. Since
with GCC (and lately Clang) function declarations have `DW_AT_object_pointer`s,
we should be able to check for the attribute's existence to determine if a
function is static (and also deduce CV-qualifiers from it). This will be useful
for cases where the object parameter is explicit (which is possible since
C++23).
This should be NFC. I added a FIXME to places where we assume an implicit
object parameter (which will be addressed in a follow-up patch).
We used to guard parsing of the CV-qualifiers of the "this" parameter with a
`encoding_mask & Type::eEncodingIsPointerUID`, which is incorrect, because
`eEncodingIsPointerUID` cannot be used as a bitmask directly (see
https://github.com/llvm/llvm-project/issues/120856). This patch corrects this,
but it should still be NFC because any parameter in C++ called "this" *is* an
implicit object parameter.
---
Full diff: https://github.com/llvm/llvm-project/pull/123951.diff
2 Files Affected:
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
(+109-60)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h (+3-5)
``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 81a1375c037182..1170ad32b64999 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -159,6 +159,79 @@ static bool TagIsRecordType(dw_tag_t tag) {
}
}
+/// Get the object parameter DIE if one exists, otherwise returns
+/// a default DWARFDIE. If \c containing_decl_ctx is not a valid
+/// C++ declaration context for class methods, assume no object
+/// parameter exists for the given \c subprogram.
+static DWARFDIE
+GetCXXObjectParameter(const DWARFDIE &subprogram,
+ clang::DeclContext const &containing_decl_ctx) {
+ assert(subprogram.Tag() == DW_TAG_subprogram ||
+ subprogram.Tag() == DW_TAG_inlined_subroutine ||
+ subprogram.Tag() == DW_TAG_subroutine_type);
+
+ if (!DeclKindIsCXXClass(containing_decl_ctx.getDeclKind()))
+return {};
+
+ if (!subprogram.HasChildren())
+return {};
+
+ // FIXME: if subprogram has a explicit DW_AT_object_pointer, use it.
+
+ // If no DW_AT_object_pointer was specified, assume the implicit object
+ // parameter is the first parameter to the function, is called "this" and is
+ // artificial (which is what most compilers would generate).
+ auto children = subprogram.children();
+ auto it = llvm::find_if(children, [](const DWARFDIE &child) {
+return child.Tag() == DW_TAG_formal_parameter;
+ });
+
+ if (it == children.end())
+return {};
+
+ DWARFDIE object_pointer = *it;
+
+ if (!object_pointer.GetAttributeValueAsUnsigned(DW_AT_artificial, 0))
+return {};
+
+ // Often times compilers omit the "this" name for the
+ // specification DIEs, so we can't rely upon the name being in
+ // the formal parameter DIE...
+ if (char const *name = object_pointer.GetName();
+ name && ::strcmp(name, "this") == 0)
+return {};
+
+ return object_pointer;
+}
+
+/// In order to determine the CV-qualifiers for a C++ class
+/// method in DWARF, we have to look at the CV-qualifiers of
+/// the object parameter's type.
+static unsigned GetCXXMethodCVQuals(DWARFDIE const &subprogram,
+DWARFDIE const &object_parameter) {
+ if (!subprogram || !object_parameter)
+return 0;
+
+ Type *this_type = subprogram.ResolveTypeUID(
+ object_parameter.GetAttributeValueAsReferenceDIE(DW_AT_type));
+ if (!this_type)
+return 0;
+
+ uint32_t encoding_mask = this_type->GetEncodingMask();
+
+ // FIXME: explicit object parameters need not to be pointers
+ if (!(encoding_mask & (1u << Type::eEncodingIsPointerUID)))
+return 0;
+
+ unsigned cv_quals = 0;
+ if (encoding_mask & (1u << Type::eEncodingIsConstUID))
+cv_quals |= clang::Qualifiers::Const;
+ if (encoding_mask & (1u << Type::eEncodingIsVolatileUID))
+cv_quals |= clang::Qualifiers::Volatile;
+
+ return cv_quals;
+}
+
TypeSP DWARFASTParserClang::ParseTypeFromClangModule(const SymbolContext &sc,
const DWARFDIE &die,
Log *log) {
@@ -1188,11 +1261,8 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
const dw_tag_t tag = die.Tag();
bool is_variadic = false;
- bool is_static = false;
bool has_template_params = false;
- unsigned type_quals = 0;
-
DEBUG_PRINTF("0x%8.8" PRIx64 ": %s (\"%s\")\n", die.GetID(),
DW_TAG_value_to_name(tag), type_name_cstr);
@@ -1215,23 +1285,15 @@
[Lldb-commits] [lldb] Reland "[lldb] Implement basic support for reverse-continue" (#123906)" (PR #123945)
@@ -0,0 +1,528 @@
+import os
+import os.path
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbproxy import *
+import lldbgdbserverutils
+import re
+
+
+class ThreadSnapshot:
+def __init__(self, thread_id, registers):
+self.thread_id = thread_id
+self.registers = registers
+
+
+class MemoryBlockSnapshot:
+def __init__(self, address, data):
+self.address = address
+self.data = data
+
+
+class StateSnapshot:
+def __init__(self, thread_snapshots, memory):
+self.thread_snapshots = thread_snapshots
+self.memory = memory
+self.thread_id = None
+
+
+class RegisterInfo:
+def __init__(self, lldb_index, bitsize, little_endian):
+self.lldb_index = lldb_index
+self.bitsize = bitsize
+self.little_endian = little_endian
+
+
+BELOW_STACK_POINTER = 16384
+ABOVE_STACK_POINTER = 4096
+
+BLOCK_SIZE = 1024
+
+SOFTWARE_BREAKPOINTS = 0
+HARDWARE_BREAKPOINTS = 1
+WRITE_WATCHPOINTS = 2
+
+
+class ReverseTestBase(GDBProxyTestBase):
+"""
+Base class for tests that need reverse execution.
+
+This class uses a gdbserver proxy to add very limited reverse-
+execution capability to lldb-server/debugserver for testing
+purposes only.
+
+To use this class, run the inferior forward until some stopping point.
+Then call `start_recording()` and execute forward again until reaching
+a software breakpoint; this class records the state before each execution
executes.
+At that point, the server will accept "bc" and "bs" packets to step
+backwards through the state.
+When executing during recording, we only allow single-step and continue
without
+delivering a signal, and only software breakpoint stops are allowed.
+
+We assume that while recording is enabled, the only effects of instructions
+are on general-purpose registers (read/written by the 'g' and 'G' packets)
+and on memory bytes between [SP - BELOW_STACK_POINTER, SP +
ABOVE_STACK_POINTER).
+"""
+
+NO_DEBUG_INFO_TESTCASE = True
+
+"""
+A list of StateSnapshots in time order.
+
+There is one snapshot per single-stepped instruction,
+representing the state before that instruction was
+executed. The last snapshot in the list is the
+snapshot before the last instruction was executed.
+This is an undo log; we snapshot a superset of the state that may have
+been changed by the instruction's execution.
+"""
+snapshots = None
+recording_enabled = False
+
+breakpoints = None
+
+pc_register_info = None
+sp_register_info = None
+general_purpose_register_info = None
+
+def __init__(self, *args, **kwargs):
+GDBProxyTestBase.__init__(self, *args, **kwargs)
+self.breakpoints = [set(), set(), set(), set(), set()]
+
+def respond(self, packet):
+if not packet:
+raise ValueError("Invalid empty packet")
+if packet == self.server.PACKET_INTERRUPT:
+# Don't send a response. We'll just run to completion.
+return []
+if self.is_command(packet, "qSupported", ":"):
+# Disable multiprocess support in the server and in LLDB
+# since Mac debugserver doesn't support it and we want lldb-server
to
+# be consistent with that
+reply = self.pass_through(packet.replace(";multiprocess", ""))
+return reply.replace(";multiprocess", "") +
";ReverseStep+;ReverseContinue+"
+if packet == "c" or packet == "s":
+packet = "vCont;" + packet
+elif (
+packet[0] == "c" or packet[0] == "s" or packet[0] == "C" or
packet[0] == "S"
+):
+raise ValueError(
+"Old-style continuation packets with address or signal not
supported yet"
+)
+if self.is_command(packet, "vCont", ";"):
+if self.recording_enabled:
+return self.continue_with_recording(packet)
+snapshots = []
+if packet == "bc":
+return self.reverse_continue()
+if packet == "bs":
+return self.reverse_step()
+if packet == "jThreadsInfo":
+# Suppress this because it contains thread stop reasons which we
might
+# need to modify, and we don't want to have to implement that.
+return ""
+if packet[0] == "z" or packet[0] == "Z":
+reply = self.pass_through(packet)
+if reply == "OK":
+self.update_breakpoints(packet)
+return reply
+return GDBProxyTestBase.respond(self, packet)
+
+def start_recording(self):
+self.recording_enabled = True
+self.snapshots = []
+
+def stop_recording(self):
+"""
+Don't record when executing foward.
+
+Reverse execution is still supported until the next forward continue.
+"""
[Lldb-commits] [lldb] Reland "[lldb] Implement basic support for reverse-continue" (#123906)" (PR #123945)
@@ -0,0 +1,528 @@
+import os
+import os.path
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.gdbclientutils import *
+from lldbsuite.test.lldbgdbproxy import *
+import lldbgdbserverutils
+import re
+
+
+class ThreadSnapshot:
+def __init__(self, thread_id, registers):
+self.thread_id = thread_id
+self.registers = registers
+
+
+class MemoryBlockSnapshot:
+def __init__(self, address, data):
+self.address = address
+self.data = data
+
+
+class StateSnapshot:
+def __init__(self, thread_snapshots, memory):
+self.thread_snapshots = thread_snapshots
+self.memory = memory
+self.thread_id = None
+
+
+class RegisterInfo:
+def __init__(self, lldb_index, bitsize, little_endian):
+self.lldb_index = lldb_index
+self.bitsize = bitsize
+self.little_endian = little_endian
+
+
+BELOW_STACK_POINTER = 16384
+ABOVE_STACK_POINTER = 4096
+
+BLOCK_SIZE = 1024
+
+SOFTWARE_BREAKPOINTS = 0
+HARDWARE_BREAKPOINTS = 1
+WRITE_WATCHPOINTS = 2
+
+
+class ReverseTestBase(GDBProxyTestBase):
+"""
+Base class for tests that need reverse execution.
+
+This class uses a gdbserver proxy to add very limited reverse-
+execution capability to lldb-server/debugserver for testing
+purposes only.
+
+To use this class, run the inferior forward until some stopping point.
+Then call `start_recording()` and execute forward again until reaching
+a software breakpoint; this class records the state before each execution
executes.
+At that point, the server will accept "bc" and "bs" packets to step
+backwards through the state.
+When executing during recording, we only allow single-step and continue
without
+delivering a signal, and only software breakpoint stops are allowed.
+
+We assume that while recording is enabled, the only effects of instructions
+are on general-purpose registers (read/written by the 'g' and 'G' packets)
+and on memory bytes between [SP - BELOW_STACK_POINTER, SP +
ABOVE_STACK_POINTER).
+"""
+
+NO_DEBUG_INFO_TESTCASE = True
+
+"""
+A list of StateSnapshots in time order.
+
+There is one snapshot per single-stepped instruction,
+representing the state before that instruction was
+executed. The last snapshot in the list is the
+snapshot before the last instruction was executed.
+This is an undo log; we snapshot a superset of the state that may have
+been changed by the instruction's execution.
+"""
+snapshots = None
+recording_enabled = False
+
+breakpoints = None
+
+pc_register_info = None
+sp_register_info = None
+general_purpose_register_info = None
+
+def __init__(self, *args, **kwargs):
+GDBProxyTestBase.__init__(self, *args, **kwargs)
+self.breakpoints = [set(), set(), set(), set(), set()]
+
+def respond(self, packet):
+if not packet:
+raise ValueError("Invalid empty packet")
+if packet == self.server.PACKET_INTERRUPT:
+# Don't send a response. We'll just run to completion.
+return []
+if self.is_command(packet, "qSupported", ":"):
+# Disable multiprocess support in the server and in LLDB
+# since Mac debugserver doesn't support it and we want lldb-server
to
+# be consistent with that
+reply = self.pass_through(packet.replace(";multiprocess", ""))
+return reply.replace(";multiprocess", "") +
";ReverseStep+;ReverseContinue+"
+if packet == "c" or packet == "s":
+packet = "vCont;" + packet
+elif (
+packet[0] == "c" or packet[0] == "s" or packet[0] == "C" or
packet[0] == "S"
+):
+raise ValueError(
+"Old-style continuation packets with address or signal not
supported yet"
+)
+if self.is_command(packet, "vCont", ";"):
+if self.recording_enabled:
+return self.continue_with_recording(packet)
+snapshots = []
+if packet == "bc":
+return self.reverse_continue()
+if packet == "bs":
+return self.reverse_step()
+if packet == "jThreadsInfo":
+# Suppress this because it contains thread stop reasons which we
might
+# need to modify, and we don't want to have to implement that.
+return ""
+if packet[0] == "z" or packet[0] == "Z":
+reply = self.pass_through(packet)
+if reply == "OK":
+self.update_breakpoints(packet)
+return reply
+return GDBProxyTestBase.respond(self, packet)
+
+def start_recording(self):
+self.recording_enabled = True
+self.snapshots = []
+
+def stop_recording(self):
+"""
+Don't record when executing foward.
+
+Reverse execution is still supported until the next forward continue.
+"""
[Lldb-commits] [lldb] [lldb] Add SymbolContext::GetAddress (PR #123340)
@@ -370,6 +370,31 @@ bool SymbolContext::GetAddressRange(uint32_t scope,
uint32_t range_idx,
return false;
}
+Address SymbolContext::GetAddress(uint32_t scope,
+ bool use_inline_block_range) const {
+ if ((scope & eSymbolContextLineEntry) && line_entry.IsValid())
+return line_entry.range.GetBaseAddress();
+
+ if (scope & eSymbolContextBlock) {
+Block *block_to_use = (block && use_inline_block_range)
+ ? block->GetContainingInlinedBlock()
+ : block;
+if (block_to_use) {
+ Address addr;
+ block_to_use->GetStartAddress(addr);
+ return addr;
+}
+ }
labath wrote:
> Giving a single address to a block considered as a logical entity is
> problematic, but the address of the start of the block referenced by a
> SymbolContext doesn't have that problem.
I guess, if you define the "start of a block" as "the lowest address in the
block". This definition is sort of what I have a problem with because we have
also defined the "start of a function" as the "lowest address in the function",
but then also used it as "the entry point of the function", and this was fine
until it wasn't. I don't think we're going to have the same problem with blocks
that we did with functions, but I don't think this is completely right either.
I also don't really have an alternative proposal, and we already do have
`Block::GetStartAddress`, which I have no intention of touching, so if you say
this is okay, I'm happy to go along with it.
https://github.com/llvm/llvm-project/pull/123340
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SymbolContext::GetAddress (PR #123340)
https://github.com/labath updated
https://github.com/llvm/llvm-project/pull/123340
>From 2d6210ad9527df5147987f856e941e61d9851a97 Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Fri, 17 Jan 2025 12:36:36 +0100
Subject: [PATCH 1/3] [lldb] Add SymbolContext::GetAddress
Many (most?) uses of SC::GetAddressRange were not interested in the
range, but in the address of the function/symbol contained inside the
symbol context. They were getting that by calling the GetBaseAddress on
the returned range, which worked well enough so far, but isn't
compatible with discontinuous functions, whose address (entry point) may
not be the lowest address in the range.
To resolve this problem, this PR creates a new function whose purpose is
return the address of the object inside the symbol context ("address of
a block" is a somewhat fuzzy concept, but I've made it so that mirrors
what the GetAddressRange function does). It also changes all of the
callers of GetAddressRange which do not actually care about the range to
call this function instead.
I've also changed all of the callers which are interested in the entry
point of a function/symbol to pass eSymbolContextFunction |
eSymbolContextSymbol instead of eSymbolContextEverything, as a block or
a line will not have a callable address.
---
lldb/include/lldb/Symbol/SymbolContext.h | 27
lldb/source/Commands/CommandObjectTarget.cpp | 17 +++---
.../Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp | 6 ++--
.../MacOSX-DYLD/DynamicLoaderDarwin.cpp | 20 +---
.../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 6 ++--
.../CPlusPlus/CPPLanguageRuntime.cpp | 6 ++--
.../Process/Utility/InferiorCallPOSIX.cpp | 32 ---
.../MacOSX/SystemRuntimeMacOSX.cpp| 24 +-
lldb/source/Symbol/SymbolContext.cpp | 25 +++
9 files changed, 95 insertions(+), 68 deletions(-)
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h
b/lldb/include/lldb/Symbol/SymbolContext.h
index 07769cd8dffae7..fde11f275dc60a 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -192,6 +192,33 @@ class SymbolContext {
bool GetAddressRange(uint32_t scope, uint32_t range_idx,
bool use_inline_block_range, AddressRange &range) const;
+ /// Get the address represented by this symbol context.
+ ///
+ /// The exact meaning of the address depends on object being queried and the
+ /// flags. Priority is as follows:
+ /// - The beginning (lowest address) of the line_entry if line_entry is
+ /// valid and eSymbolContextLineEntry is set in \a scope
+ /// - The beginning of the block if block is not nullptr and
+ /// eSymbolContextBlock is set in \a scope
+ /// - function address (entry point) if function is not nullptr and
+ /// eSymbolContextFunction is set in \a scope
+ /// - symbol address if symbol is not nullptr and eSymbolContextSymbol is
+ /// set in \a scope
+ ///
+ /// \param[in] scope
+ /// A mask of symbol context bits telling this function which
+ /// address ranges it can use when trying to extract one from
+ /// the valid (non-nullptr) symbol context classes.
+ ///
+ /// \param[in] use_inline_block_range
+ /// If \a scope has the eSymbolContextBlock bit set, and there
+ /// is a valid block in the symbol context, return the block
+ /// address range for the containing inline function block, not
+ /// the deepest most block. This allows us to extract information
+ /// for the address range of the inlined function block, not
+ /// the deepest lexical block.
+ Address GetAddress(uint32_t scope, bool use_inline_block_range) const;
+
llvm::Error GetAddressRangeFromHereToEndLine(uint32_t end_line,
AddressRange &range);
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp
b/lldb/source/Commands/CommandObjectTarget.cpp
index d8265e41a7384e..705c69167badd3 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -1621,12 +1621,8 @@ static void DumpSymbolContextList(
if (!first_module)
strm.EOL();
-AddressRange range;
-
-sc.GetAddressRange(eSymbolContextEverything, 0, true, range);
-
-DumpAddress(exe_scope, range.GetBaseAddress(), verbose, all_ranges, strm,
-settings);
+Address addr = sc.GetAddress(eSymbolContextEverything, true);
+DumpAddress(exe_scope, addr, verbose, all_ranges, strm, settings);
first_module = false;
}
strm.IndentLess();
@@ -3570,16 +3566,13 @@ class CommandObjectTargetModulesShowUnwind : public
CommandObjectParsed {
continue;
if (!sc.module_sp || sc.module_sp->GetObjectFile() == nullptr)
continue;
- AddressRange range;
- if (!sc.GetAddressRange(eSymbolContextFunction | eSymbolContextSymbol, 0,
-
[Lldb-commits] [lldb] [lldb] Add SymbolContext::GetAddress (PR #123340)
@@ -192,6 +192,33 @@ class SymbolContext {
bool GetAddressRange(uint32_t scope, uint32_t range_idx,
bool use_inline_block_range, AddressRange &range) const;
+ /// Get the address represented by this symbol context.
+ ///
+ /// The exact meaning of the address depends on object being queried and the
+ /// flags. Priority is as follows:
+ /// - The beginning (lowest address) of the line_entry if line_entry is
+ /// valid and eSymbolContextLineEntry is set in \a scope
+ /// - The beginning of the block if block is not nullptr and
+ /// eSymbolContextBlock is set in \a scope
+ /// - function address (entry point) if function is not nullptr and
+ /// eSymbolContextFunction is set in \a scope
+ /// - symbol address if symbol is not nullptr and eSymbolContextSymbol is
+ /// set in \a scope
+ ///
+ /// \param[in] scope
+ /// A mask of symbol context bits telling this function which
labath wrote:
Okay, I can do that.
https://github.com/llvm/llvm-project/pull/123340
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reland "[lldb] Implement basic support for reverse-continue" (#123906)" (PR #123945)
https://github.com/DavidSpickett created https://github.com/llvm/llvm-project/pull/123945 This reverts commit 22561cfb443267905d4190f0e2a738e6b412457f and fixes b7b9ccf44988edf49886743ae5c3cf4184db211f (#112079). The problem is that x86_64 and Arm 32-bit have memory regions above the stack that are readable but not writeable. First Arm: ``` (lldb) memory region --all <...> [0xfffcf000-0x) rw- [stack] [0x-0x1000) r-x [vectors] [0x1000-0x) --- ``` Then x86_64: ``` $ cat /proc/self/maps <...> 7ffdcd148000-7ffdcd16a000 rw-p 00:00 0 [stack] 7ffdcd193000-7ffdcd196000 r--p 00:00 0 [vvar] 7ffdcd196000-7ffdcd197000 r-xp 00:00 0 [vdso] ff60-ff601000 --xp 00:00 0 [vsyscall] ``` Compare this to AArch64 where the test did pass: ``` $ cat /proc/self/maps <...> b87dc000-b87dd000 r--p 00:00 0 [vvar] b87dd000-b87de000 r-xp 00:00 0 [vdso] b87de000-b87e r--p 0002a000 00:3c 76927217 /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1 b87e-b87e2000 rw-p 0002c000 00:3c 76927217 /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1 f4216000-f4237000 rw-p 00:00 0 [stack] ``` To solve this, look up the memory region of the stack pointer (using https://lldb.llvm.org/resources/lldbgdbremote.html#qmemoryregioninfo-addr) and constrain the read to within that region. Since we know the stack is all readable and writeable. >From ecb1b90e109df650ef1b50cc3d07b56fd302e274 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Wed, 22 Jan 2025 10:52:16 + Subject: [PATCH] Reland "[lldb] Implement basic support for reverse-continue" (#123906)" This reverts commit 22561cfb443267905d4190f0e2a738e6b412457f and fixes b7b9ccf44988edf49886743ae5c3cf4184db211f (#112079). The problem is that x86_64 and Arm 32-bit have memory regions above the stack that are readable but not writeable. First Arm: ``` (lldb) memory region --all <...> [0xfffcf000-0x) rw- [stack] [0x-0x1000) r-x [vectors] [0x1000-0x) --- ``` Then x86_64: ``` $ cat /proc/self/maps <...> 7ffdcd148000-7ffdcd16a000 rw-p 00:00 0 [stack] 7ffdcd193000-7ffdcd196000 r--p 00:00 0 [vvar] 7ffdcd196000-7ffdcd197000 r-xp 00:00 0 [vdso] ff60-ff601000 --xp 00:00 0 [vsyscall] ``` Compare this to AArch64 where the test did pass: ``` $ cat /proc/self/maps <...> b87dc000-b87dd000 r--p 00:00 0 [vvar] b87dd000-b87de000 r-xp 00:00 0 [vdso] b87de000-b87e r--p 0002a000 00:3c 76927217 /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1 b87e-b87e2000 rw-p 0002c000 00:3c 76927217 /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1 f4216000-f4237000 rw-p 00:00 0 [stack] ``` To solve this, look up the memory region of the stack pointer and constrain the read to within that region. Since we know the stack is all readable and writeable. --- lldb/include/lldb/API/SBProcess.h | 1 + lldb/include/lldb/Target/Process.h| 28 +- lldb/include/lldb/Target/StopInfo.h | 7 + lldb/include/lldb/Target/Thread.h | 9 +- lldb/include/lldb/Target/ThreadList.h | 6 +- lldb/include/lldb/Target/ThreadPlan.h | 13 + lldb/include/lldb/Target/ThreadPlanBase.h | 2 + lldb/include/lldb/lldb-enumerations.h | 6 + .../Python/lldbsuite/test/gdbclientutils.py | 5 +- .../Python/lldbsuite/test/lldbgdbproxy.py | 175 ++ .../Python/lldbsuite/test/lldbreverse.py | 528 ++ .../Python/lldbsuite/test/lldbtest.py | 2 + .../tools/lldb-server/lldbgdbserverutils.py | 14 +- lldb/source/API/SBProcess.cpp | 12 + lldb/source/API/SBThread.cpp | 2 + .../source/Interpreter/CommandInterpreter.cpp | 3 +- .../Process/Linux/NativeThreadLinux.cpp | 3 + .../Process/MacOSX-Kernel/ProcessKDP.cpp | 8 +- .../Process/MacOSX-Kernel/ProcessKDP.h| 2 +- .../Process/Windows/Common/ProcessWindows.cpp | 9 +- .../Process/Windows/Common/ProcessWindows.h | 2 +- .../GDBRemoteCommunicationClient.cpp | 20 + .../gdb-remote/GDBRemoteCommunicationClient.h | 6 + .../GDBRemoteCommunicationServerLLGS.cpp | 1 + .../Process/gdb-remote/ProcessGDBRemote.cpp | 98 +++- .../Process/gdb-remote/ProcessGDBRemote.h | 4 +- .../Process/scripted/S
[Lldb-commits] [lldb] Reland "[lldb] Implement basic support for reverse-continue" (#123906)" (PR #123945)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: David Spickett (DavidSpickett)
Changes
This reverts commit 22561cfb443267905d4190f0e2a738e6b412457f and fixes
b7b9ccf44988edf49886743ae5c3cf4184db211f (#112079).
The problem is that x86_64 and Arm 32-bit have memory regions above the stack
that are readable but not writeable. First Arm:
```
(lldb) memory region --all
<...>
[0xfffcf000-0x) rw- [stack]
[0x-0x1000) r-x [vectors]
[0x1000-0x) ---
```
Then x86_64:
```
$ cat /proc/self/maps
<...>
7ffdcd148000-7ffdcd16a000 rw-p 00:00 0 [stack]
7ffdcd193000-7ffdcd196000 r--p 00:00 0 [vvar]
7ffdcd196000-7ffdcd197000 r-xp 00:00 0 [vdso]
ff60-ff601000 --xp 00:00 0
[vsyscall]
```
Compare this to AArch64 where the test did pass:
```
$ cat /proc/self/maps
<...>
b87dc000-b87dd000 r--p 00:00 0 [vvar]
b87dd000-b87de000 r-xp 00:00 0 [vdso]
b87de000-b87e r--p 0002a000 00:3c 76927217
/usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1
b87e-b87e2000 rw-p 0002c000 00:3c 76927217
/usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1
f4216000-f4237000 rw-p 00:00 0 [stack]
```
To solve this, look up the memory region of the stack pointer (using
https://lldb.llvm.org/resources/lldbgdbremote.html#qmemoryregioninfo-addr) and
constrain the read to within that region. Since we know the stack is all
readable and writeable.
---
Patch is 85.26 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/123945.diff
40 Files Affected:
- (modified) lldb/include/lldb/API/SBProcess.h (+1)
- (modified) lldb/include/lldb/Target/Process.h (+26-2)
- (modified) lldb/include/lldb/Target/StopInfo.h (+7)
- (modified) lldb/include/lldb/Target/Thread.h (+4-5)
- (modified) lldb/include/lldb/Target/ThreadList.h (+5-1)
- (modified) lldb/include/lldb/Target/ThreadPlan.h (+13)
- (modified) lldb/include/lldb/Target/ThreadPlanBase.h (+2)
- (modified) lldb/include/lldb/lldb-enumerations.h (+6)
- (modified) lldb/packages/Python/lldbsuite/test/gdbclientutils.py (+3-2)
- (added) lldb/packages/Python/lldbsuite/test/lldbgdbproxy.py (+175)
- (added) lldb/packages/Python/lldbsuite/test/lldbreverse.py (+528)
- (modified) lldb/packages/Python/lldbsuite/test/lldbtest.py (+2)
- (modified)
lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py
(+9-5)
- (modified) lldb/source/API/SBProcess.cpp (+12)
- (modified) lldb/source/API/SBThread.cpp (+2)
- (modified) lldb/source/Interpreter/CommandInterpreter.cpp (+2-1)
- (modified) lldb/source/Plugins/Process/Linux/NativeThreadLinux.cpp (+3)
- (modified) lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp (+7-1)
- (modified) lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.h (+1-1)
- (modified) lldb/source/Plugins/Process/Windows/Common/ProcessWindows.cpp
(+8-1)
- (modified) lldb/source/Plugins/Process/Windows/Common/ProcessWindows.h (+1-1)
- (modified)
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (+20)
- (modified)
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h (+6)
- (modified)
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
(+1)
- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
(+85-13)
- (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (+3-1)
- (modified) lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp (+7-2)
- (modified) lldb/source/Plugins/Process/scripted/ScriptedProcess.h (+1-1)
- (modified) lldb/source/Target/Process.cpp (+20-4)
- (modified) lldb/source/Target/StopInfo.cpp (+28)
- (modified) lldb/source/Target/Thread.cpp (+6-3)
- (modified) lldb/source/Target/ThreadList.cpp (+29-3)
- (modified) lldb/source/Target/ThreadPlanBase.cpp (+4)
- (added) lldb/test/API/functionalities/reverse-execution/Makefile (+3)
- (added)
lldb/test/API/functionalities/reverse-execution/TestReverseContinueBreakpoints.py
(+149)
- (added)
lldb/test/API/functionalities/reverse-execution/TestReverseContinueNotSupported.py
(+31)
- (added)
lldb/test/API/functionalities/reverse-execution/TestReverseContinueWatchpoints.py
(+130)
- (added) lldb/test/API/functionalities/reverse-execution/main.c (+25)
- (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+3)
- (modified) lldb/tools/lldb-dap/LLDBUtils.cpp (+1)
``diff
diff --git a/lldb/include/lldb/API/SBProcess.h
b/lldb/include/lldb/API/SBProcess.h
index 1624e02070b1b2..882b8bd837131d 100644
--- a/lldb/include/lldb/API/SBProcess.h
+++ b/lldb/include/lldb/API/SBProcess.h
@@ -159,6 +159,7 @@ class LLDB_API SBProcess {
lldb::SBError Destroy();
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #112079)
DavidSpickett wrote: https://github.com/llvm/llvm-project/pull/123945 to fix the testing. https://github.com/llvm/llvm-project/pull/112079 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reland "[lldb] Implement basic support for reverse-continue" (#123906)" (PR #123945)
DavidSpickett wrote: I also noticed that if you don't save memory at all, the breakpoint tests pass. Which is surprising, but ok given that the test doesn't check any memory values, and watchpoint tests *do* fail if you do not save memory. https://github.com/llvm/llvm-project/pull/123945 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb/windows] Make "anonymous" pipe names more unique (PR #123905)
https://github.com/ashgti approved this pull request. https://github.com/llvm/llvm-project/pull/123905 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang][NFCI] Factor out CV-qualifier/is_static parsing from ParseChildParameters (PR #123951)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/123951
>From 99398439a91ee5130c4907e8ea08f83dc93ee699 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Wed, 22 Jan 2025 13:01:44 +
Subject: [PATCH 1/3] [lldb][DWARFASTParserClang][NFCI] Factor out
CV-qualifier/is_static parsing from ParseChildParameters
This patch continues simplifying `ParseChildParameters` by moving the logic
that parses the first parameter of a function DIE in case it's an implicit
object parameter. Since with GCC and now Clang function declarations have
`DW_AT_object_pointer`s, we should be able to check for its existence to
determine if a function is static (and also deduce CV-qualifiers from it). This
will be useful for cases where the object parameter is explicit (which is
possible since C++23).
This should be NFC. I added a FIXME to places where we assume an implicit
object parameter (which will be addressed in a follow-up patch).
We used to guard parsing of the CV-qualifiers of the "this" parameter
with a `encoding_mask & Type::eEncodingIsPointerUID`, which is
incorrect, because `eEncodingIsPointerUID` cannot be used as a bitmask
directly (see https://github.com/llvm/llvm-project/issues/120856). This
patch corrects this, but it should still be NFC because any parameter in
C++ called "this" *is* an implicit object parameter.
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 169 +++---
.../SymbolFile/DWARF/DWARFASTParserClang.h| 8 +-
2 files changed, 112 insertions(+), 65 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 81a1375c037182..1170ad32b64999 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -159,6 +159,79 @@ static bool TagIsRecordType(dw_tag_t tag) {
}
}
+/// Get the object parameter DIE if one exists, otherwise returns
+/// a default DWARFDIE. If \c containing_decl_ctx is not a valid
+/// C++ declaration context for class methods, assume no object
+/// parameter exists for the given \c subprogram.
+static DWARFDIE
+GetCXXObjectParameter(const DWARFDIE &subprogram,
+ clang::DeclContext const &containing_decl_ctx) {
+ assert(subprogram.Tag() == DW_TAG_subprogram ||
+ subprogram.Tag() == DW_TAG_inlined_subroutine ||
+ subprogram.Tag() == DW_TAG_subroutine_type);
+
+ if (!DeclKindIsCXXClass(containing_decl_ctx.getDeclKind()))
+return {};
+
+ if (!subprogram.HasChildren())
+return {};
+
+ // FIXME: if subprogram has a explicit DW_AT_object_pointer, use it.
+
+ // If no DW_AT_object_pointer was specified, assume the implicit object
+ // parameter is the first parameter to the function, is called "this" and is
+ // artificial (which is what most compilers would generate).
+ auto children = subprogram.children();
+ auto it = llvm::find_if(children, [](const DWARFDIE &child) {
+return child.Tag() == DW_TAG_formal_parameter;
+ });
+
+ if (it == children.end())
+return {};
+
+ DWARFDIE object_pointer = *it;
+
+ if (!object_pointer.GetAttributeValueAsUnsigned(DW_AT_artificial, 0))
+return {};
+
+ // Often times compilers omit the "this" name for the
+ // specification DIEs, so we can't rely upon the name being in
+ // the formal parameter DIE...
+ if (char const *name = object_pointer.GetName();
+ name && ::strcmp(name, "this") == 0)
+return {};
+
+ return object_pointer;
+}
+
+/// In order to determine the CV-qualifiers for a C++ class
+/// method in DWARF, we have to look at the CV-qualifiers of
+/// the object parameter's type.
+static unsigned GetCXXMethodCVQuals(DWARFDIE const &subprogram,
+DWARFDIE const &object_parameter) {
+ if (!subprogram || !object_parameter)
+return 0;
+
+ Type *this_type = subprogram.ResolveTypeUID(
+ object_parameter.GetAttributeValueAsReferenceDIE(DW_AT_type));
+ if (!this_type)
+return 0;
+
+ uint32_t encoding_mask = this_type->GetEncodingMask();
+
+ // FIXME: explicit object parameters need not to be pointers
+ if (!(encoding_mask & (1u << Type::eEncodingIsPointerUID)))
+return 0;
+
+ unsigned cv_quals = 0;
+ if (encoding_mask & (1u << Type::eEncodingIsConstUID))
+cv_quals |= clang::Qualifiers::Const;
+ if (encoding_mask & (1u << Type::eEncodingIsVolatileUID))
+cv_quals |= clang::Qualifiers::Volatile;
+
+ return cv_quals;
+}
+
TypeSP DWARFASTParserClang::ParseTypeFromClangModule(const SymbolContext &sc,
const DWARFDIE &die,
Log *log) {
@@ -1188,11 +1261,8 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
const dw_tag_t tag = die.Tag();
bool is_variadic = false;
- bool is_static = false;
bool has_template_params = false;
- unsigned type_qual
[Lldb-commits] [lldb] [lldb] Implement a statusline in LLDB (PR #121860)
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/121860
>From b0cf540a08958ead1c7b9e38f30a2722fe780592 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Fri, 17 Jan 2025 17:10:36 -0800
Subject: [PATCH] [lldb] Implement a statusline in LLDB
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Add a statusline to command-line LLDB to display progress events and
other information related to the current state of the debugger. The
statusline is a dedicated area displayed the bottom of the screen. The
contents of the status line are configurable through a setting
consisting of LLDB’s format strings.
The statusline is configurable through the `statusline-format` setting.
The default configuration shows the target name, the current file, the
stop reason and the current progress event.
```
(lldb) settings show statusline-format
statusline-format (format-string) =
"${ansi.bg.cyan}${ansi.fg.black}{${target.file.basename}}{ |
${line.file.basename}:${line.number}:${line.column}}{ | ${thread.stop-reason}}{
| {${progress.count} }${progress.message}}"
```
The statusline is enabled by default, but can be disabled with the
following setting:
```
(lldb) settings set show-statusline false
```
The statusline supersedes the current progress reporting implementation.
Consequently, the following settings no longer have any effect (but
continue to exist):
```
show-progress -- Whether to show progress or not if the debugger's
output is an interactive color-enabled terminal.
show-progress-ansi-prefix -- When displaying progress in a color-enabled
terminal, use the ANSI terminal code specified in this format immediately
before the progress message.
show-progress-ansi-suffix -- When displaying progress in a color-enabled
terminal, use the ANSI terminal code specified in this format immediately after
the progress message.
```
RFC: https://discourse.llvm.org/t/rfc-lldb-statusline/83948
---
lldb/include/lldb/Core/Debugger.h| 21 ++-
lldb/include/lldb/Core/FormatEntity.h| 4 +-
lldb/include/lldb/Core/Statusline.h | 63 +
lldb/include/lldb/Utility/AnsiTerminal.h | 1 +
lldb/source/Core/CMakeLists.txt | 1 +
lldb/source/Core/CoreProperties.td | 8 ++
lldb/source/Core/Debugger.cpp| 148 ++--
lldb/source/Core/FormatEntity.cpp| 46 ++-
lldb/source/Core/Statusline.cpp | 165 +++
9 files changed, 373 insertions(+), 84 deletions(-)
create mode 100644 lldb/include/lldb/Core/Statusline.h
create mode 100644 lldb/source/Core/Statusline.cpp
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 70f4c4216221c6..a4da5fd44c17fe 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -19,6 +19,7 @@
#include "lldb/Core/FormatEntity.h"
#include "lldb/Core/IOHandler.h"
#include "lldb/Core/SourceManager.h"
+#include "lldb/Core/Statusline.h"
#include "lldb/Core/UserSettingsController.h"
#include "lldb/Host/HostThread.h"
#include "lldb/Host/StreamFile.h"
@@ -308,6 +309,10 @@ class Debugger : public
std::enable_shared_from_this,
bool SetShowProgress(bool show_progress);
+ bool GetShowStatusline() const;
+
+ const FormatEntity::Entry *GetStatuslineFormat() const;
+
llvm::StringRef GetShowProgressAnsiPrefix() const;
llvm::StringRef GetShowProgressAnsiSuffix() const;
@@ -604,6 +609,14 @@ class Debugger : public
std::enable_shared_from_this,
return m_source_file_cache;
}
+ struct ProgressReport {
+uint64_t id;
+uint64_t completed;
+uint64_t total;
+std::string message;
+ };
+ std::optional GetCurrentProgressReport() const;
+
protected:
friend class CommandInterpreter;
friend class REPL;
@@ -728,7 +741,7 @@ class Debugger : public
std::enable_shared_from_this,
IOHandlerStack m_io_handler_stack;
std::recursive_mutex m_io_handler_synchronous_mutex;
- std::optional m_current_event_id;
+ std::optional m_statusline;
llvm::StringMap> m_stream_handlers;
std::shared_ptr m_callback_handler_sp;
@@ -745,6 +758,12 @@ class Debugger : public
std::enable_shared_from_this,
lldb::TargetSP m_dummy_target_sp;
Diagnostics::CallbackID m_diagnostics_callback_id;
+ /// Bookkeeping for command line progress events.
+ /// @{
+ llvm::SmallVector m_progress_reports;
+ mutable std::mutex m_progress_reports_mutex;
+ /// @}
+
std::mutex m_destroy_callback_mutex;
lldb::callback_token_t m_destroy_callback_next_token = 0;
struct DestroyCallbackInfo {
diff --git a/lldb/include/lldb/Core/FormatEntity.h
b/lldb/include/lldb/Core/FormatEntity.h
index c9d5af1f31673b..51e9ce37e54e79 100644
--- a/lldb/include/lldb/Core/FormatEntity.h
+++ b/lldb/include/lldb/Core/FormatEntity.h
@@ -100,7 +100,9 @@ struct Entry {
LineEntryColumn,
LineEntryStartAddress,
LineEntryEndAddress,
-CurrentPCArrow
+
[Lldb-commits] [lldb] [LLDB-DAP] Send Progress update message over DAP (PR #123837)
github-actions[bot] wrote:
:warning: Python code formatter, darker found issues in your code. :warning:
You can test this locally with the following command:
``bash
darker --check --diff -r
8e79ade49d68c49aeb8ba008b59f559b86d22765...56846b2fb5f491311af5ebfe98750c3159259e66
lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
``
View the diff from darker here.
``diff
--- Progress_emitter.py 2025-01-22 19:56:15.00 +
+++ Progress_emitter.py 2025-01-22 20:01:41.770338 +
@@ -58,11 +58,10 @@
def __init__(self, debugger, unused):
self.parser = self.create_options()
self.help_string = self.parser.format_help()
def __call__(self, debugger, command, exe_ctx, result):
-
command_args = shlex.split(command)
try:
(cmd_options, args) = self.parser.parse_args(command_args)
except:
result.SetError("option parsing failed")
``
https://github.com/llvm/llvm-project/pull/123837
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB-DAP] Send Progress update message over DAP (PR #123837)
https://github.com/Jlalond updated
https://github.com/llvm/llvm-project/pull/123837
>From bc7bb2471e44ca7441f592611fd293f2b8c90435 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Tue, 21 Jan 2025 14:34:49 -0800
Subject: [PATCH 1/3] Have Progress message updates include their messages when
sent over DAP
---
lldb/tools/lldb-dap/ProgressEvent.cpp | 15 ++-
lldb/tools/lldb-dap/ProgressEvent.h | 3 ++-
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/lldb/tools/lldb-dap/ProgressEvent.cpp
b/lldb/tools/lldb-dap/ProgressEvent.cpp
index 0dcc2ee81001d5..7807ecfb6c34d0 100644
--- a/lldb/tools/lldb-dap/ProgressEvent.cpp
+++ b/lldb/tools/lldb-dap/ProgressEvent.cpp
@@ -117,6 +117,10 @@ json::Value ProgressEvent::ToJSON() const {
body.try_emplace("cancellable", false);
}
+ if (m_event_type == progressUpdate) {
+EmplaceSafeString(body, "message", m_message);
+ }
+
std::string timestamp(llvm::formatv("{0:f9}", m_creation_time.count()));
EmplaceSafeString(body, "timestamp", timestamp);
@@ -164,10 +168,11 @@ const ProgressEvent
&ProgressEventManager::GetMostRecentEvent() const {
return m_last_update_event ? *m_last_update_event : m_start_event;
}
-void ProgressEventManager::Update(uint64_t progress_id, uint64_t completed,
- uint64_t total) {
- if (std::optional event = ProgressEvent::Create(
- progress_id, std::nullopt, completed, total, &GetMostRecentEvent()))
{
+void ProgressEventManager::Update(uint64_t progress_id, const char *message,
+ uint64_t completed, uint64_t total) {
+ if (std::optional event =
+ ProgressEvent::Create(progress_id, StringRef(message), completed,
+total, &GetMostRecentEvent())) {
if (event->GetEventType() == progressEnd)
m_finished = true;
@@ -227,7 +232,7 @@ void ProgressEventReporter::Push(uint64_t progress_id,
const char *message,
m_unreported_start_events.push(event_manager);
}
} else {
-it->second->Update(progress_id, completed, total);
+it->second->Update(progress_id, message, completed, total);
if (it->second->Finished())
m_event_managers.erase(it);
}
diff --git a/lldb/tools/lldb-dap/ProgressEvent.h
b/lldb/tools/lldb-dap/ProgressEvent.h
index 72317b879c803a..8494221890a1c8 100644
--- a/lldb/tools/lldb-dap/ProgressEvent.h
+++ b/lldb/tools/lldb-dap/ProgressEvent.h
@@ -99,7 +99,8 @@ class ProgressEventManager {
/// Receive a new progress event for the start event and try to report it if
/// appropriate.
- void Update(uint64_t progress_id, uint64_t completed, uint64_t total);
+ void Update(uint64_t progress_id, const char *message, uint64_t completed,
+ uint64_t total);
/// \return
/// \b true if a \a progressEnd event has been notified. There's no
>From faefdb6487eb97cca56755a20597f3d6a7348c6c Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Tue, 21 Jan 2025 15:16:46 -0800
Subject: [PATCH 2/3] PR feedback, pass string ref instead of char *
---
lldb/tools/lldb-dap/ProgressEvent.cpp | 12 +---
lldb/tools/lldb-dap/ProgressEvent.h | 2 +-
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/lldb/tools/lldb-dap/ProgressEvent.cpp
b/lldb/tools/lldb-dap/ProgressEvent.cpp
index 7807ecfb6c34d0..6a4978c055e516 100644
--- a/lldb/tools/lldb-dap/ProgressEvent.cpp
+++ b/lldb/tools/lldb-dap/ProgressEvent.cpp
@@ -117,9 +117,8 @@ json::Value ProgressEvent::ToJSON() const {
body.try_emplace("cancellable", false);
}
- if (m_event_type == progressUpdate) {
+ if (m_event_type == progressUpdate)
EmplaceSafeString(body, "message", m_message);
- }
std::string timestamp(llvm::formatv("{0:f9}", m_creation_time.count()));
EmplaceSafeString(body, "timestamp", timestamp);
@@ -168,11 +167,10 @@ const ProgressEvent
&ProgressEventManager::GetMostRecentEvent() const {
return m_last_update_event ? *m_last_update_event : m_start_event;
}
-void ProgressEventManager::Update(uint64_t progress_id, const char *message,
+void ProgressEventManager::Update(uint64_t progress_id, llvm::StringRef
message,
uint64_t completed, uint64_t total) {
- if (std::optional event =
- ProgressEvent::Create(progress_id, StringRef(message), completed,
-total, &GetMostRecentEvent())) {
+ if (std::optional event = ProgressEvent::Create(
+ progress_id, message, completed, total, &GetMostRecentEvent())) {
if (event->GetEventType() == progressEnd)
m_finished = true;
@@ -232,7 +230,7 @@ void ProgressEventReporter::Push(uint64_t progress_id,
const char *message,
m_unreported_start_events.push(event_manager);
}
} else {
-it->second->Update(progress_id, message, completed, total);
+it->second->Update(progress_id, StringRef(message), completed, total);
if (it->second->Finished())
m_event_manager
[Lldb-commits] [lldb] [lldb] Add SymbolContext::GetAddress (PR #123340)
@@ -370,6 +370,31 @@ bool SymbolContext::GetAddressRange(uint32_t scope,
uint32_t range_idx,
return false;
}
+Address SymbolContext::GetAddress(uint32_t scope,
+ bool use_inline_block_range) const {
+ if ((scope & eSymbolContextLineEntry) && line_entry.IsValid())
+return line_entry.range.GetBaseAddress();
+
+ if (scope & eSymbolContextBlock) {
+Block *block_to_use = (block && use_inline_block_range)
+ ? block->GetContainingInlinedBlock()
+ : block;
+if (block_to_use) {
+ Address addr;
+ block_to_use->GetStartAddress(addr);
+ return addr;
+}
+ }
jimingham wrote:
I needed to think about this a little more.
For functions, there's an actually useful notion of "Start" - functions get
entered from one address, and the question "where would I put a breakpoint to
capture the first instruction of this function's execution" is one we want the
answer to. Even if you have alternate entry points, those usually are named
separately, and I think you'd be surprised if you asked to stop on the function
name and got one of the alternate entry points instead. So calling it
GetStartAddress was reasonable, and when asking for the "address" of a
SymbolContext that's just got a Function, the entry point address is pretty
clearly what you want to know.
But there's no similar useful notion for blocks. After all, C++ goto's don't
make new scopes, so you can't say "this is the first bit of code that will get
executed when this block is executed." We probably should have called
Block::GetStartAddress: Block::GetLowestAddress.
I confused myself because I'm more often using LineEntry's and not Blocks.
Those entities seem similar but the are not. Blocks actually represent logical
entities and contain all the ranges for that logical entity. But LineEntry's
do not represent the logical entity "the contributions to the code from this
source line". They are "one out of many possible contributions from this
source line." So when you ask a LineEntry for it's address, you're just
getting "the lowest address of the particular range that the address you made
the SymbolContext from happened to lie within".
However, some kind of consistency would be useful. For instance, if I get the
outermost Function Block from a Function, make a SymbolContext from that Block,
and then ask for it's Address, it really should return the same answer as if I
had made the SymbolContext from the Function directly. If I'm reading the code
correctly, that won't necessarily be true, will it? For the Function, you
promise to return the entry point, but for the Function's outermost Block, we
return the start of the first address range.
That does seem potentially confusing to me.
https://github.com/llvm/llvm-project/pull/123340
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SymbolContext::GetAddress (PR #123340)
https://github.com/jimingham edited https://github.com/llvm/llvm-project/pull/123340 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement a statusline in LLDB (PR #121860)
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/121860
>From 9fd8d303ebc289e5b362e7252ae2f105a421a2e3 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Fri, 17 Jan 2025 17:10:36 -0800
Subject: [PATCH] [lldb] Implement a statusline in LLDB
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Add a statusline to command-line LLDB to display progress events and
other information related to the current state of the debugger. The
statusline is a dedicated area displayed the bottom of the screen. The
contents of the status line are configurable through a setting
consisting of LLDB’s format strings.
The statusline is configurable through the `statusline-format` setting.
The default configuration shows the target name, the current file, the
stop reason and the current progress event.
```
(lldb) settings show statusline-format
statusline-format (format-string) =
"${ansi.bg.cyan}${ansi.fg.black}{${target.file.basename}}{ |
${line.file.basename}:${line.number}:${line.column}}{ | ${thread.stop-reason}}{
| {${progress.count} }${progress.message}}"
```
The statusline is enabled by default, but can be disabled with the
following setting:
```
(lldb) settings set show-statusline false
```
The statusline supersedes the current progress reporting implementation.
Consequently, the following settings no longer have any effect (but
continue to exist):
```
show-progress -- Whether to show progress or not if the debugger's
output is an interactive color-enabled terminal.
show-progress-ansi-prefix -- When displaying progress in a color-enabled
terminal, use the ANSI terminal code specified in this format immediately
before the progress message.
show-progress-ansi-suffix -- When displaying progress in a color-enabled
terminal, use the ANSI terminal code specified in this format immediately after
the progress message.
```
RFC: https://discourse.llvm.org/t/rfc-lldb-statusline/83948
---
lldb/include/lldb/Core/Debugger.h| 21 ++-
lldb/include/lldb/Core/FormatEntity.h| 4 +-
lldb/include/lldb/Core/Statusline.h | 58 +
lldb/include/lldb/Utility/AnsiTerminal.h | 1 +
lldb/source/Core/CMakeLists.txt | 1 +
lldb/source/Core/CoreProperties.td | 8 ++
lldb/source/Core/Debugger.cpp| 148 +++---
lldb/source/Core/FormatEntity.cpp| 46 ++-
lldb/source/Core/Statusline.cpp | 155 +++
9 files changed, 358 insertions(+), 84 deletions(-)
create mode 100644 lldb/include/lldb/Core/Statusline.h
create mode 100644 lldb/source/Core/Statusline.cpp
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 70f4c4216221c6..a4da5fd44c17fe 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -19,6 +19,7 @@
#include "lldb/Core/FormatEntity.h"
#include "lldb/Core/IOHandler.h"
#include "lldb/Core/SourceManager.h"
+#include "lldb/Core/Statusline.h"
#include "lldb/Core/UserSettingsController.h"
#include "lldb/Host/HostThread.h"
#include "lldb/Host/StreamFile.h"
@@ -308,6 +309,10 @@ class Debugger : public
std::enable_shared_from_this,
bool SetShowProgress(bool show_progress);
+ bool GetShowStatusline() const;
+
+ const FormatEntity::Entry *GetStatuslineFormat() const;
+
llvm::StringRef GetShowProgressAnsiPrefix() const;
llvm::StringRef GetShowProgressAnsiSuffix() const;
@@ -604,6 +609,14 @@ class Debugger : public
std::enable_shared_from_this,
return m_source_file_cache;
}
+ struct ProgressReport {
+uint64_t id;
+uint64_t completed;
+uint64_t total;
+std::string message;
+ };
+ std::optional GetCurrentProgressReport() const;
+
protected:
friend class CommandInterpreter;
friend class REPL;
@@ -728,7 +741,7 @@ class Debugger : public
std::enable_shared_from_this,
IOHandlerStack m_io_handler_stack;
std::recursive_mutex m_io_handler_synchronous_mutex;
- std::optional m_current_event_id;
+ std::optional m_statusline;
llvm::StringMap> m_stream_handlers;
std::shared_ptr m_callback_handler_sp;
@@ -745,6 +758,12 @@ class Debugger : public
std::enable_shared_from_this,
lldb::TargetSP m_dummy_target_sp;
Diagnostics::CallbackID m_diagnostics_callback_id;
+ /// Bookkeeping for command line progress events.
+ /// @{
+ llvm::SmallVector m_progress_reports;
+ mutable std::mutex m_progress_reports_mutex;
+ /// @}
+
std::mutex m_destroy_callback_mutex;
lldb::callback_token_t m_destroy_callback_next_token = 0;
struct DestroyCallbackInfo {
diff --git a/lldb/include/lldb/Core/FormatEntity.h
b/lldb/include/lldb/Core/FormatEntity.h
index c9d5af1f31673b..51e9ce37e54e79 100644
--- a/lldb/include/lldb/Core/FormatEntity.h
+++ b/lldb/include/lldb/Core/FormatEntity.h
@@ -100,7 +100,9 @@ struct Entry {
LineEntryColumn,
LineEntryStartAddress,
LineEntryEndAddress,
-CurrentPCArrow
+
[Lldb-commits] [lldb] [lldb] Implement a statusline in LLDB (PR #121860)
@@ -0,0 +1,161 @@
+//===-- Statusline.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/Statusline.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/FormatEntity.h"
+#include "lldb/Host/ThreadLauncher.h"
+#include "lldb/Interpreter/CommandInterpreter.h"
+#include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Target/StackFrame.h"
+#include "lldb/Utility/AnsiTerminal.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/StreamString.h"
+#include "llvm/Support/Locale.h"
+
+#include
+#include
+
+#define ESCAPE "\x1b"
+#define ANSI_SAVE_CURSOR ESCAPE "7"
+#define ANSI_RESTORE_CURSOR ESCAPE "8"
+#define ANSI_CLEAR_BELOW ESCAPE "[J"
+#define ANSI_CLEAR_LINE "\r\x1B[2K"
+#define ANSI_SET_SCROLL_ROWS ESCAPE "[0;%ur"
+#define ANSI_TO_START_OF_ROW ESCAPE "[%u;0f"
+#define ANSI_UP_ROWS ESCAPE "[%dA"
+#define ANSI_DOWN_ROWS ESCAPE "[%dB"
+#define ANSI_FORWARD_COLS ESCAPE "\033[%dC"
+#define ANSI_BACKWARD_COLS ESCAPE "\033[%dD"
+
+using namespace lldb;
+using namespace lldb_private;
+
+static size_t ColumnWidth(llvm::StringRef str) {
+ std::string stripped = ansi::StripAnsiTerminalCodes(str);
+ return llvm::sys::locale::columnWidth(stripped);
+}
+
+Statusline::Statusline(Debugger &debugger) : m_debugger(debugger) {}
+
+Statusline::~Statusline() { Disable(); }
+
+bool Statusline::IsSupported() const {
+ File &file = m_debugger.GetOutputFile();
+ return file.GetIsInteractive() && file.GetIsTerminalWithColors();
+}
+
+void Statusline::Enable() {
+ if (!IsSupported())
+return;
+
+ UpdateTerminalProperties();
+
+ // Reduce the scroll window to make space for the status bar below.
+ SetScrollWindow(m_terminal_height - 1);
+
+ // Draw the statusline.
+ Update();
+}
+
+void Statusline::Disable() {
+ UpdateTerminalProperties();
+ // Clear the previous status bar if any.
+ Clear();
+ // Extend the scroll window to cover the status bar.
+ SetScrollWindow(m_terminal_height);
+}
+
+void Statusline::Draw(llvm::StringRef str) {
+ UpdateTerminalProperties();
+
+ const size_t ellipsis = 3;
+ const size_t column_width = ColumnWidth(str);
+
+ if (column_width + ellipsis >= m_terminal_width)
+str = str.substr(0, m_terminal_width - ellipsis);
+
+ StreamFile &out = m_debugger.GetOutputStream();
+ out << ANSI_SAVE_CURSOR;
+ out.Printf(ANSI_TO_START_OF_ROW, static_cast(m_terminal_height));
+ out << ANSI_CLEAR_LINE;
+ out << str;
+ out << std::string(m_terminal_width - column_width, ' ');
+ out << ansi::FormatAnsiTerminalCodes(k_ansi_suffix);
+ out << ANSI_RESTORE_CURSOR;
+}
+
+void Statusline::Reset() {
+ StreamFile &out = m_debugger.GetOutputStream();
+ out << ANSI_SAVE_CURSOR;
+ out.Printf(ANSI_TO_START_OF_ROW, static_cast(m_terminal_height));
+ out << ANSI_CLEAR_LINE;
+ out << ANSI_RESTORE_CURSOR;
+}
+
+void Statusline::Clear() { Draw(""); }
+
+void Statusline::UpdateTerminalProperties() {
+ if (m_terminal_size_has_changed == 0)
+return;
+
+ // Clear the previous statusline.
+ Reset();
+
+ // Purposely ignore the terminal settings. If the setting doesn't match
+ // reality and we draw the status bar over existing text, we have no way to
+ // recover.
+ struct winsize window_size;
+ if ((isatty(STDIN_FILENO) != 0) &&
+ ::ioctl(STDIN_FILENO, TIOCGWINSZ, &window_size) == 0) {
JDevlieghere wrote:
I don't remember why, but I was mimicking the equivalent in `Editline`.
https://github.com/llvm/llvm-project/pull/121860
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SBThread.selected_frame property (PR #123981)
https://github.com/kastiglione created
https://github.com/llvm/llvm-project/pull/123981
None
>From 3929895879a226bf8f0e407fead4524597a429af Mon Sep 17 00:00:00 2001
From: Dave Lee
Date: Wed, 22 Jan 2025 08:18:36 -0800
Subject: [PATCH] [lldb] Add SBThread.selected_frame property
---
lldb/bindings/interface/SBThreadExtensions.i | 7 +++
.../commands/frame/recognizer/TestFrameRecognizer.py | 8 +++-
.../location-list-lookup/TestLocationListLookup.py| 4 ++--
.../TestStdFunctionRecognizer.py | 11 ---
lldb/test/API/lang/objc/print-obj/TestPrintObj.py | 9 +++--
5 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/lldb/bindings/interface/SBThreadExtensions.i
b/lldb/bindings/interface/SBThreadExtensions.i
index 860a2d765a6695..926e5b1623c407 100644
--- a/lldb/bindings/interface/SBThreadExtensions.i
+++ b/lldb/bindings/interface/SBThreadExtensions.i
@@ -51,6 +51,12 @@ STRING_EXTENSION_OUTSIDE(SBThread)
for idx in range(self.GetStopReasonDataCount())
]
+def set_selected_frame(self, frame):
+if isinstance(frame, SBFrame):
+self.SetSelectedFrame(frame.idx)
+else:
+self.SetSelectedFrame(frame)
+
id = property(GetThreadID, None, doc='''A read only property that
returns the thread ID as an integer.''')
idx = property(GetIndexID, None, doc='''A read only property that
returns the thread index ID as an integer. Thread index ID values start at 1
and increment as threads come and go and can be used to uniquely identify
threads.''')
return_value = property(GetStopReturnValue, None, doc='''A read only
property that returns an lldb object that represents the return value from the
last stop (lldb.SBValue) if we just stopped due to stepping out of a
function.''')
@@ -65,6 +71,7 @@ STRING_EXTENSION_OUTSIDE(SBThread)
stop_reason_data = property(get_stop_reason_data, None, doc='''A read
only property that returns the stop reason data as a list.''')
is_suspended = property(IsSuspended, None, doc='''A read only property
that returns a boolean value that indicates if this thread is suspended.''')
is_stopped = property(IsStopped, None, doc='''A read only property
that returns a boolean value that indicates if this thread is stopped but not
exited.''')
+selected_frame = property(GetSelectedFrame, set_selected_frame,
doc='''A read/write property that gets and sets the selected frame of this
SBThread.''')
%}
#endif
}
diff --git a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
index aa2a448087431e..3e9dbfe6d85552 100644
--- a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
+++ b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
@@ -20,7 +20,7 @@ def test_frame_recognizer_1(self):
target, process, thread, _ = lldbutil.run_to_name_breakpoint(
self, "foo", exe_name=exe
)
-frame = thread.GetSelectedFrame()
+frame = thread.selected_frame
# Clear internal & plugins recognizers that get initialized at launch
self.runCmd("frame recognizer clear")
@@ -166,7 +166,7 @@ def test_frame_recognizer_hiding(self):
self.build()
target, process, thread, _ = lldbutil.run_to_name_breakpoint(self,
"nested")
-frame = thread.GetSelectedFrame()
+frame = thread.selected_frame
# Sanity check.
self.expect(
@@ -229,7 +229,6 @@ def test_frame_recognizer_multi_symbol(self):
target, process, thread, _ = lldbutil.run_to_name_breakpoint(
self, "foo", exe_name=exe
)
-frame = thread.GetSelectedFrame()
self.expect(
"frame recognizer info 0",
@@ -239,7 +238,6 @@ def test_frame_recognizer_multi_symbol(self):
target, process, thread, _ = lldbutil.run_to_name_breakpoint(
self, "bar", exe_name=exe
)
-frame = thread.GetSelectedFrame()
self.expect(
"frame recognizer info 0",
@@ -374,7 +372,7 @@ def test_frame_recognizer_not_only_first_instruction(self):
opts = lldb.SBVariablesOptions()
opts.SetIncludeRecognizedArguments(True)
-frame = thread.GetSelectedFrame()
+frame = thread.selected_frame
variables = frame.GetVariables(opts)
self.assertEqual(variables.GetSize(), 2)
diff --git
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index 84033daff77308..a97f4fc5e3d799 100644
---
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
+++
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -25,7 +25,7 @@ def check_local_vars(self, process: lldb.SBProcess,
check
[Lldb-commits] [lldb] [lldb] Add SBThread.selected_frame property (PR #123981)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Dave Lee (kastiglione)
Changes
---
Full diff: https://github.com/llvm/llvm-project/pull/123981.diff
5 Files Affected:
- (modified) lldb/bindings/interface/SBThreadExtensions.i (+7)
- (modified) lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
(+3-5)
- (modified)
lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
(+2-2)
- (modified)
lldb/test/API/lang/cpp/std-function-recognizer/TestStdFunctionRecognizer.py
(+4-7)
- (modified) lldb/test/API/lang/objc/print-obj/TestPrintObj.py (+3-6)
``diff
diff --git a/lldb/bindings/interface/SBThreadExtensions.i
b/lldb/bindings/interface/SBThreadExtensions.i
index 860a2d765a6695..926e5b1623c407 100644
--- a/lldb/bindings/interface/SBThreadExtensions.i
+++ b/lldb/bindings/interface/SBThreadExtensions.i
@@ -51,6 +51,12 @@ STRING_EXTENSION_OUTSIDE(SBThread)
for idx in range(self.GetStopReasonDataCount())
]
+def set_selected_frame(self, frame):
+if isinstance(frame, SBFrame):
+self.SetSelectedFrame(frame.idx)
+else:
+self.SetSelectedFrame(frame)
+
id = property(GetThreadID, None, doc='''A read only property that
returns the thread ID as an integer.''')
idx = property(GetIndexID, None, doc='''A read only property that
returns the thread index ID as an integer. Thread index ID values start at 1
and increment as threads come and go and can be used to uniquely identify
threads.''')
return_value = property(GetStopReturnValue, None, doc='''A read only
property that returns an lldb object that represents the return value from the
last stop (lldb.SBValue) if we just stopped due to stepping out of a
function.''')
@@ -65,6 +71,7 @@ STRING_EXTENSION_OUTSIDE(SBThread)
stop_reason_data = property(get_stop_reason_data, None, doc='''A read
only property that returns the stop reason data as a list.''')
is_suspended = property(IsSuspended, None, doc='''A read only property
that returns a boolean value that indicates if this thread is suspended.''')
is_stopped = property(IsStopped, None, doc='''A read only property
that returns a boolean value that indicates if this thread is stopped but not
exited.''')
+selected_frame = property(GetSelectedFrame, set_selected_frame,
doc='''A read/write property that gets and sets the selected frame of this
SBThread.''')
%}
#endif
}
diff --git a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
index aa2a448087431e..3e9dbfe6d85552 100644
--- a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
+++ b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
@@ -20,7 +20,7 @@ def test_frame_recognizer_1(self):
target, process, thread, _ = lldbutil.run_to_name_breakpoint(
self, "foo", exe_name=exe
)
-frame = thread.GetSelectedFrame()
+frame = thread.selected_frame
# Clear internal & plugins recognizers that get initialized at launch
self.runCmd("frame recognizer clear")
@@ -166,7 +166,7 @@ def test_frame_recognizer_hiding(self):
self.build()
target, process, thread, _ = lldbutil.run_to_name_breakpoint(self,
"nested")
-frame = thread.GetSelectedFrame()
+frame = thread.selected_frame
# Sanity check.
self.expect(
@@ -229,7 +229,6 @@ def test_frame_recognizer_multi_symbol(self):
target, process, thread, _ = lldbutil.run_to_name_breakpoint(
self, "foo", exe_name=exe
)
-frame = thread.GetSelectedFrame()
self.expect(
"frame recognizer info 0",
@@ -239,7 +238,6 @@ def test_frame_recognizer_multi_symbol(self):
target, process, thread, _ = lldbutil.run_to_name_breakpoint(
self, "bar", exe_name=exe
)
-frame = thread.GetSelectedFrame()
self.expect(
"frame recognizer info 0",
@@ -374,7 +372,7 @@ def test_frame_recognizer_not_only_first_instruction(self):
opts = lldb.SBVariablesOptions()
opts.SetIncludeRecognizedArguments(True)
-frame = thread.GetSelectedFrame()
+frame = thread.selected_frame
variables = frame.GetVariables(opts)
self.assertEqual(variables.GetSize(), 2)
diff --git
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index 84033daff77308..a97f4fc5e3d799 100644
---
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
+++
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -25,7 +25,7 @@ def check_local_vars(self, process: lldb.SBProcess,
check_expr: bool):
# Find `bar` on the stack, then
# mak
[Lldb-commits] [lldb] [lldb] Implement a statusline in LLDB (PR #121860)
@@ -0,0 +1,165 @@ +//===-- Statusline.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "lldb/Core/Statusline.h" +#include "lldb/Core/Debugger.h" +#include "lldb/Core/FormatEntity.h" +#include "lldb/Host/ThreadLauncher.h" +#include "lldb/Interpreter/CommandInterpreter.h" +#include "lldb/Symbol/SymbolContext.h" +#include "lldb/Target/StackFrame.h" +#include "lldb/Utility/AnsiTerminal.h" +#include "lldb/Utility/LLDBLog.h" +#include "lldb/Utility/Log.h" +#include "lldb/Utility/StreamString.h" +#include "llvm/Support/Locale.h" + +#include +#include bulbazord wrote: Are these headers portable? I have a feeling it won't work on windows... https://github.com/llvm/llvm-project/pull/121860 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB-DAP] Send Progress update message over DAP (PR #123837)
https://github.com/Jlalond closed https://github.com/llvm/llvm-project/pull/123837 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] a939a9f - [LLDB-DAP] Send Progress update message over DAP (#123837)
Author: Jacob Lalonde
Date: 2025-01-22T15:49:13-08:00
New Revision: a939a9fd53d98f33b94f9121646d5906a2b9f598
URL:
https://github.com/llvm/llvm-project/commit/a939a9fd53d98f33b94f9121646d5906a2b9f598
DIFF:
https://github.com/llvm/llvm-project/commit/a939a9fd53d98f33b94f9121646d5906a2b9f598.diff
LOG: [LLDB-DAP] Send Progress update message over DAP (#123837)
When testing my SBProgress DAP PR (#123826), I noticed Progress update
messages aren't sent over DAP. This patch adds the lldb progress event's
message to the body when sent over DAP.
Before
Now
Tested with my [progress tester
command](https://gist.github.com/Jlalond/48d85e75a91f7a137e3142e6a13d0947),
testing 10 events 5 seconds apart 1-10
Added:
lldb/test/API/tools/lldb-dap/progress/Makefile
lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
lldb/test/API/tools/lldb-dap/progress/main.cpp
Modified:
lldb/tools/lldb-dap/ProgressEvent.cpp
lldb/tools/lldb-dap/ProgressEvent.h
Removed:
diff --git a/lldb/test/API/tools/lldb-dap/progress/Makefile
b/lldb/test/API/tools/lldb-dap/progress/Makefile
new file mode 100644
index 00..8b20bcb050
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/progress/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
new file mode 100644
index 00..7f4055cab9ddda
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/progress/Progress_emitter.py
@@ -0,0 +1,84 @@
+import inspect
+import optparse
+import shlex
+import sys
+import time
+
+import lldb
+
+
+class ProgressTesterCommand:
+program = "test-progress"
+
+@classmethod
+def register_lldb_command(cls, debugger, module_name):
+parser = cls.create_options()
+cls.__doc__ = parser.format_help()
+# Add any commands contained in this module to LLDB
+command = "command script add -c %s.%s %s" % (
+module_name,
+cls.__name__,
+cls.program,
+)
+debugger.HandleCommand(command)
+print(
+'The "{0}" command has been installed, type "help {0}" or "{0} '
+'--help" for detailed help.'.format(cls.program)
+)
+
+@classmethod
+def create_options(cls):
+usage = "usage: %prog [options]"
+description = "SBProgress testing tool"
+# Opt parse is deprecated, but leaving this the way it is because it
allows help formating
+# Additionally all our commands use optparse right now, ideally we
migrate them all in one go.
+parser = optparse.OptionParser(
+description=description, prog=cls.program, usage=usage
+)
+
+parser.add_option(
+"--total", dest="total", help="Total to count up.", type="int"
+)
+
+parser.add_option(
+"--seconds",
+dest="seconds",
+help="Total number of seconds to wait between increments",
+type="int",
+)
+
+return parser
+
+def get_short_help(self):
+return "Progress Tester"
+
+def get_long_help(self):
+return self.help_string
+
+def __init__(self, debugger, unused):
+self.parser = self.create_options()
+self.help_string = self.parser.format_help()
+
+def __call__(self, debugger, command, exe_ctx, result):
+command_args = shlex.split(command)
+try:
+(cmd_options, args) = self.parser.parse_args(command_args)
+except:
+result.SetError("option parsing failed")
+return
+
+total = cmd_options.total
+progress = lldb.SBProgress("Progress tester", "Detail", total,
debugger)
+
+for i in range(1, total):
+progress.Increment(1, f"Step {i}")
+time.sleep(cmd_options.seconds)
+
+
+def __lldb_init_module(debugger, dict):
+# Register all classes that have a register_lldb_command method
+for _name, cls in inspect.getmembers(sys.modules[__name__]):
+if inspect.isclass(cls) and callable(
+getattr(cls, "register_lldb_command", None)
+):
+cls.register_lldb_command(debugger, __name__)
diff --git a/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
new file mode 100755
index 00..36c0cef9c47143
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/progress/TestDAP_Progress.py
@@ -0,0 +1,49 @@
+"""
+Test lldb-dap output events
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test
[Lldb-commits] [lldb] Reland "[lldb] Implement basic support for reverse-continue" (#123906)" (PR #123945)
rocallahan wrote: > I also noticed that if you don't save memory at all, the breakpoint tests > pass. Which is surprising, but ok given that the test doesn't check any > memory values, and watchpoint tests _do_ fail if you do not save memory. Yeah, this is why I think it was a good idea to add a test that actually needs the memory save/restore. Even though it caused the PR to bounce. https://github.com/llvm/llvm-project/pull/123945 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reland "[lldb] Implement basic support for reverse-continue" (#123906)" (PR #123945)
rocallahan wrote: @DavidSpickett this is your branch now ... can you add @skipIfRemote to the tests? https://github.com/llvm/llvm-project/pull/123945 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement a statusline in LLDB (PR #121860)
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/121860
>From ecd2f911e597e02c0e3bb82384e701ada09e811e Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Fri, 17 Jan 2025 17:10:36 -0800
Subject: [PATCH] [lldb] Implement a statusline in LLDB
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Add a statusline to command-line LLDB to display progress events and
other information related to the current state of the debugger. The
statusline is a dedicated area displayed the bottom of the screen. The
contents of the status line are configurable through a setting
consisting of LLDB’s format strings.
The statusline is configurable through the `statusline-format` setting.
The default configuration shows the target name, the current file, the
stop reason and the current progress event.
```
(lldb) settings show statusline-format
statusline-format (format-string) =
"${ansi.bg.cyan}${ansi.fg.black}{${target.file.basename}}{ |
${line.file.basename}:${line.number}:${line.column}}{ | ${thread.stop-reason}}{
| {${progress.count} }${progress.message}}"
```
The statusline is enabled by default, but can be disabled with the
following setting:
```
(lldb) settings set show-statusline false
```
The statusline supersedes the current progress reporting implementation.
Consequently, the following settings no longer have any effect (but
continue to exist):
```
show-progress -- Whether to show progress or not if the debugger's
output is an interactive color-enabled terminal.
show-progress-ansi-prefix -- When displaying progress in a color-enabled
terminal, use the ANSI terminal code specified in this format immediately
before the progress message.
show-progress-ansi-suffix -- When displaying progress in a color-enabled
terminal, use the ANSI terminal code specified in this format immediately after
the progress message.
```
RFC: https://discourse.llvm.org/t/rfc-lldb-statusline/83948
---
lldb/include/lldb/Core/Debugger.h| 21 ++-
lldb/include/lldb/Core/FormatEntity.h| 4 +-
lldb/include/lldb/Core/Statusline.h | 60
lldb/include/lldb/Utility/AnsiTerminal.h | 1 +
lldb/source/Core/CMakeLists.txt | 1 +
lldb/source/Core/CoreProperties.td | 8 ++
lldb/source/Core/Debugger.cpp| 148 ++--
lldb/source/Core/FormatEntity.cpp| 46 ++-
lldb/source/Core/Statusline.cpp | 168 +++
9 files changed, 373 insertions(+), 84 deletions(-)
create mode 100644 lldb/include/lldb/Core/Statusline.h
create mode 100644 lldb/source/Core/Statusline.cpp
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 70f4c4216221c6..a4da5fd44c17fe 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -19,6 +19,7 @@
#include "lldb/Core/FormatEntity.h"
#include "lldb/Core/IOHandler.h"
#include "lldb/Core/SourceManager.h"
+#include "lldb/Core/Statusline.h"
#include "lldb/Core/UserSettingsController.h"
#include "lldb/Host/HostThread.h"
#include "lldb/Host/StreamFile.h"
@@ -308,6 +309,10 @@ class Debugger : public
std::enable_shared_from_this,
bool SetShowProgress(bool show_progress);
+ bool GetShowStatusline() const;
+
+ const FormatEntity::Entry *GetStatuslineFormat() const;
+
llvm::StringRef GetShowProgressAnsiPrefix() const;
llvm::StringRef GetShowProgressAnsiSuffix() const;
@@ -604,6 +609,14 @@ class Debugger : public
std::enable_shared_from_this,
return m_source_file_cache;
}
+ struct ProgressReport {
+uint64_t id;
+uint64_t completed;
+uint64_t total;
+std::string message;
+ };
+ std::optional GetCurrentProgressReport() const;
+
protected:
friend class CommandInterpreter;
friend class REPL;
@@ -728,7 +741,7 @@ class Debugger : public
std::enable_shared_from_this,
IOHandlerStack m_io_handler_stack;
std::recursive_mutex m_io_handler_synchronous_mutex;
- std::optional m_current_event_id;
+ std::optional m_statusline;
llvm::StringMap> m_stream_handlers;
std::shared_ptr m_callback_handler_sp;
@@ -745,6 +758,12 @@ class Debugger : public
std::enable_shared_from_this,
lldb::TargetSP m_dummy_target_sp;
Diagnostics::CallbackID m_diagnostics_callback_id;
+ /// Bookkeeping for command line progress events.
+ /// @{
+ llvm::SmallVector m_progress_reports;
+ mutable std::mutex m_progress_reports_mutex;
+ /// @}
+
std::mutex m_destroy_callback_mutex;
lldb::callback_token_t m_destroy_callback_next_token = 0;
struct DestroyCallbackInfo {
diff --git a/lldb/include/lldb/Core/FormatEntity.h
b/lldb/include/lldb/Core/FormatEntity.h
index c9d5af1f31673b..51e9ce37e54e79 100644
--- a/lldb/include/lldb/Core/FormatEntity.h
+++ b/lldb/include/lldb/Core/FormatEntity.h
@@ -100,7 +100,9 @@ struct Entry {
LineEntryColumn,
LineEntryStartAddress,
LineEntryEndAddress,
-CurrentPCArrow
+
[Lldb-commits] [lldb] [lldb] Implement a statusline in LLDB (PR #121860)
@@ -0,0 +1,165 @@
+//===-- Statusline.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "lldb/Core/Statusline.h"
+#include "lldb/Core/Debugger.h"
+#include "lldb/Core/FormatEntity.h"
+#include "lldb/Host/ThreadLauncher.h"
+#include "lldb/Interpreter/CommandInterpreter.h"
+#include "lldb/Symbol/SymbolContext.h"
+#include "lldb/Target/StackFrame.h"
+#include "lldb/Utility/AnsiTerminal.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/StreamString.h"
+#include "llvm/Support/Locale.h"
+
+#include
+#include
+
+#define ESCAPE "\x1b"
+#define ANSI_SAVE_CURSOR ESCAPE "7"
+#define ANSI_RESTORE_CURSOR ESCAPE "8"
+#define ANSI_CLEAR_BELOW ESCAPE "[J"
+#define ANSI_CLEAR_LINE "\r\x1B[2K"
+#define ANSI_SET_SCROLL_ROWS ESCAPE "[0;%ur"
+#define ANSI_TO_START_OF_ROW ESCAPE "[%u;0f"
+#define ANSI_UP_ROWS ESCAPE "[%dA"
+#define ANSI_DOWN_ROWS ESCAPE "[%dB"
+#define ANSI_FORWARD_COLS ESCAPE "\033[%dC"
+#define ANSI_BACKWARD_COLS ESCAPE "\033[%dD"
+
+using namespace lldb;
+using namespace lldb_private;
+
+static size_t ColumnWidth(llvm::StringRef str) {
+ std::string stripped = ansi::StripAnsiTerminalCodes(str);
+ return llvm::sys::locale::columnWidth(stripped);
+}
+
+Statusline::Statusline(Debugger &debugger) : m_debugger(debugger) { Enable(); }
+
+Statusline::~Statusline() { Disable(); }
+
+void Statusline::TerminalSizeChanged() {
+ m_terminal_size_has_changed = 1;
+
+ // FIXME: This probably isn't safe?
bulbazord wrote:
A little more detail in this FIXME would be helpful. Why wouldn't it be safe?
https://github.com/llvm/llvm-project/pull/121860
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement a statusline in LLDB (PR #121860)
@@ -0,0 +1,60 @@ +//===-- Statusline.h -===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#ifndef LLDB_CORE_STATUSBAR_H +#define LLDB_CORE_STATUSBAR_H bulbazord wrote: nit: header guard does not match filename https://github.com/llvm/llvm-project/pull/121860 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement a statusline in LLDB (PR #121860)
@@ -1198,13 +1207,13 @@ bool FormatEntity::Format(const Entry &entry, Stream &s,
// FormatEntity::Entry::Definition encoding
return false;
case Entry::Type::EscapeCode:
-if (exe_ctx) {
- if (Target *target = exe_ctx->GetTargetPtr()) {
-Debugger &debugger = target->GetDebugger();
-if (debugger.GetUseColor()) {
- s.PutCString(entry.string);
-}
+if (Target *target = Target::GetTargetFromContexts(exe_ctx, sc)) {
+ Debugger &debugger = target->GetDebugger();
+ if (debugger.GetUseColor()) {
+s.PutCString(entry.string);
}
+} else {
+ assert(false);
bulbazord wrote:
Looks like you were using this assertion for debugging purposes?
https://github.com/llvm/llvm-project/pull/121860
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [StrTable] Switch the option parser to `llvm::StringTable` (PR #123308)
@@ -117,13 +121,13 @@ class OptTable {
private:
// A unified string table for these options. Individual strings are stored as
// null terminated C-strings at offsets within this table.
- const char *StrTable;
+ const StringTable *StrTable;
chandlerc wrote:
Using a const reference makes the type unassignable (without UB) sadly, because
references can't be re-bound.
Technically, this type could probably be unassignable, but I always prefer
regular types that don't have complex constraints like that and so I much
prefer pointers rather than references as members.
That said, if there is a strong preference, I can try switching to a reference
and see if I can get it all to compile.
https://github.com/llvm/llvm-project/pull/123308
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [StrTable] Switch the option parser to `llvm::StringTable` (PR #123308)
@@ -33,25 +33,26 @@ using namespace llvm::opt;
namespace {
struct OptNameLess {
- const char *StrTable;
- ArrayRef PrefixesTable;
+ const StringTable *StrTable;
chandlerc wrote:
(same as above)
https://github.com/llvm/llvm-project/pull/123308
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [StrTable] Switch the option parser to `llvm::StringTable` (PR #123308)
@@ -117,13 +121,13 @@ class OptTable {
private:
// A unified string table for these options. Individual strings are stored as
// null terminated C-strings at offsets within this table.
- const char *StrTable;
+ const StringTable *StrTable;
rnk wrote:
Can this be a const reference instead? Is there a reason you chose to make this
a pointer?
https://github.com/llvm/llvm-project/pull/123308
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [StrTable] Switch the option parser to `llvm::StringTable` (PR #123308)
https://github.com/rnk edited https://github.com/llvm/llvm-project/pull/123308 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [StrTable] Switch the option parser to `llvm::StringTable` (PR #123308)
@@ -33,25 +33,26 @@ using namespace llvm::opt;
namespace {
struct OptNameLess {
- const char *StrTable;
- ArrayRef PrefixesTable;
+ const StringTable *StrTable;
rnk wrote:
Can this be a const reference instead? Is there a reason you chose to make this
a pointer?
https://github.com/llvm/llvm-project/pull/123308
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [StrTable] Switch the option parser to `llvm::StringTable` (PR #123308)
https://github.com/rnk commented: Thanks, I just had one question. https://github.com/llvm/llvm-project/pull/123308 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB-DAP] Send Progress update message over DAP (PR #123837)
@@ -0,0 +1,49 @@
+"""
+Test lldb-dap output events
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import os
+import time
+
+import lldbdap_testcase
+
+
+class TestDAP_progress(lldbdap_testcase.DAPTestCaseBase):
+@skipIfWindows
+def test_output(self):
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py")
+print(f"Progress emitter path: {progress_emitter}")
+source = "main.cpp"
+# Set breakpoint in the thread function so we can step the threads
+breakpoint_ids = self.set_source_breakpoints(
+source, [line_number(source, "// break here")]
+)
+self.continue_to_breakpoints(breakpoint_ids)
+self.dap_server.request_evaluate(
+f"`command script import {progress_emitter}", context="repl"
+)
+self.dap_server.request_evaluate(
+"`test-progress --total 3 --seconds 1", context="repl"
+)
+
+self.dap_server.wait_for_event("progressEnd", 15)
+# Expect at least a start, an update, and end event
+# However because the progress is an RAII object and we can't guaruntee
Jlalond wrote:
I did, but I believe because we'll free that memory when the python object is
destroyed we can't depend on the deterministic behavior of the RAII object.
https://github.com/llvm/llvm-project/pull/123837
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB-DAP] Send Progress update message over DAP (PR #123837)
@@ -0,0 +1,85 @@
+import inspect
+import optparse
+import shlex
+import sys
+import time
+
+import lldb
+
+
+class ProgressTesterCommand:
+program = "test-progress"
+
+@classmethod
+def register_lldb_command(cls, debugger, module_name):
+parser = cls.create_options()
+cls.__doc__ = parser.format_help()
+# Add any commands contained in this module to LLDB
+command = "command script add -c %s.%s %s" % (
+module_name,
+cls.__name__,
+cls.program,
+)
+debugger.HandleCommand(command)
+print(
+'The "{0}" command has been installed, type "help {0}" or "{0} '
+'--help" for detailed help.'.format(cls.program)
+)
+
+@classmethod
+def create_options(cls):
+usage = "usage: %prog [options]"
+description = "Jacob Lalonde's sbprogress testing tool"
+# Opt parse is deprecated, but leaving this the way it is because it
allows help formating
+# Additionally all our commands use optparse right now, ideally we
migrate them all in one go.
+parser = optparse.OptionParser(
+description=description, prog=cls.program, usage=usage
+)
+
+parser.add_option(
+"--total", dest="total", help="Total to count up.", type="int"
+)
+
+parser.add_option(
+"--seconds",
+dest="seconds",
+help="Total number of seconds to wait between increments",
+type="int",
+)
+
+return parser
+
+def get_short_help(self):
+return "Progress Tester"
+
+def get_long_help(self):
+return self.help_string
+
+def __init__(self, debugger, unused):
+self.parser = self.create_options()
+self.help_string = self.parser.format_help()
+
+def __call__(self, debugger, command, exe_ctx, result):
+command_args = shlex.split(command)
+try:
+(cmd_options, args) = self.parser.parse_args(command_args)
+except:
+result.SetError("option parsing failed")
+return
+
+total = cmd_options.total
+progress = lldb.SBProgress("Progress tester", "Detail", total,
debugger)
+
+# This actually should start at 1 but it's 6:30 on a Friday...
JDevlieghere wrote:
? :D
https://github.com/llvm/llvm-project/pull/123837
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB-DAP] Send Progress update message over DAP (PR #123837)
@@ -0,0 +1,49 @@
+"""
+Test lldb-dap output events
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import os
+import time
+
+import lldbdap_testcase
+
+
+class TestDAP_progress(lldbdap_testcase.DAPTestCaseBase):
+@skipIfWindows
+def test_output(self):
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py")
+print(f"Progress emitter path: {progress_emitter}")
+source = "main.cpp"
+# Set breakpoint in the thread function so we can step the threads
+breakpoint_ids = self.set_source_breakpoints(
+source, [line_number(source, "// break here")]
+)
+self.continue_to_breakpoints(breakpoint_ids)
+self.dap_server.request_evaluate(
+f"`command script import {progress_emitter}", context="repl"
+)
+self.dap_server.request_evaluate(
+"`test-progress --total 3 --seconds 1", context="repl"
+)
+
+self.dap_server.wait_for_event("progressEnd", 15)
+# Expect at least a start, an update, and end event
+# However because the progress is an RAII object and we can't guaruntee
JDevlieghere wrote:
Maybe something like this would make it more obvious that you're talking about
the `lldb_private::Progress` class:
```suggestion
# However because the underlying Progress instance is an RAII object
and we can't guaruntee
```
https://github.com/llvm/llvm-project/pull/123837
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB-DAP] Send Progress update message over DAP (PR #123837)
@@ -0,0 +1,85 @@
+import inspect
+import optparse
+import shlex
+import sys
+import time
+
+import lldb
+
+
+class ProgressTesterCommand:
+program = "test-progress"
+
+@classmethod
+def register_lldb_command(cls, debugger, module_name):
+parser = cls.create_options()
+cls.__doc__ = parser.format_help()
+# Add any commands contained in this module to LLDB
+command = "command script add -c %s.%s %s" % (
+module_name,
+cls.__name__,
+cls.program,
+)
+debugger.HandleCommand(command)
+print(
+'The "{0}" command has been installed, type "help {0}" or "{0} '
+'--help" for detailed help.'.format(cls.program)
+)
+
+@classmethod
+def create_options(cls):
+usage = "usage: %prog [options]"
+description = "Jacob Lalonde's sbprogress testing tool"
JDevlieghere wrote:
```suggestion
description = "SBProgress Testing Tool"
```
https://github.com/llvm/llvm-project/pull/123837
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB-DAP] Send Progress update message over DAP (PR #123837)
@@ -0,0 +1,49 @@
+"""
+Test lldb-dap output events
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import os
+import time
+
+import lldbdap_testcase
+
+
+class TestDAP_progress(lldbdap_testcase.DAPTestCaseBase):
+@skipIfWindows
+def test_output(self):
+program = self.getBuildArtifact("a.out")
+self.build_and_launch(program)
+progress_emitter = os.path.join(os.getcwd(), "Progress_emitter.py")
+print(f"Progress emitter path: {progress_emitter}")
+source = "main.cpp"
+# Set breakpoint in the thread function so we can step the threads
+breakpoint_ids = self.set_source_breakpoints(
+source, [line_number(source, "// break here")]
+)
+self.continue_to_breakpoints(breakpoint_ids)
+self.dap_server.request_evaluate(
+f"`command script import {progress_emitter}", context="repl"
+)
+self.dap_server.request_evaluate(
+"`test-progress --total 3 --seconds 1", context="repl"
+)
+
+self.dap_server.wait_for_event("progressEnd", 15)
+# Expect at least a start, an update, and end event
+# However because the progress is an RAII object and we can't guaruntee
Jlalond wrote:
Good suggestion
https://github.com/llvm/llvm-project/pull/123837
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB-DAP] Send Progress update message over DAP (PR #123837)
https://github.com/Jlalond edited https://github.com/llvm/llvm-project/pull/123837 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB-DAP] Send Progress update message over DAP (PR #123837)
@@ -0,0 +1,85 @@
+import inspect
+import optparse
+import shlex
+import sys
+import time
+
+import lldb
+
+
+class ProgressTesterCommand:
+program = "test-progress"
+
+@classmethod
+def register_lldb_command(cls, debugger, module_name):
+parser = cls.create_options()
+cls.__doc__ = parser.format_help()
+# Add any commands contained in this module to LLDB
+command = "command script add -c %s.%s %s" % (
+module_name,
+cls.__name__,
+cls.program,
+)
+debugger.HandleCommand(command)
+print(
+'The "{0}" command has been installed, type "help {0}" or "{0} '
+'--help" for detailed help.'.format(cls.program)
+)
+
+@classmethod
+def create_options(cls):
+usage = "usage: %prog [options]"
+description = "Jacob Lalonde's sbprogress testing tool"
Jlalond wrote:
Haha! Wow that's embarassing, I copied over my gist and forgot I left that in.
https://github.com/llvm/llvm-project/pull/123837
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement a statusline in LLDB (PR #121860)
https://github.com/JDevlieghere updated
https://github.com/llvm/llvm-project/pull/121860
>From b0cf540a08958ead1c7b9e38f30a2722fe780592 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere
Date: Fri, 17 Jan 2025 17:10:36 -0800
Subject: [PATCH 1/2] [lldb] Implement a statusline in LLDB
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Add a statusline to command-line LLDB to display progress events and
other information related to the current state of the debugger. The
statusline is a dedicated area displayed the bottom of the screen. The
contents of the status line are configurable through a setting
consisting of LLDB’s format strings.
The statusline is configurable through the `statusline-format` setting.
The default configuration shows the target name, the current file, the
stop reason and the current progress event.
```
(lldb) settings show statusline-format
statusline-format (format-string) =
"${ansi.bg.cyan}${ansi.fg.black}{${target.file.basename}}{ |
${line.file.basename}:${line.number}:${line.column}}{ | ${thread.stop-reason}}{
| {${progress.count} }${progress.message}}"
```
The statusline is enabled by default, but can be disabled with the
following setting:
```
(lldb) settings set show-statusline false
```
The statusline supersedes the current progress reporting implementation.
Consequently, the following settings no longer have any effect (but
continue to exist):
```
show-progress -- Whether to show progress or not if the debugger's
output is an interactive color-enabled terminal.
show-progress-ansi-prefix -- When displaying progress in a color-enabled
terminal, use the ANSI terminal code specified in this format immediately
before the progress message.
show-progress-ansi-suffix -- When displaying progress in a color-enabled
terminal, use the ANSI terminal code specified in this format immediately after
the progress message.
```
RFC: https://discourse.llvm.org/t/rfc-lldb-statusline/83948
---
lldb/include/lldb/Core/Debugger.h| 21 ++-
lldb/include/lldb/Core/FormatEntity.h| 4 +-
lldb/include/lldb/Core/Statusline.h | 63 +
lldb/include/lldb/Utility/AnsiTerminal.h | 1 +
lldb/source/Core/CMakeLists.txt | 1 +
lldb/source/Core/CoreProperties.td | 8 ++
lldb/source/Core/Debugger.cpp| 148 ++--
lldb/source/Core/FormatEntity.cpp| 46 ++-
lldb/source/Core/Statusline.cpp | 165 +++
9 files changed, 373 insertions(+), 84 deletions(-)
create mode 100644 lldb/include/lldb/Core/Statusline.h
create mode 100644 lldb/source/Core/Statusline.cpp
diff --git a/lldb/include/lldb/Core/Debugger.h
b/lldb/include/lldb/Core/Debugger.h
index 70f4c4216221c6..a4da5fd44c17fe 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -19,6 +19,7 @@
#include "lldb/Core/FormatEntity.h"
#include "lldb/Core/IOHandler.h"
#include "lldb/Core/SourceManager.h"
+#include "lldb/Core/Statusline.h"
#include "lldb/Core/UserSettingsController.h"
#include "lldb/Host/HostThread.h"
#include "lldb/Host/StreamFile.h"
@@ -308,6 +309,10 @@ class Debugger : public
std::enable_shared_from_this,
bool SetShowProgress(bool show_progress);
+ bool GetShowStatusline() const;
+
+ const FormatEntity::Entry *GetStatuslineFormat() const;
+
llvm::StringRef GetShowProgressAnsiPrefix() const;
llvm::StringRef GetShowProgressAnsiSuffix() const;
@@ -604,6 +609,14 @@ class Debugger : public
std::enable_shared_from_this,
return m_source_file_cache;
}
+ struct ProgressReport {
+uint64_t id;
+uint64_t completed;
+uint64_t total;
+std::string message;
+ };
+ std::optional GetCurrentProgressReport() const;
+
protected:
friend class CommandInterpreter;
friend class REPL;
@@ -728,7 +741,7 @@ class Debugger : public
std::enable_shared_from_this,
IOHandlerStack m_io_handler_stack;
std::recursive_mutex m_io_handler_synchronous_mutex;
- std::optional m_current_event_id;
+ std::optional m_statusline;
llvm::StringMap> m_stream_handlers;
std::shared_ptr m_callback_handler_sp;
@@ -745,6 +758,12 @@ class Debugger : public
std::enable_shared_from_this,
lldb::TargetSP m_dummy_target_sp;
Diagnostics::CallbackID m_diagnostics_callback_id;
+ /// Bookkeeping for command line progress events.
+ /// @{
+ llvm::SmallVector m_progress_reports;
+ mutable std::mutex m_progress_reports_mutex;
+ /// @}
+
std::mutex m_destroy_callback_mutex;
lldb::callback_token_t m_destroy_callback_next_token = 0;
struct DestroyCallbackInfo {
diff --git a/lldb/include/lldb/Core/FormatEntity.h
b/lldb/include/lldb/Core/FormatEntity.h
index c9d5af1f31673b..51e9ce37e54e79 100644
--- a/lldb/include/lldb/Core/FormatEntity.h
+++ b/lldb/include/lldb/Core/FormatEntity.h
@@ -100,7 +100,9 @@ struct Entry {
LineEntryColumn,
LineEntryStartAddress,
LineEntryEndAddress,
-CurrentPCArrow
[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)
@@ -111,8 +111,10 @@ Error
RunInTerminalLauncherCommChannel::WaitUntilDebugAdaptorAttaches(
return message.takeError();
}
-Error RunInTerminalLauncherCommChannel::NotifyPid() {
- return m_io.SendJSON(RunInTerminalMessagePid(getpid()).ToJSON());
+Error RunInTerminalLauncherCommChannel::NotifyPid(lldb::pid_t pid) {
+ if (pid == 0)
+pid = getpid();
walter-erquinigo wrote:
can you explain here why you added this?
https://github.com/llvm/llvm-project/pull/121269
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)
https://github.com/walter-erquinigo edited https://github.com/llvm/llvm-project/pull/121269 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)
https://github.com/walter-erquinigo commented: To be honest it's a bit hard to review this because I'm not versed in these low level APIs, let along Windows. Could you make sure that your changes pass mac, linux and windows CI? IIRC you can trigger manually buildbots if you want to run custom tests. https://github.com/llvm/llvm-project/pull/121269 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)
@@ -21,21 +23,22 @@ namespace lldb_dap {
/// The file is destroyed when the destructor is invoked.
struct FifoFile {
FifoFile(llvm::StringRef path);
+ FifoFile(llvm::StringRef path, FILE *f);
+ // FifoFile(llvm::StringRef path, FILE *f);
+ FifoFile(FifoFile &&other);
+
+ FifoFile(const FifoFile &) = delete;
+ FifoFile &operator=(const FifoFile &) = delete;
~FifoFile();
std::string m_path;
+ FILE *m_file;
};
-/// Create a fifo file in the filesystem.
-///
-/// \param[in] path
-/// The path for the fifo file.
-///
-/// \return
-/// A \a std::shared_ptr if the file could be created, or an
-/// \a llvm::Error in case of failures.
-llvm::Expected> CreateFifoFile(llvm::StringRef path);
+std::error_code createNamedPipe(const llvm::Twine &Prefix,
walter-erquinigo wrote:
This API is unnecessarily complex. The Prefix can just be a StringRef. Also use
lower case for names. You also never use the Suffix.
Finally, please write some documentation mentioning that a unique named pipe
with the given prefix will be created.
You may consider renaming this function as `createUniqueNamedPipe`
https://github.com/llvm/llvm-project/pull/121269
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB-DAP] Send Progress update message over DAP (PR #123837)
https://github.com/Jlalond updated
https://github.com/llvm/llvm-project/pull/123837
>From bc7bb2471e44ca7441f592611fd293f2b8c90435 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Tue, 21 Jan 2025 14:34:49 -0800
Subject: [PATCH 1/5] Have Progress message updates include their messages when
sent over DAP
---
lldb/tools/lldb-dap/ProgressEvent.cpp | 15 ++-
lldb/tools/lldb-dap/ProgressEvent.h | 3 ++-
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/lldb/tools/lldb-dap/ProgressEvent.cpp
b/lldb/tools/lldb-dap/ProgressEvent.cpp
index 0dcc2ee81001d5..7807ecfb6c34d0 100644
--- a/lldb/tools/lldb-dap/ProgressEvent.cpp
+++ b/lldb/tools/lldb-dap/ProgressEvent.cpp
@@ -117,6 +117,10 @@ json::Value ProgressEvent::ToJSON() const {
body.try_emplace("cancellable", false);
}
+ if (m_event_type == progressUpdate) {
+EmplaceSafeString(body, "message", m_message);
+ }
+
std::string timestamp(llvm::formatv("{0:f9}", m_creation_time.count()));
EmplaceSafeString(body, "timestamp", timestamp);
@@ -164,10 +168,11 @@ const ProgressEvent
&ProgressEventManager::GetMostRecentEvent() const {
return m_last_update_event ? *m_last_update_event : m_start_event;
}
-void ProgressEventManager::Update(uint64_t progress_id, uint64_t completed,
- uint64_t total) {
- if (std::optional event = ProgressEvent::Create(
- progress_id, std::nullopt, completed, total, &GetMostRecentEvent()))
{
+void ProgressEventManager::Update(uint64_t progress_id, const char *message,
+ uint64_t completed, uint64_t total) {
+ if (std::optional event =
+ ProgressEvent::Create(progress_id, StringRef(message), completed,
+total, &GetMostRecentEvent())) {
if (event->GetEventType() == progressEnd)
m_finished = true;
@@ -227,7 +232,7 @@ void ProgressEventReporter::Push(uint64_t progress_id,
const char *message,
m_unreported_start_events.push(event_manager);
}
} else {
-it->second->Update(progress_id, completed, total);
+it->second->Update(progress_id, message, completed, total);
if (it->second->Finished())
m_event_managers.erase(it);
}
diff --git a/lldb/tools/lldb-dap/ProgressEvent.h
b/lldb/tools/lldb-dap/ProgressEvent.h
index 72317b879c803a..8494221890a1c8 100644
--- a/lldb/tools/lldb-dap/ProgressEvent.h
+++ b/lldb/tools/lldb-dap/ProgressEvent.h
@@ -99,7 +99,8 @@ class ProgressEventManager {
/// Receive a new progress event for the start event and try to report it if
/// appropriate.
- void Update(uint64_t progress_id, uint64_t completed, uint64_t total);
+ void Update(uint64_t progress_id, const char *message, uint64_t completed,
+ uint64_t total);
/// \return
/// \b true if a \a progressEnd event has been notified. There's no
>From faefdb6487eb97cca56755a20597f3d6a7348c6c Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Tue, 21 Jan 2025 15:16:46 -0800
Subject: [PATCH 2/5] PR feedback, pass string ref instead of char *
---
lldb/tools/lldb-dap/ProgressEvent.cpp | 12 +---
lldb/tools/lldb-dap/ProgressEvent.h | 2 +-
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/lldb/tools/lldb-dap/ProgressEvent.cpp
b/lldb/tools/lldb-dap/ProgressEvent.cpp
index 7807ecfb6c34d0..6a4978c055e516 100644
--- a/lldb/tools/lldb-dap/ProgressEvent.cpp
+++ b/lldb/tools/lldb-dap/ProgressEvent.cpp
@@ -117,9 +117,8 @@ json::Value ProgressEvent::ToJSON() const {
body.try_emplace("cancellable", false);
}
- if (m_event_type == progressUpdate) {
+ if (m_event_type == progressUpdate)
EmplaceSafeString(body, "message", m_message);
- }
std::string timestamp(llvm::formatv("{0:f9}", m_creation_time.count()));
EmplaceSafeString(body, "timestamp", timestamp);
@@ -168,11 +167,10 @@ const ProgressEvent
&ProgressEventManager::GetMostRecentEvent() const {
return m_last_update_event ? *m_last_update_event : m_start_event;
}
-void ProgressEventManager::Update(uint64_t progress_id, const char *message,
+void ProgressEventManager::Update(uint64_t progress_id, llvm::StringRef
message,
uint64_t completed, uint64_t total) {
- if (std::optional event =
- ProgressEvent::Create(progress_id, StringRef(message), completed,
-total, &GetMostRecentEvent())) {
+ if (std::optional event = ProgressEvent::Create(
+ progress_id, message, completed, total, &GetMostRecentEvent())) {
if (event->GetEventType() == progressEnd)
m_finished = true;
@@ -232,7 +230,7 @@ void ProgressEventReporter::Push(uint64_t progress_id,
const char *message,
m_unreported_start_events.push(event_manager);
}
} else {
-it->second->Update(progress_id, message, completed, total);
+it->second->Update(progress_id, StringRef(message), completed, total);
if (it->second->Finished())
m_event_manager
[Lldb-commits] [lldb] [lldb] Add SBThread.selected_frame property (PR #123981)
@@ -99,11 +99,8 @@ def test_api(self):
(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
self, "// break here", lldb.SBFileSpec("main.cpp")
)
-frame = thread.GetSelectedFrame()
num_hidden = 0
-for i in range(1, thread.GetNumFrames()):
-thread.SetSelectedFrame(i)
-frame = thread.GetSelectedFrame()
kastiglione wrote:
this code didn't need to set the selected frame, it only needs to inspect
whether a frame is hidden (which can be done without selecting it).
https://github.com/llvm/llvm-project/pull/123981
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SBThread.selected_frame property (PR #123981)
https://github.com/kastiglione edited https://github.com/llvm/llvm-project/pull/123981 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SBThread.selected_frame property (PR #123981)
github-actions[bot] wrote:
:warning: Python code formatter, darker found issues in your code. :warning:
You can test this locally with the following command:
``bash
darker --check --diff -r
ec15b242505a46ea7d195a6520fb869a80a2cd10...3929895879a226bf8f0e407fead4524597a429af
lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
lldb/test/API/lang/cpp/std-function-recognizer/TestStdFunctionRecognizer.py
lldb/test/API/lang/objc/print-obj/TestPrintObj.py
``
View the diff from darker here.
``diff
--- lang/objc/print-obj/TestPrintObj.py 2025-01-22 18:39:47.00 +
+++ lang/objc/print-obj/TestPrintObj.py 2025-01-22 18:42:50.579312 +
@@ -68,11 +68,11 @@
# We want to traverse the frame to the one corresponding to blocked.m
to
# issue our 'po lock_me' command.
for frame in other_thread.frames:
- if frame.name == "main":
+if frame.name == "main":
other_thread.selected_frame = frame
if self.TraceOn():
print("selected frame:" + lldbutil.get_description(frame))
break
``
https://github.com/llvm/llvm-project/pull/123981
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SBThread.selected_frame property (PR #123981)
https://github.com/kastiglione updated
https://github.com/llvm/llvm-project/pull/123981
>From 3929895879a226bf8f0e407fead4524597a429af Mon Sep 17 00:00:00 2001
From: Dave Lee
Date: Wed, 22 Jan 2025 08:18:36 -0800
Subject: [PATCH 1/2] [lldb] Add SBThread.selected_frame property
---
lldb/bindings/interface/SBThreadExtensions.i | 7 +++
.../commands/frame/recognizer/TestFrameRecognizer.py | 8 +++-
.../location-list-lookup/TestLocationListLookup.py| 4 ++--
.../TestStdFunctionRecognizer.py | 11 ---
lldb/test/API/lang/objc/print-obj/TestPrintObj.py | 9 +++--
5 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/lldb/bindings/interface/SBThreadExtensions.i
b/lldb/bindings/interface/SBThreadExtensions.i
index 860a2d765a6695..926e5b1623c407 100644
--- a/lldb/bindings/interface/SBThreadExtensions.i
+++ b/lldb/bindings/interface/SBThreadExtensions.i
@@ -51,6 +51,12 @@ STRING_EXTENSION_OUTSIDE(SBThread)
for idx in range(self.GetStopReasonDataCount())
]
+def set_selected_frame(self, frame):
+if isinstance(frame, SBFrame):
+self.SetSelectedFrame(frame.idx)
+else:
+self.SetSelectedFrame(frame)
+
id = property(GetThreadID, None, doc='''A read only property that
returns the thread ID as an integer.''')
idx = property(GetIndexID, None, doc='''A read only property that
returns the thread index ID as an integer. Thread index ID values start at 1
and increment as threads come and go and can be used to uniquely identify
threads.''')
return_value = property(GetStopReturnValue, None, doc='''A read only
property that returns an lldb object that represents the return value from the
last stop (lldb.SBValue) if we just stopped due to stepping out of a
function.''')
@@ -65,6 +71,7 @@ STRING_EXTENSION_OUTSIDE(SBThread)
stop_reason_data = property(get_stop_reason_data, None, doc='''A read
only property that returns the stop reason data as a list.''')
is_suspended = property(IsSuspended, None, doc='''A read only property
that returns a boolean value that indicates if this thread is suspended.''')
is_stopped = property(IsStopped, None, doc='''A read only property
that returns a boolean value that indicates if this thread is stopped but not
exited.''')
+selected_frame = property(GetSelectedFrame, set_selected_frame,
doc='''A read/write property that gets and sets the selected frame of this
SBThread.''')
%}
#endif
}
diff --git a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
index aa2a448087431e..3e9dbfe6d85552 100644
--- a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
+++ b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
@@ -20,7 +20,7 @@ def test_frame_recognizer_1(self):
target, process, thread, _ = lldbutil.run_to_name_breakpoint(
self, "foo", exe_name=exe
)
-frame = thread.GetSelectedFrame()
+frame = thread.selected_frame
# Clear internal & plugins recognizers that get initialized at launch
self.runCmd("frame recognizer clear")
@@ -166,7 +166,7 @@ def test_frame_recognizer_hiding(self):
self.build()
target, process, thread, _ = lldbutil.run_to_name_breakpoint(self,
"nested")
-frame = thread.GetSelectedFrame()
+frame = thread.selected_frame
# Sanity check.
self.expect(
@@ -229,7 +229,6 @@ def test_frame_recognizer_multi_symbol(self):
target, process, thread, _ = lldbutil.run_to_name_breakpoint(
self, "foo", exe_name=exe
)
-frame = thread.GetSelectedFrame()
self.expect(
"frame recognizer info 0",
@@ -239,7 +238,6 @@ def test_frame_recognizer_multi_symbol(self):
target, process, thread, _ = lldbutil.run_to_name_breakpoint(
self, "bar", exe_name=exe
)
-frame = thread.GetSelectedFrame()
self.expect(
"frame recognizer info 0",
@@ -374,7 +372,7 @@ def test_frame_recognizer_not_only_first_instruction(self):
opts = lldb.SBVariablesOptions()
opts.SetIncludeRecognizedArguments(True)
-frame = thread.GetSelectedFrame()
+frame = thread.selected_frame
variables = frame.GetVariables(opts)
self.assertEqual(variables.GetSize(), 2)
diff --git
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
index 84033daff77308..a97f4fc5e3d799 100644
---
a/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
+++
b/lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
@@ -25,7 +25,7 @@ def check_local_vars(self, process: lldb.SBProcess,
check_e
[Lldb-commits] [clang] [lldb] [llvm] [StrTable] Switch the option parser to `llvm::StringTable` (PR #123308)
@@ -117,13 +121,13 @@ class OptTable {
private:
// A unified string table for these options. Individual strings are stored as
// null terminated C-strings at offsets within this table.
- const char *StrTable;
+ const StringTable *StrTable;
rnk wrote:
Got it, and I do seem to recall we sometimes use OptTable as a value type, even
if that is somewhat questionable.
https://github.com/llvm/llvm-project/pull/123308
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)
@@ -0,0 +1,205 @@
+//===-- DILLexer.cpp
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// This implements the recursive descent parser for the Data Inspection
+// Language (DIL), and its helper functions, which will eventually underlie the
+// 'frame variable' command. The language that this parser recognizes is
+// described in lldb/docs/dil-expr-lang.ebnf
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace lldb_private {
+
+namespace dil {
+
+// For fast keyword lookup. More keywords will be added later.
+const llvm::StringMap Keywords = {
+{"namespace", dil::TokenKind::kw_namespace},
+};
+
+const std::string DILToken::getTokenName(dil::TokenKind kind) {
+ switch (kind) {
+ case dil::TokenKind::coloncolon:
+return "coloncolon";
+ case dil::TokenKind::eof:
+return "eof";
+ case dil::TokenKind::identifier:
+return "identifier";
+ case dil::TokenKind::kw_namespace:
+return "namespace";
+ case dil::TokenKind::l_paren:
+return "l_paren";
+ case dil::TokenKind::r_paren:
+return "r_paren";
+ case dil::TokenKind::unknown:
+return "unknown";
+ default:
+return "token_name";
+ }
+}
+
+static bool Is_Letter(char c) {
+ if (('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z'))
+return true;
+ return false;
+}
+
+static bool Is_Digit(char c) { return ('0' <= c && c <= '9'); }
+
+// A word starts with a letter, underscore, or dollar sign, followed by
+// letters ('a'..'z','A'..'Z'), digits ('0'..'9'), and/or underscores.
+bool DILLexer::Is_Word(std::string::iterator start, uint32_t &length) {
+ bool done = false;
+ bool dollar_start = false;
+
+ // Must not start with a digit.
+ if (m_cur_pos == m_expr.end() || Is_Digit(*m_cur_pos))
+return false;
+
+ // First character *may* be a '$', for a register name or convenience
+ // variable.
+ if (*m_cur_pos == '$') {
+dollar_start = true;
+++m_cur_pos;
+length++;
+ }
+
+ // Contains only letters, digits or underscores
+ for (; m_cur_pos != m_expr.end() && !done; ++m_cur_pos) {
+char c = *m_cur_pos;
+if (!Is_Letter(c) && !Is_Digit(c) && c != '_') {
+ done = true;
+ break;
+} else
+ length++;
+ }
+
+ if (dollar_start && length > 1) // Must have something besides just '$'
+return true;
+
+ if (!dollar_start && length > 0)
+return true;
+
+ // Not a valid word, so re-set the lexing position.
+ m_cur_pos = start;
+ return false;
+}
+
+void DILLexer::UpdateLexedTokens(DILToken &result, dil::TokenKind tok_kind,
+ std::string tok_str, uint32_t tok_pos) {
+ DILToken new_token;
+ result.setValues(tok_kind, tok_str, tok_pos);
+ new_token = result;
+ m_lexed_tokens.push_back(std::move(new_token));
+}
+
+bool DILLexer::Lex(DILToken &result, bool look_ahead) {
+ bool retval = true;
+
+ if (!look_ahead) {
+// We're being asked for the 'next' token, and not a part of a LookAhead.
+// Check to see if we've already lexed it and pushed it onto our tokens
+// vector; if so, return the next token from the vector, rather than doing
+// more lexing.
+if ((m_tokens_idx != UINT_MAX) &&
+(m_tokens_idx < m_lexed_tokens.size() - 1)) {
+ result = m_lexed_tokens[m_tokens_idx + 1];
+ return retval;
+}
+ }
+
+ // Skip over whitespace (spaces).
+ while (m_cur_pos != m_expr.end() && *m_cur_pos == ' ')
+m_cur_pos++;
+
+ // Check to see if we've reached the end of our input string.
+ if (m_cur_pos == m_expr.end()) {
+UpdateLexedTokens(result, dil::TokenKind::eof, "", m_expr.length());
+return retval;
+ }
+
+ uint32_t position = m_cur_pos - m_expr.begin();
+ ;
+ std::string::iterator start = m_cur_pos;
+ uint32_t length = 0;
+ if (Is_Word(start, length)) {
+dil::TokenKind kind;
+std::string word = m_expr.substr(position, length);
+auto iter = Keywords.find(word);
+if (iter != Keywords.end())
+ kind = iter->second;
+else
+ kind = dil::TokenKind::identifier;
+
+UpdateLexedTokens(result, kind, word, position);
+return true;
+ }
+
+ switch (*m_cur_pos) {
+ case '(':
+m_cur_pos++;
+UpdateLexedTokens(result, dil::TokenKind::l_paren, "(", position);
+return true;
+ case ')':
+m_cur_pos++;
+UpdateLexedTokens(result, dil::TokenKind::r_paren, ")", position);
+return true;
+ case ':':
+if (position + 1 < m_expr.size() && m_expr[position + 1] == ':') {
+ m_cur_pos += 2;
+ UpdateLexedTokens(result, dil::TokenKind::coloncolon, "::", position);
+ return true;
+}
+break;
+ default:
+break;
+ }
+ // Empty Token
+ result.setValues(dil::TokenKind::none,
[Lldb-commits] [clang] [lldb] [llvm] [StrTable] Switch the option parser to `llvm::StringTable` (PR #123308)
https://github.com/rnk approved this pull request. https://github.com/llvm/llvm-project/pull/123308 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][NFC] Calculate the region size of an in memory image if size isn't specified (PR #123148)
clayborg wrote: > The reason this code looks the way it does is that we've had crashes when > trying to read corrupted elf files from memory, where we load the size of the > elf file from the process memory, find that out that it's size is 935872395 > GB, try to allocate a host buffer of that size, and then explode when the > allocation failed. How do you load the size of the ELF file from process memory? Are you saying the memory region information was corrupted in such cases? > > Since the elf code wasn't really ready to work with files loaded from memory > (it assumed section headers would always be present -- something which you're > changing now), They are not present in memory and never have been. So I changed the ELF code to not rely on requiring sections when the PT_DYNAMIC info or PT_NOTE info points to the same data without requiring section info. > we removed the code which tries to dynamically size the buffer for reading > the file. The only case we needed to support was reading the vdso image from > memory (which worked because the vdso is a complete elf image with section > headers and all, and it fits into a single page of memory), which is why > we've added this workaround to determine the region size from the outside. > > This patch institutionalizes that workaround, but I'm unsure if that's the > right direction for this. I'd like to understand where you want to take this > afterwards. If the next step is to teach ObjectFileELF to read from the > remaining memory regions (which I think you'll have to do, because most ELF > files will not consist of a single memory region), then I think this change > is unnecessary -- and maybe even harmful. Unnecessary, because if the code > will know how to handle the additional memory regions, then it should be easy > to teach it to extend the first region as well. Harmful, because if anything > goes wrong (there is no ELF file at the place where we think it should be), > then we could end up reading a huge block of memory for nothing (since the > memory region data comes from a more trusted source, we shouldn't end up with > the ridiculous values as we did before, but we could still end up reading > several gigabytes of data only to find out that it doesn't actually represent > an ELF file). Right now reading 512 bytes means if we ask for program headers, some or all program headers might be returned as all PT_NULL program headers due to truncation and the way the parsing code is right now. This patch is really to get ELF core files to be able to load what it needs effectively from memory like the PT_DYNAMIC and entries in the dynamic section like: - dynamic symbol table - hash table for the symbol table - the DT_DEBUG entry for the shared library list And so we can access the PT_NOTE data so we can get to the GNU build ID information and anything else that is encoded into PT_LOAD[0]. If we stick to 512 bytes, it often isn't enough to get all the data for the program headers, and this was causing issues when we couldn't locate the shared library list in the dynamic loader using this ELF file, or get to the note data. > So, I think it would be better to keep the current algorithm, which is to > read a small chunk of memory -- just enough to check whether we're truly > dealing with an object file, and leave the rest of the work to the object > file plugin (if I'm not mistaken, that's the logic we use when reading from a > file as well). Once the elf memory reading code gets sufficiently robust, we > can remove the vdso GetMemoryRegion call as well. Or we can keep it as a > performance optimization. We can see how it goes, but either way, I don't > think that a GetMemoryRegion call should be necessary for loading of _other_ > object files. > > What do you make of all this? How were you planning to approach things? This patch is needed before we can start reading all ELF files from memory when loading a core file. Right now if you don't specify an executable when loading a core file you get nothing. No shared libraries or anything. If a fix can be made to be able to correctly load ELF files from memory, we suddenly can not have a full image list with just a core file and no other executables because we will be able to correctly load them. We will also get the GNU build IDs from this memory ELF to ensure we don't allow someone to load the incorrect /usr/lib/libc.so from the current machine. If we can't figure out what the UUID was supposed to be, loading a core file often just loads what is on the current host and backtraces don't work at all due to using the wrong files. So with a new patch I really just want to ensure we can load all program header data and the things that the program headers point to for any PT_DYNAMIC and PT_NOTE stuff. The current object file stuff assumes we have the entire file mapped into memory if it is read from disk, so some accesses start failing if there are
[Lldb-commits] [lldb] 511dc26 - [lldb][DWARFASTParserClang][NFCI] Factor out CV-qualifier/is_static parsing from ParseChildParameters (#123951)
Author: Michael Buch
Date: 2025-01-22T17:47:26Z
New Revision: 511dc261ab94da7db6e67b05cdcef9dcff44798a
URL:
https://github.com/llvm/llvm-project/commit/511dc261ab94da7db6e67b05cdcef9dcff44798a
DIFF:
https://github.com/llvm/llvm-project/commit/511dc261ab94da7db6e67b05cdcef9dcff44798a.diff
LOG: [lldb][DWARFASTParserClang][NFCI] Factor out CV-qualifier/is_static
parsing from ParseChildParameters (#123951)
This patch continues simplifying `ParseChildParameters` by moving out
the logic that parses the first parameter of a function DIE into a
helper function. Since with GCC (and lately Clang) function declarations
have `DW_AT_object_pointer`s, we should be able to check for the
attribute's existence to determine if a function is static (and also
deduce CV-qualifiers from it). This will be useful for cases where the
object parameter is explicit (which is possible since C++23).
This should be NFC. I added a FIXME to places where we assume an
implicit object parameter (which will be addressed in a follow-up
patch).
We used to guard parsing of the CV-qualifiers of the "this" parameter
with a `encoding_mask & Type::eEncodingIsPointerUID`, which is
incorrect, because `eEncodingIsPointerUID` cannot be used as a bitmask
directly (see https://github.com/llvm/llvm-project/issues/120856). This
patch corrects this, but it should still be NFC because any parameter in
C++ called "this" *is* an implicit object parameter.
Added:
Modified:
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h
Removed:
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 81a1375c037182..f54b7fc9cdad24 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -159,6 +159,76 @@ static bool TagIsRecordType(dw_tag_t tag) {
}
}
+/// Get the object parameter DIE if one exists, otherwise returns
+/// a default DWARFDIE. If \c containing_decl_ctx is not a valid
+/// C++ declaration context for class methods, assume no object
+/// parameter exists for the given \c subprogram.
+static DWARFDIE
+GetCXXObjectParameter(const DWARFDIE &subprogram,
+ const clang::DeclContext &containing_decl_ctx) {
+ assert(subprogram.Tag() == DW_TAG_subprogram ||
+ subprogram.Tag() == DW_TAG_inlined_subroutine ||
+ subprogram.Tag() == DW_TAG_subroutine_type);
+
+ if (!DeclKindIsCXXClass(containing_decl_ctx.getDeclKind()))
+return {};
+
+ // FIXME: if subprogram has a explicit DW_AT_object_pointer, use it.
+
+ // If no DW_AT_object_pointer was specified, assume the implicit object
+ // parameter is the first parameter to the function, is called "this" and is
+ // artificial (which is what most compilers would generate).
+ auto children = subprogram.children();
+ auto it = llvm::find_if(children, [](const DWARFDIE &child) {
+return child.Tag() == DW_TAG_formal_parameter;
+ });
+
+ if (it == children.end())
+return {};
+
+ DWARFDIE object_pointer = *it;
+
+ if (!object_pointer.GetAttributeValueAsUnsigned(DW_AT_artificial, 0))
+return {};
+
+ // Often times compilers omit the "this" name for the
+ // specification DIEs, so we can't rely upon the name being in
+ // the formal parameter DIE...
+ if (const char *name = object_pointer.GetName();
+ name && ::strcmp(name, "this") != 0)
+return {};
+
+ return object_pointer;
+}
+
+/// In order to determine the CV-qualifiers for a C++ class
+/// method in DWARF, we have to look at the CV-qualifiers of
+/// the object parameter's type.
+static unsigned GetCXXMethodCVQuals(const DWARFDIE &subprogram,
+const DWARFDIE &object_parameter) {
+ if (!subprogram || !object_parameter)
+return 0;
+
+ Type *this_type = subprogram.ResolveTypeUID(
+ object_parameter.GetAttributeValueAsReferenceDIE(DW_AT_type));
+ if (!this_type)
+return 0;
+
+ uint32_t encoding_mask = this_type->GetEncodingMask();
+
+ // FIXME: explicit object parameters need not to be pointers
+ if (!(encoding_mask & (1u << Type::eEncodingIsPointerUID)))
+return 0;
+
+ unsigned cv_quals = 0;
+ if (encoding_mask & (1u << Type::eEncodingIsConstUID))
+cv_quals |= clang::Qualifiers::Const;
+ if (encoding_mask & (1u << Type::eEncodingIsVolatileUID))
+cv_quals |= clang::Qualifiers::Volatile;
+
+ return cv_quals;
+}
+
TypeSP DWARFASTParserClang::ParseTypeFromClangModule(const SymbolContext &sc,
const DWARFDIE &die,
Log *log) {
@@ -1188,11 +1258,8 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
const dw_tag_t tag = die.Tag();
bool is_variadic = false;
- bool
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang][NFCI] Factor out CV-qualifier/is_static parsing from ParseChildParameters (PR #123951)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/123951 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)
@@ -0,0 +1,156 @@
+//===-- DILLexer.h --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_VALUEOBJECT_DILLEXER_H_
+#define LLDB_VALUEOBJECT_DILLEXER_H_
+
+#include "llvm/ADT/StringRef.h"
+#include
+#include
+#include
+#include
+#include
+
+namespace lldb_private {
+
+namespace dil {
+
+enum class TokenKind {
+ coloncolon,
+ eof,
+ identifier,
+ invalid,
+ kw_namespace,
+ l_paren,
+ none,
+ r_paren,
+ unknown,
+};
+
+/// Class defining the tokens generated by the DIL lexer and used by the
+/// DIL parser.
+class DILToken {
+public:
+ DILToken(dil::TokenKind kind, std::string spelling, uint32_t start)
+ : m_kind(kind), m_spelling(spelling), m_start_pos(start) {}
+
+ DILToken() : m_kind(dil::TokenKind::none), m_spelling(""), m_start_pos(0) {}
+
+ void setKind(dil::TokenKind kind) { m_kind = kind; }
+ dil::TokenKind getKind() const { return m_kind; }
+
+ std::string getSpelling() const { return m_spelling; }
+
+ uint32_t getLength() const { return m_spelling.size(); }
+
+ bool is(dil::TokenKind kind) const { return m_kind == kind; }
+
+ bool isNot(dil::TokenKind kind) const { return m_kind != kind; }
+
+ bool isOneOf(dil::TokenKind kind1, dil::TokenKind kind2) const {
+return is(kind1) || is(kind2);
+ }
+
+ template bool isOneOf(dil::TokenKind kind, Ts... Ks) const {
+return is(kind) || isOneOf(Ks...);
+ }
+
+ uint32_t getLocation() const { return m_start_pos; }
+
+ void setValues(dil::TokenKind kind, std::string spelling, uint32_t start) {
+m_kind = kind;
+m_spelling = spelling;
+m_start_pos = start;
+ }
+
+ static const std::string getTokenName(dil::TokenKind kind);
+
+private:
+ dil::TokenKind m_kind;
+ std::string m_spelling;
+ uint32_t m_start_pos; // within entire expression string
+};
+
+/// Class for doing the simple lexing required by DIL.
+class DILLexer {
+public:
+ DILLexer(llvm::StringRef dil_expr) : m_expr(dil_expr.str()) {
+m_cur_pos = m_expr.begin();
+// Use UINT_MAX to indicate invalid/uninitialized value.
+m_tokens_idx = UINT_MAX;
+ }
+
+ bool Lex(DILToken &result, bool look_ahead = false);
+
+ bool Is_Word(std::string::iterator start, uint32_t &length);
+
+ uint32_t GetLocation() { return m_cur_pos - m_expr.begin(); }
+
+ /// Update 'result' with the other paremeter values, create a
+ /// duplicate token, and push the duplicate token onto the vector of
+ /// lexed tokens.
+ void UpdateLexedTokens(DILToken &result, dil::TokenKind tok_kind,
+ std::string tok_str, uint32_t tok_pos);
+
+ /// Return the lexed token N+1 positions ahead of the 'current' token
+ /// being handled by the DIL parser.
+ const DILToken &LookAhead(uint32_t N);
+
+ const DILToken &AcceptLookAhead(uint32_t N);
+
+ /// Return the index for the 'current' token being handled by the DIL parser.
+ uint32_t GetCurrentTokenIdx() { return m_tokens_idx; }
+
+ /// Return the current token to be handled by the DIL parser.
+ DILToken &GetCurrentToken() { return m_lexed_tokens[m_tokens_idx]; }
+
+ /// Update the index for the 'current' token, to point to the next lexed
+ /// token.
+ bool IncrementTokenIdx() {
cmtice wrote:
Not really. Lex() is called from LookAhead, when we definitely do not want to
automatically increment the token index.
https://github.com/llvm/llvm-project/pull/123521
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)
@@ -0,0 +1,205 @@
+//===-- DILLexer.cpp
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// This implements the recursive descent parser for the Data Inspection
+// Language (DIL), and its helper functions, which will eventually underlie the
+// 'frame variable' command. The language that this parser recognizes is
+// described in lldb/docs/dil-expr-lang.ebnf
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace lldb_private {
+
+namespace dil {
+
+// For fast keyword lookup. More keywords will be added later.
+const llvm::StringMap Keywords = {
+{"namespace", dil::TokenKind::kw_namespace},
+};
cmtice wrote:
We have on the order of 25 keywords. llvm::StringSwitch will be as fast as
StringMap.find()?
https://github.com/llvm/llvm-project/pull/123521
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)
@@ -0,0 +1,205 @@
+//===-- DILLexer.cpp
--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+// This implements the recursive descent parser for the Data Inspection
+// Language (DIL), and its helper functions, which will eventually underlie the
+// 'frame variable' command. The language that this parser recognizes is
+// described in lldb/docs/dil-expr-lang.ebnf
+//
+//===--===//
+
+#include "lldb/ValueObject/DILLexer.h"
+#include "llvm/ADT/StringMap.h"
+
+namespace lldb_private {
+
+namespace dil {
+
+// For fast keyword lookup. More keywords will be added later.
+const llvm::StringMap Keywords = {
+{"namespace", dil::TokenKind::kw_namespace},
+};
+
+const std::string DILToken::getTokenName(dil::TokenKind kind) {
+ switch (kind) {
+ case dil::TokenKind::coloncolon:
+return "coloncolon";
+ case dil::TokenKind::eof:
+return "eof";
+ case dil::TokenKind::identifier:
+return "identifier";
+ case dil::TokenKind::kw_namespace:
+return "namespace";
+ case dil::TokenKind::l_paren:
+return "l_paren";
+ case dil::TokenKind::r_paren:
+return "r_paren";
+ case dil::TokenKind::unknown:
+return "unknown";
+ default:
+return "token_name";
+ }
+}
+
+static bool Is_Letter(char c) {
+ if (('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z'))
+return true;
+ return false;
+}
+
+static bool Is_Digit(char c) { return ('0' <= c && c <= '9'); }
+
+// A word starts with a letter, underscore, or dollar sign, followed by
+// letters ('a'..'z','A'..'Z'), digits ('0'..'9'), and/or underscores.
+bool DILLexer::Is_Word(std::string::iterator start, uint32_t &length) {
+ bool done = false;
cmtice wrote:
It's used on lines 74 & 77/
https://github.com/llvm/llvm-project/pull/123521
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)
https://github.com/cmtice edited https://github.com/llvm/llvm-project/pull/123521 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Formatters] Do not recursively dereference pointer type when creating formatter candicates list. (PR #124048)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Zequan Wu (ZequanWu) Changes Currently, the type `T`'s formatter will be matched for `T`, `T*`, `T**` and so on. This is inconsistent with the behaviour of `SBValue::GetChildMemberWithName` which can only dereference the pointer type at most once if the sbvalue is a pointer type, because the API eventually calls `TypeSystemClang::GetIndexOfChildMemberWithName` (https://github.com/llvm/llvm-project/blob/3ef90f843fee74ff811ef88246734475f50e2073/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp#L6927-L6934) for C/C++. The current behaviour might cause crash on pretty printers. If the pretty printer calls `SBValue::GetChildMemberWithName` when compute the synthetic children or summary string and its type is `T**`, this might cause crash as it will return nullptr or None in this case. An example is the built-in pretty printer for libstdc++ `std::optional` when it calls `GetChildMemberWithName` on a nullptr returned from the previous call to `GetChildMemberWithName` (https://github.com/llvm/llvm-project/blob/3ef90f843fee74ff811ef88246734475f50e2073/lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp#L74-L75): ``` $ cat main.cpp #includeint main() { std::optional o_null; auto po_null = &o_null; auto ppo_null = &po_null; auto pppo_null = &ppo_null; return 0; } $ clang++ -g main.cpp && lldb -o "b 8" -o "r" -o "v pppo_null" [lldb crash] ``` After this change, data formatter for T can only be used for `T` and `T*` (if skip pointer is set to false). --- Full diff: https://github.com/llvm/llvm-project/pull/124048.diff 4 Files Affected: - (modified) lldb/include/lldb/DataFormatters/FormatManager.h (+2-1) - (modified) lldb/source/DataFormatters/FormatManager.cpp (+6-4) - (modified) lldb/test/API/functionalities/data-formatter/ptr_ref_typedef/TestPtrRef2Typedef.py (+12) - (modified) lldb/test/API/functionalities/data-formatter/ptr_ref_typedef/main.cpp (+3) ``diff diff --git a/lldb/include/lldb/DataFormatters/FormatManager.h b/lldb/include/lldb/DataFormatters/FormatManager.h index db2fe99c44cafc..9329ee930125a6 100644 --- a/lldb/include/lldb/DataFormatters/FormatManager.h +++ b/lldb/include/lldb/DataFormatters/FormatManager.h @@ -163,7 +163,7 @@ class FormatManager : public IFormatChangeListener { GetPossibleMatches(ValueObject &valobj, lldb::DynamicValueType use_dynamic) { FormattersMatchVector matches; GetPossibleMatches(valobj, valobj.GetCompilerType(), use_dynamic, matches, - FormattersMatchCandidate::Flags(), true); + FormattersMatchCandidate::Flags(), true, true); return matches; } @@ -180,6 +180,7 @@ class FormatManager : public IFormatChangeListener { lldb::DynamicValueType use_dynamic, FormattersMatchVector &entries, FormattersMatchCandidate::Flags current_flags, + bool dereference_ptr = true, bool root_level = false); std::atomic m_last_revision; diff --git a/lldb/source/DataFormatters/FormatManager.cpp b/lldb/source/DataFormatters/FormatManager.cpp index 3b891cecb1c8b9..d6d6935f3e002c 100644 --- a/lldb/source/DataFormatters/FormatManager.cpp +++ b/lldb/source/DataFormatters/FormatManager.cpp @@ -174,7 +174,8 @@ void FormatManager::DisableAllCategories() { void FormatManager::GetPossibleMatches( ValueObject &valobj, CompilerType compiler_type, lldb::DynamicValueType use_dynamic, FormattersMatchVector &entries, -FormattersMatchCandidate::Flags current_flags, bool root_level) { +FormattersMatchCandidate::Flags current_flags, bool dereference_ptr, +bool root_level) { compiler_type = compiler_type.GetTypeForFormatters(); ConstString type_name(compiler_type.GetTypeName()); // A ValueObject that couldn't be made correctly won't necessarily have a @@ -222,14 +223,15 @@ void FormatManager::GetPossibleMatches( if (compiler_type.IsPointerType()) { CompilerType non_ptr_type = compiler_type.GetPointeeType(); -GetPossibleMatches(valobj, non_ptr_type, use_dynamic, entries, - current_flags.WithStrippedPointer()); +if (dereference_ptr) + GetPossibleMatches(valobj, non_ptr_type, use_dynamic, entries, +current_flags.WithStrippedPointer(), false); if (non_ptr_type.IsTypedefType()) { CompilerType deffed_pointed_type = non_ptr_type.GetTypedefedType().GetPointerType(); // this is not exactly the usual meaning of stripping typedefs GetPossibleMatches(valobj, deffed_pointed_type, use_dynamic, entries, - current_flags.WithStrippedTypedef()); + current_flags.WithStrippedTypedef(), dereference_ptr); } } diff --git a/lldb/test/API/functionalities/data-formatter/ptr_ref_type
[Lldb-commits] [lldb] [lldb][Formatters] Do not recursively dereference pointer type when creating formatter candicates list. (PR #124048)
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 dd860bcfb57df429c0a1ad2e2d869ff3b795bc4d
1948805894e006d84fbb78299574b3c7618959d8 --extensions h,cpp --
lldb/include/lldb/DataFormatters/FormatManager.h
lldb/source/DataFormatters/FormatManager.cpp
lldb/test/API/functionalities/data-formatter/ptr_ref_typedef/main.cpp
``
View the diff from clang-format here.
``diff
diff --git a/lldb/source/DataFormatters/FormatManager.cpp
b/lldb/source/DataFormatters/FormatManager.cpp
index d6d6935f3e..d6b621fe41 100644
--- a/lldb/source/DataFormatters/FormatManager.cpp
+++ b/lldb/source/DataFormatters/FormatManager.cpp
@@ -225,7 +225,7 @@ void FormatManager::GetPossibleMatches(
CompilerType non_ptr_type = compiler_type.GetPointeeType();
if (dereference_ptr)
GetPossibleMatches(valobj, non_ptr_type, use_dynamic, entries,
-current_flags.WithStrippedPointer(), false);
+ current_flags.WithStrippedPointer(), false);
if (non_ptr_type.IsTypedefType()) {
CompilerType deffed_pointed_type =
non_ptr_type.GetTypedefedType().GetPointerType();
diff --git
a/lldb/test/API/functionalities/data-formatter/ptr_ref_typedef/main.cpp
b/lldb/test/API/functionalities/data-formatter/ptr_ref_typedef/main.cpp
index 41b1d28234..5c330ac2ae 100644
--- a/lldb/test/API/functionalities/data-formatter/ptr_ref_typedef/main.cpp
+++ b/lldb/test/API/functionalities/data-formatter/ptr_ref_typedef/main.cpp
@@ -6,8 +6,8 @@ int main() {
Foo& y = lval;
Foo&& z = 1;
- // Test lldb doesn't dereference pointer more than once.
- Foo** xp = &x;
- return 0; // Set breakpoint here
+// Test lldb doesn't dereference pointer more than once.
+Foo **xp = &x;
+return 0; // Set breakpoint here
}
``
https://github.com/llvm/llvm-project/pull/124048
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Formatters] Do not recursively dereference pointer type when creating formatter candicates list. (PR #124048)
github-actions[bot] wrote:
:warning: Python code formatter, darker found issues in your code. :warning:
You can test this locally with the following command:
``bash
darker --check --diff -r
dd860bcfb57df429c0a1ad2e2d869ff3b795bc4d...1948805894e006d84fbb78299574b3c7618959d8
lldb/test/API/functionalities/data-formatter/ptr_ref_typedef/TestPtrRef2Typedef.py
``
View the diff from darker here.
``diff
--- TestPtrRef2Typedef.py 2025-01-23 02:32:11.00 +
+++ TestPtrRef2Typedef.py 2025-01-23 02:57:00.503348 +
@@ -53,11 +53,11 @@
# the match.
self.expect("frame variable y", substrs=["(Foo &", ") y = 0x",
"IntLRef"])
self.expect("frame variable z", substrs=["(Foo &&", ") z = 0x",
"IntRRef"])
# Test lldb doesn't dereference pointer more than once.
-# xp has type Foo**, so it can only uses summary for Foo* or int*, not
+# xp has type Foo**, so it can only uses summary for Foo* or int*, not
# summary for Foo or int.
self.expect("frame variable xp", substrs=["(Foo **) xp = 0x",
"IntPointer"])
self.runCmd('type summary delete "int *"')
self.runCmd('type summary add --cascade true -s "MyInt" "int"')
``
https://github.com/llvm/llvm-project/pull/124048
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Formatters] Do not recursively dereference pointer type when creating formatter candicates list. (PR #124048)
https://github.com/ZequanWu created
https://github.com/llvm/llvm-project/pull/124048
Currently, the type `T`'s formatter will be matched for `T`, `T*`, `T**` and so
on. This is inconsistent with the behaviour of
`SBValue::GetChildMemberWithName` which can only dereference the pointer type
at most once if the sbvalue is a pointer type, because the API eventually calls
`TypeSystemClang::GetIndexOfChildMemberWithName`
(https://github.com/llvm/llvm-project/blob/3ef90f843fee74ff811ef88246734475f50e2073/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp#L6927-L6934)
for C/C++.
The current behaviour might cause crash on pretty printers. If the pretty
printer calls `SBValue::GetChildMemberWithName` when compute the synthetic
children or summary string and its type is `T**`, this might cause crash as it
will return nullptr or None in this case.
An example is the built-in pretty printer for libstdc++ `std::optional` when it
calls `GetChildMemberWithName` on a nullptr returned from the previous call to
`GetChildMemberWithName`
(https://github.com/llvm/llvm-project/blob/3ef90f843fee74ff811ef88246734475f50e2073/lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp#L74-L75):
```
$ cat main.cpp
#include
int main() {
std::optional o_null;
auto po_null = &o_null;
auto ppo_null = &po_null;
auto pppo_null = &ppo_null;
return 0;
}
$ clang++ -g main.cpp && lldb -o "b 8" -o "r" -o "v pppo_null"
[lldb crash]
```
After this change, data formatter for T can only be used for `T` and `T*` (if
skip pointer is set to false).
>From 1948805894e006d84fbb78299574b3c7618959d8 Mon Sep 17 00:00:00 2001
From: Zequan Wu
Date: Wed, 22 Jan 2025 18:32:11 -0800
Subject: [PATCH] [lldb][Formatters] Do not recursively dereference pointer
type when creating formatter candicates list.
---
lldb/include/lldb/DataFormatters/FormatManager.h | 3 ++-
lldb/source/DataFormatters/FormatManager.cpp | 10 ++
.../ptr_ref_typedef/TestPtrRef2Typedef.py| 12
.../data-formatter/ptr_ref_typedef/main.cpp | 3 +++
4 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/lldb/include/lldb/DataFormatters/FormatManager.h
b/lldb/include/lldb/DataFormatters/FormatManager.h
index db2fe99c44cafc..9329ee930125a6 100644
--- a/lldb/include/lldb/DataFormatters/FormatManager.h
+++ b/lldb/include/lldb/DataFormatters/FormatManager.h
@@ -163,7 +163,7 @@ class FormatManager : public IFormatChangeListener {
GetPossibleMatches(ValueObject &valobj, lldb::DynamicValueType use_dynamic) {
FormattersMatchVector matches;
GetPossibleMatches(valobj, valobj.GetCompilerType(), use_dynamic, matches,
- FormattersMatchCandidate::Flags(), true);
+ FormattersMatchCandidate::Flags(), true, true);
return matches;
}
@@ -180,6 +180,7 @@ class FormatManager : public IFormatChangeListener {
lldb::DynamicValueType use_dynamic,
FormattersMatchVector &entries,
FormattersMatchCandidate::Flags current_flags,
+ bool dereference_ptr = true,
bool root_level = false);
std::atomic m_last_revision;
diff --git a/lldb/source/DataFormatters/FormatManager.cpp
b/lldb/source/DataFormatters/FormatManager.cpp
index 3b891cecb1c8b9..d6d6935f3e002c 100644
--- a/lldb/source/DataFormatters/FormatManager.cpp
+++ b/lldb/source/DataFormatters/FormatManager.cpp
@@ -174,7 +174,8 @@ void FormatManager::DisableAllCategories() {
void FormatManager::GetPossibleMatches(
ValueObject &valobj, CompilerType compiler_type,
lldb::DynamicValueType use_dynamic, FormattersMatchVector &entries,
-FormattersMatchCandidate::Flags current_flags, bool root_level) {
+FormattersMatchCandidate::Flags current_flags, bool dereference_ptr,
+bool root_level) {
compiler_type = compiler_type.GetTypeForFormatters();
ConstString type_name(compiler_type.GetTypeName());
// A ValueObject that couldn't be made correctly won't necessarily have a
@@ -222,14 +223,15 @@ void FormatManager::GetPossibleMatches(
if (compiler_type.IsPointerType()) {
CompilerType non_ptr_type = compiler_type.GetPointeeType();
-GetPossibleMatches(valobj, non_ptr_type, use_dynamic, entries,
- current_flags.WithStrippedPointer());
+if (dereference_ptr)
+ GetPossibleMatches(valobj, non_ptr_type, use_dynamic, entries,
+current_flags.WithStrippedPointer(), false);
if (non_ptr_type.IsTypedefType()) {
CompilerType deffed_pointed_type =
non_ptr_type.GetTypedefedType().GetPointerType();
// this is not exactly the usual meaning of stripping typedefs
GetPossibleMatches(valobj, deffed_pointed_type, use_dynamic, entries,
- current_flags.WithStrippedTypedef());
+ current_flags.With
[Lldb-commits] [lldb] [lldb][LoongArch] Complete register alias name in `AugmentRegisterInfo` (PR #124059)
https://github.com/wangleiat created
https://github.com/llvm/llvm-project/pull/124059
Fixes: https://github.com/llvm/llvm-project/issues/123903
>From f404df6b2ac7b7ab33e3baadd3830154904a Mon Sep 17 00:00:00 2001
From: Ray Wang
Date: Thu, 23 Jan 2025 12:13:32 +0800
Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?=
=?UTF-8?q?l=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.5-bogner
---
.../ABI/LoongArch/ABISysV_loongarch.cpp | 38 +++
1 file changed, 14 insertions(+), 24 deletions(-)
diff --git a/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp
b/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp
index dc7e9bba000676..272c6a6be529ff 100644
--- a/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp
+++ b/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp
@@ -644,32 +644,22 @@ void ABISysV_loongarch::AugmentRegisterInfo(
std::vector ®s) {
lldb_private::RegInfoBasedABI::AugmentRegisterInfo(regs);
+ static const std::unordered_map reg_aliases = {
+ {"r0", "zero"}, {"r1", "ra"}, {"r2", "tp"}, {"r3", "sp"},
+ {"r4", "a0"}, {"r5", "a1"}, {"r6", "a2"}, {"r7", "a3"},
+ {"r8", "a4"}, {"r9", "a5"}, {"r10", "a6"}, {"r11", "a7"},
+ {"r12", "t0"}, {"r13", "t1"}, {"r14", "t2"}, {"r15", "t3"},
+ {"r16", "t4"}, {"r17", "t5"}, {"r18", "t6"}, {"r19", "t7"},
+ {"r20", "t8"}, {"r22", "fp"}, {"r23", "s0"}, {"r24", "s1"},
+ {"r25", "s2"}, {"r26", "s3"}, {"r27", "s4"}, {"r28", "s5"},
+ {"r29", "s6"}, {"r30", "s7"}, {"r31", "s8"}};
+
for (auto it : llvm::enumerate(regs)) {
// Set alt name for certain registers for convenience
-if (it.value().name == "r0")
- it.value().alt_name.SetCString("zero");
-else if (it.value().name == "r1")
- it.value().alt_name.SetCString("ra");
-else if (it.value().name == "r3")
- it.value().alt_name.SetCString("sp");
-else if (it.value().name == "r22")
- it.value().alt_name.SetCString("fp");
-else if (it.value().name == "r4")
- it.value().alt_name.SetCString("a0");
-else if (it.value().name == "r5")
- it.value().alt_name.SetCString("a1");
-else if (it.value().name == "r6")
- it.value().alt_name.SetCString("a2");
-else if (it.value().name == "r7")
- it.value().alt_name.SetCString("a3");
-else if (it.value().name == "r8")
- it.value().alt_name.SetCString("a4");
-else if (it.value().name == "r9")
- it.value().alt_name.SetCString("a5");
-else if (it.value().name == "r10")
- it.value().alt_name.SetCString("a6");
-else if (it.value().name == "r11")
- it.value().alt_name.SetCString("a7");
+std::string reg_name = it.value().name.GetStringRef().str();
+if (auto alias = reg_aliases.find(reg_name); alias != reg_aliases.end()) {
+ it.value().alt_name.SetCString(alias->second.c_str());
+}
// Set generic regnum so lldb knows what the PC, etc is
it.value().regnum_generic = GetGenericNum(it.value().name.GetStringRef());
___
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)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: wanglei (wangleiat)
Changes
Fixes: https://github.com/llvm/llvm-project/issues/123903
---
Full diff: https://github.com/llvm/llvm-project/pull/124059.diff
1 Files Affected:
- (modified) lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp (+14-24)
``diff
diff --git a/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp
b/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp
index dc7e9bba000676..272c6a6be529ff 100644
--- a/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp
+++ b/lldb/source/Plugins/ABI/LoongArch/ABISysV_loongarch.cpp
@@ -644,32 +644,22 @@ void ABISysV_loongarch::AugmentRegisterInfo(
std::vector ®s) {
lldb_private::RegInfoBasedABI::AugmentRegisterInfo(regs);
+ static const std::unordered_map reg_aliases = {
+ {"r0", "zero"}, {"r1", "ra"}, {"r2", "tp"}, {"r3", "sp"},
+ {"r4", "a0"}, {"r5", "a1"}, {"r6", "a2"}, {"r7", "a3"},
+ {"r8", "a4"}, {"r9", "a5"}, {"r10", "a6"}, {"r11", "a7"},
+ {"r12", "t0"}, {"r13", "t1"}, {"r14", "t2"}, {"r15", "t3"},
+ {"r16", "t4"}, {"r17", "t5"}, {"r18", "t6"}, {"r19", "t7"},
+ {"r20", "t8"}, {"r22", "fp"}, {"r23", "s0"}, {"r24", "s1"},
+ {"r25", "s2"}, {"r26", "s3"}, {"r27", "s4"}, {"r28", "s5"},
+ {"r29", "s6"}, {"r30", "s7"}, {"r31", "s8"}};
+
for (auto it : llvm::enumerate(regs)) {
// Set alt name for certain registers for convenience
-if (it.value().name == "r0")
- it.value().alt_name.SetCString("zero");
-else if (it.value().name == "r1")
- it.value().alt_name.SetCString("ra");
-else if (it.value().name == "r3")
- it.value().alt_name.SetCString("sp");
-else if (it.value().name == "r22")
- it.value().alt_name.SetCString("fp");
-else if (it.value().name == "r4")
- it.value().alt_name.SetCString("a0");
-else if (it.value().name == "r5")
- it.value().alt_name.SetCString("a1");
-else if (it.value().name == "r6")
- it.value().alt_name.SetCString("a2");
-else if (it.value().name == "r7")
- it.value().alt_name.SetCString("a3");
-else if (it.value().name == "r8")
- it.value().alt_name.SetCString("a4");
-else if (it.value().name == "r9")
- it.value().alt_name.SetCString("a5");
-else if (it.value().name == "r10")
- it.value().alt_name.SetCString("a6");
-else if (it.value().name == "r11")
- it.value().alt_name.SetCString("a7");
+std::string reg_name = it.value().name.GetStringRef().str();
+if (auto alias = reg_aliases.find(reg_name); alias != reg_aliases.end()) {
+ it.value().alt_name.SetCString(alias->second.c_str());
+}
// Set generic regnum so lldb knows what the PC, etc is
it.value().regnum_generic = GetGenericNum(it.value().name.GetStringRef());
``
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] Avoid repeated map lookups (NFC) (PR #124077)
https://github.com/JDevlieghere approved this pull request. https://github.com/llvm/llvm-project/pull/124077 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [llvm] [StrTable] Switch the option parser to `llvm::StringTable` (PR #123308)
https://github.com/chandlerc closed https://github.com/llvm/llvm-project/pull/123308 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] cd5694e - [StrTable] Switch the option parser to `llvm::StringTable` (#123308)
Author: Chandler Carruth
Date: 2025-01-22T23:19:47-08:00
New Revision: cd5694ecea2da1990365f46f9737be1b29d94f0c
URL:
https://github.com/llvm/llvm-project/commit/cd5694ecea2da1990365f46f9737be1b29d94f0c
DIFF:
https://github.com/llvm/llvm-project/commit/cd5694ecea2da1990365f46f9737be1b29d94f0c.diff
LOG: [StrTable] Switch the option parser to `llvm::StringTable` (#123308)
Now that we have a dedicated abstraction for string tables, switch the
option parser library's string table over to it rather than using a raw
`const char*`. Also try to use the `StringTable::Offset` type rather
than a raw `unsigned` where we can to avoid accidental increments or
other issues.
This is based on review feedback for the initial switch of options to a
string table. Happy to tweak or adjust if desired here.
Added:
Modified:
clang/lib/Frontend/CompilerInvocation.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
llvm/include/llvm/Option/OptTable.h
llvm/lib/Option/OptTable.cpp
llvm/tools/llvm-objdump/llvm-objdump.cpp
llvm/unittests/Option/OptionMarshallingTest.cpp
llvm/utils/TableGen/OptionParserEmitter.cpp
Removed:
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp
b/clang/lib/Frontend/CompilerInvocation.cpp
index 58658dedbaf1ee..3bf124e4827be9 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -282,7 +282,7 @@ using ArgumentConsumer =
CompilerInvocation::ArgumentConsumer;
#undef OPTTABLE_STR_TABLE_CODE
static llvm::StringRef lookupStrInTable(unsigned Offset) {
- return &OptionStrTable[Offset];
+ return OptionStrTable[Offset];
}
#define SIMPLE_ENUM_VALUE_TABLE
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 2a36f95c94d0ce..51e9a6d81b8390 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -42,6 +42,7 @@
#include "lldb/Utility/Status.h"
#include "lldb/Utility/Timer.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringTable.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Threading.h"
@@ -1083,7 +1084,7 @@ void
PlatformDarwin::AddClangModuleCompilationOptionsForSDKType(
if (!version.empty() && sdk_type != XcodeSDK::Type::Linux &&
sdk_type != XcodeSDK::Type::XROS) {
#define OPTION(PREFIX_OFFSET, NAME_OFFSET, VAR, ...)
\
- llvm::StringRef opt_##VAR = &OptionStrTable[NAME_OFFSET];
\
+ llvm::StringRef opt_##VAR = OptionStrTable[NAME_OFFSET];
\
(void)opt_##VAR;
#include "clang/Driver/Options.inc"
#undef OPTION
diff --git a/llvm/include/llvm/Option/OptTable.h
b/llvm/include/llvm/Option/OptTable.h
index 38a03fef7ae124..61a58aa304ecb4 100644
--- a/llvm/include/llvm/Option/OptTable.h
+++ b/llvm/include/llvm/Option/OptTable.h
@@ -12,6 +12,7 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringTable.h"
#include "llvm/Option/OptSpecifier.h"
#include "llvm/Support/StringSaver.h"
#include
@@ -54,7 +55,7 @@ class OptTable {
/// Entry for a single option instance in the option data table.
struct Info {
unsigned PrefixesOffset;
-unsigned PrefixedNameOffset;
+StringTable::Offset PrefixedNameOffset;
const char *HelpText;
// Help text for specific visibilities. A list of pairs, where each pair
// is a list of visibilities and a specific help string for those
@@ -80,34 +81,37 @@ class OptTable {
bool hasNoPrefix() const { return PrefixesOffset == 0; }
-unsigned getNumPrefixes(ArrayRef PrefixesTable) const {
- return PrefixesTable[PrefixesOffset];
+unsigned getNumPrefixes(ArrayRef PrefixesTable) const
{
+ // We embed the number of prefixes in the value of the first offset.
+ return PrefixesTable[PrefixesOffset].value();
}
-ArrayRef
-getPrefixOffsets(ArrayRef PrefixesTable) const {
- return hasNoPrefix() ? ArrayRef()
+ArrayRef
+getPrefixOffsets(ArrayRef PrefixesTable) const {
+ return hasNoPrefix() ? ArrayRef()
: PrefixesTable.slice(PrefixesOffset + 1,
getNumPrefixes(PrefixesTable));
}
-void appendPrefixes(const char *StrTable, ArrayRef PrefixesTable,
+void appendPrefixes(const StringTable &StrTable,
+ArrayRef PrefixesTable,
SmallVectorImpl &Prefixes) const {
- for (unsigned PrefixOffset : getPrefixOffsets(PrefixesTable))
-Prefixes.push_back(&StrTable[PrefixOffset]);
+ for (auto PrefixOffset : getPrefixOffsets(PrefixesTable))
+Prefixes.push_back(StrTable[PrefixOffset]);
}
-StringRef
[Lldb-commits] [lldb] [lldb] Add SBThread.selected_frame property (PR #123981)
@@ -51,6 +51,12 @@ STRING_EXTENSION_OUTSIDE(SBThread) for idx in range(self.GetStopReasonDataCount()) ] +def set_selected_frame(self, frame): +if isinstance(frame, SBFrame): +self.SetSelectedFrame(frame.idx) labath wrote: Maybe it should check that the frame really belongs to this thread before selecting a completely unrelated frame? https://github.com/llvm/llvm-project/pull/123981 ___ 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)
@@ -644,32 +644,22 @@ void ABISysV_loongarch::AugmentRegisterInfo(
std::vector ®s) {
lldb_private::RegInfoBasedABI::AugmentRegisterInfo(regs);
+ static const std::unordered_map reg_aliases = {
am11 wrote:
A nit: perhaps call it `isa_to_abi_alias_map`; r0-r31 is ISA convention and
a/s/t is ABI convention. (I think mostly ABI convention is favored in
discussions about LA64 and RV64)
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] Avoid repeated map lookups (NFC) (PR #124077)
https://github.com/kazutakahirata created
https://github.com/llvm/llvm-project/pull/124077
None
>From ef8125e2022772009b72d4b8b0da5db1b611d2c6 Mon Sep 17 00:00:00 2001
From: Kazu Hirata
Date: Wed, 22 Jan 2025 00:47:57 -0800
Subject: [PATCH] [lldb] Avoid repeated map lookups (NFC)
---
lldb/source/Target/DynamicRegisterInfo.cpp | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/lldb/source/Target/DynamicRegisterInfo.cpp
b/lldb/source/Target/DynamicRegisterInfo.cpp
index 1a817449fa9589..9ad98a41c688c8 100644
--- a/lldb/source/Target/DynamicRegisterInfo.cpp
+++ b/lldb/source/Target/DynamicRegisterInfo.cpp
@@ -460,8 +460,8 @@ void DynamicRegisterInfo::Finalize(const ArchSpec &arch) {
// Now update all value_regs with each register info as needed
const size_t num_regs = m_regs.size();
for (size_t i = 0; i < num_regs; ++i) {
-if (m_value_regs_map.find(i) != m_value_regs_map.end())
- m_regs[i].value_regs = m_value_regs_map[i].data();
+if (auto it = m_value_regs_map.find(i); it != m_value_regs_map.end())
+ m_regs[i].value_regs = it->second.data();
else
m_regs[i].value_regs = nullptr;
}
@@ -509,8 +509,9 @@ void DynamicRegisterInfo::Finalize(const ArchSpec &arch) {
// Now update all invalidate_regs with each register info as needed
for (size_t i = 0; i < num_regs; ++i) {
-if (m_invalidate_regs_map.find(i) != m_invalidate_regs_map.end())
- m_regs[i].invalidate_regs = m_invalidate_regs_map[i].data();
+if (auto it = m_invalidate_regs_map.find(i);
+it != m_invalidate_regs_map.end())
+ m_regs[i].invalidate_regs = it->second.data();
else
m_regs[i].invalidate_regs = nullptr;
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement a statusline in LLDB (PR #121860)
@@ -0,0 +1,165 @@ +//===-- Statusline.cpp ---===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#include "lldb/Core/Statusline.h" +#include "lldb/Core/Debugger.h" +#include "lldb/Core/FormatEntity.h" +#include "lldb/Host/ThreadLauncher.h" +#include "lldb/Interpreter/CommandInterpreter.h" +#include "lldb/Symbol/SymbolContext.h" +#include "lldb/Target/StackFrame.h" +#include "lldb/Utility/AnsiTerminal.h" +#include "lldb/Utility/LLDBLog.h" +#include "lldb/Utility/Log.h" +#include "lldb/Utility/StreamString.h" +#include "llvm/Support/Locale.h" + +#include +#include JDevlieghere wrote: I removed the corresponding code so this isn't necessary any longer. https://github.com/llvm/llvm-project/pull/121860 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Avoid repeated map lookups (NFC) (PR #124077)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Kazu Hirata (kazutakahirata)
Changes
---
Full diff: https://github.com/llvm/llvm-project/pull/124077.diff
1 Files Affected:
- (modified) lldb/source/Target/DynamicRegisterInfo.cpp (+5-4)
``diff
diff --git a/lldb/source/Target/DynamicRegisterInfo.cpp
b/lldb/source/Target/DynamicRegisterInfo.cpp
index 1a817449fa9589..9ad98a41c688c8 100644
--- a/lldb/source/Target/DynamicRegisterInfo.cpp
+++ b/lldb/source/Target/DynamicRegisterInfo.cpp
@@ -460,8 +460,8 @@ void DynamicRegisterInfo::Finalize(const ArchSpec &arch) {
// Now update all value_regs with each register info as needed
const size_t num_regs = m_regs.size();
for (size_t i = 0; i < num_regs; ++i) {
-if (m_value_regs_map.find(i) != m_value_regs_map.end())
- m_regs[i].value_regs = m_value_regs_map[i].data();
+if (auto it = m_value_regs_map.find(i); it != m_value_regs_map.end())
+ m_regs[i].value_regs = it->second.data();
else
m_regs[i].value_regs = nullptr;
}
@@ -509,8 +509,9 @@ void DynamicRegisterInfo::Finalize(const ArchSpec &arch) {
// Now update all invalidate_regs with each register info as needed
for (size_t i = 0; i < num_regs; ++i) {
-if (m_invalidate_regs_map.find(i) != m_invalidate_regs_map.end())
- m_regs[i].invalidate_regs = m_invalidate_regs_map[i].data();
+if (auto it = m_invalidate_regs_map.find(i);
+it != m_invalidate_regs_map.end())
+ m_regs[i].invalidate_regs = it->second.data();
else
m_regs[i].invalidate_regs = nullptr;
}
``
https://github.com/llvm/llvm-project/pull/124077
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
