[Lldb-commits] [lldb] Define Telemetry plugin for LLDB. (PR #126588)
labath wrote:
> This was what we were doing in the [initial
> PR](https://github.com/llvm/llvm-project/pull/98528/files#diff-20a2060f8e87c6742d6f2c7ae97e919f8485995d7808bd9fccbfbede697a9ec7)
> but Pavel had correctly pointed out that the architecture was unnecessarily
> "baroque". GIven there will be only one instance of the TelemetrManager at a
> time and that we will not be creating a new manager on the fly, none of those
> complex hooks are necessary.
I think you've summarized this correctly. I just want to elaborate/add some
colour a bit:
The (baroque) way that the other plugins work serves two purposes:
1. controlling the type (class) of the plugin being created based on some
runtime property
2. creating an arbitrary number of plugin instances
For telemetry "plugins", I don't think we want/need (1) because the type of the
plugin is determined by the vendor -- at build time. A user can't change that
at runtime. They may be able to disable collection, but they can't say "well,
today I feel like reporting my telemetry to company " and change a setting
or something to make that happen. I mean, it's probably doable, but I don't
think anyone has that as a requirement (if you do, let us know).
The second question is I think more interesting, and I think it boils down to
"what is the 'scope' of a TelemetryManager object". I can see two options, and
I think both of them are reasonable. One (which is what this PR does) is to
make it a process-wide object (a singleton). The other is to create one
instance per Debugger object. The advantage of the first one is that the object
is always available -- there's no problem with accessing the object from parts
of code (e.g. everything under the `Module` class) which are not tied to a
specific debugger instance (I'm sure you remember the contortions that we go
through to report progress events related to debug info parsing). The advantage
of the second one is that it fits in better with our object model and the
events we do report are automatically organized into a user session.
The reason I think the first one is better is that organizing events into
"sessions" is still possible with this model (just add a debugger_id field to
the reported event), but it still allows you to report debugger-less events,
but I could be convinced otherwise. However, even if we do that, I still don't
think we need a *list* of plugins (due to the first item). All we'd need to do
is replace `setInstance(std::unique_ptr)` with something like
`setInstanceCreateCallback(std::unique_ptr (*)(Debugger&))`.
> More of a question for @labath but do we have an example of another place
> where we have "semi plugins" like this and if not, what are existing plugins
> that could be reworked to be lighter weight?
We sort of do have a precedent for that, but it's in an unlikely place -- the
Platform plugins. It sounds hard to believe because Platforms are probably the
most complicated kinds of plugins, but part of that complexity comes from the
fact that they are doing two mostly separate things:
1. Registering themselves to handle the remote systems of the given kind. This
part is pretty much the same as all other plugins and uses the PluginManager,
iteration, and whatnot.
2. Registering themselves as the "Host" platform, if they match the current
build host. This part is achieved by a call to Platform::SetPlatform (and
that's an analogue to TelemetryManager::setInstance).
Here's how the PlatformLinux::Initialize looks like. The first part is just us
being "baroque":
```
PlatformPOSIX::Initialize();
if (g_initialize_count++ == 0) {
```
Next it does the the item (2)
```
#if defined(__linux__) && !defined(__ANDROID__)
PlatformSP default_platform_sp(new PlatformLinux(true));
default_platform_sp->SetSystemArchitecture(HostInfo::GetArchitecture());
Platform::SetHostPlatform(default_platform_sp);
#endif
```
And finally item (1)
```
PluginManager::RegisterPlugin(
PlatformLinux::GetPluginNameStatic(false),
PlatformLinux::GetPluginDescriptionStatic(false),
PlatformLinux::CreateInstance, nullptr);
}
```
Another slightly similar mechanism are the process plugins. ProcessLinux and
Process***BSD are a sort of a plugin, but they aren't even linked into lldb.
They are used by lldb-server, which only links the plugin which matches the
current build host (there's no runtime choice). I think it's reasonable to call
these "plugins" even though their choice is completely static.
> I think we need to have a sufficiently strong motivation to diverge from the
> existing way of writing plugins (or have a renaissance where we improve it
> for all the plugins).
Does this sound compelling enough?
Another similar mechanism we have is the ["static
polymorphism"](https://github.com/llvm/llvm-project/blob/55b0fde20a2ba1c67313cb4c8d6a30316facd6ad/lldb/include/lldb/Host/HostInfo.h#L35)
used in the Host classes. I would like to
[Lldb-commits] [lldb] [lldb] Fix broken pipe error (PR #127100)
@@ -397,21 +413,23 @@ def connect_to_debug_monitor(self, attach_pid=None):
# Schedule debug monitor to be shut down during teardown.
logger = self.logger
-connect_attemps = 0
+connect_attempts = 0
MAX_CONNECT_ATTEMPTS = 10
-while connect_attemps < MAX_CONNECT_ATTEMPTS:
-# Create a socket to talk to the server
-try:
-logger.info("Connect attempt %d", connect_attemps + 1)
-self.sock = self.create_socket()
sga-sc wrote:
Python somehow manages to create a socket when the server-side port is not yet
open. Because of this, the first time we try to read data from the socket the
code crashes with an exception:
```
4: test_qHostInfo_returns_at_least_one_key_val_pair_llgs
(TestGdbRemoteHostInfo.TestGdbRemoteHostInfo.test_qHostInfo_returns_at_least_one_key_val_pair_llgs)
... ERROR
FAIL: LLDB (/home/jorik/work/llvm-project/build/Release/bin/clang-rv64gc) ::
test_qHostInfo_returns_at_least_one_key_val_pair_llgs
(TestGdbRemoteHostInfo.TestGdbRemoteHostInfo.test_qHostInfo_returns_at_least_one_key_val_pair_llgs)
2025-02-14 12:04:31,718 WARNING failed to send kill packet to debug monitor:
; ignoring
==
ERROR: test_qHostInfo_returns_at_least_one_key_val_pair_llgs
(TestGdbRemoteHostInfo.TestGdbRemoteHostInfo.test_qHostInfo_returns_at_least_one_key_val_pair_llgs)
--
Traceback (most recent call last):
File
"/home/jorik/work/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py",
line 48, in test_method
return attrvalue(self)
^^^
File
"/home/jorik/work/llvm-project/lldb/test/API/tools/lldb-server/TestGdbRemoteHostInfo.py",
line 122, in test_qHostInfo_returns_at_least_one_key_val_pair
self.get_qHostInfo_response()
File
"/home/jorik/work/llvm-project/lldb/test/API/tools/lldb-server/TestGdbRemoteHostInfo.py",
line 90, in get_qHostInfo_response
self.do_handshake()
File
"/home/jorik/work/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py",
line 546, in do_handshake
self.assertEqual(server.get_normal_packet(), b"+")
^^
File
"/home/jorik/work/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py",
line 943, in get_normal_packet
frame = self.get_raw_normal_packet()
File
"/home/jorik/work/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py",
line 932, in get_raw_normal_packet
return self._read(self._normal_queue)
^^
File
"/home/jorik/work/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-server/lldbgdbserverutils.py",
line 885, in _read
new_bytes = self._sock.recv(4096)
^
ConnectionResetError: [Errno 104] Connection reset by peer
```
https://github.com/llvm/llvm-project/pull/127100
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add terminfo dependency for ncurses support (PR #126810)
@@ -2,23 +2,54 @@
# FindCursesAndPanel
# ---
#
-# Find the curses and panel library as a whole.
+# Find the curses, terminfo, and panel library as a whole.
-if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND PANEL_LIBRARIES)
+include(CMakePushCheckState)
+
+function(lldb_check_curses_tinfo CURSES_LIBRARIES CURSES_HAS_TINFO)
+ cmake_reset_check_state()
+ set(CMAKE_REQUIRED_LIBRARIES "${CURSES_LIBRARIES}")
+ # acs_map is one of many symbols that are part of tinfo but could
+ # be bundled in curses.
+ check_symbol_exists(acs_map "curses.h" CURSES_HAS_TINFO)
+endfunction()
+
+if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND TINFO_LIBRARIES AND
PANEL_LIBRARIES)
set(CURSESANDPANEL_FOUND TRUE)
else()
find_package(Curses QUIET)
find_library(PANEL_LIBRARIES NAMES panel DOC "The curses panel library"
QUIET)
include(FindPackageHandleStandardArgs)
+
+ # Sometimes the curses libraries define their own terminfo symbols,
+ # other times they're extern and are defined by a separate terminfo library.
+ # Auto-detect which.
+ lldb_check_curses_tinfo("${CURSES_LIBRARIES}" CURSES_HAS_TINFO)
+ if(CURSES_FOUND AND PANEL_LIBRARIES)
+if (NOT CURSES_HAS_TINFO)
JDevlieghere wrote:
```suggestion
if(CURSES_FOUND AND PANEL_LIBRARIES)
# Sometimes the curses libraries define their own terminfo symbols,
# other times they're extern and are defined by a separate terminfo library.
# Auto-detect which by looking for acs_map is one of many symbols that are
part
# of tinfo but could be bundled in curses.
check_symbol_exists(acs_map "curses.h" CURSES_HAS_TINFO)
if (NOT CURSES_HAS_TINFO)
```
https://github.com/llvm/llvm-project/pull/126810
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add terminfo dependency for ncurses support (PR #126810)
@@ -2,23 +2,54 @@
# FindCursesAndPanel
# ---
#
-# Find the curses and panel library as a whole.
+# Find the curses, terminfo, and panel library as a whole.
-if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND PANEL_LIBRARIES)
+include(CMakePushCheckState)
+
+function(lldb_check_curses_tinfo CURSES_LIBRARIES CURSES_HAS_TINFO)
+ cmake_reset_check_state()
+ set(CMAKE_REQUIRED_LIBRARIES "${CURSES_LIBRARIES}")
JDevlieghere wrote:
Fair enough, I didn't really consider the care of someone else having a
`curses.h` but this would definitely make this module more hermetic. Thanks for
explaining 👍
https://github.com/llvm/llvm-project/pull/126810
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Mach-O] Read dyld_all_image_infos addr from `main bin spec` LC_NOTE (PR #127156)
https://github.com/JDevlieghere edited https://github.com/llvm/llvm-project/pull/127156 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Mach-O] Read dyld_all_image_infos addr from `main bin spec` LC_NOTE (PR #127156)
@@ -81,9 +81,12 @@ class ObjectFile : public
std::enable_shared_from_this,
enum BinaryType {
eBinaryTypeInvalid = 0,
eBinaryTypeUnknown,
-eBinaryTypeKernel,/// kernel binary
-eBinaryTypeUser, /// user process binary
-eBinaryTypeStandalone /// standalone binary / firmware
+eBinaryTypeKernel,/// kernel binary
+eBinaryTypeUser, /// user process binary,
+ /// dyld addr
+eBinaryTypeUserAllImageInfos, /// user process binary,
+ /// dyld_all_image_infos addr
+eBinaryTypeStandalone /// standalone binary / firmware
JDevlieghere wrote:
```suggestion
/// kernel binary
eBinaryTypeKernel,
/// user process binary, dyld addr
eBinaryTypeUser,
/// user process binary, dyld_all_image_infos addr
eBinaryTypeUserAllImageInfos,
/// standalone binary / firmware
eBinaryTypeStandalone
```
Doxygen requires `///<` for inline comments like this. I don't know how that
works with multiline commetns and given our 80 col limit, documenting them
above the value is more readable anyway.
https://github.com/llvm/llvm-project/pull/127156
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add terminfo dependency for ncurses support (PR #126810)
https://github.com/JDevlieghere edited https://github.com/llvm/llvm-project/pull/126810 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Mach-O] Read dyld_all_image_infos addr from `main bin spec` LC_NOTE (PR #127156)
@@ -5669,6 +5673,10 @@ bool ObjectFileMachO::GetCorefileMainBinaryInfo(addr_t &value, type = eBinaryTypeStandalone; typestr = "standalone"; break; + case 4: +type = eBinaryTypeUserAllImageInfos; +typestr = "userland dyld_all_image_infos"; +break; JDevlieghere wrote: Should this have a default case that sets `type` to `eBinaryTypeInvalid`? https://github.com/llvm/llvm-project/pull/127156 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Provide default impl for MightHaveChildren (NFC) (PR #119977)
kastiglione wrote: @jimingham does this seem ok to you? https://github.com/llvm/llvm-project/pull/119977 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Synchronize the debugger's stdout and stderr streams (PR #126630)
JDevlieghere wrote: > > A random idea, I don't know if it's a good one: have one object > > (LockableStreamPair?) that holds the two streams and their mutex. One less > > argument to pass around? > > Sounds good, that simplifies the signatures (which this PR is already > touching) and keeps the mutex abstracted away. I did implement this and I'm not happy with the result: even though you only pass one object, you still need to get the output or error stream out of it which ends up being more verbose. And because of the `IOHandler` and `Debugger::AdoptTopIOHandlerFilesIfInvalid` it needs to be a shared pointer. Here's what that looks like: https://github.com/llvm/llvm-project/commit/d62fdaec4cd322574e1ab78c8cacd1effe2c29c0. It compiles but some tests are failing. Probably something small but given I don't like the approach I haven't looked into it. https://github.com/llvm/llvm-project/pull/126630 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add terminfo dependency for ncurses support (PR #126810)
@@ -2,23 +2,54 @@
# FindCursesAndPanel
# ---
#
-# Find the curses and panel library as a whole.
+# Find the curses, terminfo, and panel library as a whole.
-if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND PANEL_LIBRARIES)
+include(CMakePushCheckState)
+
+function(lldb_check_curses_tinfo CURSES_LIBRARIES CURSES_HAS_TINFO)
+ cmake_reset_check_state()
+ set(CMAKE_REQUIRED_LIBRARIES "${CURSES_LIBRARIES}")
ajordanr-google wrote:
This function is primarily here so that we don't have scope leak on modifying
`CMAKE_REQUIRED_LIBRARIES` (which we want to restrict to only our given curses
`.so`, and no other).
I'm not super sure how scope guarding works with how these `Find*` modules
work, but I wanted to be sure that we don't actually modify
CMAKE_REQUIRED_LIBRARIES outside of any other scope.
Not sure if that makes too much sense, but we _could_ move it out if you're
okay with other libraries providing `acs_map`, not just the found
`libcurses.so`.
https://github.com/llvm/llvm-project/pull/126810
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Mach-O] Read dyld_all_image_infos addr from `main bin spec` LC_NOTE (PR #127156)
https://github.com/JDevlieghere approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/127156 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add terminfo dependency for ncurses support (PR #126810)
@@ -11,6 +11,9 @@ set(LLDB_LIBEDIT_LIBS)
if (LLDB_ENABLE_CURSES)
list(APPEND LLDB_CURSES_LIBS ${PANEL_LIBRARIES} ${CURSES_LIBRARIES})
+ if(NOT CURSES_HAS_TINFO)
+list(APPEND LLDB_CURSES_LIBS ${TINFO_LIBRARIES})
+ endif()
ajordanr-google wrote:
I think that should be fine. Will test, and upload!
https://github.com/llvm/llvm-project/pull/126810
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add terminfo dependency for ncurses support (PR #126810)
@@ -2,23 +2,54 @@
# FindCursesAndPanel
# ---
#
-# Find the curses and panel library as a whole.
+# Find the curses, terminfo, and panel library as a whole.
-if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND PANEL_LIBRARIES)
+include(CMakePushCheckState)
+
+function(lldb_check_curses_tinfo CURSES_LIBRARIES CURSES_HAS_TINFO)
+ cmake_reset_check_state()
+ set(CMAKE_REQUIRED_LIBRARIES "${CURSES_LIBRARIES}")
JDevlieghere wrote:
Could we eliminate this if we moved and inlined the `check_symbol_exists` in
the if-check on line 28? Presumably at that point we know curses was found?
https://github.com/llvm/llvm-project/pull/126810
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add terminfo dependency for ncurses support (PR #126810)
@@ -11,6 +11,9 @@ set(LLDB_LIBEDIT_LIBS)
if (LLDB_ENABLE_CURSES)
list(APPEND LLDB_CURSES_LIBS ${PANEL_LIBRARIES} ${CURSES_LIBRARIES})
+ if(NOT CURSES_HAS_TINFO)
+list(APPEND LLDB_CURSES_LIBS ${TINFO_LIBRARIES})
+ endif()
JDevlieghere wrote:
How about we append `TINFO_LIBRARIES` to `CURSES_LIBRARIES`? That way we don't
need this at all, and nobody needs to know about `$CURSES_HAS_TINFO`?
https://github.com/llvm/llvm-project/pull/126810
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Allow option to ignore module load errors in ScriptedProcess (PR #127153)
rchamala wrote: Note that currently, I am returning failure if a wrong type is passed in to the dictionary via "get_loaded_images" or if "load_addr" is missing even for one module. Once the type checks are done, I am conditionally ignoring module load errors. https://github.com/llvm/llvm-project/pull/127153 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Allow option to ignore module load errors in ScriptedProcess (PR #127153)
https://github.com/rchamala edited https://github.com/llvm/llvm-project/pull/127153 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Allow option to ignore module load errors in ScriptedProcess (PR #127153)
https://github.com/rchamala edited https://github.com/llvm/llvm-project/pull/127153 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Add support for floating point template argument constants (PR #127206)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/127206
>From 759f0569807d00a059a78aeb3bd1eddeffcbdf36 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Fri, 14 Feb 2025 11:43:20 +
Subject: [PATCH 1/5] Init
---
lldb/include/lldb/Symbol/CompilerType.h | 3 +-
lldb/source/API/SBType.cpp| 8 +++-
.../Language/CPlusPlus/GenericBitset.cpp | 2 +-
.../Plugins/Language/CPlusPlus/LibCxxSpan.cpp | 2 +-
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 33 ++---
.../TypeSystem/Clang/TypeSystemClang.cpp | 47 +--
.../TestCppTemplateArguments.py | 31 ++--
.../API/lang/cpp/template-arguments/main.cpp | 6 +++
lldb/unittests/Symbol/TestTypeSystemClang.cpp | 42 -
9 files changed, 143 insertions(+), 31 deletions(-)
diff --git a/lldb/include/lldb/Symbol/CompilerType.h
b/lldb/include/lldb/Symbol/CompilerType.h
index 096a8f1ab68e8..f7e3e552f3e45 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -15,6 +15,7 @@
#include
#include "lldb/lldb-private.h"
+#include "clang/AST/APValue.h"
#include "llvm/ADT/APSInt.h"
#include "llvm/Support/Casting.h"
@@ -544,7 +545,7 @@ bool operator==(const CompilerType &lhs, const CompilerType
&rhs);
bool operator!=(const CompilerType &lhs, const CompilerType &rhs);
struct CompilerType::IntegralTemplateArgument {
- llvm::APSInt value;
+ clang::APValue value;
CompilerType type;
};
diff --git a/lldb/source/API/SBType.cpp b/lldb/source/API/SBType.cpp
index 6401d32c85795..72f590947dff6 100644
--- a/lldb/source/API/SBType.cpp
+++ b/lldb/source/API/SBType.cpp
@@ -697,6 +697,7 @@ lldb::SBValue
SBType::GetTemplateArgumentValue(lldb::SBTarget target,
std::optional arg;
const bool expand_pack = true;
switch (GetTemplateArgumentKind(idx)) {
+ case eTemplateArgumentKindStructuralValue:
case eTemplateArgumentKindIntegral:
arg = m_opaque_sp->GetCompilerType(false).GetIntegralTemplateArgument(
idx, expand_pack);
@@ -708,7 +709,12 @@ lldb::SBValue
SBType::GetTemplateArgumentValue(lldb::SBTarget target,
if (!arg)
return {};
- Scalar value{arg->value};
+ Scalar value;
+ if (arg->value.isFloat())
+value = arg->value.getFloat();
+ else
+value = arg->value.getInt();
+
DataExtractor data;
value.GetData(data);
diff --git a/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
b/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
index 33955dccb6ccc..99ff975825c71 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
@@ -91,7 +91,7 @@ lldb::ChildCacheState GenericBitsetFrontEnd::Update() {
size_t size = 0;
if (auto arg = m_backend.GetCompilerType().GetIntegralTemplateArgument(0))
-size = arg->value.getLimitedValue();
+size = arg->value.getInt().getLimitedValue();
m_elements.assign(size, ValueObjectSP());
m_first =
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
index 15040295efe6d..687ef1739ad11 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
@@ -119,7 +119,7 @@
lldb_private::formatters::LibcxxStdSpanSyntheticFrontEnd::Update() {
} else if (auto arg =
m_backend.GetCompilerType().GetIntegralTemplateArgument(1))
{
- m_num_elements = arg->value.getLimitedValue();
+ m_num_elements = arg->value.getInt().getLimitedValue();
}
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index ec0004c70c6da..70af283ab7443 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1973,6 +1973,27 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
ClangASTMetadata m_metadata;
};
+static clang::APValue MakeAPValue(CompilerType clang_type, uint64_t bit_width,
+ uint64_t value) {
+ bool is_signed = false;
+ const bool is_integral = clang_type.IsIntegerOrEnumerationType(is_signed);
+
+ llvm::APSInt apint(bit_width, !is_signed);
+ apint = value;
+
+ if (is_integral)
+return clang::APValue(apint);
+
+ uint32_t count;
+ bool is_complex;
+ assert(clang_type.IsFloatingPointType(count, is_complex));
+
+ if (bit_width == 32)
+return clang::APValue(llvm::APFloat(apint.bitsToFloat()));
+
+ return clang::APValue(llvm::APFloat(apint.bitsToDouble()));
+}
+
bool DWARFASTParserClang::ParseTemplateDIE(
const DWARFDIE &die,
TypeSystemClang::TemplateParameterInfos &template_param_infos) {
@@ -2050,9 +2071,6 @@ bool DWARFASTParserClang::ParseTemplateDIE(
clang_type = m_ast.GetBasicType(eBasicTypeVoid);
if (!is_templat
[Lldb-commits] [lldb] [llvm] [lldb-dap] Add: show return value on step out (PR #106907)
https://github.com/Da-Viper updated
https://github.com/llvm/llvm-project/pull/106907
>From aeb8854bbe7695e576257c8403e04d4b8268b679 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka
Date: Sun, 1 Sep 2024 13:48:41 +0100
Subject: [PATCH 01/10] Add: show return value on step out
---
lldb/tools/lldb-dap/lldb-dap.cpp | 11 +++
1 file changed, 11 insertions(+)
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index e323990d8b6ed..3cacc16dedb25 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4225,6 +4225,17 @@ void request_variables(DAP &dap, const
llvm::json::Object &request) {
variable_name_counts[GetNonNullVariableName(variable)]++;
}
+// Show return value if there is any ( in the top frame )
+auto process = g_dap.target.GetProcess();
+auto selectedThread = process.GetSelectedThread();
+lldb::SBValue stopReturnValue = selectedThread.GetStopReturnValue();
+if (stopReturnValue.IsValid() &&
+(selectedThread.GetSelectedFrame().GetFrameID() == 0)) {
+ auto renamedReturnValue = stopReturnValue.Clone("(Return Value)");
+ variables.emplace_back(
+ CreateVariable(renamedReturnValue,0, UINT64_MAX, hex, false));
+}
+
// Now we construct the result with unique display variable names
for (auto i = start_idx; i < end_idx; ++i) {
lldb::SBValue variable = top_scope->GetValueAtIndex(i);
>From 07c3c56bc987cdd947b8de126be6497f45f1da7b Mon Sep 17 00:00:00 2001
From: Ezike Ebuka
Date: Sun, 1 Sep 2024 14:10:54 +0100
Subject: [PATCH 02/10] format file
---
lldb/tools/lldb-dap/lldb-dap.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 3cacc16dedb25..30b3354c184b0 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4233,7 +4233,7 @@ void request_variables(DAP &dap, const llvm::json::Object
&request) {
(selectedThread.GetSelectedFrame().GetFrameID() == 0)) {
auto renamedReturnValue = stopReturnValue.Clone("(Return Value)");
variables.emplace_back(
- CreateVariable(renamedReturnValue,0, UINT64_MAX, hex, false));
+ CreateVariable(renamedReturnValue, 0, UINT64_MAX, hex, false));
}
// Now we construct the result with unique display variable names
>From c9ef0294def8919536ade9f8097c3471b94b0355 Mon Sep 17 00:00:00 2001
From: Ezike Ebuka
Date: Sun, 15 Sep 2024 22:22:09 +0100
Subject: [PATCH 03/10] [lldb-dap] Add: Show children for return values
---
lldb/tools/lldb-dap/lldb-dap.cpp | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 30b3354c184b0..00584f31848cf 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -4227,13 +4227,19 @@ void request_variables(DAP &dap, const
llvm::json::Object &request) {
// Show return value if there is any ( in the top frame )
auto process = g_dap.target.GetProcess();
-auto selectedThread = process.GetSelectedThread();
-lldb::SBValue stopReturnValue = selectedThread.GetStopReturnValue();
-if (stopReturnValue.IsValid() &&
-(selectedThread.GetSelectedFrame().GetFrameID() == 0)) {
- auto renamedReturnValue = stopReturnValue.Clone("(Return Value)");
- variables.emplace_back(
- CreateVariable(renamedReturnValue, 0, UINT64_MAX, hex, false));
+auto selected_thread = process.GetSelectedThread();
+lldb::SBValue stop_return_value = selected_thread.GetStopReturnValue();
+if (stop_return_value.IsValid() &&
+(selected_thread.GetSelectedFrame().GetFrameID() == 0)) {
+ auto renamed_return_value = stop_return_value.Clone("(Return Value)");
+ int64_t return_ref = 0;
+ if (stop_return_value.MightHaveChildren() ||
+ stop_return_value.IsSynthetic()) {
+return_ref = g_dap.variables.InsertExpandableVariable(
+stop_return_value, /*is_permanent=*/false);
+ }
+ variables.emplace_back(CreateVariable(renamed_return_value, return_ref,
+UINT64_MAX, hex, false));
}
// Now we construct the result with unique display variable names
>From 91e7042f9c3438b13ff8b476d07057da52b2589f Mon Sep 17 00:00:00 2001
From: Ezike Ebuka
Date: Wed, 12 Feb 2025 00:16:46 +
Subject: [PATCH 04/10] [lldb-dap] Add Tests: Show children for return values
---
.../lldb-dap/variables/TestDAP_variables.py | 57 +++
.../children/TestDAP_variables_children.py| 55 ++
.../lldb-dap/variables/children/main.cpp | 12
.../API/tools/lldb-dap/variables/main.cpp | 9 +++
4 files changed, 122 insertions(+), 11 deletions(-)
diff --git a/lldb/test/API/tools/lldb-dap/variables/TestDAP_variables.py
b/lldb/test/API/tools/lldb-dap/variables/TestDAP_varia
[Lldb-commits] [lldb] [lldb] Add terminfo dependency for ncurses support (PR #126810)
https://github.com/ajordanr-google updated
https://github.com/llvm/llvm-project/pull/126810
>From ddd3febff5b77cc7b2101996d49729added00f2b Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead
Date: Tue, 1 Oct 2024 18:41:28 +
Subject: [PATCH 1/3] [lldb] Add terminfo dependency for ncurses support
For some operating systems (e.g. chromiumos), terminfo is a separate
package and library from ncurses. Both are still requirements for curses
support in lldb, individually.
This is a rebase of this original spack commit:
https://github.com/spack/spack/commit/9ea261265010eacd250691a8361f661d0576f25c
Without this fix, LLDB cannot be built on these systems.
Fixes #101368
---
lldb/cmake/modules/FindCursesAndPanel.cmake | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/lldb/cmake/modules/FindCursesAndPanel.cmake
b/lldb/cmake/modules/FindCursesAndPanel.cmake
index aaadf214bf54b..df4980cc5e0d1 100644
--- a/lldb/cmake/modules/FindCursesAndPanel.cmake
+++ b/lldb/cmake/modules/FindCursesAndPanel.cmake
@@ -2,12 +2,15 @@
# FindCursesAndPanel
# ---
#
-# Find the curses and panel library as a whole.
+# Find the curses, terminfo, and panel library as a whole.
+# NOTE: terminfo and curses libraries are required separately, as
+# some systems do not bundle them together.
-if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND PANEL_LIBRARIES)
+if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND TINFO_LIBRARIES AND
PANEL_LIBRARIES)
set(CURSESANDPANEL_FOUND TRUE)
else()
find_package(Curses QUIET)
+ find_package(TINFO_LIBRARIES NAMES tinfo DOC "The curses tinfo library"
QUIET)
find_library(PANEL_LIBRARIES NAMES panel DOC "The curses panel library"
QUIET)
include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(CursesAndPanel
@@ -16,9 +19,10 @@ else()
REQUIRED_VARS
CURSES_INCLUDE_DIRS
CURSES_LIBRARIES
+ TINFO_LIBRARIES
PANEL_LIBRARIES)
- if(CURSES_FOUND AND PANEL_LIBRARIES)
-mark_as_advanced(CURSES_INCLUDE_DIRS CURSES_LIBRARIES PANEL_LIBRARIES)
+ if(CURSES_FOUND AND TINFO_LIBRARIES AND PANEL_LIBRARIES)
+mark_as_advanced(CURSES_INCLUDE_DIRS CURSES_LIBRARIES TINFO_LIBRARIES
PANEL_LIBRARIES)
endif()
endif()
>From 41ad79112bc2242a1cb10ae688353863155d9038 Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead
Date: Wed, 12 Feb 2025 18:22:09 +
Subject: [PATCH 2/3] fixup! [lldb] Add terminfo dependency for ncurses support
---
lldb/source/Core/CMakeLists.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index cf5f6ac9da489..6c7a50056f751 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -10,7 +10,7 @@ set(LLDB_CURSES_LIBS)
set(LLDB_LIBEDIT_LIBS)
if (LLDB_ENABLE_CURSES)
- list(APPEND LLDB_CURSES_LIBS ${PANEL_LIBRARIES} ${CURSES_LIBRARIES})
+ list(APPEND LLDB_CURSES_LIBS ${PANEL_LIBRARIES} ${CURSES_LIBRARIES}
${TINFO_LIBRARIES})
if (LLVM_BUILD_STATIC)
list(APPEND LLDB_CURSES_LIBS gpm)
endif()
>From 9ffc89356cc635ac31f11c66dcb6f32cd73380fc Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead
Date: Wed, 12 Feb 2025 19:29:23 +
Subject: [PATCH 3/3] fixup! fixup! [lldb] Add terminfo dependency for ncurses
support
---
lldb/cmake/modules/FindCursesAndPanel.cmake | 41 +
lldb/cmake/modules/FindTerminfo.cmake | 21 +++
lldb/source/Core/CMakeLists.txt | 5 ++-
3 files changed, 59 insertions(+), 8 deletions(-)
create mode 100644 lldb/cmake/modules/FindTerminfo.cmake
diff --git a/lldb/cmake/modules/FindCursesAndPanel.cmake
b/lldb/cmake/modules/FindCursesAndPanel.cmake
index df4980cc5e0d1..646113155343d 100644
--- a/lldb/cmake/modules/FindCursesAndPanel.cmake
+++ b/lldb/cmake/modules/FindCursesAndPanel.cmake
@@ -3,26 +3,53 @@
# ---
#
# Find the curses, terminfo, and panel library as a whole.
-# NOTE: terminfo and curses libraries are required separately, as
-# some systems do not bundle them together.
+
+include(CMakePushCheckState)
+
+function(lldb_check_curses_tinfo CURSES_LIBRARIES CURSES_HAS_TINFO)
+ cmake_reset_check_state()
+ set(CMAKE_REQUIRED_LIBRARIES "${CURSES_LIBRARIES}")
+ # acs_map is one of many symbols that are part of tinfo but could
+ # be bundled in curses.
+ check_symbol_exists(acs_map "curses.h" CURSES_HAS_TINFO)
+endfunction()
if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND TINFO_LIBRARIES AND
PANEL_LIBRARIES)
set(CURSESANDPANEL_FOUND TRUE)
else()
find_package(Curses QUIET)
- find_package(TINFO_LIBRARIES NAMES tinfo DOC "The curses tinfo library"
QUIET)
find_library(PANEL_LIBRARIES NAMES panel DOC "The curses panel library"
QUIET)
include(FindPackageHandleStandardArgs)
+
+ # Sometimes the curs
[Lldb-commits] [lldb] [lldb] Add terminfo dependency for ncurses support (PR #126810)
https://github.com/ajordanr-google updated
https://github.com/llvm/llvm-project/pull/126810
>From ddd3febff5b77cc7b2101996d49729added00f2b Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead
Date: Tue, 1 Oct 2024 18:41:28 +
Subject: [PATCH 1/5] [lldb] Add terminfo dependency for ncurses support
For some operating systems (e.g. chromiumos), terminfo is a separate
package and library from ncurses. Both are still requirements for curses
support in lldb, individually.
This is a rebase of this original spack commit:
https://github.com/spack/spack/commit/9ea261265010eacd250691a8361f661d0576f25c
Without this fix, LLDB cannot be built on these systems.
Fixes #101368
---
lldb/cmake/modules/FindCursesAndPanel.cmake | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/lldb/cmake/modules/FindCursesAndPanel.cmake
b/lldb/cmake/modules/FindCursesAndPanel.cmake
index aaadf214bf54b..df4980cc5e0d1 100644
--- a/lldb/cmake/modules/FindCursesAndPanel.cmake
+++ b/lldb/cmake/modules/FindCursesAndPanel.cmake
@@ -2,12 +2,15 @@
# FindCursesAndPanel
# ---
#
-# Find the curses and panel library as a whole.
+# Find the curses, terminfo, and panel library as a whole.
+# NOTE: terminfo and curses libraries are required separately, as
+# some systems do not bundle them together.
-if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND PANEL_LIBRARIES)
+if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND TINFO_LIBRARIES AND
PANEL_LIBRARIES)
set(CURSESANDPANEL_FOUND TRUE)
else()
find_package(Curses QUIET)
+ find_package(TINFO_LIBRARIES NAMES tinfo DOC "The curses tinfo library"
QUIET)
find_library(PANEL_LIBRARIES NAMES panel DOC "The curses panel library"
QUIET)
include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(CursesAndPanel
@@ -16,9 +19,10 @@ else()
REQUIRED_VARS
CURSES_INCLUDE_DIRS
CURSES_LIBRARIES
+ TINFO_LIBRARIES
PANEL_LIBRARIES)
- if(CURSES_FOUND AND PANEL_LIBRARIES)
-mark_as_advanced(CURSES_INCLUDE_DIRS CURSES_LIBRARIES PANEL_LIBRARIES)
+ if(CURSES_FOUND AND TINFO_LIBRARIES AND PANEL_LIBRARIES)
+mark_as_advanced(CURSES_INCLUDE_DIRS CURSES_LIBRARIES TINFO_LIBRARIES
PANEL_LIBRARIES)
endif()
endif()
>From 41ad79112bc2242a1cb10ae688353863155d9038 Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead
Date: Wed, 12 Feb 2025 18:22:09 +
Subject: [PATCH 2/5] fixup! [lldb] Add terminfo dependency for ncurses support
---
lldb/source/Core/CMakeLists.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index cf5f6ac9da489..6c7a50056f751 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -10,7 +10,7 @@ set(LLDB_CURSES_LIBS)
set(LLDB_LIBEDIT_LIBS)
if (LLDB_ENABLE_CURSES)
- list(APPEND LLDB_CURSES_LIBS ${PANEL_LIBRARIES} ${CURSES_LIBRARIES})
+ list(APPEND LLDB_CURSES_LIBS ${PANEL_LIBRARIES} ${CURSES_LIBRARIES}
${TINFO_LIBRARIES})
if (LLVM_BUILD_STATIC)
list(APPEND LLDB_CURSES_LIBS gpm)
endif()
>From 9ffc89356cc635ac31f11c66dcb6f32cd73380fc Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead
Date: Wed, 12 Feb 2025 19:29:23 +
Subject: [PATCH 3/5] fixup! fixup! [lldb] Add terminfo dependency for ncurses
support
---
lldb/cmake/modules/FindCursesAndPanel.cmake | 41 +
lldb/cmake/modules/FindTerminfo.cmake | 21 +++
lldb/source/Core/CMakeLists.txt | 5 ++-
3 files changed, 59 insertions(+), 8 deletions(-)
create mode 100644 lldb/cmake/modules/FindTerminfo.cmake
diff --git a/lldb/cmake/modules/FindCursesAndPanel.cmake
b/lldb/cmake/modules/FindCursesAndPanel.cmake
index df4980cc5e0d1..646113155343d 100644
--- a/lldb/cmake/modules/FindCursesAndPanel.cmake
+++ b/lldb/cmake/modules/FindCursesAndPanel.cmake
@@ -3,26 +3,53 @@
# ---
#
# Find the curses, terminfo, and panel library as a whole.
-# NOTE: terminfo and curses libraries are required separately, as
-# some systems do not bundle them together.
+
+include(CMakePushCheckState)
+
+function(lldb_check_curses_tinfo CURSES_LIBRARIES CURSES_HAS_TINFO)
+ cmake_reset_check_state()
+ set(CMAKE_REQUIRED_LIBRARIES "${CURSES_LIBRARIES}")
+ # acs_map is one of many symbols that are part of tinfo but could
+ # be bundled in curses.
+ check_symbol_exists(acs_map "curses.h" CURSES_HAS_TINFO)
+endfunction()
if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND TINFO_LIBRARIES AND
PANEL_LIBRARIES)
set(CURSESANDPANEL_FOUND TRUE)
else()
find_package(Curses QUIET)
- find_package(TINFO_LIBRARIES NAMES tinfo DOC "The curses tinfo library"
QUIET)
find_library(PANEL_LIBRARIES NAMES panel DOC "The curses panel library"
QUIET)
include(FindPackageHandleStandardArgs)
+
+ # Sometimes the curs
[Lldb-commits] [lldb] [lldb] Add terminfo dependency for ncurses support (PR #126810)
https://github.com/ajordanr-google edited https://github.com/llvm/llvm-project/pull/126810 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add terminfo dependency for ncurses support (PR #126810)
https://github.com/ajordanr-google updated
https://github.com/llvm/llvm-project/pull/126810
>From ddd3febff5b77cc7b2101996d49729added00f2b Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead
Date: Tue, 1 Oct 2024 18:41:28 +
Subject: [PATCH 1/4] [lldb] Add terminfo dependency for ncurses support
For some operating systems (e.g. chromiumos), terminfo is a separate
package and library from ncurses. Both are still requirements for curses
support in lldb, individually.
This is a rebase of this original spack commit:
https://github.com/spack/spack/commit/9ea261265010eacd250691a8361f661d0576f25c
Without this fix, LLDB cannot be built on these systems.
Fixes #101368
---
lldb/cmake/modules/FindCursesAndPanel.cmake | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/lldb/cmake/modules/FindCursesAndPanel.cmake
b/lldb/cmake/modules/FindCursesAndPanel.cmake
index aaadf214bf54b..df4980cc5e0d1 100644
--- a/lldb/cmake/modules/FindCursesAndPanel.cmake
+++ b/lldb/cmake/modules/FindCursesAndPanel.cmake
@@ -2,12 +2,15 @@
# FindCursesAndPanel
# ---
#
-# Find the curses and panel library as a whole.
+# Find the curses, terminfo, and panel library as a whole.
+# NOTE: terminfo and curses libraries are required separately, as
+# some systems do not bundle them together.
-if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND PANEL_LIBRARIES)
+if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND TINFO_LIBRARIES AND
PANEL_LIBRARIES)
set(CURSESANDPANEL_FOUND TRUE)
else()
find_package(Curses QUIET)
+ find_package(TINFO_LIBRARIES NAMES tinfo DOC "The curses tinfo library"
QUIET)
find_library(PANEL_LIBRARIES NAMES panel DOC "The curses panel library"
QUIET)
include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(CursesAndPanel
@@ -16,9 +19,10 @@ else()
REQUIRED_VARS
CURSES_INCLUDE_DIRS
CURSES_LIBRARIES
+ TINFO_LIBRARIES
PANEL_LIBRARIES)
- if(CURSES_FOUND AND PANEL_LIBRARIES)
-mark_as_advanced(CURSES_INCLUDE_DIRS CURSES_LIBRARIES PANEL_LIBRARIES)
+ if(CURSES_FOUND AND TINFO_LIBRARIES AND PANEL_LIBRARIES)
+mark_as_advanced(CURSES_INCLUDE_DIRS CURSES_LIBRARIES TINFO_LIBRARIES
PANEL_LIBRARIES)
endif()
endif()
>From 41ad79112bc2242a1cb10ae688353863155d9038 Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead
Date: Wed, 12 Feb 2025 18:22:09 +
Subject: [PATCH 2/4] fixup! [lldb] Add terminfo dependency for ncurses support
---
lldb/source/Core/CMakeLists.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index cf5f6ac9da489..6c7a50056f751 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -10,7 +10,7 @@ set(LLDB_CURSES_LIBS)
set(LLDB_LIBEDIT_LIBS)
if (LLDB_ENABLE_CURSES)
- list(APPEND LLDB_CURSES_LIBS ${PANEL_LIBRARIES} ${CURSES_LIBRARIES})
+ list(APPEND LLDB_CURSES_LIBS ${PANEL_LIBRARIES} ${CURSES_LIBRARIES}
${TINFO_LIBRARIES})
if (LLVM_BUILD_STATIC)
list(APPEND LLDB_CURSES_LIBS gpm)
endif()
>From 9ffc89356cc635ac31f11c66dcb6f32cd73380fc Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead
Date: Wed, 12 Feb 2025 19:29:23 +
Subject: [PATCH 3/4] fixup! fixup! [lldb] Add terminfo dependency for ncurses
support
---
lldb/cmake/modules/FindCursesAndPanel.cmake | 41 +
lldb/cmake/modules/FindTerminfo.cmake | 21 +++
lldb/source/Core/CMakeLists.txt | 5 ++-
3 files changed, 59 insertions(+), 8 deletions(-)
create mode 100644 lldb/cmake/modules/FindTerminfo.cmake
diff --git a/lldb/cmake/modules/FindCursesAndPanel.cmake
b/lldb/cmake/modules/FindCursesAndPanel.cmake
index df4980cc5e0d1..646113155343d 100644
--- a/lldb/cmake/modules/FindCursesAndPanel.cmake
+++ b/lldb/cmake/modules/FindCursesAndPanel.cmake
@@ -3,26 +3,53 @@
# ---
#
# Find the curses, terminfo, and panel library as a whole.
-# NOTE: terminfo and curses libraries are required separately, as
-# some systems do not bundle them together.
+
+include(CMakePushCheckState)
+
+function(lldb_check_curses_tinfo CURSES_LIBRARIES CURSES_HAS_TINFO)
+ cmake_reset_check_state()
+ set(CMAKE_REQUIRED_LIBRARIES "${CURSES_LIBRARIES}")
+ # acs_map is one of many symbols that are part of tinfo but could
+ # be bundled in curses.
+ check_symbol_exists(acs_map "curses.h" CURSES_HAS_TINFO)
+endfunction()
if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND TINFO_LIBRARIES AND
PANEL_LIBRARIES)
set(CURSESANDPANEL_FOUND TRUE)
else()
find_package(Curses QUIET)
- find_package(TINFO_LIBRARIES NAMES tinfo DOC "The curses tinfo library"
QUIET)
find_library(PANEL_LIBRARIES NAMES panel DOC "The curses panel library"
QUIET)
include(FindPackageHandleStandardArgs)
+
+ # Sometimes the curs
[Lldb-commits] [lldb] [lldb] Add terminfo dependency for ncurses support (PR #126810)
ajordanr-google wrote: Tested with a system that has bundled terminfo in curses and one without locally! https://github.com/llvm/llvm-project/pull/126810 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 776fa2d - [lldb] Gardening in IOHandlerCurses (NFC)
Author: Jonas Devlieghere
Date: 2025-02-14T22:28:42-08:00
New Revision: 776fa2d731c17d6ba0afad2554ebc89cf5e3e5ef
URL:
https://github.com/llvm/llvm-project/commit/776fa2d731c17d6ba0afad2554ebc89cf5e3e5ef
DIFF:
https://github.com/llvm/llvm-project/commit/776fa2d731c17d6ba0afad2554ebc89cf5e3e5ef.diff
LOG: [lldb] Gardening in IOHandlerCurses (NFC)
- Remove _ap (auto_ptr) suffix with _up (unique_ptr) suffix
- Move forward declaration from IOHandler.h to IOHandlerCursesGUI.h
- Move curses namespace under lldb_private
Motivated by Alex' comment in #126630.
Added:
Modified:
lldb/include/lldb/Core/IOHandler.h
lldb/include/lldb/Core/IOHandlerCursesGUI.h
lldb/source/Core/IOHandlerCursesGUI.cpp
Removed:
diff --git a/lldb/include/lldb/Core/IOHandler.h
b/lldb/include/lldb/Core/IOHandler.h
index cb14d72413209..d6ac1cc8b5a14 100644
--- a/lldb/include/lldb/Core/IOHandler.h
+++ b/lldb/include/lldb/Core/IOHandler.h
@@ -32,11 +32,6 @@ namespace lldb_private {
class Debugger;
} // namespace lldb_private
-namespace curses {
-class Application;
-typedef std::unique_ptr ApplicationAP;
-} // namespace curses
-
namespace lldb_private {
class IOHandler {
diff --git a/lldb/include/lldb/Core/IOHandlerCursesGUI.h
b/lldb/include/lldb/Core/IOHandlerCursesGUI.h
index 22ca735063ba1..e9871e0532194 100644
--- a/lldb/include/lldb/Core/IOHandlerCursesGUI.h
+++ b/lldb/include/lldb/Core/IOHandlerCursesGUI.h
@@ -12,6 +12,9 @@
#include "lldb/Core/IOHandler.h"
namespace lldb_private {
+namespace curses {
+class Application;
+} // namespace curses
class IOHandlerCursesGUI : public IOHandler {
public:
@@ -34,7 +37,7 @@ class IOHandlerCursesGUI : public IOHandler {
void TerminalSizeChanged() override;
protected:
- curses::ApplicationAP m_app_ap;
+ std::unique_ptr m_app_up;
};
} // namespace lldb_private
diff --git a/lldb/source/Core/IOHandlerCursesGUI.cpp
b/lldb/source/Core/IOHandlerCursesGUI.cpp
index 456ce7d16e102..c5eed0c0b4089 100644
--- a/lldb/source/Core/IOHandlerCursesGUI.cpp
+++ b/lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -94,6 +94,7 @@ using llvm::StringRef;
#define KEY_SHIFT_TAB (KEY_MAX + 1)
#define KEY_ALT_ENTER (KEY_MAX + 2)
+namespace lldb_private {
namespace curses {
class Menu;
class MenuDelegate;
@@ -4479,8 +4480,9 @@ class Application {
};
} // namespace curses
+} // namespace lldb_private
-using namespace curses;
+using namespace lldb_private::curses;
struct Row {
ValueObjectUpdater value;
@@ -7573,12 +7575,12 @@ IOHandlerCursesGUI::IOHandlerCursesGUI(Debugger
&debugger)
void IOHandlerCursesGUI::Activate() {
IOHandler::Activate();
- if (!m_app_ap) {
-m_app_ap = std::make_unique(GetInputFILE(), GetOutputFILE());
+ if (!m_app_up) {
+m_app_up = std::make_unique(GetInputFILE(), GetOutputFILE());
// This is both a window and a menu delegate
std::shared_ptr app_delegate_sp(
-new ApplicationDelegate(*m_app_ap, m_debugger));
+new ApplicationDelegate(*m_app_up, m_debugger));
MenuDelegateSP app_menu_delegate_sp =
std::static_pointer_cast(app_delegate_sp);
@@ -7652,8 +7654,8 @@ void IOHandlerCursesGUI::Activate() {
help_menu_sp->AddSubmenu(MenuSP(new Menu(
"GUI Help", nullptr, 'g', ApplicationDelegate::eMenuID_HelpGUIHelp)));
-m_app_ap->Initialize();
-WindowSP &main_window_sp = m_app_ap->GetMainWindow();
+m_app_up->Initialize();
+WindowSP &main_window_sp = m_app_up->GetMainWindow();
MenuSP menubar_sp(new Menu(Menu::Type::Bar));
menubar_sp->AddSubmenu(lldb_menu_sp);
@@ -7734,10 +7736,10 @@ void IOHandlerCursesGUI::Activate() {
}
}
-void IOHandlerCursesGUI::Deactivate() { m_app_ap->Terminate(); }
+void IOHandlerCursesGUI::Deactivate() { m_app_up->Terminate(); }
void IOHandlerCursesGUI::Run() {
- m_app_ap->Run(m_debugger);
+ m_app_up->Run(m_debugger);
SetIsDone(true);
}
@@ -7752,7 +7754,7 @@ bool IOHandlerCursesGUI::Interrupt() {
void IOHandlerCursesGUI::GotEOF() {}
void IOHandlerCursesGUI::TerminalSizeChanged() {
- m_app_ap->TerminalSizeChanged();
+ m_app_up->TerminalSizeChanged();
}
#endif // LLDB_ENABLE_CURSES
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Synchronize the debugger's stdout and stderr streams (PR #126630)
@@ -7574,7 +7574,9 @@ IOHandlerCursesGUI::IOHandlerCursesGUI(Debugger &debugger)
void IOHandlerCursesGUI::Activate() {
IOHandler::Activate();
if (!m_app_ap) {
-m_app_ap = std::make_unique(GetInputFILE(), GetOutputFILE());
+m_app_ap = std::make_unique(
JDevlieghere wrote:
Addressed in 776fa2d731c17d6ba0afad2554ebc89cf5e3e5ef
https://github.com/llvm/llvm-project/pull/126630
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Synchronize the debugger's stdout and stderr streams (PR #126630)
@@ -500,19 +502,16 @@ bool
CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
void CommandObjectExpression::IOHandlerInputComplete(IOHandler &io_handler,
std::string &line) {
io_handler.SetIsDone(true);
- StreamFileSP output_sp = io_handler.GetOutputStreamFileSP();
- StreamFileSP error_sp = io_handler.GetErrorStreamFileSP();
+ LockedStreamFile locked_output_stream =
+ io_handler.GetOutputStreamFileSP()->Lock();
+ LockedStreamFile locked_error_stream =
+ io_handler.GetErrorStreamFileSP()->Lock();
JDevlieghere wrote:
Fixed in ed32d85d31999756602a7d5c4647cb6771d8f857
https://github.com/llvm/llvm-project/pull/126630
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Define Telemetry plugin for LLDB. (PR #126588)
oontvoo wrote: > > The (baroque) way that the other plugins work serves two purposes: > > > > 1. controlling the type (class) of the plugin being created based on some > > runtime property > > 2. creating an arbitrary number of plugin instances > > > > For telemetry "plugins", I don't think we want/need (1) because the type of > > the plugin is determined by the vendor -- at build time. A user can't > > change that at runtime. They may be able to disable collection, but they > > can't say "well, today I feel like reporting my telemetry to company ``" > > and change a setting or something to make that happen. I mean, it's > > probably doable, but I don't think anyone has that as a requirement (if you > > do, let us know). > > I don't think anyone has that requirement but I could see a few situations > where that could be useful. For example we could have a "textual telemetry" > fallback plugin that we use for testing and users could turn on the audit > what's being reported. This sounds like it could be done via configuration on the `Destination` (ie., where the collected data ended up), rather than the TelemetryManager (plugin) itself. > We don't need to inherit from `PluginInterface` as it really doesn't give you > anything. >From my understanding, the benefit of inheriting from the interface is you >can re-use the existing cmake and C++ macros magic for defining vendor impl - >that is, you just put `LLDB_PLUGIN_DEFINE(MyPlugin)` in the vendor code, along >with the cmake declaration and you're done! The framework takes care of >ensuring the `::Initialize()` is called to set the instance. > How does that sound to you @oontvoo & @labath? SG, either way - the simpler the better :) https://github.com/llvm/llvm-project/pull/126588 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Provide default impl for MightHaveChildren (NFC) (PR #119977)
https://github.com/jimingham approved this pull request. Ack, sorry I missed this... This is fine. You won't miss the fact that you should have overridden it to return false when you meant true, since then you'll get asked to make children but you can't. So having a default implementation shouldn't cause mysterious failures because you didn't notice the function. Good to remove all that boilerplate too! https://github.com/llvm/llvm-project/pull/119977 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Addressed additional review comments from PR/119716. (PR #126757)
https://github.com/oontvoo updated
https://github.com/llvm/llvm-project/pull/126757
>From 5a8b91422a017dcda595efa614a018f0a432df12 Mon Sep 17 00:00:00 2001
From: Vy Nguyen
Date: Tue, 11 Feb 2025 11:05:21 -0500
Subject: [PATCH 1/4] Addressed additional review comments from PR/119716.
---
lldb/include/lldb/Core/Telemetry.h | 2 +-
lldb/source/Core/Telemetry.cpp | 14 +-
2 files changed, 6 insertions(+), 10 deletions(-)
diff --git a/lldb/include/lldb/Core/Telemetry.h
b/lldb/include/lldb/Core/Telemetry.h
index 60a7097de5eee..d6c9e42ca8139 100644
--- a/lldb/include/lldb/Core/Telemetry.h
+++ b/lldb/include/lldb/Core/Telemetry.h
@@ -56,7 +56,7 @@ struct LLDBBaseTelemetryInfo : public
llvm::telemetry::TelemetryInfo {
void serialize(llvm::telemetry::Serializer &serializer) const override;
};
-/// The base Telemetry manager instance in LLDB
+/// The base Telemetry manager instance in LLDB.
/// This class declares additional instrumentation points
/// applicable to LLDB.
class TelemetryManager : public llvm::telemetry::Manager {
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 99f5d43ccbaf0..d0d7014d9ae83 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -27,10 +27,7 @@
namespace lldb_private {
namespace telemetry {
-using ::llvm::Error;
-using ::llvm::telemetry::Destination;
-using ::llvm::telemetry::Serializer;
-using ::llvm::telemetry::TelemetryInfo;
+namespace llvm::telemetry;
static uint64_t ToNanosec(const SteadyTimePoint Point) {
return std::chrono::nanoseconds(Point.time_since_epoch()).count();
@@ -44,21 +41,20 @@ void LLDBBaseTelemetryInfo::serialize(Serializer
&serializer) const {
serializer.write("end_time", ToNanosec(end_time.value()));
}
-[[maybe_unused]] static std::string MakeUUID(lldb_private::Debugger *debugger)
{
+[[maybe_unused]] static std::string MakeUUID(Debugger *debugger) {
uint8_t random_bytes[16];
if (auto ec = llvm::getRandomBytes(random_bytes, 16)) {
LLDB_LOG(GetLog(LLDBLog::Object),
"Failed to generate random bytes for UUID: {0}", ec.message());
-// fallback to using timestamp + debugger ID.
+// Fallback to using timestamp + debugger ID.
return llvm::formatv(
"{0}_{1}", std::chrono::steady_clock::now().time_since_epoch().count(),
debugger->GetID());
}
- return lldb_private::UUID(random_bytes).GetAsString();
+ return MakeUUID(random_bytes).GetAsString();
}
-TelemetryManager::TelemetryManager(
-std::unique_ptr config)
+TelemetryManager::TelemetryManager(std::unique_ptr config)
: m_config(std::move(config)) {}
llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
>From 8cb7a81b2e1afba54c520bf9dcbd532680bebdbc Mon Sep 17 00:00:00 2001
From: Vy Nguyen
Date: Tue, 11 Feb 2025 14:34:07 -0500
Subject: [PATCH 2/4] typo
---
lldb/source/Core/Telemetry.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index d0d7014d9ae83..1a829e09ac7c0 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -27,7 +27,7 @@
namespace lldb_private {
namespace telemetry {
-namespace llvm::telemetry;
+using namespace llvm::telemetry;
static uint64_t ToNanosec(const SteadyTimePoint Point) {
return std::chrono::nanoseconds(Point.time_since_epoch()).count();
>From a3f51ca9c6b094e9c0339c7c25faea1c5f0b17b9 Mon Sep 17 00:00:00 2001
From: Vy Nguyen
Date: Wed, 12 Feb 2025 09:42:08 -0500
Subject: [PATCH 3/4] change MakeUUID back to UUID
---
lldb/source/Core/Telemetry.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 1a829e09ac7c0..7a15ee4558693 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -51,7 +51,7 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer)
const {
"{0}_{1}", std::chrono::steady_clock::now().time_since_epoch().count(),
debugger->GetID());
}
- return MakeUUID(random_bytes).GetAsString();
+ return UUID(random_bytes).GetAsString();
}
TelemetryManager::TelemetryManager(std::unique_ptr config)
>From d52e2aae795fc7664675c55463a94708f406ef09 Mon Sep 17 00:00:00 2001
From: Vy Nguyen
Date: Thu, 13 Feb 2025 15:56:16 -0500
Subject: [PATCH 4/4] add llvm prefix
---
lldb/source/Core/Telemetry.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 7a15ee4558693..1ab26e0cc49f0 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -61,7 +61,7 @@ llvm::Error TelemetryManager::preDispatch(TelemetryInfo
*entry) {
// Do nothing for now.
// In up-coming patch, this would be where the manager
// attach the session_uuid to the entry.
- return Error::success();
+ return llvm::Error::success();
}
} // names
[Lldb-commits] [lldb] ed32d85 - [lldb] Use async output & error stream for EvaluateExpression
Author: Jonas Devlieghere
Date: 2025-02-14T22:11:03-08:00
New Revision: ed32d85d31999756602a7d5c4647cb6771d8f857
URL:
https://github.com/llvm/llvm-project/commit/ed32d85d31999756602a7d5c4647cb6771d8f857
DIFF:
https://github.com/llvm/llvm-project/commit/ed32d85d31999756602a7d5c4647cb6771d8f857.diff
LOG: [lldb] Use async output & error stream for EvaluateExpression
Similar to #126821, in support of #126630.
Added:
Modified:
lldb/source/Commands/CommandObjectExpression.cpp
Removed:
diff --git a/lldb/source/Commands/CommandObjectExpression.cpp
b/lldb/source/Commands/CommandObjectExpression.cpp
index 13491b5c79442..7e26381c92405 100644
--- a/lldb/source/Commands/CommandObjectExpression.cpp
+++ b/lldb/source/Commands/CommandObjectExpression.cpp
@@ -500,19 +500,17 @@ bool
CommandObjectExpression::EvaluateExpression(llvm::StringRef expr,
void CommandObjectExpression::IOHandlerInputComplete(IOHandler &io_handler,
std::string &line) {
io_handler.SetIsDone(true);
- StreamFileSP output_sp = io_handler.GetOutputStreamFileSP();
- StreamFileSP error_sp = io_handler.GetErrorStreamFileSP();
+ StreamSP output_stream =
+ GetCommandInterpreter().GetDebugger().GetAsyncOutputStream();
+ StreamSP error_stream =
+ GetCommandInterpreter().GetDebugger().GetAsyncErrorStream();
CommandReturnObject return_obj(
GetCommandInterpreter().GetDebugger().GetUseColor());
- EvaluateExpression(line.c_str(), *output_sp, *error_sp, return_obj);
+ EvaluateExpression(line.c_str(), *output_stream, *error_stream, return_obj);
- if (output_sp)
-output_sp->Flush();
- if (error_sp) {
-*error_sp << return_obj.GetErrorString();
-error_sp->Flush();
- }
+ output_stream->Flush();
+ *error_stream << return_obj.GetErrorString();
}
bool CommandObjectExpression::IOHandlerIsInputComplete(IOHandler &io_handler,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add terminfo dependency for ncurses support (PR #126810)
https://github.com/JDevlieghere approved this pull request. LGTM. Thanks for bearing with me! https://github.com/llvm/llvm-project/pull/126810 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix broken pipe error (PR #127100)
https://github.com/sga-sc edited https://github.com/llvm/llvm-project/pull/127100 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Allow option to ignore module load errors in ScriptedProcess (PR #127153)
labath wrote: Let's see what @medismailben has to say, but I think it would be fine to "ignore" (meaning, to continue trying to load other modules) load errors even without the special flags, as I think that's how our non-scripted processes work (do they?) https://github.com/llvm/llvm-project/pull/127153 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Change lldb's breakpoint handling behavior (PR #96260)
mstorsjo wrote: > For the record, this PR was finally re-landed as #126988 after four separate > commits to address issues found in CI testing when I originally merged this. FWIW, it looks like my smoke tests are still passing fine after these changes have been relanded now: https://github.com/mstorsjo/llvm-mingw/actions/runs/13320200475/job/37215994344 Thanks for sorting it out! https://github.com/llvm/llvm-project/pull/96260 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix broken pipe error (PR #127100)
https://github.com/sga-sc updated
https://github.com/llvm/llvm-project/pull/127100
>From 1fdc2f811c239bc8992fba9aa8e3d4eb9c76e96a Mon Sep 17 00:00:00 2001
From: Georgiy Samoylov
Date: Thu, 6 Feb 2025 13:23:13 +0300
Subject: [PATCH 1/2] [lldb] Fixed a typo
---
.../test/tools/lldb-server/gdbremote_testcase.py | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
index cbe430c92fa7f..cf94bf08a5bce 100644
---
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -397,13 +397,13 @@ def connect_to_debug_monitor(self, attach_pid=None):
# Schedule debug monitor to be shut down during teardown.
logger = self.logger
-connect_attemps = 0
+connect_attempts = 0
MAX_CONNECT_ATTEMPTS = 10
-while connect_attemps < MAX_CONNECT_ATTEMPTS:
+while connect_attempts < MAX_CONNECT_ATTEMPTS:
# Create a socket to talk to the server
try:
-logger.info("Connect attempt %d", connect_attemps + 1)
+logger.info("Connect attempt %d", connect_attempts + 1)
self.sock = self.create_socket()
self._server = Server(self.sock, server)
return server
@@ -411,7 +411,7 @@ def connect_to_debug_monitor(self, attach_pid=None):
# Ignore, and try again.
pass
time.sleep(0.5)
-connect_attemps += 1
+connect_attempts += 1
# We should close the server here to be safe.
server.terminate()
>From 9aab5677f83b826b796c3884c4c09364b27a38f2 Mon Sep 17 00:00:00 2001
From: Georgiy Samoylov
Date: Fri, 14 Feb 2025 15:53:04 +0300
Subject: [PATCH 2/2] [lldb] Made socket verification process more generic
---
.../lldbsuite/test/tools/lldb-server/gdbremote_testcase.py | 7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
index cf94bf08a5bce..fb96b3d4de560 100644
---
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -249,14 +249,11 @@ def remove_port_forward():
def _verify_socket(self, sock):
# Normally, when the remote stub is not ready, we will get
ECONNREFUSED during the
-# connect() attempt. However, due to the way how ADB forwarding works,
on android targets
+# connect() attempt. However, due to the way how port forwarding can
work, on some targets
# the connect() will always be successful, but the connection will be
immediately dropped
-# if ADB could not connect on the remote side. This function tries to
detect this
+# if we could not connect on the remote side. This function tries to
detect this
# situation, and report it as "connection refused" so that the upper
layers attempt the
# connection again.
-triple = self.dbg.GetSelectedPlatform().GetTriple()
-if not re.match(".*-.*-.*-android", triple):
-return # Not android.
can_read, _, _ = select.select([sock], [], [], 0.1)
if sock not in can_read:
return # Data is not available, but the connection is alive.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Add support for floating point template argument constants (PR #127206)
@@ -1973,6 +1973,27 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
ClangASTMetadata m_metadata;
};
+static clang::APValue MakeAPValue(CompilerType clang_type, uint64_t bit_width,
+ uint64_t value) {
+ bool is_signed = false;
+ const bool is_integral = clang_type.IsIntegerOrEnumerationType(is_signed);
+
+ llvm::APSInt apint(bit_width, !is_signed);
+ apint = value;
+
+ if (is_integral)
+return clang::APValue(apint);
+
+ uint32_t count;
+ bool is_complex;
+ assert(clang_type.IsFloatingPointType(count, is_complex));
+
+ if (bit_width == 32)
+return clang::APValue(llvm::APFloat(apint.bitsToFloat()));
+
+ return clang::APValue(llvm::APFloat(apint.bitsToDouble()));
Michael137 wrote:
Hmmm good question
The only differentiator between the different float types we have in DWARF is
the `DW_AT_name` of the type and the `DW_AT_byte_size`. So we could set the
semantics based on the floating point type, so we probably won't be able to
support all the different available `fltSemantics`. We already create Clang
types from the `DW_AT_name`, so we could, e.g., differentiate between
`_Float16` and `__bf16` semantics by looking at the typename. Then 32-bits and
64-bits we would assume IEEE. And if none of those work then we could return a
`std::nullopt` here? Wdyt?
https://github.com/llvm/llvm-project/pull/127206
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Add support for floating point template argument constants (PR #127206)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/127206 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix broken pipe error (PR #127100)
@@ -343,6 +343,22 @@ def get_target_byte_order(self):
target = self.dbg.CreateTarget(inferior_exe_path)
return target.GetByteOrder()
+def is_port_opened(self):
+connect_port = self.port
+
+err, retcode, cmd_output = self.run_platform_command(f"netstat -ltn |
grep {connect_port} | grep LISTEN")
sga-sc wrote:
Removed this function
https://github.com/llvm/llvm-project/pull/127100
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix broken pipe error (PR #127100)
@@ -343,6 +343,22 @@ def get_target_byte_order(self):
target = self.dbg.CreateTarget(inferior_exe_path)
return target.GetByteOrder()
+def is_port_opened(self):
+connect_port = self.port
+
+err, retcode, cmd_output = self.run_platform_command(f"netstat -ltn |
grep {connect_port} | grep LISTEN")
sga-sc wrote:
I agree with Jonas about dependency on `netstat`. But llgs tests themselves are
very linux-oriented:
https://github.com/llvm/llvm-project/blob/8844cd67967e7a55682f2b0fd06e8bebe63dd604/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py#L199
I also agree that filtering should be done using Python
https://github.com/llvm/llvm-project/pull/127100
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix "in function" detection in "thread until" (PR #123622)
labath wrote: Good question. I think I could use it in immediately in #126526 if I make the result exact (return the precise set of ranges instead of the high/low water mark). Let me see how that would look like. https://github.com/llvm/llvm-project/pull/123622 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Add support for floating point template argument constants (PR #127206)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/127206 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix broken pipe error (PR #127100)
@@ -343,6 +343,22 @@ def get_target_byte_order(self):
target = self.dbg.CreateTarget(inferior_exe_path)
return target.GetByteOrder()
+def is_port_opened(self):
+connect_port = self.port
+
+err, retcode, cmd_output = self.run_platform_command(f"netstat -ltn |
grep {connect_port} | grep LISTEN")
labath wrote:
That may be true, but I don't think its a reason to make things worse,
particularly if we don't understand why does this work. I'll also note that the
code you point to in a block of code that does not execute on darwin (the other
main user of these tests) while this code does.
https://github.com/llvm/llvm-project/pull/127100
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix broken pipe error (PR #127100)
@@ -397,21 +413,23 @@ def connect_to_debug_monitor(self, attach_pid=None):
# Schedule debug monitor to be shut down during teardown.
logger = self.logger
-connect_attemps = 0
+connect_attempts = 0
MAX_CONNECT_ATTEMPTS = 10
-while connect_attemps < MAX_CONNECT_ATTEMPTS:
-# Create a socket to talk to the server
-try:
-logger.info("Connect attempt %d", connect_attemps + 1)
-self.sock = self.create_socket()
sga-sc wrote:
@labath Thank you for your advice! After removing check for android platform
tests passed. I execute tests on `qemu-system-riscv64` and it's port forwarding
behaves probably like android's (judging by passed tests). What should I do
with this check for android?
https://github.com/llvm/llvm-project/pull/127100
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Add support for floating point template argument constants (PR #127206)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/127206
This patch adds support for the recently added
`clang::TemplateArgument::ArgKind::StructuralValue`
(https://github.com/llvm/llvm-project/pull/78041). These are used for non-type
template parameters such as floating point constants. When LLDB created
`clang::NonTypeTemplateParmDecl`s, it previously assumed integral values, this
patch accounts for structural values too.
Anywhere LLDB assumed a `DW_TAG_template_value_parameter` was `Integral`, it
will now also check for `StructuralValue`, and will unpack the
`TemplateArgument` value and type accordingly.
We can rely on the fact that any `TemplateArgument` of `StructuralValue` kind
that the `DWARFASTParserClang` creates will have a valid value, because it gets
those from `DW_AT_const_value`.
>From 759f0569807d00a059a78aeb3bd1eddeffcbdf36 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Fri, 14 Feb 2025 11:43:20 +
Subject: [PATCH] Init
---
lldb/include/lldb/Symbol/CompilerType.h | 3 +-
lldb/source/API/SBType.cpp| 8 +++-
.../Language/CPlusPlus/GenericBitset.cpp | 2 +-
.../Plugins/Language/CPlusPlus/LibCxxSpan.cpp | 2 +-
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 33 ++---
.../TypeSystem/Clang/TypeSystemClang.cpp | 47 +--
.../TestCppTemplateArguments.py | 31 ++--
.../API/lang/cpp/template-arguments/main.cpp | 6 +++
lldb/unittests/Symbol/TestTypeSystemClang.cpp | 42 -
9 files changed, 143 insertions(+), 31 deletions(-)
diff --git a/lldb/include/lldb/Symbol/CompilerType.h
b/lldb/include/lldb/Symbol/CompilerType.h
index 096a8f1ab68e8..f7e3e552f3e45 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -15,6 +15,7 @@
#include
#include "lldb/lldb-private.h"
+#include "clang/AST/APValue.h"
#include "llvm/ADT/APSInt.h"
#include "llvm/Support/Casting.h"
@@ -544,7 +545,7 @@ bool operator==(const CompilerType &lhs, const CompilerType
&rhs);
bool operator!=(const CompilerType &lhs, const CompilerType &rhs);
struct CompilerType::IntegralTemplateArgument {
- llvm::APSInt value;
+ clang::APValue value;
CompilerType type;
};
diff --git a/lldb/source/API/SBType.cpp b/lldb/source/API/SBType.cpp
index 6401d32c85795..72f590947dff6 100644
--- a/lldb/source/API/SBType.cpp
+++ b/lldb/source/API/SBType.cpp
@@ -697,6 +697,7 @@ lldb::SBValue
SBType::GetTemplateArgumentValue(lldb::SBTarget target,
std::optional arg;
const bool expand_pack = true;
switch (GetTemplateArgumentKind(idx)) {
+ case eTemplateArgumentKindStructuralValue:
case eTemplateArgumentKindIntegral:
arg = m_opaque_sp->GetCompilerType(false).GetIntegralTemplateArgument(
idx, expand_pack);
@@ -708,7 +709,12 @@ lldb::SBValue
SBType::GetTemplateArgumentValue(lldb::SBTarget target,
if (!arg)
return {};
- Scalar value{arg->value};
+ Scalar value;
+ if (arg->value.isFloat())
+value = arg->value.getFloat();
+ else
+value = arg->value.getInt();
+
DataExtractor data;
value.GetData(data);
diff --git a/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
b/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
index 33955dccb6ccc..99ff975825c71 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
@@ -91,7 +91,7 @@ lldb::ChildCacheState GenericBitsetFrontEnd::Update() {
size_t size = 0;
if (auto arg = m_backend.GetCompilerType().GetIntegralTemplateArgument(0))
-size = arg->value.getLimitedValue();
+size = arg->value.getInt().getLimitedValue();
m_elements.assign(size, ValueObjectSP());
m_first =
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
index 15040295efe6d..687ef1739ad11 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
@@ -119,7 +119,7 @@
lldb_private::formatters::LibcxxStdSpanSyntheticFrontEnd::Update() {
} else if (auto arg =
m_backend.GetCompilerType().GetIntegralTemplateArgument(1))
{
- m_num_elements = arg->value.getLimitedValue();
+ m_num_elements = arg->value.getInt().getLimitedValue();
}
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index ec0004c70c6da..70af283ab7443 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1973,6 +1973,27 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
ClangASTMetadata m_metadata;
};
+static clang::APValue MakeAPValue(CompilerType clang_type, uint64_t bit_width,
+ uint64_t value) {
+ bool is_signed =
[Lldb-commits] [lldb] [lldb] Fix broken pipe error (PR #127100)
@@ -397,21 +413,23 @@ def connect_to_debug_monitor(self, attach_pid=None):
# Schedule debug monitor to be shut down during teardown.
logger = self.logger
-connect_attemps = 0
+connect_attempts = 0
MAX_CONNECT_ATTEMPTS = 10
-while connect_attemps < MAX_CONNECT_ATTEMPTS:
-# Create a socket to talk to the server
-try:
-logger.info("Connect attempt %d", connect_attemps + 1)
-self.sock = self.create_socket()
labath wrote:
Cool. Glad we could sort that out. I think we can remove that android check
(and reword the comment to be more generic). I think this is a general
property/design bug of port forwarders (they only attempt the outgoing
connection after they receive the incoming one, at which point it is too late
to abort, so the only thing they really can do is drop the connection). The
only reason that check is android-specific is because we didn't need it
anywhere else (yet).
https://github.com/llvm/llvm-project/pull/127100
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Add support for floating point template argument constants (PR #127206)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Michael Buch (Michael137)
Changes
This patch adds support for the recently added
`clang::TemplateArgument::ArgKind::StructuralValue`
(https://github.com/llvm/llvm-project/pull/78041). These are used for non-type
template parameters such as floating point constants. When LLDB created
`clang::NonTypeTemplateParmDecl`s, it previously assumed integral values, this
patch accounts for structural values too.
Anywhere LLDB assumed a `DW_TAG_template_value_parameter` was `Integral`, it
will now also check for `StructuralValue`, and will unpack the
`TemplateArgument` value and type accordingly.
We can rely on the fact that any `TemplateArgument` of `StructuralValue` kind
that the `DWARFASTParserClang` creates will have a valid value, because it gets
those from `DW_AT_const_value`.
---
Full diff: https://github.com/llvm/llvm-project/pull/127206.diff
9 Files Affected:
- (modified) lldb/include/lldb/Symbol/CompilerType.h (+2-1)
- (modified) lldb/source/API/SBType.cpp (+7-1)
- (modified) lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp (+1-1)
- (modified) lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp (+1-1)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
(+26-7)
- (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+32-15)
- (modified)
lldb/test/API/lang/cpp/template-arguments/TestCppTemplateArguments.py (+28-3)
- (modified) lldb/test/API/lang/cpp/template-arguments/main.cpp (+6)
- (modified) lldb/unittests/Symbol/TestTypeSystemClang.cpp (+40-2)
``diff
diff --git a/lldb/include/lldb/Symbol/CompilerType.h
b/lldb/include/lldb/Symbol/CompilerType.h
index 096a8f1ab68e8..f7e3e552f3e45 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -15,6 +15,7 @@
#include
#include "lldb/lldb-private.h"
+#include "clang/AST/APValue.h"
#include "llvm/ADT/APSInt.h"
#include "llvm/Support/Casting.h"
@@ -544,7 +545,7 @@ bool operator==(const CompilerType &lhs, const CompilerType
&rhs);
bool operator!=(const CompilerType &lhs, const CompilerType &rhs);
struct CompilerType::IntegralTemplateArgument {
- llvm::APSInt value;
+ clang::APValue value;
CompilerType type;
};
diff --git a/lldb/source/API/SBType.cpp b/lldb/source/API/SBType.cpp
index 6401d32c85795..72f590947dff6 100644
--- a/lldb/source/API/SBType.cpp
+++ b/lldb/source/API/SBType.cpp
@@ -697,6 +697,7 @@ lldb::SBValue
SBType::GetTemplateArgumentValue(lldb::SBTarget target,
std::optional arg;
const bool expand_pack = true;
switch (GetTemplateArgumentKind(idx)) {
+ case eTemplateArgumentKindStructuralValue:
case eTemplateArgumentKindIntegral:
arg = m_opaque_sp->GetCompilerType(false).GetIntegralTemplateArgument(
idx, expand_pack);
@@ -708,7 +709,12 @@ lldb::SBValue
SBType::GetTemplateArgumentValue(lldb::SBTarget target,
if (!arg)
return {};
- Scalar value{arg->value};
+ Scalar value;
+ if (arg->value.isFloat())
+value = arg->value.getFloat();
+ else
+value = arg->value.getInt();
+
DataExtractor data;
value.GetData(data);
diff --git a/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
b/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
index 33955dccb6ccc..99ff975825c71 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
@@ -91,7 +91,7 @@ lldb::ChildCacheState GenericBitsetFrontEnd::Update() {
size_t size = 0;
if (auto arg = m_backend.GetCompilerType().GetIntegralTemplateArgument(0))
-size = arg->value.getLimitedValue();
+size = arg->value.getInt().getLimitedValue();
m_elements.assign(size, ValueObjectSP());
m_first =
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
index 15040295efe6d..687ef1739ad11 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
@@ -119,7 +119,7 @@
lldb_private::formatters::LibcxxStdSpanSyntheticFrontEnd::Update() {
} else if (auto arg =
m_backend.GetCompilerType().GetIntegralTemplateArgument(1))
{
- m_num_elements = arg->value.getLimitedValue();
+ m_num_elements = arg->value.getInt().getLimitedValue();
}
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index ec0004c70c6da..70af283ab7443 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1973,6 +1973,27 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
ClangASTMetadata m_metadata;
};
+static clang::APValue MakeAPValue(CompilerType clang_type, uint64_t bit_width,
+ uint64_t value) {
+ bool is_signe
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Add support for floating point template argument constants (PR #127206)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/127206 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Add support for floating point template argument constants (PR #127206)
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
134a94ae0b6cb8dccf1114fb2eefb172e9697905...759f0569807d00a059a78aeb3bd1eddeffcbdf36
lldb/test/API/lang/cpp/template-arguments/TestCppTemplateArguments.py
``
View the diff from darker here.
``diff
--- TestCppTemplateArguments.py 2025-02-14 12:14:22.00 +
+++ TestCppTemplateArguments.py 2025-02-14 12:17:53.809209 +
@@ -82,11 +82,13 @@
template_param_value =
value.GetType().GetTemplateArgumentValue(target, 1)
self.assertEqual(template_param_value.GetTypeName(), "double")
# FIXME: this should return a float
self.assertEqual(template_param_value.GetValueAsSigned(), 1)
-value = self.expect_expr("temp8", result_type="Bar")
+value = self.expect_expr(
+"temp8", result_type="Bar"
+)
template_param_value =
value.GetType().GetTemplateArgumentValue(target, 1)
self.assertEqual(template_param_value.GetTypeName(), "float")
# FIXME: this should return a float
self.assertEqual(template_param_value.GetValueAsSigned(), 1)
``
https://github.com/llvm/llvm-project/pull/127206
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Add support for floating point template argument constants (PR #127206)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/127206
>From 759f0569807d00a059a78aeb3bd1eddeffcbdf36 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Fri, 14 Feb 2025 11:43:20 +
Subject: [PATCH 1/2] Init
---
lldb/include/lldb/Symbol/CompilerType.h | 3 +-
lldb/source/API/SBType.cpp| 8 +++-
.../Language/CPlusPlus/GenericBitset.cpp | 2 +-
.../Plugins/Language/CPlusPlus/LibCxxSpan.cpp | 2 +-
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 33 ++---
.../TypeSystem/Clang/TypeSystemClang.cpp | 47 +--
.../TestCppTemplateArguments.py | 31 ++--
.../API/lang/cpp/template-arguments/main.cpp | 6 +++
lldb/unittests/Symbol/TestTypeSystemClang.cpp | 42 -
9 files changed, 143 insertions(+), 31 deletions(-)
diff --git a/lldb/include/lldb/Symbol/CompilerType.h
b/lldb/include/lldb/Symbol/CompilerType.h
index 096a8f1ab68e8..f7e3e552f3e45 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -15,6 +15,7 @@
#include
#include "lldb/lldb-private.h"
+#include "clang/AST/APValue.h"
#include "llvm/ADT/APSInt.h"
#include "llvm/Support/Casting.h"
@@ -544,7 +545,7 @@ bool operator==(const CompilerType &lhs, const CompilerType
&rhs);
bool operator!=(const CompilerType &lhs, const CompilerType &rhs);
struct CompilerType::IntegralTemplateArgument {
- llvm::APSInt value;
+ clang::APValue value;
CompilerType type;
};
diff --git a/lldb/source/API/SBType.cpp b/lldb/source/API/SBType.cpp
index 6401d32c85795..72f590947dff6 100644
--- a/lldb/source/API/SBType.cpp
+++ b/lldb/source/API/SBType.cpp
@@ -697,6 +697,7 @@ lldb::SBValue
SBType::GetTemplateArgumentValue(lldb::SBTarget target,
std::optional arg;
const bool expand_pack = true;
switch (GetTemplateArgumentKind(idx)) {
+ case eTemplateArgumentKindStructuralValue:
case eTemplateArgumentKindIntegral:
arg = m_opaque_sp->GetCompilerType(false).GetIntegralTemplateArgument(
idx, expand_pack);
@@ -708,7 +709,12 @@ lldb::SBValue
SBType::GetTemplateArgumentValue(lldb::SBTarget target,
if (!arg)
return {};
- Scalar value{arg->value};
+ Scalar value;
+ if (arg->value.isFloat())
+value = arg->value.getFloat();
+ else
+value = arg->value.getInt();
+
DataExtractor data;
value.GetData(data);
diff --git a/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
b/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
index 33955dccb6ccc..99ff975825c71 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
@@ -91,7 +91,7 @@ lldb::ChildCacheState GenericBitsetFrontEnd::Update() {
size_t size = 0;
if (auto arg = m_backend.GetCompilerType().GetIntegralTemplateArgument(0))
-size = arg->value.getLimitedValue();
+size = arg->value.getInt().getLimitedValue();
m_elements.assign(size, ValueObjectSP());
m_first =
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
index 15040295efe6d..687ef1739ad11 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
@@ -119,7 +119,7 @@
lldb_private::formatters::LibcxxStdSpanSyntheticFrontEnd::Update() {
} else if (auto arg =
m_backend.GetCompilerType().GetIntegralTemplateArgument(1))
{
- m_num_elements = arg->value.getLimitedValue();
+ m_num_elements = arg->value.getInt().getLimitedValue();
}
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index ec0004c70c6da..70af283ab7443 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1973,6 +1973,27 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
ClangASTMetadata m_metadata;
};
+static clang::APValue MakeAPValue(CompilerType clang_type, uint64_t bit_width,
+ uint64_t value) {
+ bool is_signed = false;
+ const bool is_integral = clang_type.IsIntegerOrEnumerationType(is_signed);
+
+ llvm::APSInt apint(bit_width, !is_signed);
+ apint = value;
+
+ if (is_integral)
+return clang::APValue(apint);
+
+ uint32_t count;
+ bool is_complex;
+ assert(clang_type.IsFloatingPointType(count, is_complex));
+
+ if (bit_width == 32)
+return clang::APValue(llvm::APFloat(apint.bitsToFloat()));
+
+ return clang::APValue(llvm::APFloat(apint.bitsToDouble()));
+}
+
bool DWARFASTParserClang::ParseTemplateDIE(
const DWARFDIE &die,
TypeSystemClang::TemplateParameterInfos &template_param_infos) {
@@ -2050,9 +2071,6 @@ bool DWARFASTParserClang::ParseTemplateDIE(
clang_type = m_ast.GetBasicType(eBasicTypeVoid);
if (!is_templat
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Add support for floating point template argument constants (PR #127206)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/127206
>From 759f0569807d00a059a78aeb3bd1eddeffcbdf36 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Fri, 14 Feb 2025 11:43:20 +
Subject: [PATCH 1/3] Init
---
lldb/include/lldb/Symbol/CompilerType.h | 3 +-
lldb/source/API/SBType.cpp| 8 +++-
.../Language/CPlusPlus/GenericBitset.cpp | 2 +-
.../Plugins/Language/CPlusPlus/LibCxxSpan.cpp | 2 +-
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 33 ++---
.../TypeSystem/Clang/TypeSystemClang.cpp | 47 +--
.../TestCppTemplateArguments.py | 31 ++--
.../API/lang/cpp/template-arguments/main.cpp | 6 +++
lldb/unittests/Symbol/TestTypeSystemClang.cpp | 42 -
9 files changed, 143 insertions(+), 31 deletions(-)
diff --git a/lldb/include/lldb/Symbol/CompilerType.h
b/lldb/include/lldb/Symbol/CompilerType.h
index 096a8f1ab68e8..f7e3e552f3e45 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -15,6 +15,7 @@
#include
#include "lldb/lldb-private.h"
+#include "clang/AST/APValue.h"
#include "llvm/ADT/APSInt.h"
#include "llvm/Support/Casting.h"
@@ -544,7 +545,7 @@ bool operator==(const CompilerType &lhs, const CompilerType
&rhs);
bool operator!=(const CompilerType &lhs, const CompilerType &rhs);
struct CompilerType::IntegralTemplateArgument {
- llvm::APSInt value;
+ clang::APValue value;
CompilerType type;
};
diff --git a/lldb/source/API/SBType.cpp b/lldb/source/API/SBType.cpp
index 6401d32c85795..72f590947dff6 100644
--- a/lldb/source/API/SBType.cpp
+++ b/lldb/source/API/SBType.cpp
@@ -697,6 +697,7 @@ lldb::SBValue
SBType::GetTemplateArgumentValue(lldb::SBTarget target,
std::optional arg;
const bool expand_pack = true;
switch (GetTemplateArgumentKind(idx)) {
+ case eTemplateArgumentKindStructuralValue:
case eTemplateArgumentKindIntegral:
arg = m_opaque_sp->GetCompilerType(false).GetIntegralTemplateArgument(
idx, expand_pack);
@@ -708,7 +709,12 @@ lldb::SBValue
SBType::GetTemplateArgumentValue(lldb::SBTarget target,
if (!arg)
return {};
- Scalar value{arg->value};
+ Scalar value;
+ if (arg->value.isFloat())
+value = arg->value.getFloat();
+ else
+value = arg->value.getInt();
+
DataExtractor data;
value.GetData(data);
diff --git a/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
b/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
index 33955dccb6ccc..99ff975825c71 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
@@ -91,7 +91,7 @@ lldb::ChildCacheState GenericBitsetFrontEnd::Update() {
size_t size = 0;
if (auto arg = m_backend.GetCompilerType().GetIntegralTemplateArgument(0))
-size = arg->value.getLimitedValue();
+size = arg->value.getInt().getLimitedValue();
m_elements.assign(size, ValueObjectSP());
m_first =
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
index 15040295efe6d..687ef1739ad11 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
@@ -119,7 +119,7 @@
lldb_private::formatters::LibcxxStdSpanSyntheticFrontEnd::Update() {
} else if (auto arg =
m_backend.GetCompilerType().GetIntegralTemplateArgument(1))
{
- m_num_elements = arg->value.getLimitedValue();
+ m_num_elements = arg->value.getInt().getLimitedValue();
}
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index ec0004c70c6da..70af283ab7443 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1973,6 +1973,27 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
ClangASTMetadata m_metadata;
};
+static clang::APValue MakeAPValue(CompilerType clang_type, uint64_t bit_width,
+ uint64_t value) {
+ bool is_signed = false;
+ const bool is_integral = clang_type.IsIntegerOrEnumerationType(is_signed);
+
+ llvm::APSInt apint(bit_width, !is_signed);
+ apint = value;
+
+ if (is_integral)
+return clang::APValue(apint);
+
+ uint32_t count;
+ bool is_complex;
+ assert(clang_type.IsFloatingPointType(count, is_complex));
+
+ if (bit_width == 32)
+return clang::APValue(llvm::APFloat(apint.bitsToFloat()));
+
+ return clang::APValue(llvm::APFloat(apint.bitsToDouble()));
+}
+
bool DWARFASTParserClang::ParseTemplateDIE(
const DWARFDIE &die,
TypeSystemClang::TemplateParameterInfos &template_param_infos) {
@@ -2050,9 +2071,6 @@ bool DWARFASTParserClang::ParseTemplateDIE(
clang_type = m_ast.GetBasicType(eBasicTypeVoid);
if (!is_templat
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Add support for floating point template argument constants (PR #127206)
@@ -91,7 +91,7 @@ lldb::ChildCacheState GenericBitsetFrontEnd::Update() {
size_t size = 0;
if (auto arg = m_backend.GetCompilerType().GetIntegralTemplateArgument(0))
-size = arg->value.getLimitedValue();
+size = arg->value.getInt().getLimitedValue();
labath wrote:
Technically I think these need to check that the value is an integral type as a
malicious could crash lldb by declaring a `std::bitset` with a floating point
template argument.
https://github.com/llvm/llvm-project/pull/127206
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Add support for floating point template argument constants (PR #127206)
@@ -1973,6 +1973,27 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
ClangASTMetadata m_metadata;
};
+static clang::APValue MakeAPValue(CompilerType clang_type, uint64_t bit_width,
+ uint64_t value) {
+ bool is_signed = false;
+ const bool is_integral = clang_type.IsIntegerOrEnumerationType(is_signed);
+
+ llvm::APSInt apint(bit_width, !is_signed);
+ apint = value;
+
+ if (is_integral)
+return clang::APValue(apint);
+
+ uint32_t count;
+ bool is_complex;
+ assert(clang_type.IsFloatingPointType(count, is_complex));
+
+ if (bit_width == 32)
+return clang::APValue(llvm::APFloat(apint.bitsToFloat()));
+
+ return clang::APValue(llvm::APFloat(apint.bitsToDouble()));
labath wrote:
Any chance this could be something like `return
clang::APValue(llvm::APFloat(fltSemantics_I_somehow_got_from_clang_type,
apint))`
Reason being `float`s and `double`s aren't the only floating point type, so
this will likely not work on any of the fancier float types.
https://github.com/llvm/llvm-project/pull/127206
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Synchronize the debugger's stdout and stderr streams (PR #126630)
labath wrote: > > I do want to spend some time discussing the relationship between stdout and > > stderr. The current implementation uses a separate mutex for each stream, > > which is not _un_reasonable, but I can see at least two other possible > > philosophies: > > > > 1. Since both of the streams will be going to the same terminal most of the > > time, it might make sense to use a single mutex for both of them, as that's > > sort of the only way to ensure consistent output. (Imagine the situation of > > printing the "stderr" of a CLI command while the status line is being > > updated) > > 2. Alternatively, we could say that stderr is for immediate and exceptional > > output where it is more important to get the message to the user than it > > being formatted beautifully. In this world, we could keep "stderr" as a > > regular unlocked stream. In specific cases, where synchronizing the output > > is particularly important to us (like the beforementioned case of "stderr" > > of a CLI command) we could synchronize its output by holding the "stdout" > > mutex while writing it. > > > > What do you think of that? I'm sort of leaning towards option two since > > stderr might be used in contexts where holding a mutex might not be > > completely safe (and the first option would make that even worse), and the > > current implementation with separate mutexes doesn't really guarantee > > "reasonable" stdout/err sequencing. > > I had a feeling this was going to come up... The tricky part is that, as you > pointed out, most of the time the output is going to same terminal. But on > the other hand, we totally support having those two streams go to totally > separate files and it feels wrong to force them to be synchronized. How about > a compromise between (1) and (2) where we use the same mutex when the > debugger is creating the StreamFile for stdout and stderr, but we use > separate murexes when they're set through the setter (and the SB API)? How would that work? We'd check the value of the passed-in `FILE*` and choose a mutex based on whether it happens to be `stdout` ? I don't think I'd like that for several reasons: - it doesn't handle the case where `stdout/err` is redirected externally (it still locks even though it shouldn't) - it's doesn't let you create a terminal-like experience when embedding lldb-as-a-library (it doesn't lock even though it should) > Conceptually I'd also lean towards (2) but I worry about losing the benefits > the current approach brings. Deciding whether "the output is particularly > important" is a judgement call that we can't enforce at the API level. I like > how the current implementation makes it easy to do the right thing and hard > to do the wrong thing (or at least make you think about locking). If the > default is to not lock the stdout (the alternative of always locking is > equivalent to (1)) then I'm skeptical that we won't just end up with the same > outcome that we have with two mutexes. But with two mutexes you still don't get stdout<=>stderr synchronization, which means that stderr output can corrupt things you're writing to stdout. What the stderr mutex buys you is stderr<=>stderr synchronization, which is... maybe nice, but maybe also not necessary? FWIW, there is some precedent for unconditional stdout-stderr interaction: writes to `std::cerr` will automatically flush `std::cout` regardless of whether the two streams refer to the same terminal. And the `iostream` library allows you to connect arbitrary two streams in this way. So, I think we *could* say we "tie" debuggers output and error streams in this way, even if they are completely independent, because they are "our" stdout and stderr. https://github.com/llvm/llvm-project/pull/126630 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix broken pipe error (PR #127100)
@@ -397,21 +413,23 @@ def connect_to_debug_monitor(self, attach_pid=None):
# Schedule debug monitor to be shut down during teardown.
logger = self.logger
-connect_attemps = 0
+connect_attempts = 0
MAX_CONNECT_ATTEMPTS = 10
-while connect_attemps < MAX_CONNECT_ATTEMPTS:
-# Create a socket to talk to the server
-try:
-logger.info("Connect attempt %d", connect_attemps + 1)
-self.sock = self.create_socket()
labath wrote:
This is interesting, but doesn't quite explain how could the connection be
established without the other side being present. We may need to dig deeper.
One way I can see this happening is if there is some kind of a port forwarder
sitting between the test and the server. Are you sure you don't have one of
those?
We already have a
[hackaround](https://github.com/llvm/llvm-project/blob/2fdf191e244b62409fd73fa9bb717466d6e683b5/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py#L258)
for that, but it's only enabled on android. Could you check what happens if
you remove the android check (and maybe increase the timeout in the sleep call)?
https://github.com/llvm/llvm-project/pull/127100
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 134a94a - [lldb][test] TestCppTemplateArguments.py: skip on older compilers
Author: Michael Buch
Date: 2025-02-14T10:26:47Z
New Revision: 134a94ae0b6cb8dccf1114fb2eefb172e9697905
URL:
https://github.com/llvm/llvm-project/commit/134a94ae0b6cb8dccf1114fb2eefb172e9697905
DIFF:
https://github.com/llvm/llvm-project/commit/134a94ae0b6cb8dccf1114fb2eefb172e9697905.diff
LOG: [lldb][test] TestCppTemplateArguments.py: skip on older compilers
This test needs to be compiled with compilers that support floating point NTTP.
Added:
Modified:
lldb/test/API/lang/cpp/template-arguments/TestCppTemplateArguments.py
Removed:
diff --git
a/lldb/test/API/lang/cpp/template-arguments/TestCppTemplateArguments.py
b/lldb/test/API/lang/cpp/template-arguments/TestCppTemplateArguments.py
index c276aae449920..db5388b8bcc6d 100644
--- a/lldb/test/API/lang/cpp/template-arguments/TestCppTemplateArguments.py
+++ b/lldb/test/API/lang/cpp/template-arguments/TestCppTemplateArguments.py
@@ -6,6 +6,7 @@
class TestCase(TestBase):
@no_debug_info_test
+@skipIf(compiler="clang", compiler_version=["<", "20.0"])
def test(self):
self.build()
target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Add support for floating point template argument constants (PR #127206)
@@ -1973,6 +1973,27 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
ClangASTMetadata m_metadata;
};
+static clang::APValue MakeAPValue(CompilerType clang_type, uint64_t bit_width,
+ uint64_t value) {
+ bool is_signed = false;
+ const bool is_integral = clang_type.IsIntegerOrEnumerationType(is_signed);
+
+ llvm::APSInt apint(bit_width, !is_signed);
+ apint = value;
+
+ if (is_integral)
+return clang::APValue(apint);
+
+ uint32_t count;
+ bool is_complex;
+ assert(clang_type.IsFloatingPointType(count, is_complex));
+
+ if (bit_width == 32)
+return clang::APValue(llvm::APFloat(apint.bitsToFloat()));
+
+ return clang::APValue(llvm::APFloat(apint.bitsToDouble()));
labath wrote:
I don't think we need to solve the problem of parsing all floating point types
here. What I'm mainly interested in is seeing if there's a way to write this
code such that it is automatically correct for any floating point type that we
do support. I'm assuming/hoping that there is *a* way to get floating point
semantics object out of a clang type without relying on the bitwidth...
https://github.com/llvm/llvm-project/pull/127206
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix broken pipe error (PR #127100)
https://github.com/labath approved this pull request. Looks good. Thanks. (But be sure to wait for Jonas since he clicked the "request changes" button) https://github.com/llvm/llvm-project/pull/127100 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix broken pipe error (PR #127100)
@@ -397,21 +413,23 @@ def connect_to_debug_monitor(self, attach_pid=None):
# Schedule debug monitor to be shut down during teardown.
logger = self.logger
-connect_attemps = 0
+connect_attempts = 0
MAX_CONNECT_ATTEMPTS = 10
-while connect_attemps < MAX_CONNECT_ATTEMPTS:
-# Create a socket to talk to the server
-try:
-logger.info("Connect attempt %d", connect_attemps + 1)
-self.sock = self.create_socket()
sga-sc wrote:
Addressed.
https://github.com/llvm/llvm-project/pull/127100
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Define Telemetry plugin for LLDB. (PR #126588)
JDevlieghere wrote:
> The (baroque) way that the other plugins work serves two purposes:
>
> 1. controlling the type (class) of the plugin being created based on some
> runtime property
> 2. creating an arbitrary number of plugin instances
>
> For telemetry "plugins", I don't think we want/need (1) because the type of
> the plugin is determined by the vendor -- at build time. A user can't change
> that at runtime. They may be able to disable collection, but they can't say
> "well, today I feel like reporting my telemetry to company ``" and change
> a setting or something to make that happen. I mean, it's probably doable, but
> I don't think anyone has that as a requirement (if you do, let us know).
I don't think anyone has that requirement but I could see a few situations
where that could be useful. For example we could have a "textual telemetry"
fallback plugin that we use for testing and users could turn on the audit
what's being reported. Also with the work I did a few years ago, it's also
super easy to disable plugins are compile time or do something like:
```
if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
add_subdirectory(DarwinTelemetry)
endif()
```
Anyway, to be clear, I'm not saying we **need** to support that, but I could
see a reason to make it work like that. The overhead/complexity of not
preventing that is small (admittedly that's subjective) in exchange for
consistency and if this was the only argument I'd feel it would not warrant
diverging from the established way of doing this. I think (2) is more
compelling.
> The second question is I think more interesting, and I think it boils down to
> "what is the 'scope' of a TelemetryManager object". I can see two options,
> and I think both of them are reasonable. One (which is what this PR does) is
> to make it a process-wide object (a singleton). The other is to create one
> instance per Debugger object. The advantage of the first one is that the
> object is always available -- there's no problem with accessing the object
> from parts of code (e.g. everything under the `Module` class) which are not
> tied to a specific debugger instance (I'm sure you remember the contortions
> that we go through to report progress events related to debug info parsing).
> The advantage of the second one is that it fits in better with our object
> model and the events we do report are automatically organized into a user
> session.
>
> The reason I think the first one is better is that organizing events into
> "sessions" is still possible with this model (just add a debugger_id field to
> the reported event), but it still allows you to report debugger-less events,
> but I could be convinced otherwise. However, even if we do that, I still
> don't think we need a _list_ of plugins (due to the first item). All we'd
> need to do is replace `setInstance(std::unique_ptr)` with
> something like `setInstanceCreateCallback(std::unique_ptr
> (*)(Debugger&))`.
Yes, I like the `debugger_id` approach we've taken with the progress events and
the debugger diagnostics. I agree we should do the same for the telemetry as it
has worked well. In other words, I understand we only need a single instance.
But that's totally something we can handle through the PluginManager interface,
which could return you the same one with the snippet of code I posted earlier.
What I'm hearing is that Telemetry _could_ use the plugin mechanism but that it
offers you to do things that we don't need (pick at runtime, have multiple
instances) and it adds more complexity to do it that way. I follow both
arguments and to be clear, I have the same qualms with the Plugin machinery as
everyone else. I'm not really advocating for it beyond "this could be used to
make this work".
> > More of a question for @labath but do we have an example of another place
> > where we have "semi plugins" like this and if not, what are existing
> > plugins that could be reworked to be lighter weight?
>
> We sort of do have a precedent for that, but it's in an unlikely place -- the
> Platform plugins. It sounds hard to believe because Platforms are probably
> the most complicated kinds of plugins, but part of that complexity comes from
> the fact that they are doing two mostly separate things:
>
> 1. Registering themselves to handle the remote systems of the given kind.
> This part is pretty much the same as all other plugins and uses the
> PluginManager, iteration, and whatnot.
> 2. Registering themselves as the "Host" platform, if they match the current
> build host. This part is achieved by a call to Platform::SetPlatform (and
> that's an analogue to TelemetryManager::setInstance).
>
> Here's how the PlatformLinux::Initialize looks like. The first part is just
> us being "baroque":
>
> ```
> PlatformPOSIX::Initialize();
>
> if (g_initialize_count++ == 0) {
> ```
>
> Next it does the the item (2)
>
> ```
> #if defined(__linux__) && !defined(__ANDROID__)
>
[Lldb-commits] [lldb] [lldb] Synchronize the debugger's stdout and stderr streams (PR #126630)
JDevlieghere wrote: > I guess so, but I think we're drawing different conclusions. My take on this > is that if the only thing we get from locking stderr is the ability to > mutually exclude two stderr accesses, then maybe we don't need to bother with > locking stderr at all. That's what I understood and I agree. We don't use the error stream all that much, so I don't feel strongly either way. If it were up to me I would've gone with doing it anyway for parity with stdout. > > > FWIW, there is some precedent for unconditional stdout-stderr > > > interaction: writes to `std::cerr` will automatically flush `std::cout` > > > regardless of whether the two streams refer to the same terminal. And the > > > `iostream` library allows you to connect arbitrary two streams in this > > > way. So, I think we _could_ say we "tie" debuggers output and error > > > streams in this way, even if they are completely independent, because > > > they are "our" stdout and stderr. > > > > > > Fair enough. What I'm hearing is that you think it's important to > > synchronize the two somehow and I'd rather do it consistently rather than > > doing it only sometimes which makes it hard to know what the expectations > > both for us as the people writing that code and for our users. > > Yes, I think that tying them _always_ is better than tying them _sometimes_. > However, I don't really have a clear preference between the other options > flying around. The options being: > > * tying them always > * not tying them at all > * not tying them and not locking stderr > > If it sounds like I'm in favor of one of them, it's because I'm comparing its > (dis)advantages relative to some other approach. However, I think all of > these approaches have their (dis)advantages. Ack, I'm sorry if I was reading too much into it. Given that neither of us has a clear preference, and given that we need to make a decision, I'll suggest that we go with (1) as that's really the only thing that guarantees we don't mess up the statusline, which was the motivation behind this PR. The other two options will probably still do the right thing most of the time so it's really about picking a tie-breaker. > > It should be fairly straightforward to support tying the two streams > > together. I'll see how that goes and update the PR. > > A random idea, I don't know if it's a good one: have one object > (LockableStreamPair?) that holds the two streams and their mutex. One less > argument to pass around? Sounds good, that simplifies the signatures (which this PR is already touching) and keeps the mutex abstracted away. https://github.com/llvm/llvm-project/pull/126630 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Add support for floating point template argument constants (PR #127206)
@@ -1973,6 +1973,27 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
ClangASTMetadata m_metadata;
};
+static clang::APValue MakeAPValue(CompilerType clang_type, uint64_t bit_width,
+ uint64_t value) {
+ bool is_signed = false;
+ const bool is_integral = clang_type.IsIntegerOrEnumerationType(is_signed);
+
+ llvm::APSInt apint(bit_width, !is_signed);
+ apint = value;
+
+ if (is_integral)
+return clang::APValue(apint);
+
+ uint32_t count;
+ bool is_complex;
+ assert(clang_type.IsFloatingPointType(count, is_complex));
+
+ if (bit_width == 32)
+return clang::APValue(llvm::APFloat(apint.bitsToFloat()));
+
+ return clang::APValue(llvm::APFloat(apint.bitsToDouble()));
Michael137 wrote:
Ahh I misunderstood the suggestion. Yes, getting the semantics from the
QualType makes sense here. Done in the latest commit
https://github.com/llvm/llvm-project/pull/127206
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystemClang] Add support for floating point template argument constants (PR #127206)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/127206
>From 759f0569807d00a059a78aeb3bd1eddeffcbdf36 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Fri, 14 Feb 2025 11:43:20 +
Subject: [PATCH 1/4] Init
---
lldb/include/lldb/Symbol/CompilerType.h | 3 +-
lldb/source/API/SBType.cpp| 8 +++-
.../Language/CPlusPlus/GenericBitset.cpp | 2 +-
.../Plugins/Language/CPlusPlus/LibCxxSpan.cpp | 2 +-
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 33 ++---
.../TypeSystem/Clang/TypeSystemClang.cpp | 47 +--
.../TestCppTemplateArguments.py | 31 ++--
.../API/lang/cpp/template-arguments/main.cpp | 6 +++
lldb/unittests/Symbol/TestTypeSystemClang.cpp | 42 -
9 files changed, 143 insertions(+), 31 deletions(-)
diff --git a/lldb/include/lldb/Symbol/CompilerType.h
b/lldb/include/lldb/Symbol/CompilerType.h
index 096a8f1ab68e8..f7e3e552f3e45 100644
--- a/lldb/include/lldb/Symbol/CompilerType.h
+++ b/lldb/include/lldb/Symbol/CompilerType.h
@@ -15,6 +15,7 @@
#include
#include "lldb/lldb-private.h"
+#include "clang/AST/APValue.h"
#include "llvm/ADT/APSInt.h"
#include "llvm/Support/Casting.h"
@@ -544,7 +545,7 @@ bool operator==(const CompilerType &lhs, const CompilerType
&rhs);
bool operator!=(const CompilerType &lhs, const CompilerType &rhs);
struct CompilerType::IntegralTemplateArgument {
- llvm::APSInt value;
+ clang::APValue value;
CompilerType type;
};
diff --git a/lldb/source/API/SBType.cpp b/lldb/source/API/SBType.cpp
index 6401d32c85795..72f590947dff6 100644
--- a/lldb/source/API/SBType.cpp
+++ b/lldb/source/API/SBType.cpp
@@ -697,6 +697,7 @@ lldb::SBValue
SBType::GetTemplateArgumentValue(lldb::SBTarget target,
std::optional arg;
const bool expand_pack = true;
switch (GetTemplateArgumentKind(idx)) {
+ case eTemplateArgumentKindStructuralValue:
case eTemplateArgumentKindIntegral:
arg = m_opaque_sp->GetCompilerType(false).GetIntegralTemplateArgument(
idx, expand_pack);
@@ -708,7 +709,12 @@ lldb::SBValue
SBType::GetTemplateArgumentValue(lldb::SBTarget target,
if (!arg)
return {};
- Scalar value{arg->value};
+ Scalar value;
+ if (arg->value.isFloat())
+value = arg->value.getFloat();
+ else
+value = arg->value.getInt();
+
DataExtractor data;
value.GetData(data);
diff --git a/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
b/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
index 33955dccb6ccc..99ff975825c71 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/GenericBitset.cpp
@@ -91,7 +91,7 @@ lldb::ChildCacheState GenericBitsetFrontEnd::Update() {
size_t size = 0;
if (auto arg = m_backend.GetCompilerType().GetIntegralTemplateArgument(0))
-size = arg->value.getLimitedValue();
+size = arg->value.getInt().getLimitedValue();
m_elements.assign(size, ValueObjectSP());
m_first =
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
b/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
index 15040295efe6d..687ef1739ad11 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxxSpan.cpp
@@ -119,7 +119,7 @@
lldb_private::formatters::LibcxxStdSpanSyntheticFrontEnd::Update() {
} else if (auto arg =
m_backend.GetCompilerType().GetIntegralTemplateArgument(1))
{
- m_num_elements = arg->value.getLimitedValue();
+ m_num_elements = arg->value.getInt().getLimitedValue();
}
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index ec0004c70c6da..70af283ab7443 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1973,6 +1973,27 @@ class DWARFASTParserClang::DelayedAddObjCClassProperty {
ClangASTMetadata m_metadata;
};
+static clang::APValue MakeAPValue(CompilerType clang_type, uint64_t bit_width,
+ uint64_t value) {
+ bool is_signed = false;
+ const bool is_integral = clang_type.IsIntegerOrEnumerationType(is_signed);
+
+ llvm::APSInt apint(bit_width, !is_signed);
+ apint = value;
+
+ if (is_integral)
+return clang::APValue(apint);
+
+ uint32_t count;
+ bool is_complex;
+ assert(clang_type.IsFloatingPointType(count, is_complex));
+
+ if (bit_width == 32)
+return clang::APValue(llvm::APFloat(apint.bitsToFloat()));
+
+ return clang::APValue(llvm::APFloat(apint.bitsToDouble()));
+}
+
bool DWARFASTParserClang::ParseTemplateDIE(
const DWARFDIE &die,
TypeSystemClang::TemplateParameterInfos &template_param_infos) {
@@ -2050,9 +2071,6 @@ bool DWARFASTParserClang::ParseTemplateDIE(
clang_type = m_ast.GetBasicType(eBasicTypeVoid);
if (!is_templat
[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap port listening mode to allow multiple connections. (PR #116392)
ashgti wrote: @JDevlieghere Any thoughts on the server mode? https://github.com/llvm/llvm-project/pull/116392 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix broken pipe error (PR #127100)
https://github.com/mgorny approved this pull request. https://github.com/llvm/llvm-project/pull/127100 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Allow option to ignore module load errors in ScriptedProcess (PR #127153)
https://github.com/rchamala edited https://github.com/llvm/llvm-project/pull/127153 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Synchronize the debugger's stdout and stderr streams (PR #126630)
JDevlieghere wrote: > How would that work? We'd check the value of the passed-in `FILE*` and choose > a mutex based on whether it happens to be `stdout` ? I don't think I'd like > that for several reasons: > > * it doesn't handle the case where `stdout/err` is redirected externally (it > still locks even though it shouldn't) > * it's doesn't let you create a terminal-like experience when embedding > lldb-as-a-library (it doesn't lock even though it should) What I had in mind was much simpler than that: it would only apply when the debugger is the one creating the streams in its constructor. I was specifically referring to: ``` Debugger::Debugger(lldb::LogOutputCallback log_callback, void *baton) : UserID(g_unique_id++), Properties(std::make_shared()), m_input_file_sp(std::make_shared(stdin, false)), m_output_stream_sp(std::make_shared(stdout, false)), m_error_stream_sp(std::make_shared(stderr, false)), ``` While not doing this when someone calls `SetOutputFile` or `SetErrorFile`. > > Conceptually I'd also lean towards (2) but I worry about losing the > > benefits the current approach brings. Deciding whether "the output is > > particularly important" is a judgement call that we can't enforce at the > > API level. I like how the current implementation makes it easy to do the > > right thing and hard to do the wrong thing (or at least make you think > > about locking). If the default is to not lock the stdout (the alternative > > of always locking is equivalent to (1)) then I'm skeptical that we won't > > just end up with the same outcome that we have with two mutexes. > > But with two mutexes you still don't get stdout<=>stderr synchronization, > which means that stderr output can corrupt things you're writing to stdout. > What the stderr mutex buys you is stderr<=>stderr synchronization, which > is... maybe nice, but maybe also not necessary? Yes, I think we're saying the same thing here. > FWIW, there is some precedent for unconditional stdout-stderr interaction: > writes to `std::cerr` will automatically flush `std::cout` regardless of > whether the two streams refer to the same terminal. And the `iostream` > library allows you to connect arbitrary two streams in this way. So, I think > we _could_ say we "tie" debuggers output and error streams in this way, even > if they are completely independent, because they are "our" stdout and stderr. Fair enough. What I'm hearing is that you think it's important to synchronize the two somehow and I'd rather do it consistently rather than doing it only sometimes which makes it hard to know what the expectations both for us as the people writing that code and for our users. It should be fairly straightforward to support tying the two streams together. I'll see how that goes and update the PR. https://github.com/llvm/llvm-project/pull/126630 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix broken pipe error (PR #127100)
sga-sc wrote: Thank you for a review, guys, can you merge it, please? I don't have merge rights https://github.com/llvm/llvm-project/pull/127100 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix broken pipe error (PR #127100)
https://github.com/JDevlieghere approved this pull request. Thanks! https://github.com/llvm/llvm-project/pull/127100 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 1042bd7 - [lldb] Fix broken pipe error (#127100)
Author: Georgiy Samoylov
Date: 2025-02-14T10:39:23-08:00
New Revision: 1042bd79722a08b989e034c644c35f3a4556d83c
URL:
https://github.com/llvm/llvm-project/commit/1042bd79722a08b989e034c644c35f3a4556d83c
DIFF:
https://github.com/llvm/llvm-project/commit/1042bd79722a08b989e034c644c35f3a4556d83c.diff
LOG: [lldb] Fix broken pipe error (#127100)
During LLDB testing on slow machines with the remote-linux platform all
tests from llgs category fail with python exception `BrokenPipeError`.
The main reason of these failures is slow start of lldb-server in
gdbserver mode. Due to this desired gdbserver socket does not have time
to open by the time the Python script tries to establish a connection.
List of failed tests:
```
TestAppleSimulatorOSType.py
TestGdbRemoteAttach.py
TestGdbRemoteAuxvSupport.py
TestGdbRemoteCompletion.py
TestGdbRemoteExitCode.py
TestGdbRemoteExpeditedRegisters.py
TestGdbRemoteHostInfo.py
TestGdbRemoteKill.py
TestGdbRemoteLaunch.py
TestGdbRemoteModuleInfo.py
TestGdbRemotePlatformFile.py
TestGdbRemoteProcessInfo.py
TestGdbRemoteRegisterState.py
TestGdbRemoteSaveCore.py
TestGdbRemoteSingleStep.py
TestGdbRemoteThreadsInStopReply.py
TestGdbRemote_qThreadStopInfo.py
TestGdbRemote_vCont.py
TestLldbGdbServer.py
TestNonStop.py
TestPtyServer.py
TestGdbRemoteAttachWait.py
TestGdbRemoteConnection.py
TestStubSetSID.py
TestGdbRemoteAbort.py
TestGdbRemoteSegFault.py
TestGdbRemoteLibrariesSvr4Support.py
TestGdbRemoteMemoryAllocation.py
TestGdbRemoteMemoryTagging.py
TestGdbRemoteGPacket.py
TestGdbRemoteTargetXmlPacket.py
TestGdbRemote_QPassSignals.py
TestGdbRemoteThreadName.py
TestPartialResume.py
TestSignal.py
```
This patch implements an additional check for the opened socket on
lldb-server side and fixes this error.
Added:
Modified:
lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
Removed:
diff --git
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
index cbe430c92fa7f..fb96b3d4de560 100644
---
a/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
+++
b/lldb/packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
@@ -249,14 +249,11 @@ def remove_port_forward():
def _verify_socket(self, sock):
# Normally, when the remote stub is not ready, we will get
ECONNREFUSED during the
-# connect() attempt. However, due to the way how ADB forwarding works,
on android targets
+# connect() attempt. However, due to the way how port forwarding can
work, on some targets
# the connect() will always be successful, but the connection will be
immediately dropped
-# if ADB could not connect on the remote side. This function tries to
detect this
+# if we could not connect on the remote side. This function tries to
detect this
# situation, and report it as "connection refused" so that the upper
layers attempt the
# connection again.
-triple = self.dbg.GetSelectedPlatform().GetTriple()
-if not re.match(".*-.*-.*-android", triple):
-return # Not android.
can_read, _, _ = select.select([sock], [], [], 0.1)
if sock not in can_read:
return # Data is not available, but the connection is alive.
@@ -397,13 +394,13 @@ def connect_to_debug_monitor(self, attach_pid=None):
# Schedule debug monitor to be shut down during teardown.
logger = self.logger
-connect_attemps = 0
+connect_attempts = 0
MAX_CONNECT_ATTEMPTS = 10
-while connect_attemps < MAX_CONNECT_ATTEMPTS:
+while connect_attempts < MAX_CONNECT_ATTEMPTS:
# Create a socket to talk to the server
try:
-logger.info("Connect attempt %d", connect_attemps + 1)
+logger.info("Connect attempt %d", connect_attempts + 1)
self.sock = self.create_socket()
self._server = Server(self.sock, server)
return server
@@ -411,7 +408,7 @@ def connect_to_debug_monitor(self, attach_pid=None):
# Ignore, and try again.
pass
time.sleep(0.5)
-connect_attemps += 1
+connect_attempts += 1
# We should close the server here to be safe.
server.terminate()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix broken pipe error (PR #127100)
https://github.com/JDevlieghere closed https://github.com/llvm/llvm-project/pull/127100 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Synchronize the debugger's stdout and stderr streams (PR #126630)
labath wrote: > > How would that work? We'd check the value of the passed-in `FILE*` and > > choose a mutex based on whether it happens to be `stdout` ? I don't think > > I'd like that for several reasons: > > > > * it doesn't handle the case where `stdout/err` is redirected externally > > (it still locks even though it shouldn't) > > * it's doesn't let you create a terminal-like experience when embedding > > lldb-as-a-library (it doesn't lock even though it should) > > What I had in mind was much simpler than that: it would only apply when the > debugger is the one creating the streams in its constructor. I was > specifically referring to: > > ``` > Debugger::Debugger(lldb::LogOutputCallback log_callback, void *baton) > : UserID(g_unique_id++), > Properties(std::make_shared()), > m_input_file_sp(std::make_shared(stdin, false)), > m_output_stream_sp(std::make_shared(stdout, false)), > m_error_stream_sp(std::make_shared(stderr, false)), > ``` > > While not doing this when someone calls `SetOutputFile` or `SetErrorFile`. I see. Well.. that's slightly cleaner, but it's currently a no-op because even our own driver calls [SetOutputFileHandle](https://github.com/llvm/llvm-project/blob/1199bbb396fb9554401ad5ae1816b6648bab76a9/lldb/tools/driver/Driver.cpp#L442). I guess we could change that, but I also don't particularly like the inaccessibility of this feature. > > > > Conceptually I'd also lean towards (2) but I worry about losing the > > > benefits the current approach brings. Deciding whether "the output is > > > particularly important" is a judgement call that we can't enforce at the > > > API level. I like how the current implementation makes it easy to do the > > > right thing and hard to do the wrong thing (or at least make you think > > > about locking). If the default is to not lock the stdout (the alternative > > > of always locking is equivalent to (1)) then I'm skeptical that we won't > > > just end up with the same outcome that we have with two mutexes. > > > > > > But with two mutexes you still don't get stdout<=>stderr synchronization, > > which means that stderr output can corrupt things you're writing to stdout. > > What the stderr mutex buys you is stderr<=>stderr synchronization, which > > is... maybe nice, but maybe also not necessary? > > Yes, I think we're saying the same thing here. I guess so, but I think we're drawing different conclusions. My take on this is that if the only thing we get from locking stderr is the ability to mutually exclude two stderr accesses, then maybe we don't need to bother with locking stderr at all. > > FWIW, there is some precedent for unconditional stdout-stderr interaction: > > writes to `std::cerr` will automatically flush `std::cout` regardless of > > whether the two streams refer to the same terminal. And the `iostream` > > library allows you to connect arbitrary two streams in this way. So, I > > think we _could_ say we "tie" debuggers output and error streams in this > > way, even if they are completely independent, because they are "our" stdout > > and stderr. > > Fair enough. What I'm hearing is that you think it's important to synchronize > the two somehow and I'd rather do it consistently rather than doing it only > sometimes which makes it hard to know what the expectations both for us as > the people writing that code and for our users. Yes, I think that tying them *always* is better than tying them *sometimes*. However, I don't really have a clear preference between the other options flying around. The options being: - tying them always - not tying them at all - not tying them and not locking stderr If it sounds like I'm in favor of one of them, it's because I'm comparing its (dis)advantages relative to some other approach. However, I think all of these approaches have their (dis)advantages. > It should be fairly straightforward to support tying the two streams > together. I'll see how that goes and update the PR. A random idea, I don't know if it's a good one: have one object (LockableStreamPair?) that holds the two streams and their mutex. One less argument to pass around? https://github.com/llvm/llvm-project/pull/126630 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][LoongArch] Complete register alias name in `AugmentRegisterInfo` (PR #124059)
wangleiat wrote: gentle ping 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] Add terminfo dependency for ncurses support (PR #126810)
https://github.com/ajordanr-google updated
https://github.com/llvm/llvm-project/pull/126810
>From ddd3febff5b77cc7b2101996d49729added00f2b Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead
Date: Tue, 1 Oct 2024 18:41:28 +
Subject: [PATCH 1/6] [lldb] Add terminfo dependency for ncurses support
For some operating systems (e.g. chromiumos), terminfo is a separate
package and library from ncurses. Both are still requirements for curses
support in lldb, individually.
This is a rebase of this original spack commit:
https://github.com/spack/spack/commit/9ea261265010eacd250691a8361f661d0576f25c
Without this fix, LLDB cannot be built on these systems.
Fixes #101368
---
lldb/cmake/modules/FindCursesAndPanel.cmake | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/lldb/cmake/modules/FindCursesAndPanel.cmake
b/lldb/cmake/modules/FindCursesAndPanel.cmake
index aaadf214bf54b..df4980cc5e0d1 100644
--- a/lldb/cmake/modules/FindCursesAndPanel.cmake
+++ b/lldb/cmake/modules/FindCursesAndPanel.cmake
@@ -2,12 +2,15 @@
# FindCursesAndPanel
# ---
#
-# Find the curses and panel library as a whole.
+# Find the curses, terminfo, and panel library as a whole.
+# NOTE: terminfo and curses libraries are required separately, as
+# some systems do not bundle them together.
-if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND PANEL_LIBRARIES)
+if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND TINFO_LIBRARIES AND
PANEL_LIBRARIES)
set(CURSESANDPANEL_FOUND TRUE)
else()
find_package(Curses QUIET)
+ find_package(TINFO_LIBRARIES NAMES tinfo DOC "The curses tinfo library"
QUIET)
find_library(PANEL_LIBRARIES NAMES panel DOC "The curses panel library"
QUIET)
include(FindPackageHandleStandardArgs)
find_package_handle_standard_args(CursesAndPanel
@@ -16,9 +19,10 @@ else()
REQUIRED_VARS
CURSES_INCLUDE_DIRS
CURSES_LIBRARIES
+ TINFO_LIBRARIES
PANEL_LIBRARIES)
- if(CURSES_FOUND AND PANEL_LIBRARIES)
-mark_as_advanced(CURSES_INCLUDE_DIRS CURSES_LIBRARIES PANEL_LIBRARIES)
+ if(CURSES_FOUND AND TINFO_LIBRARIES AND PANEL_LIBRARIES)
+mark_as_advanced(CURSES_INCLUDE_DIRS CURSES_LIBRARIES TINFO_LIBRARIES
PANEL_LIBRARIES)
endif()
endif()
>From 41ad79112bc2242a1cb10ae688353863155d9038 Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead
Date: Wed, 12 Feb 2025 18:22:09 +
Subject: [PATCH 2/6] fixup! [lldb] Add terminfo dependency for ncurses support
---
lldb/source/Core/CMakeLists.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index cf5f6ac9da489..6c7a50056f751 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -10,7 +10,7 @@ set(LLDB_CURSES_LIBS)
set(LLDB_LIBEDIT_LIBS)
if (LLDB_ENABLE_CURSES)
- list(APPEND LLDB_CURSES_LIBS ${PANEL_LIBRARIES} ${CURSES_LIBRARIES})
+ list(APPEND LLDB_CURSES_LIBS ${PANEL_LIBRARIES} ${CURSES_LIBRARIES}
${TINFO_LIBRARIES})
if (LLVM_BUILD_STATIC)
list(APPEND LLDB_CURSES_LIBS gpm)
endif()
>From 9ffc89356cc635ac31f11c66dcb6f32cd73380fc Mon Sep 17 00:00:00 2001
From: Jordan R Abrahams-Whitehead
Date: Wed, 12 Feb 2025 19:29:23 +
Subject: [PATCH 3/6] fixup! fixup! [lldb] Add terminfo dependency for ncurses
support
---
lldb/cmake/modules/FindCursesAndPanel.cmake | 41 +
lldb/cmake/modules/FindTerminfo.cmake | 21 +++
lldb/source/Core/CMakeLists.txt | 5 ++-
3 files changed, 59 insertions(+), 8 deletions(-)
create mode 100644 lldb/cmake/modules/FindTerminfo.cmake
diff --git a/lldb/cmake/modules/FindCursesAndPanel.cmake
b/lldb/cmake/modules/FindCursesAndPanel.cmake
index df4980cc5e0d1..646113155343d 100644
--- a/lldb/cmake/modules/FindCursesAndPanel.cmake
+++ b/lldb/cmake/modules/FindCursesAndPanel.cmake
@@ -3,26 +3,53 @@
# ---
#
# Find the curses, terminfo, and panel library as a whole.
-# NOTE: terminfo and curses libraries are required separately, as
-# some systems do not bundle them together.
+
+include(CMakePushCheckState)
+
+function(lldb_check_curses_tinfo CURSES_LIBRARIES CURSES_HAS_TINFO)
+ cmake_reset_check_state()
+ set(CMAKE_REQUIRED_LIBRARIES "${CURSES_LIBRARIES}")
+ # acs_map is one of many symbols that are part of tinfo but could
+ # be bundled in curses.
+ check_symbol_exists(acs_map "curses.h" CURSES_HAS_TINFO)
+endfunction()
if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND TINFO_LIBRARIES AND
PANEL_LIBRARIES)
set(CURSESANDPANEL_FOUND TRUE)
else()
find_package(Curses QUIET)
- find_package(TINFO_LIBRARIES NAMES tinfo DOC "The curses tinfo library"
QUIET)
find_library(PANEL_LIBRARIES NAMES panel DOC "The curses panel library"
QUIET)
include(FindPackageHandleStandardArgs)
+
+ # Sometimes the curs
[Lldb-commits] [lldb] [lldb] Add terminfo dependency for ncurses support (PR #126810)
@@ -2,23 +2,54 @@
# FindCursesAndPanel
# ---
#
-# Find the curses and panel library as a whole.
+# Find the curses, terminfo, and panel library as a whole.
-if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND PANEL_LIBRARIES)
+include(CMakePushCheckState)
+
+function(lldb_check_curses_tinfo CURSES_LIBRARIES CURSES_HAS_TINFO)
+ cmake_reset_check_state()
+ set(CMAKE_REQUIRED_LIBRARIES "${CURSES_LIBRARIES}")
+ # acs_map is one of many symbols that are part of tinfo but could
+ # be bundled in curses.
+ check_symbol_exists(acs_map "curses.h" CURSES_HAS_TINFO)
+endfunction()
+
+if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND TINFO_LIBRARIES AND
PANEL_LIBRARIES)
set(CURSESANDPANEL_FOUND TRUE)
else()
find_package(Curses QUIET)
find_library(PANEL_LIBRARIES NAMES panel DOC "The curses panel library"
QUIET)
include(FindPackageHandleStandardArgs)
+
+ # Sometimes the curses libraries define their own terminfo symbols,
+ # other times they're extern and are defined by a separate terminfo library.
+ # Auto-detect which.
+ lldb_check_curses_tinfo("${CURSES_LIBRARIES}" CURSES_HAS_TINFO)
+ if(CURSES_FOUND AND PANEL_LIBRARIES)
+if (NOT CURSES_HAS_TINFO)
ajordanr-google wrote:
As above, I'm going to keep `lldb_check_curses_tinfo()`, but I should indeed
move this into the `if`. Thanks!
https://github.com/llvm/llvm-project/pull/126810
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 8fff0c1 - [lldb] Add terminfo dependency for ncurses support (#126810)
Author: Jordan R AW
Date: 2025-02-14T21:37:39-08:00
New Revision: 8fff0c181f26a5e8b2344c061ebf2559118b1160
URL:
https://github.com/llvm/llvm-project/commit/8fff0c181f26a5e8b2344c061ebf2559118b1160
DIFF:
https://github.com/llvm/llvm-project/commit/8fff0c181f26a5e8b2344c061ebf2559118b1160.diff
LOG: [lldb] Add terminfo dependency for ncurses support (#126810)
For some operating systems (e.g. chromiumos), terminfo is a separate
package and library from ncurses. Both are still requirements for curses
support in lldb, individually.
This is a rework of this original spack commit:
https://github.com/spack/spack/commit/9ea261265010eacd250691a8361f661d0576f25c
Instead though, this PR uses CMake to detect whether the symbol is
present and defined in the curses library, and only falls back to a separate
tinfo if not found.
Without this fix, LLDB cannot be built on these systems.
Fixes #101368
Added:
Modified:
lldb/cmake/modules/FindCursesAndPanel.cmake
Removed:
diff --git a/lldb/cmake/modules/FindCursesAndPanel.cmake
b/lldb/cmake/modules/FindCursesAndPanel.cmake
index aaadf214bf54b..75ebaa35d7ea1 100644
--- a/lldb/cmake/modules/FindCursesAndPanel.cmake
+++ b/lldb/cmake/modules/FindCursesAndPanel.cmake
@@ -2,23 +2,55 @@
# FindCursesAndPanel
# ---
#
-# Find the curses and panel library as a whole.
+# Find the curses, terminfo, and panel library as a whole.
-if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND PANEL_LIBRARIES)
+include(CMakePushCheckState)
+
+function(lldb_check_curses_tinfo CURSES_LIBRARIES CURSES_HAS_TINFO)
+ cmake_reset_check_state()
+ set(CMAKE_REQUIRED_LIBRARIES "${CURSES_LIBRARIES}")
+ # acs_map is one of many symbols that are part of tinfo but could
+ # be bundled in curses.
+ check_symbol_exists(acs_map "curses.h" CURSES_HAS_TINFO)
+endfunction()
+
+if(CURSES_INCLUDE_DIRS AND CURSES_LIBRARIES AND TINFO_LIBRARIES AND
PANEL_LIBRARIES)
set(CURSESANDPANEL_FOUND TRUE)
else()
find_package(Curses QUIET)
find_library(PANEL_LIBRARIES NAMES panel DOC "The curses panel library"
QUIET)
include(FindPackageHandleStandardArgs)
+
+ if(CURSES_FOUND AND PANEL_LIBRARIES)
+# Sometimes the curses libraries define their own terminfo symbols,
+# other times they're extern and are defined by a separate terminfo
library.
+# Auto-detect which.
+lldb_check_curses_tinfo("${CURSES_LIBRARIES}" CURSES_HAS_TINFO)
+if (NOT CURSES_HAS_TINFO)
+ message(STATUS "curses library missing terminfo symbols, looking for
tinfo separately")
+ find_library(TINFO_LIBRARIES NAMES tinfo DOC "The curses tinfo library"
QUIET)
+ list(APPEND CURSES_LIBRARIES "${TINFO_LIBRARIES}")
+endif()
+set(HAS_TERMINFO_SYMBOLS
"$,$>")
+ endif()
+
find_package_handle_standard_args(CursesAndPanel
FOUND_VAR
CURSESANDPANEL_FOUND
REQUIRED_VARS
CURSES_INCLUDE_DIRS
CURSES_LIBRARIES
- PANEL_LIBRARIES)
- if(CURSES_FOUND AND PANEL_LIBRARIES)
-mark_as_advanced(CURSES_INCLUDE_DIRS CURSES_LIBRARIES PANEL_LIBRARIES)
+ PANEL_LIBRARIES
+ HAS_TERMINFO_SYMBOLS)
+
+ if(CURSES_FOUND AND PANEL_LIBRARIES AND HAS_TERMINFO_SYMBOLS)
+mark_as_advanced(CURSES_INCLUDE_DIRS
+ PANEL_LIBRARIES
+ HAS_TERMINFO_SYMBOLS
+ CURSES_HAS_TINFO)
+ endif()
+ if(TINFO_LIBRARIES)
+mark_as_advanced(TINFO_LIBRARIES)
endif()
endif()
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add terminfo dependency for ncurses support (PR #126810)
https://github.com/JDevlieghere closed https://github.com/llvm/llvm-project/pull/126810 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
