[Lldb-commits] [lldb] [lldb][DWARF] Remove obsolete calls to Supports_DW_AT_APPLE_objc_complete_type and DW_AT_decl_file_attributes_are_invalid (PR #120226)
labath wrote: > That would be easy to find out on Compiler Explorer :-) I was curious, so I [tried it out](https://godbolt.org/z/E5j5afqso). The answer appears to be "no". :) https://github.com/llvm/llvm-project/pull/120226 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AIX] Introducing ALL_SOURCE macro into driver CMakeLists (PR #120607)
DavidSpickett wrote: I assume the macro is `_ALL_SOURCE` with the leading underscore, you put `ALL_SOURCE` in the title and description. I see that https://github.com/llvm/llvm-project/blob/d66f653c8db90d0c643f8f2740bbdc01bf647f18/third-party/unittest/CMakeLists.txt#L18 also does this. https://github.com/llvm/llvm-project/blob/d66f653c8db90d0c643f8f2740bbdc01bf647f18/clang/tools/libclang/CMakeLists.txt#L132 says it does the same thing but it doesn't add `_ALL_SOURCE` it only removes the other macro. The one in third-party seems more logical and matches the one you're adding, so this is fine. https://github.com/llvm/llvm-project/pull/120607 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AIX] Adding AIX version of ptrace64 (PR #120390)
labath wrote: I think this is fine. We could come up with an abstraction for this, but I don't think it's necessary for a simple change like this. Like @DhruvSrivastavaX said, the rest of the ptrace calls are going to be in platform-specific code. https://github.com/llvm/llvm-project/pull/120390 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AIX] Adding AIX version of ptrace64 (PR #120390)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/120390 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] cf0bc8d - [lldb][AIX] Adding AIX version of ptrace64 (#120390)
Author: Dhruv Srivastava
Date: 2024-12-20T11:08:17Z
New Revision: cf0bc8d0321a55f3f166131ec3fe45cdef7d5e3c
URL:
https://github.com/llvm/llvm-project/commit/cf0bc8d0321a55f3f166131ec3fe45cdef7d5e3c
DIFF:
https://github.com/llvm/llvm-project/commit/cf0bc8d0321a55f3f166131ec3fe45cdef7d5e3c.diff
LOG: [lldb][AIX] Adding AIX version of ptrace64 (#120390)
This PR is in reference to porting LLDB on AIX.
Link to discussions on llvm discourse and github:
1. https://discourse.llvm.org/t/port-lldb-to-ibm-aix/80640
2. https://github.com/llvm/llvm-project/issues/101657
The complete changes for porting are present in this draft PR:
https://github.com/llvm/llvm-project/pull/102601
Adding changes for minimal build for lldb binary on AIX. ptrace64 is
needed to debug 64-bit AIX debuggee, and its format is different than
the traditional ptrace on other platforms: [AIX
ptrace](https://www.ibm.com/docs/en/aix/7.3?topic=p-ptrace-ptracex-ptrace64-subroutine)
Added:
Modified:
lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
Removed:
diff --git a/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
b/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
index 4a0437b634ee39..7b8b42a4b7fe07 100644
--- a/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
+++ b/lldb/source/Host/posix/ProcessLauncherPosixFork.cpp
@@ -192,7 +192,11 @@ struct ForkLaunchInfo {
}
// Start tracing this child that is about to exec.
+#ifdef _AIX
+if (ptrace64(PT_TRACE_ME, 0, 0, 0, nullptr) == -1)
+#else
if (ptrace(PT_TRACE_ME, 0, nullptr, 0) == -1)
+#endif
ExitWithError(error_fd, "ptrace");
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AIX] Adding AIX version of ptrace64 (PR #120390)
https://github.com/DavidSpickett closed https://github.com/llvm/llvm-project/pull/120390 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][SymbolFileDWARF] CompleteType: Lookup type in the declaration DIE's SymbolFile (PR #120569)
Michael137 wrote: > > In which case, GetDIEToType().lookup(decl_die) will return a nullptr. This > > is already a bit iffy because some of the surrounding code assumes we don't > > call CompleteTypeFromDWARF with a nullptr Type*. E.g., CompleteEnumType > > blindly dereferences it (though enums will never encounter this because > > their definition is fetched in ParseEnum, unlike for structures). > > Should we bail out early if the Type* is null and return false to tell > `SymbolFileDWARFDebugMap::CompleteType` that it can not complete this type > and let it iterate to the symbol file that has the entry in its map. Just tried this. Unfortunately [we remove the CompilerType from the `GetForwardDeclCompilerTypeToDIE`](https://github.com/llvm/llvm-project/blob/d7ddc976d544528fe7f16882f5bec66c3b2a7884/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L1577) map inside of `CompleteType`. So in the next iteration of the `SymbolFileDWARFDebugMap::CompleteType` loop, it [won't ever call into `SymbolFileDWARF::CompleteType` again](https://github.com/llvm/llvm-project/blob/d7ddc976d544528fe7f16882f5bec66c3b2a7884/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp#L808) :( https://github.com/llvm/llvm-project/pull/120569 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix address to read segment data (PR #120655)
https://github.com/labath commented: I'm pretty sure this is not correct. `p_offset` is an offset in the file. It tells you nothing about how the file is mapped into memory. What you're *probably* running into is an executable whose base address (`p_vaddr` of the first `PT_LOAD` segment) is not zero. This is true for all non-PIE executables, but it can be true for other kinds of files as well. Since the value you get for `address` here is the actual address of the first load segment (rather than the *delta* between the virtual and actual addresses, which is used in some other places), what you need to do here is to subtract the address of the first segment from the result. This way you get zero for the first segment, and then a delta for all the others. Take a look at [this](https://github.com/llvm/llvm-project/blob/d7ddc976d544528fe7f16882f5bec66c3b2a7884/lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp#L64) code doing something similar. https://github.com/llvm/llvm-project/pull/120655 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AIX] Introducing _ALL_SOURCE macro into driver CMakeLists (PR #120607)
@@ -11,6 +11,11 @@ if(APPLE)
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS}
-Wl,-sectcreate,__TEXT,__info_plist,${CMAKE_CURRENT_BINARY_DIR}/lldb-Info.plist")
endif()
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+ remove_definitions("-D_XOPEN_SOURCE=700")
labath wrote:
This is fairly fragile. It may be better to do a `-U_XOPEN_SOURCE`.
Some comment about why you're undoing the changes
[here](https://github.com/llvm/llvm-project/blob/d7ddc976d544528fe7f16882f5bec66c3b2a7884/llvm/CMakeLists.txt#L1173)
might be in order as well.
https://github.com/llvm/llvm-project/pull/120607
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][SymbolFileDWARF] CompleteType: Lookup type in the declaration DIE's SymbolFile (PR #120569)
Michael137 wrote: > As an alternative, we might be able to make the die-to-type map shared > between all symbol files in a debug map (similar to how "unique dwarf ast > type map" is shared). Yea I considered that, but I was concerned with the unforeseen complications this might have. Particularly I'm not sure how the `TypeSP` ownership should be managed. IIUC, a `TypeSP` is solely supposed to be owned by a single `SymbolFile` (`MakeType` kind of binds the two together). But then if we bundled `TypeSP`s from different modules into a single map on `SymbolFileDWARFDebugMap`, could we run into issues if one of the `SymbolFile`'s gets torn down (if that can happen)? Though I'm also not certain that we *only* store `TypeSP`s inside the `SymbolFileDWARF`. I don't think there's anything asserting/enforcing this. https://github.com/llvm/llvm-project/pull/120569 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][SymbolFileDWARF] CompleteType: Lookup type in the declaration DIE's SymbolFile (PR #120569)
Michael137 wrote: > Maybe, but I don't expect the test to be pretty. What I (sometimes) do in > these cases is to make the test bidirectional, i.e., have two compile unit > where each one has a definition for a type declared/used in the other one. > This way, you're guaranteed to hit the error code path regardless of the > order of the two units. This is easier to do if you don't actually need to > run the binary (which you probably don't need to do here). Great idea, let me try this out https://github.com/llvm/llvm-project/pull/120569 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 000febd - [lldb][test] Add test-coverage for DW_AT_APPLE_objc_complete_type parsing (#120279)
Author: Michael Buch
Date: 2024-12-20T12:16:19Z
New Revision: 000febd0290698728abd9e23da6b27969c529177
URL:
https://github.com/llvm/llvm-project/commit/000febd0290698728abd9e23da6b27969c529177
DIFF:
https://github.com/llvm/llvm-project/commit/000febd0290698728abd9e23da6b27969c529177.diff
LOG: [lldb][test] Add test-coverage for DW_AT_APPLE_objc_complete_type parsing
(#120279)
When given a DIE for an Objective-C interface (which doesn't have a
`DW_AT_APPLE_objc_complete_type`), the `DWARFASTParserClang` will try to
find the DIE which corresponds to the implementation to complete the
interface DIE. The code is here:
https://github.com/llvm/llvm-project/blob/d2e7ee77d33e8b3be3b1d4e9bc5bc4c60b62b554/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp#L1718-L1738
However, this was currently not exercised in our test-suite (removing
the code above didn't fail any LLDB test).
This patch adds a test which exercises this codepath (it will fail if we
don't fetch the implementation DIE in the `DWARFASTParserClang`).
Something that's not currently clear to me is why `frame var *f`
succeeds even without the `DW_AT_APPLE_objc_complete_type`
infrastructure. If it's using the ObjC runtime, we should make `expr` do
the same, in which case we can remove this code from
`DWARFASTParserClang`.
Added:
lldb/test/Shell/Expr/TestObjCHiddenIvars.test
Modified:
Removed:
diff --git a/lldb/test/Shell/Expr/TestObjCHiddenIvars.test
b/lldb/test/Shell/Expr/TestObjCHiddenIvars.test
new file mode 100644
index 00..18c496e4d2d271
--- /dev/null
+++ b/lldb/test/Shell/Expr/TestObjCHiddenIvars.test
@@ -0,0 +1,65 @@
+# REQUIRES: system-darwin
+#
+# Tests that LLDB correctly finds the implementation
+# DIE (with a `DW_AT_APPLE_objc_complete_type`)
+# given an interface DIE (without said attribute).
+#
+# RUN: split-file %s %t
+# RUN: %clangxx_host %t/lib.m -c -g -o %t/lib.o
+# RUN: %clangxx_host %t/main.m -c -g -o %t/main.o
+# RUN: %clangxx_host %t/main.o %t/lib.o -o %t/a.out -framework Foundation
+#
+# RUN: %lldb %t/a.out \
+# RUN: -o "breakpoint set -p 'return' -X main" \
+# RUN: -o run \
+# RUN: -o "expression *f" \
+# RUN: -o exit | FileCheck %s
+
+# CHECK: (lldb) expression *f
+# CHECK: (Foo) ${{[0-9]+}} = {
+# CHECK: y = 2
+# CHECK-NEXT:i = 1
+
+#--- main.m
+#import
+#import "lib.h"
+
+extern Foo * func();
+
+int main() {
+Foo * f = func();
+return 0;
+}
+
+#--- lib.m
+#import
+#import "lib.h"
+
+@implementation Foo {
+int i;
+}
+
+- (id)init {
+ self->i = 1;
+ self->y = 2;
+
+ return self;
+}
+@end
+
+Foo * func() {
+ return [[Foo alloc] init];
+}
+
+#--- lib.h
+#ifndef LIB_H_IN
+#define LIB_H_IN
+
+#import
+
+@interface Foo : NSObject {
+int y;
+}
+@end
+
+#endif // _H_IN
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Add test-coverage for DW_AT_APPLE_objc_complete_type parsing (PR #120279)
https://github.com/Michael137 closed https://github.com/llvm/llvm-project/pull/120279 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #112079)
labath wrote: > I would recommend not squashing, because that will make it much easier to > find the incorrect change if there are regressions. I agree, but "squash and merge" is the only strategy enabled for this repository. Could you create a separate PR (or two, but probably not eight) with the refactoring changes, and we can rebase this PR on top of that when they land? https://github.com/llvm/llvm-project/pull/112079 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][SymbolFileDWARF] Share GetDIEToType between SymbolFiles of a SymbolFileDWARFDebugMap (PR #120569)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/120569 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][SymbolFileDWARF] Share GetDIEToType between SymbolFiles of a SymbolFileDWARFDebugMap (PR #120569)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/120569 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Docs] Add equivalents of GDB's "skip" to command map (PR #120740)
@@ -235,6 +235,40 @@ Do a source level single step in the currently selected thread (lldb) step (lldb) s +Ignore a function when doing a source level single step in +~~ + +.. code-block:: shell + + (gdb) skip abc + Function abc will be skipped when stepping. + +.. code-block:: shell + + (lldb) settings show target.process.thread.step-avoid-regexp + target.process.thread.step-avoid-regexp (regex) = ^std:: + (lldb) settings set target.process.thread.step-avoid-regexp (^std::)|(^abc) + +Get the default value, make it into a capture group, then add another capture +group for the new function name. + +You can ignore a function once using: + +.. code-block:: shell + + (lldb) thread step-in -r ^abc + +Or you can do the opposite, only step into functions with a certain name: + +.. code-block:: shell + + (lldb) sif abc + # Which is equivalent to: + (lldb) thread step-in -t abc kastiglione wrote: It may be worth noting that the string is matched as a substring. The benefit is that for long function names, it's only necessary to type a uniquely identifiable substring. https://github.com/llvm/llvm-project/pull/120740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][DWARFASTParserClang][NFC] Remove unused parameter to CompleteRecordType (PR #120456)
https://github.com/labath approved this pull request. Nice. https://github.com/llvm/llvm-project/pull/120456 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][LoongArch] Add LSX and LASX register definitions and operations (PR #120664)
https://github.com/SixWeining commented: Could you add some tests in `lldb/test/Shell/Register/` ? https://github.com/llvm/llvm-project/pull/120664 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][LoongArch] Add LSX and LASX register definitions and operations (PR #120664)
@@ -27,6 +27,14 @@ // struct iovec definition #include +#ifndef NT_LARCH_LSX SixWeining wrote: Why not use the `NT_LOONGARCH_LSX` defined in elf.h? https://github.com/llvm/llvm-project/pull/120664 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][LoongArch] Add LSX and LASX register definitions and operations (PR #120664)
https://github.com/SixWeining edited https://github.com/llvm/llvm-project/pull/120664 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][test] Prefer gmake to make and warn for potentially non-GNU make (PR #119573)
labath wrote: > > However, it shows an issue that is a concern actually: Do we really want to > > prefer gmake over make on all platforms? > > I was using RHEL9 today and it has gmake and make but they are both GNU make. > So I think we're safe using gmake if we find it. On my system (Gentoo), `gmake` is a symlink to `make`. ``` $ ls -l `which make` lrwxr-xr-x 1 root root 5 Nov 12 19:29 /usr/bin/make -> gmake ``` I'd be willing to bet that the version of the code before this patch (which preferred `make`) would fail on a linux system (if it even exists) where `make` is *NOT* "GNU make" . (In other words, I agree with you :) ) https://github.com/llvm/llvm-project/pull/119573 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] do not show misleading error when there is no frame (PR #119103)
oltolm wrote: Is there still some TODO for me? https://github.com/llvm/llvm-project/pull/119103 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Debuginfod cache use index cache settings and include real file name. (PR #120814)
https://github.com/GeorgeHuyubo created
https://github.com/llvm/llvm-project/pull/120814
This PR include two changes:
1. Change debuginfod cache file name to include origin file name, the new file
name would be something like:
llvmcache-13267c5f5d2e3df472c133c8efa45fb3331ef1ea-liblzma.so.5.2.2.debuginfo.dwp
So it will provide more information in image list instead of a plain
llvmcache-123
2. Switch debuginfod cache to use lldb index cache settings. Currently we don't
have proper settings for setting the cache path or the cache expiration time
for debuginfod cache. We want to use the lldb index cache settings, as they
make sense to be in the same place and have the same TTL.
>From 6923737d728191816567e7874a01c5dfce68bfde Mon Sep 17 00:00:00 2001
From: George Hu
Date: Fri, 20 Dec 2024 15:20:00 -0800
Subject: [PATCH 1/2] [lldb] Change debuginfod cache file name to include
origin file name
---
.../Debuginfod/SymbolLocatorDebuginfod.cpp| 27 +--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git
a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
index 2cd7244902..c103c98d20ac27 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
@@ -141,6 +141,25 @@ SymbolLocator *SymbolLocatorDebuginfod::CreateInstance() {
return new SymbolLocatorDebuginfod();
}
+static llvm::StringRef getFileName(const ModuleSpec &module_spec,
+ std::string url_path) {
+ // Check if the URL path requests an executable file or a symbol file
+ bool is_executable = url_path.find("debuginfo") == std::string::npos;
+ if (is_executable) {
+return module_spec.GetFileSpec().GetFilename().GetStringRef();
+ }
+ llvm::StringRef symbol_file =
+ module_spec.GetSymbolFileSpec().GetFilename().GetStringRef();
+ // Remove llvmcache- prefix and hash, keep origin file name
+ if (symbol_file.starts_with("llvmcache-")) {
+size_t pos = symbol_file.rfind('-');
+if (pos != llvm::StringRef::npos) {
+ symbol_file = symbol_file.substr(pos + 1);
+}
+ }
+ return symbol_file;
+}
+
static std::optional
GetFileForModule(const ModuleSpec &module_spec,
std::function UrlBuilder)
{
@@ -166,9 +185,13 @@ GetFileForModule(const ModuleSpec &module_spec,
// We're ready to ask the Debuginfod library to find our file.
llvm::object::BuildID build_id(module_uuid.GetBytes());
std::string url_path = UrlBuilder(build_id);
- std::string cache_key = llvm::getDebuginfodCacheKey(url_path);
+ llvm::StringRef file_name = getFileName(module_spec, url_path);
+ std::string cache_file_name = llvm::toHex(build_id, true);
+ if (!file_name.empty()) {
+cache_file_name += "-" + file_name.str();
+ }
llvm::Expected result = llvm::getCachedOrDownloadArtifact(
- cache_key, url_path, cache_path, debuginfod_urls, timeout);
+ cache_file_name, url_path, cache_path, debuginfod_urls, timeout);
if (result)
return FileSpec(*result);
>From 58134726a34790fa23db0fe18e3cb4cc178a389b Mon Sep 17 00:00:00 2001
From: George Hu
Date: Fri, 20 Dec 2024 16:37:50 -0800
Subject: [PATCH 2/2] [lldb] Switch debuginfod cache to use lldb index cache
settings
---
.../Debuginfod/SymbolLocatorDebuginfod.cpp| 16 ++
llvm/include/llvm/Debuginfod/Debuginfod.h | 4 ++-
llvm/lib/Debuginfod/Debuginfod.cpp| 29 +--
llvm/unittests/Debuginfod/DebuginfodTests.cpp | 3 +-
4 files changed, 35 insertions(+), 17 deletions(-)
diff --git
a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
index c103c98d20ac27..f459825a61a562 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
@@ -8,6 +8,7 @@
#include "SymbolLocatorDebuginfod.h"
+#include "lldb/Core/DataFileCache.h"
#include "lldb/Core/PluginManager.h"
#include "lldb/Interpreter/OptionValueString.h"
#include "lldb/Utility/Args.h"
@@ -173,11 +174,14 @@ GetFileForModule(const ModuleSpec &module_spec,
// Grab LLDB's Debuginfod overrides from the
// plugin.symbol-locator.debuginfod.* settings.
PluginProperties &plugin_props = GetGlobalPluginProperties();
- llvm::Expected cache_path_or_err = plugin_props.GetCachePath();
- // A cache location is *required*.
- if (!cache_path_or_err)
-return {};
- std::string cache_path = *cache_path_or_err;
+ // Grab the lldb index cache settings from the global module list properties.
+ ModuleListProperties &properties =
+ ModuleList::GetGlobalModuleListProperties();
+ std::string cache_path = properties.GetLLDBIndexCachePath().GetPath();
+
+ llvm::CachePruningPolicy pruning_policy =
+ D
[Lldb-commits] [lldb] [lldb][SymbolFileDWARF] Ignore Declaration when searching through UniqueDWARFASTTypeList in C++ (PR #120809)
ZequanWu wrote: > This particular example was fixed by > https://github.com/llvm/llvm-project/pull/120569 but this still feels a > little inconsistent. I'm not entirely sure we can get into that situation > after that patch anymore. So the only test-case I could come up with was the > unit-test which calls MapDeclDIEToDefDIE directly. That sounds still possible to happens without this change: parse decl die -> complete its type -> update the map with def die and non-zeroed def die -> parse a second decl die for the same type -> without this change, it will fail to find the existing type in the map because zeroed declaration. https://github.com/llvm/llvm-project/pull/120809 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][SymbolFileDWARF] Ignore Declaration when searching through UniqueDWARFASTTypeList in C++ (PR #120809)
https://github.com/ZequanWu approved this pull request. LGTM, thanks for fixing it. https://github.com/llvm/llvm-project/pull/120809 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][SymbolFileDWARF] Ignore Declaration when searching through UniqueDWARFASTTypeList in C++ (PR #120809)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Michael Buch (Michael137)
Changes
This addresses an issue encountered when investigating
https://github.com/llvm/llvm-project/pull/120569.
In `ParseTypeFromDWARF`, we insert the parsed type into `UniqueDWARFASTTypeMap`
using the typename and `Declaration` obtained with
`GetUniqueTypeNameAndDeclaration`. For C++ `GetUniqueTypeNameAndDeclaration`
will zero the `Declaration` info (presumably because forward declaration may
not have it, and for C++, ODR means we can rely on fully qualified typenames
for uniqueing). When we then called `CompleteType`, we would first
`MapDeclDIEToDefDIE`, updating the `UniqueDWARFASTType` we inserted previously
with the `Declaration` of the definition DIE (without zeroing it). We would
then call into `ParseTypeFromDWARF` for the same type again, and search the
`UniqueDWARFASTTypeMap`. But we do the search using a zeroed declaration we get
from `GetUniqueTypeNameAndDeclaration`.
This particular example was fixed by
https://github.com/llvm/llvm-project/pull/120569 but this still feels a little
inconsistent. I'm not entirely sure we can get into that situation after that
patch anymore. So the only test-case I could come up with was the unit-test
which calls `MapDeclDIEToDefDIE` directly.
---
Full diff: https://github.com/llvm/llvm-project/pull/120809.diff
2 Files Affected:
- (modified) lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
(+33-10)
- (modified) lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
(+143)
``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
index 3d201e96f92c3c..b598768b6e49f9 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
@@ -7,8 +7,10 @@
//===--===//
#include "UniqueDWARFASTType.h"
+#include "SymbolFileDWARF.h"
#include "lldb/Core/Declaration.h"
+#include "lldb/Target/Language.h"
using namespace lldb_private::dwarf;
using namespace lldb_private::plugin::dwarf;
@@ -18,6 +20,33 @@ static bool IsStructOrClassTag(llvm::dwarf::Tag Tag) {
Tag == llvm::dwarf::Tag::DW_TAG_structure_type;
}
+static bool IsSizeAndDeclarationMatching(UniqueDWARFASTType const &udt,
+ DWARFDIE const &die,
+ const lldb_private::Declaration &decl,
+ const int32_t byte_size,
+ bool is_forward_declaration) {
+
+ // If they are not both definition DIEs or both declaration DIEs, then
+ // don't check for byte size and declaration location, because declaration
+ // DIEs usually don't have those info.
+ if (udt.m_is_forward_declaration != is_forward_declaration)
+return true;
+
+ if (udt.m_byte_size > 0 && byte_size > 0 && udt.m_byte_size != byte_size)
+return false;
+
+ // For C++, we match the behaviour of
+ // DWARFASTParserClang::GetUniqueTypeNameAndDeclaration. We rely on the
+ // one-definition-rule: for a given fully qualified name there exists only
one
+ // definition, and there should only be one entry for such name, so ignore
+ // location of where it was declared vs. defined.
+ if (lldb_private::Language::LanguageIsCPlusPlus(
+ SymbolFileDWARF::GetLanguage(*die.GetCU(
+return true;
+
+ return udt.m_declaration == decl;
+}
+
UniqueDWARFASTType *UniqueDWARFASTTypeList::Find(
const DWARFDIE &die, const lldb_private::Declaration &decl,
const int32_t byte_size, bool is_forward_declaration) {
@@ -25,17 +54,11 @@ UniqueDWARFASTType *UniqueDWARFASTTypeList::Find(
// Make sure the tags match
if (udt.m_die.Tag() == die.Tag() || (IsStructOrClassTag(udt.m_die.Tag()) &&
IsStructOrClassTag(die.Tag( {
- // If they are not both definition DIEs or both declaration DIEs, then
- // don't check for byte size and declaration location, because
declaration
- // DIEs usually don't have those info.
- bool matching_size_declaration =
- udt.m_is_forward_declaration != is_forward_declaration
- ? true
- : (udt.m_byte_size < 0 || byte_size < 0 ||
- udt.m_byte_size == byte_size) &&
-udt.m_declaration == decl;
- if (!matching_size_declaration)
+
+ if (!IsSizeAndDeclarationMatching(udt, die, decl, byte_size,
+is_forward_declaration))
continue;
+
// The type has the same name, and was defined on the same file and
// line. Now verify all of the parent DIEs match.
DWARFDIE parent_arg_die = die.GetParent();
diff --git a/lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
b/lldb/unittests/SymbolFile/
[Lldb-commits] [lldb] [lldb][SymbolFileDWARF] Ignore Declaration when searching through UniqueDWARFASTTypeList in C++ (PR #120809)
Michael137 wrote: If we don't want to introduce language specifics into `UniqueDWARFASTTypeList`, we could just not update `MapDeclDIEToDefDIE` with the `Declaration` info of the definition when we're dealing with C++. On the other hand, assuming the debuggee doesn't violate ODR might be a bit too strong of an assumption for more complex setups. https://github.com/llvm/llvm-project/pull/120809 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expose structured errors in SBError (PR #120784)
slydiman wrote: The test `lldb-api::TestExprDiagnostics.py` is broken after this patch. https://lab.llvm.org/buildbot/#/builders/195/builds/2683 https://github.com/llvm/llvm-project/pull/120784 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][SymbolFileDWARF] Ignore Declaration when searching through UniqueDWARFASTTypeList in C++ (PR #120809)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/120809
This addresses an issue encountered when investigating
https://github.com/llvm/llvm-project/pull/120569.
In `ParseTypeFromDWARF`, we insert the parsed type into `UniqueDWARFASTTypeMap`
using the typename and `Declaration` obtained with
`GetUniqueTypeNameAndDeclaration`. For C++ `GetUniqueTypeNameAndDeclaration`
will zero the `Declaration` info (presumably because forward declaration may
not have it, and for C++, ODR means we can rely on fully qualified typenames
for uniqueing). When we then called `CompleteType`, we would first
`MapDeclDIEToDefDIE`, updating the `UniqueDWARFASTType` we inserted previously
with the `Declaration` of the definition DIE (without zeroing it). We would
then call into `ParseTypeFromDWARF` for the same type again, and search the
`UniqueDWARFASTTypeMap`. But we do the search using a zeroed declaration we get
from `GetUniqueTypeNameAndDeclaration`.
This particular example was fixed by
https://github.com/llvm/llvm-project/pull/120569 but this still feels a little
inconsistent. I'm not entirely sure we can get into that situation after that
patch anymore. So the only test-case I could come up with was the unit-test
which calls `MapDeclDIEToDefDIE` directly.
>From 010f1492f3050eb851a10954a3143f56e2dd3df5 Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Fri, 20 Dec 2024 23:37:49 +
Subject: [PATCH] [lldb][SymbolFileDWARF] Ignore Declaration when searching
through UniqueDWARFASTTypeList in C++
---
.../SymbolFile/DWARF/UniqueDWARFASTType.cpp | 43 --
.../DWARF/DWARFASTParserClangTests.cpp| 143 ++
2 files changed, 176 insertions(+), 10 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
index 3d201e96f92c3c..b598768b6e49f9 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/UniqueDWARFASTType.cpp
@@ -7,8 +7,10 @@
//===--===//
#include "UniqueDWARFASTType.h"
+#include "SymbolFileDWARF.h"
#include "lldb/Core/Declaration.h"
+#include "lldb/Target/Language.h"
using namespace lldb_private::dwarf;
using namespace lldb_private::plugin::dwarf;
@@ -18,6 +20,33 @@ static bool IsStructOrClassTag(llvm::dwarf::Tag Tag) {
Tag == llvm::dwarf::Tag::DW_TAG_structure_type;
}
+static bool IsSizeAndDeclarationMatching(UniqueDWARFASTType const &udt,
+ DWARFDIE const &die,
+ const lldb_private::Declaration &decl,
+ const int32_t byte_size,
+ bool is_forward_declaration) {
+
+ // If they are not both definition DIEs or both declaration DIEs, then
+ // don't check for byte size and declaration location, because declaration
+ // DIEs usually don't have those info.
+ if (udt.m_is_forward_declaration != is_forward_declaration)
+return true;
+
+ if (udt.m_byte_size > 0 && byte_size > 0 && udt.m_byte_size != byte_size)
+return false;
+
+ // For C++, we match the behaviour of
+ // DWARFASTParserClang::GetUniqueTypeNameAndDeclaration. We rely on the
+ // one-definition-rule: for a given fully qualified name there exists only
one
+ // definition, and there should only be one entry for such name, so ignore
+ // location of where it was declared vs. defined.
+ if (lldb_private::Language::LanguageIsCPlusPlus(
+ SymbolFileDWARF::GetLanguage(*die.GetCU(
+return true;
+
+ return udt.m_declaration == decl;
+}
+
UniqueDWARFASTType *UniqueDWARFASTTypeList::Find(
const DWARFDIE &die, const lldb_private::Declaration &decl,
const int32_t byte_size, bool is_forward_declaration) {
@@ -25,17 +54,11 @@ UniqueDWARFASTType *UniqueDWARFASTTypeList::Find(
// Make sure the tags match
if (udt.m_die.Tag() == die.Tag() || (IsStructOrClassTag(udt.m_die.Tag()) &&
IsStructOrClassTag(die.Tag( {
- // If they are not both definition DIEs or both declaration DIEs, then
- // don't check for byte size and declaration location, because
declaration
- // DIEs usually don't have those info.
- bool matching_size_declaration =
- udt.m_is_forward_declaration != is_forward_declaration
- ? true
- : (udt.m_byte_size < 0 || byte_size < 0 ||
- udt.m_byte_size == byte_size) &&
-udt.m_declaration == decl;
- if (!matching_size_declaration)
+
+ if (!IsSizeAndDeclarationMatching(udt, die, decl, byte_size,
+is_forward_declaration))
continue;
+
// The type has the same name, and was defined on the same file and
// line. Now verify all of the parent DIEs m
[Lldb-commits] [lldb] [lldb] Expose structured errors in SBError (PR #120784)
https://github.com/adrian-prantl updated
https://github.com/llvm/llvm-project/pull/120784
>From 88f6c19a87e82f1cc0c589029d8eb288ec53eaba Mon Sep 17 00:00:00 2001
From: Adrian Prantl
Date: Fri, 20 Dec 2024 10:35:47 -0800
Subject: [PATCH] [lldb] Expose structured errors in SBError
Building on top of previous work that exposed expression diagnostics
via SBCommandReturnObject, this patch generalizes the support to
expose any SBError as machine-readable structured data. One use-case
of this is to allow IDEs to better visualize expression diagnostics.
rdar://139997604
---
lldb/include/lldb/API/SBError.h | 5 ++
lldb/include/lldb/API/SBStructuredData.h | 1 +
.../lldb/Utility/DiagnosticsRendering.h | 14 +++-
lldb/include/lldb/Utility/Status.h| 6 ++
lldb/source/API/SBError.cpp | 14
.../Interpreter/CommandReturnObject.cpp | 52 +
lldb/source/Utility/DiagnosticsRendering.cpp | 40 ++
lldb/source/Utility/Status.cpp| 24 ++
.../diagnostics/TestExprDiagnostics.py| 75 ---
.../API/commands/frame/var/TestFrameVar.py| 16 +++-
10 files changed, 162 insertions(+), 85 deletions(-)
diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h
index 9f55f92131c06e..58b45ebd793af5 100644
--- a/lldb/include/lldb/API/SBError.h
+++ b/lldb/include/lldb/API/SBError.h
@@ -44,8 +44,13 @@ class LLDB_API SBError {
bool Success() const;
+ /// Get the error code.
uint32_t GetError() const;
+ /// Get the error in machine-readable form. Particularly useful for
+ /// compiler diagnostics.
+ SBStructuredData GetErrorData() const;
+
lldb::ErrorType GetType() const;
void SetError(uint32_t err, lldb::ErrorType type);
diff --git a/lldb/include/lldb/API/SBStructuredData.h
b/lldb/include/lldb/API/SBStructuredData.h
index c0d214a7374c65..f96e169f236edd 100644
--- a/lldb/include/lldb/API/SBStructuredData.h
+++ b/lldb/include/lldb/API/SBStructuredData.h
@@ -115,6 +115,7 @@ class SBStructuredData {
friend class SBLaunchInfo;
friend class SBDebugger;
friend class SBFrame;
+ friend class SBError;
friend class SBTarget;
friend class SBProcess;
friend class SBThread;
diff --git a/lldb/include/lldb/Utility/DiagnosticsRendering.h
b/lldb/include/lldb/Utility/DiagnosticsRendering.h
index 33aded602e3a77..e5f35c4611c20a 100644
--- a/lldb/include/lldb/Utility/DiagnosticsRendering.h
+++ b/lldb/include/lldb/Utility/DiagnosticsRendering.h
@@ -57,6 +57,13 @@ struct DiagnosticDetail {
std::string rendered;
};
+StructuredData::ObjectSP Serialize(llvm::ArrayRef details);
+
+void RenderDiagnosticDetails(Stream &stream,
+ std::optional offset_in_command,
+ bool show_inline,
+ llvm::ArrayRef details);
+
class DiagnosticError
: public llvm::ErrorInfo {
public:
@@ -64,12 +71,11 @@ class DiagnosticError
DiagnosticError(std::error_code ec) : ErrorInfo(ec) {}
lldb::ErrorType GetErrorType() const override;
virtual llvm::ArrayRef GetDetails() const = 0;
+ StructuredData::ObjectSP GetErrorData() const override {
+return Serialize(GetDetails());
+ }
static char ID;
};
-void RenderDiagnosticDetails(Stream &stream,
- std::optional offset_in_command,
- bool show_inline,
- llvm::ArrayRef details);
} // namespace lldb_private
#endif
diff --git a/lldb/include/lldb/Utility/Status.h
b/lldb/include/lldb/Utility/Status.h
index b8993dff3bb18a..681a56b7a155e1 100644
--- a/lldb/include/lldb/Utility/Status.h
+++ b/lldb/include/lldb/Utility/Status.h
@@ -10,6 +10,7 @@
#define LLDB_UTILITY_STATUS_H
#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/StructuredData.h"
#include "lldb/lldb-defines.h"
#include "lldb/lldb-enumerations.h"
#include "llvm/ADT/StringRef.h"
@@ -38,6 +39,7 @@ class CloneableError
CloneableError() : ErrorInfo() {}
virtual std::unique_ptr Clone() const = 0;
virtual lldb::ErrorType GetErrorType() const = 0;
+ virtual StructuredData::ObjectSP GetErrorData() const = 0;
static char ID;
};
@@ -49,6 +51,7 @@ class CloneableECError
std::error_code convertToErrorCode() const override { return EC; }
void log(llvm::raw_ostream &OS) const override { OS << EC.message(); }
lldb::ErrorType GetErrorType() const override;
+ virtual StructuredData::ObjectSP GetErrorData() const override;
static char ID;
protected:
@@ -183,6 +186,9 @@ class Status {
/// NULL otherwise.
const char *AsCString(const char *default_error_str = "unknown error") const;
+ /// Get the error in machine-readable form.
+ StructuredData::ObjectSP GetErrorData() const;
+
/// Clear the object state.
///
/// Reverts the state of this object to contain a generic success value and
diff --git a/lldb/source/API/SBError.cpp b/lldb/source/API/SBError.cpp
in
[Lldb-commits] [lldb] [lldb][Docs] Add equivalents of GDB's "skip" to command map (PR #120740)
kastiglione wrote: for anyone interested: I override `s` to take an optional argument, so that it behaves like `sif`. In my .lldbinit: ``` command unalias s command regex s s/^$/step/ s/(.+)/sif %1/ ``` https://github.com/llvm/llvm-project/pull/120740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][SymbolFileDWARF] Share GetDIEToType between SymbolFiles of a SymbolFileDWARFDebugMap (PR #120569)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/120569
>From 9aa49efdcde5887e8de6bcd6cfbb08c0a499e24b Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 19 Dec 2024 12:25:19 +
Subject: [PATCH 1/5] [lldb][SymbolFileDWARF] CompleteType: Lookup type in the
declaration DIE's SymbolFile
The problem here manifests as follows:
1. We are stopped in main.o, so the first `ParseTypeFromDWARF`
on `FooImpl` gets called on `main.o`'s SymbolFile. This
adds a mapping from *declaration die* -> `TypeSP` into `main.o`'s
`GetDIEToType` map.
2. We then `CompleteType(FooImpl)`. Depending on the order of
entries in the debug-map, this might call `CompleteType` on `lib.o`'s
SymbolFile. In which case, `GetDIEToType().lookup(decl_die)` will
return a `nullptr`. This is already a bit iffy because surrounding code
used to assume we don't call `CompleteTypeFromDWARF` with a `nullptr` `Type*`.
This used to be more of an issue when `CompleteRecordType` actually made use of
the `Type*` parameter (see https://github.com/llvm/llvm-project/pull/120456).
3. While in `CompleteTypeFromDWARF`, we call `ParseTypeFromDWARF` again.
This will parse the member function `FooImpl::Create` and its return
type which is a typedef to `FooImpl*`. But now we're inside `lib.o`'s
SymbolFile,
so we call it on the definition DIE. In step (2) we just inserted a `nullptr`
into
`GetDIEToType` for the definition DIE, so we trivially return a `nullptr` from
`ParseTypeFromDWARF`. Instead of reporting back this parse failure to
the user LLDB trucks on and marks `FooImpl::Ref` to be `void*`.
This test-case will trigger an assert in `TypeSystemClang::VerifyDecl`
even if we just `frame var` (but only in debug-builds). In release
builds where this function is a no-op, we'll create an incorrect Clang
AST node for the `Ref` typedef.
---
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 4 ++-
.../lang/cpp/typedef-to-outer-fwd/Makefile| 3 ++
.../TestTypedefToOuterFwd.py | 32 +++
.../API/lang/cpp/typedef-to-outer-fwd/lib.cpp | 15 +
.../API/lang/cpp/typedef-to-outer-fwd/lib.h | 10 ++
.../lang/cpp/typedef-to-outer-fwd/main.cpp| 8 +
6 files changed, 71 insertions(+), 1 deletion(-)
create mode 100644 lldb/test/API/lang/cpp/typedef-to-outer-fwd/Makefile
create mode 100644
lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py
create mode 100644 lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.cpp
create mode 100644 lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.h
create mode 100644 lldb/test/API/lang/cpp/typedef-to-outer-fwd/main.cpp
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 68e50902d641a2..936a44865b4a7e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1593,7 +1593,9 @@ bool SymbolFileDWARF::CompleteType(CompilerType
&compiler_type) {
DWARFASTParser *dwarf_ast = GetDWARFParser(*def_die.GetCU());
if (!dwarf_ast)
return false;
- Type *type = GetDIEToType().lookup(decl_die.GetDIE());
+ Type *type = decl_die.GetDWARF()->GetDIEToType().lookup(decl_die.GetDIE());
+ assert (type);
+
if (decl_die != def_die) {
GetDIEToType()[def_die.GetDIE()] = type;
DWARFASTParserClang *ast_parser =
diff --git a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/Makefile
b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/Makefile
new file mode 100644
index 00..d9db5666f9b6cd
--- /dev/null
+++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := lib.cpp main.cpp
+
+include Makefile.rules
diff --git
a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py
b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py
new file mode 100644
index 00..ad26d503c545b2
--- /dev/null
+++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py
@@ -0,0 +1,32 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCaseTypedefToOuterFwd(TestBase):
+'''
+We are stopped in main.o, which only sees a forward declaration
+of FooImpl. We then try to get the FooImpl::Ref typedef (whose
+definition is in lib.o). Make sure we correctly resolve this
+typedef.
+'''
+def test(self):
+self.build()
+(_, _, thread, _) = lldbutil.run_to_source_breakpoint(
+self, "return", lldb.SBFileSpec("main.cpp")
+)
+
+foo = thread.frames[0].FindVariable('foo')
+self.assertSuccess(foo.GetError(), "Found foo")
+
+foo_type = foo.GetType()
+self.assertTrue(foo_type)
+
+impl = foo_type.GetPointeeType()
+self.assertTrue(impl)
+
+ref = impl.FindDirectNestedType('Ref')
+self.assertTrue(ref)
+
+self.assertEq
[Lldb-commits] [lldb] [lldb] Expose structured errors in SBError (PR #120784)
https://github.com/JDevlieghere edited https://github.com/llvm/llvm-project/pull/120784 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expose structured errors in SBError (PR #120784)
@@ -183,6 +186,9 @@ class Status {
/// NULL otherwise.
const char *AsCString(const char *default_error_str = "unknown error") const;
+ /// Get the error in machine-readable form.
+ StructuredData::ObjectSP GetErrorData() const;
JDevlieghere wrote:
We have prior art of calling these methods `GetAsStructuredData`. Let's use
that for consistency, unless there's a good reason not to.
https://github.com/llvm/llvm-project/pull/120784
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expose structured errors in SBError (PR #120784)
adrian-prantl wrote: > LGTM. nit: I'd have called the API `SBError::GetStructuredError`. I have no strong feelings about this — I chose GetErrorData, because that's what `SBStructuredData SBCommandReturnObject::GetErrorData()` is called. https://github.com/llvm/llvm-project/pull/120784 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)
https://github.com/kuilpd updated
https://github.com/llvm/llvm-project/pull/115005
>From 5290832b802d98b9d293b6910c0837911ec490c4 Mon Sep 17 00:00:00 2001
From: Ilia Kuklin
Date: Mon, 4 Nov 2024 14:33:45 +0500
Subject: [PATCH 1/6] [lldb] Analyze enum promotion type during parsing
---
clang/include/clang/AST/Decl.h| 2 +-
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 95 ++-
.../TypeSystem/Clang/TypeSystemClang.cpp | 29 +-
3 files changed, 100 insertions(+), 26 deletions(-)
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index 8c39ef3d5a9fa6..a78e6c92abb22b 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -3891,6 +3891,7 @@ class EnumDecl : public TagDecl {
void setInstantiationOfMemberEnum(ASTContext &C, EnumDecl *ED,
TemplateSpecializationKind TSK);
+public:
/// Sets the width in bits required to store all the
/// non-negative enumerators of this enum.
void setNumPositiveBits(unsigned Num) {
@@ -3902,7 +3903,6 @@ class EnumDecl : public TagDecl {
/// negative enumerators of this enum. (see getNumNegativeBits)
void setNumNegativeBits(unsigned Num) { EnumDeclBits.NumNegativeBits = Num; }
-public:
/// True if this tag declaration is a scoped enumeration. Only
/// possible in C++11 mode.
void setScoped(bool Scoped = true) { EnumDeclBits.IsScoped = Scoped; }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index a30d898a93cc4d..a21607c2020161 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2248,6 +2248,8 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
return 0;
size_t enumerators_added = 0;
+ unsigned NumNegativeBits = 0;
+ unsigned NumPositiveBits = 0;
for (DWARFDIE die : parent_die.children()) {
const dw_tag_t tag = die.Tag();
@@ -2299,11 +2301,102 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
}
if (name && name[0] && got_value) {
- m_ast.AddEnumerationValueToEnumerationType(
+ auto ECD = m_ast.AddEnumerationValueToEnumerationType(
clang_type, decl, name, enum_value, enumerator_byte_size * 8);
++enumerators_added;
+
+ llvm::APSInt InitVal = ECD->getInitVal();
+ // Keep track of the size of positive and negative values.
+ if (InitVal.isUnsigned() || InitVal.isNonNegative()) {
+// If the enumerator is zero that should still be counted as a positive
+// bit since we need a bit to store the value zero.
+unsigned ActiveBits = InitVal.getActiveBits();
+NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u});
+ } else {
+NumNegativeBits =
+std::max(NumNegativeBits, (unsigned)InitVal.getSignificantBits());
+ }
}
}
+
+ /// The following code follows the same logic as in Sema::ActOnEnumBody
+ /// clang/lib/Sema/SemaDecl.cpp
+ // If we have an empty set of enumerators we still need one bit.
+ // From [dcl.enum]p8
+ // If the enumerator-list is empty, the values of the enumeration are as if
+ // the enumeration had a single enumerator with value 0
+ if (!NumPositiveBits && !NumNegativeBits)
+NumPositiveBits = 1;
+
+ clang::QualType qual_type(ClangUtil::GetQualType(clang_type));
+ clang::EnumDecl *enum_decl = qual_type->getAs()->getDecl();
+ enum_decl->setNumPositiveBits(NumPositiveBits);
+ enum_decl->setNumNegativeBits(NumNegativeBits);
+
+ // C++0x N3000 [conv.prom]p3:
+ // An rvalue of an unscoped enumeration type whose underlying
+ // type is not fixed can be converted to an rvalue of the first
+ // of the following types that can represent all the values of
+ // the enumeration: int, unsigned int, long int, unsigned long
+ // int, long long int, or unsigned long long int.
+ // C99 6.4.4.3p2:
+ // An identifier declared as an enumeration constant has type int.
+ // The C99 rule is modified by C23.
+ clang::QualType BestPromotionType;
+ unsigned BestWidth;
+
+ auto &Context = m_ast.getASTContext();
+ unsigned LongWidth = Context.getTargetInfo().getLongWidth();
+ unsigned IntWidth = Context.getTargetInfo().getIntWidth();
+ unsigned CharWidth = Context.getTargetInfo().getCharWidth();
+ unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
+
+ bool is_cpp = Language::LanguageIsCPlusPlus(
+ SymbolFileDWARF::GetLanguage(*parent_die.GetCU()));
+
+ if (NumNegativeBits) {
+// If there is a negative value, figure out the smallest integer type (of
+// int/long/longlong) that fits.
+if (NumNegativeBits <= CharWidth && NumPositiveBits < CharWidth) {
+ BestWidth = CharWidth;
+} else if (NumNegativeBits <= ShortWidth && NumPositiveBits < ShortWidth) {
+ BestWidth = ShortWidth;
+} else if (NumNegativeBits <= IntWidth &
[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)
@@ -2299,11 +2301,103 @@ size_t DWARFASTParserClang::ParseChildEnumerators(
}
if (name && name[0] && got_value) {
- m_ast.AddEnumerationValueToEnumerationType(
+ auto ECD = m_ast.AddEnumerationValueToEnumerationType(
clang_type, decl, name, enum_value, enumerator_byte_size * 8);
++enumerators_added;
+
+ llvm::APSInt InitVal = ECD->getInitVal();
+ // Keep track of the size of positive and negative values.
+ if (InitVal.isUnsigned() || InitVal.isNonNegative()) {
+// If the enumerator is zero that should still be counted as a positive
+// bit since we need a bit to store the value zero.
+unsigned ActiveBits = InitVal.getActiveBits();
+NumPositiveBits = std::max({NumPositiveBits, ActiveBits, 1u});
+ } else {
+NumNegativeBits =
+std::max(NumNegativeBits, (unsigned)InitVal.getSignificantBits());
+ }
}
}
+
+ /// The following code follows the same logic as in Sema::ActOnEnumBody
+ /// clang/lib/Sema/SemaDecl.cpp
+ // If we have an empty set of enumerators we still need one bit.
+ // From [dcl.enum]p8
+ // If the enumerator-list is empty, the values of the enumeration are as if
+ // the enumeration had a single enumerator with value 0
+ if (!NumPositiveBits && !NumNegativeBits)
+NumPositiveBits = 1;
+
+ clang::EnumDecl *enum_decl =
+ ClangUtil::GetQualType(clang_type)->getAs()->getDecl();
+ enum_decl->setNumPositiveBits(NumPositiveBits);
+ enum_decl->setNumNegativeBits(NumNegativeBits);
+
+ // C++0x N3000 [conv.prom]p3:
+ // An rvalue of an unscoped enumeration type whose underlying
+ // type is not fixed can be converted to an rvalue of the first
+ // of the following types that can represent all the values of
+ // the enumeration: int, unsigned int, long int, unsigned long
+ // int, long long int, or unsigned long long int.
+ // C99 6.4.4.3p2:
+ // An identifier declared as an enumeration constant has type int.
+ // The C99 rule is modified by C23.
+ clang::QualType BestPromotionType;
+ unsigned BestWidth;
+
+ auto &Context = m_ast.getASTContext();
+ unsigned LongWidth = Context.getTargetInfo().getLongWidth();
+ unsigned IntWidth = Context.getTargetInfo().getIntWidth();
+ unsigned CharWidth = Context.getTargetInfo().getCharWidth();
+ unsigned ShortWidth = Context.getTargetInfo().getShortWidth();
+
+ bool is_cpp = Language::LanguageIsCPlusPlus(
+ SymbolFileDWARF::GetLanguage(*parent_die.GetCU()));
+
+ if (NumNegativeBits) {
kuilpd wrote:
If I understood you correctly, I created this separate function in clang's
SemaDecl.cpp, and then called it from both LLDB and Sema itself. Seems to be
working, but it needed a lot of arguments, since when called from LLDB Sema
doesn't have any information stored at all, so ASTContext, EnumDecl and other
arguments have to come from LLDB, and it also returns 3 values.
This looks clunky because of it, any suggestions how to make it better?
https://github.com/llvm/llvm-project/pull/115005
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expose structured errors in SBError (PR #120784)
https://github.com/adrian-prantl closed https://github.com/llvm/llvm-project/pull/120784 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] d9cc37f - [lldb] Expose structured errors in SBError (#120784)
Author: Adrian Prantl
Date: 2024-12-20T13:02:54-08:00
New Revision: d9cc37fea7b02954079ca59e8f7f28cffacc7e9e
URL:
https://github.com/llvm/llvm-project/commit/d9cc37fea7b02954079ca59e8f7f28cffacc7e9e
DIFF:
https://github.com/llvm/llvm-project/commit/d9cc37fea7b02954079ca59e8f7f28cffacc7e9e.diff
LOG: [lldb] Expose structured errors in SBError (#120784)
Building on top of previous work that exposed expression diagnostics via
SBCommandReturnObject, this patch generalizes the support to expose any
SBError as machine-readable structured data. One use-case of this is to
allow IDEs to better visualize expression diagnostics.
rdar://139997604
Added:
Modified:
lldb/include/lldb/API/SBError.h
lldb/include/lldb/API/SBStructuredData.h
lldb/include/lldb/Utility/DiagnosticsRendering.h
lldb/include/lldb/Utility/Status.h
lldb/source/API/SBError.cpp
lldb/source/Interpreter/CommandReturnObject.cpp
lldb/source/Utility/DiagnosticsRendering.cpp
lldb/source/Utility/Status.cpp
lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
lldb/test/API/commands/frame/var/TestFrameVar.py
Removed:
diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h
index 9f55f92131c06e..58b45ebd793af5 100644
--- a/lldb/include/lldb/API/SBError.h
+++ b/lldb/include/lldb/API/SBError.h
@@ -44,8 +44,13 @@ class LLDB_API SBError {
bool Success() const;
+ /// Get the error code.
uint32_t GetError() const;
+ /// Get the error in machine-readable form. Particularly useful for
+ /// compiler diagnostics.
+ SBStructuredData GetErrorData() const;
+
lldb::ErrorType GetType() const;
void SetError(uint32_t err, lldb::ErrorType type);
diff --git a/lldb/include/lldb/API/SBStructuredData.h
b/lldb/include/lldb/API/SBStructuredData.h
index c0d214a7374c65..f96e169f236edd 100644
--- a/lldb/include/lldb/API/SBStructuredData.h
+++ b/lldb/include/lldb/API/SBStructuredData.h
@@ -115,6 +115,7 @@ class SBStructuredData {
friend class SBLaunchInfo;
friend class SBDebugger;
friend class SBFrame;
+ friend class SBError;
friend class SBTarget;
friend class SBProcess;
friend class SBThread;
diff --git a/lldb/include/lldb/Utility/DiagnosticsRendering.h
b/lldb/include/lldb/Utility/DiagnosticsRendering.h
index 33aded602e3a77..dd33d671c24a5f 100644
--- a/lldb/include/lldb/Utility/DiagnosticsRendering.h
+++ b/lldb/include/lldb/Utility/DiagnosticsRendering.h
@@ -57,6 +57,13 @@ struct DiagnosticDetail {
std::string rendered;
};
+StructuredData::ObjectSP Serialize(llvm::ArrayRef details);
+
+void RenderDiagnosticDetails(Stream &stream,
+ std::optional offset_in_command,
+ bool show_inline,
+ llvm::ArrayRef details);
+
class DiagnosticError
: public llvm::ErrorInfo {
public:
@@ -64,12 +71,11 @@ class DiagnosticError
DiagnosticError(std::error_code ec) : ErrorInfo(ec) {}
lldb::ErrorType GetErrorType() const override;
virtual llvm::ArrayRef GetDetails() const = 0;
+ StructuredData::ObjectSP GetAsStructuredData() const override {
+return Serialize(GetDetails());
+ }
static char ID;
};
-void RenderDiagnosticDetails(Stream &stream,
- std::optional offset_in_command,
- bool show_inline,
- llvm::ArrayRef details);
} // namespace lldb_private
#endif
diff --git a/lldb/include/lldb/Utility/Status.h
b/lldb/include/lldb/Utility/Status.h
index b8993dff3bb18a..212282cca1f3ed 100644
--- a/lldb/include/lldb/Utility/Status.h
+++ b/lldb/include/lldb/Utility/Status.h
@@ -10,6 +10,7 @@
#define LLDB_UTILITY_STATUS_H
#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/StructuredData.h"
#include "lldb/lldb-defines.h"
#include "lldb/lldb-enumerations.h"
#include "llvm/ADT/StringRef.h"
@@ -38,6 +39,7 @@ class CloneableError
CloneableError() : ErrorInfo() {}
virtual std::unique_ptr Clone() const = 0;
virtual lldb::ErrorType GetErrorType() const = 0;
+ virtual StructuredData::ObjectSP GetAsStructuredData() const = 0;
static char ID;
};
@@ -49,6 +51,7 @@ class CloneableECError
std::error_code convertToErrorCode() const override { return EC; }
void log(llvm::raw_ostream &OS) const override { OS << EC.message(); }
lldb::ErrorType GetErrorType() const override;
+ virtual StructuredData::ObjectSP GetAsStructuredData() const override;
static char ID;
protected:
@@ -183,6 +186,9 @@ class Status {
/// NULL otherwise.
const char *AsCString(const char *default_error_str = "unknown error") const;
+ /// Get the error in machine-readable form.
+ StructuredData::ObjectSP GetAsStructuredData() const;
+
/// Clear the object state.
///
/// Reverts the state of this object to contain a generic success value and
diff --git a/lldb/source
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)
https://github.com/ashgti updated
https://github.com/llvm/llvm-project/pull/120457
>From 4131b8c5af83a219efb2513a1ac90e8b9877d2ef Mon Sep 17 00:00:00 2001
From: John Harrison
Date: Tue, 17 Dec 2024 17:45:34 -0800
Subject: [PATCH 1/5] [lldb-dap] Ensure the IO forwarding threads are managed
by the DAP object lifecycle.
This moves the ownership of the threads that forward stdout/stderr to the DAP
object itself to ensure that the threads are joined and that the forwarding is
cleaned up when the DAP connection is disconnected.
This is part of a larger refactor to allow lldb-dap to run in a listening mode
and accept multiple connections.
---
lldb/tools/lldb-dap/CMakeLists.txt | 6 +-
lldb/tools/lldb-dap/DAP.cpp | 114 ++
lldb/tools/lldb-dap/DAP.h| 67 +++
lldb/tools/lldb-dap/IOStream.cpp | 8 +-
lldb/tools/lldb-dap/IOStream.h | 14 ++-
lldb/tools/lldb-dap/OutputRedirector.cpp | 63 --
lldb/tools/lldb-dap/OutputRedirector.h | 26
lldb/tools/lldb-dap/lldb-dap.cpp | 146 +--
8 files changed, 231 insertions(+), 213 deletions(-)
delete mode 100644 lldb/tools/lldb-dap/OutputRedirector.cpp
delete mode 100644 lldb/tools/lldb-dap/OutputRedirector.h
diff --git a/lldb/tools/lldb-dap/CMakeLists.txt
b/lldb/tools/lldb-dap/CMakeLists.txt
index d68098bf7b3266..00906e8ac10904 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -1,7 +1,3 @@
-if ( CMAKE_SYSTEM_NAME MATCHES "Windows" OR CMAKE_SYSTEM_NAME MATCHES "NetBSD"
)
- list(APPEND extra_libs lldbHost)
-endif ()
-
if (HAVE_LIBPTHREAD)
list(APPEND extra_libs pthread)
endif ()
@@ -32,7 +28,6 @@ add_lldb_tool(lldb-dap
IOStream.cpp
JSONUtils.cpp
LLDBUtils.cpp
- OutputRedirector.cpp
ProgressEvent.cpp
RunInTerminal.cpp
SourceBreakpoint.cpp
@@ -42,6 +37,7 @@ add_lldb_tool(lldb-dap
LINK_LIBS
liblldb
+lldbHost
${extra_libs}
LINK_COMPONENTS
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 35250d9eef608a..4e0de6fa3a4e90 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -6,34 +6,53 @@
//
//===--===//
-#include
-#include
-#include
-#include
-
#include "DAP.h"
#include "JSONUtils.h"
#include "LLDBUtils.h"
+#include "lldb/API/SBBreakpoint.h"
#include "lldb/API/SBCommandInterpreter.h"
#include "lldb/API/SBLanguageRuntime.h"
#include "lldb/API/SBListener.h"
#include "lldb/API/SBStream.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBProcess.h"
+#include "lldb/Utility/Status.h"
+#include "lldb/lldb-defines.h"
+#include "lldb/lldb-enumerations.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Twine.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/raw_ostream.h"
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
#if defined(_WIN32)
#define NOMINMAX
#include
#include
#include
+#else
+#include
#endif
using namespace lldb_dap;
namespace lldb_dap {
-DAP::DAP(llvm::StringRef path, ReplMode repl_mode)
-: debug_adaptor_path(path), broadcaster("lldb-dap"),
+DAP::DAP(llvm::StringRef path, std::optional &log,
+ ReplMode repl_mode, StreamDescriptor input, StreamDescriptor output)
+: debug_adaptor_path(path), log(log), input(std::move(input)),
+ output(std::move(output)), broadcaster("lldb-dap"),
exception_breakpoints(), focus_tid(LLDB_INVALID_THREAD_ID),
stop_at_entry(false), is_attach(false),
enable_auto_variable_summaries(false),
@@ -43,21 +62,7 @@ DAP::DAP(llvm::StringRef path, ReplMode repl_mode)
configuration_done_sent(false), waiting_for_run_in_terminal(false),
progress_event_reporter(
[&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }),
- reverse_request_seq(0), repl_mode(repl_mode) {
- const char *log_file_path = getenv("LLDBDAP_LOG");
-#if defined(_WIN32)
- // Windows opens stdout and stdin in text mode which converts \n to 13,10
- // while the value is just 10 on Darwin/Linux. Setting the file mode to
binary
- // fixes this.
- int result = _setmode(fileno(stdout), _O_BINARY);
- assert(result);
- result = _setmode(fileno(stdin), _O_BINARY);
- UNUSED_IF_ASSERT_DISABLED(result);
- assert(result);
-#endif
- if (log_file_path)
-log.reset(new std::ofstream(log_file_path));
-}
+ reverse_request_seq(0), repl_mode(repl_mode) {}
DAP::~DAP() = default;
@@ -173,6 +178,63 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const
lldb::break_id_t bp_id) {
return nullptr;
}
+llvm::Error DAP::ConfigureIO(std::optional overrideOut,
+ std::optional overrideErr) {
+
[Lldb-commits] [lldb] [llvm] Debuginfod cache use index cache settings and include real file name. (PR #120814)
llvmbot wrote:
@llvm/pr-subscribers-debuginfo
Author: None (GeorgeHuyubo)
Changes
This PR include two changes:
1. Change debuginfod cache file name to include origin file name, the new file
name would be something like:
llvmcache-13267c5f5d2e3df472c133c8efa45fb3331ef1ea-liblzma.so.5.2.2.debuginfo.dwp
So it will provide more information in image list instead of a plain
llvmcache-123
2. Switch debuginfod cache to use lldb index cache settings. Currently we don't
have proper settings for setting the cache path or the cache expiration time
for debuginfod cache. We want to use the lldb index cache settings, as they
make sense to be in the same place and have the same TTL.
---
Full diff: https://github.com/llvm/llvm-project/pull/120814.diff
4 Files Affected:
- (modified)
lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
(+34-7)
- (modified) llvm/include/llvm/Debuginfod/Debuginfod.h (+3-1)
- (modified) llvm/lib/Debuginfod/Debuginfod.cpp (+20-9)
- (modified) llvm/unittests/Debuginfod/DebuginfodTests.cpp (+2-1)
``diff
diff --git
a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
index 2cd7244902..f459825a61a562 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
@@ -8,6 +8,7 @@
#include "SymbolLocatorDebuginfod.h"
+#include "lldb/Core/DataFileCache.h"
#include "lldb/Core/PluginManager.h"
#include "lldb/Interpreter/OptionValueString.h"
#include "lldb/Utility/Args.h"
@@ -141,6 +142,25 @@ SymbolLocator *SymbolLocatorDebuginfod::CreateInstance() {
return new SymbolLocatorDebuginfod();
}
+static llvm::StringRef getFileName(const ModuleSpec &module_spec,
+ std::string url_path) {
+ // Check if the URL path requests an executable file or a symbol file
+ bool is_executable = url_path.find("debuginfo") == std::string::npos;
+ if (is_executable) {
+return module_spec.GetFileSpec().GetFilename().GetStringRef();
+ }
+ llvm::StringRef symbol_file =
+ module_spec.GetSymbolFileSpec().GetFilename().GetStringRef();
+ // Remove llvmcache- prefix and hash, keep origin file name
+ if (symbol_file.starts_with("llvmcache-")) {
+size_t pos = symbol_file.rfind('-');
+if (pos != llvm::StringRef::npos) {
+ symbol_file = symbol_file.substr(pos + 1);
+}
+ }
+ return symbol_file;
+}
+
static std::optional
GetFileForModule(const ModuleSpec &module_spec,
std::function UrlBuilder)
{
@@ -154,11 +174,14 @@ GetFileForModule(const ModuleSpec &module_spec,
// Grab LLDB's Debuginfod overrides from the
// plugin.symbol-locator.debuginfod.* settings.
PluginProperties &plugin_props = GetGlobalPluginProperties();
- llvm::Expected cache_path_or_err = plugin_props.GetCachePath();
- // A cache location is *required*.
- if (!cache_path_or_err)
-return {};
- std::string cache_path = *cache_path_or_err;
+ // Grab the lldb index cache settings from the global module list properties.
+ ModuleListProperties &properties =
+ ModuleList::GetGlobalModuleListProperties();
+ std::string cache_path = properties.GetLLDBIndexCachePath().GetPath();
+
+ llvm::CachePruningPolicy pruning_policy =
+ DataFileCache::GetLLDBIndexCachePolicy();
+
llvm::SmallVector debuginfod_urls =
llvm::getDefaultDebuginfodUrls();
std::chrono::milliseconds timeout = plugin_props.GetTimeout();
@@ -166,9 +189,13 @@ GetFileForModule(const ModuleSpec &module_spec,
// We're ready to ask the Debuginfod library to find our file.
llvm::object::BuildID build_id(module_uuid.GetBytes());
std::string url_path = UrlBuilder(build_id);
- std::string cache_key = llvm::getDebuginfodCacheKey(url_path);
+ llvm::StringRef file_name = getFileName(module_spec, url_path);
+ std::string cache_file_name = llvm::toHex(build_id, true);
+ if (!file_name.empty()) {
+cache_file_name += "-" + file_name.str();
+ }
llvm::Expected result = llvm::getCachedOrDownloadArtifact(
- cache_key, url_path, cache_path, debuginfod_urls, timeout);
+ cache_file_name, url_path, cache_path, debuginfod_urls, timeout,
pruning_policy);
if (result)
return FileSpec(*result);
diff --git a/llvm/include/llvm/Debuginfod/Debuginfod.h
b/llvm/include/llvm/Debuginfod/Debuginfod.h
index 99fe15ad859794..aebcf31cd48227 100644
--- a/llvm/include/llvm/Debuginfod/Debuginfod.h
+++ b/llvm/include/llvm/Debuginfod/Debuginfod.h
@@ -25,6 +25,7 @@
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Object/BuildID.h"
+#include "llvm/Support/CachePruning.h"
#include "llvm/Support/Error.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Mutex.h"
@@ -95,7 +96,8 @@ Expected getCachedOrDownloadArtifact(StringRef
UniqueKey,
/// found, uses the UniqueKey for the local cache f
[Lldb-commits] [lldb] [llvm] Debuginfod cache use index cache settings and include real file name. (PR #120814)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff c1e7e4500c6e3b921f5e0cda8ba8d8d66e086db6 58134726a34790fa23db0fe18e3cb4cc178a389b --extensions cpp,h -- lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp llvm/include/llvm/Debuginfod/Debuginfod.h llvm/lib/Debuginfod/Debuginfod.cpp llvm/unittests/Debuginfod/DebuginfodTests.cpp `` View the diff from clang-format here. ``diff diff --git a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp index f459825a61..00b4c9a45b 100644 --- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp +++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp @@ -195,7 +195,8 @@ GetFileForModule(const ModuleSpec &module_spec, cache_file_name += "-" + file_name.str(); } llvm::Expected result = llvm::getCachedOrDownloadArtifact( - cache_file_name, url_path, cache_path, debuginfod_urls, timeout, pruning_policy); + cache_file_name, url_path, cache_path, debuginfod_urls, timeout, + pruning_policy); if (result) return FileSpec(*result); `` https://github.com/llvm/llvm-project/pull/120814 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Debuginfod cache use index cache settings and include real file name. (PR #120814)
https://github.com/GeorgeHuyubo converted_to_draft https://github.com/llvm/llvm-project/pull/120814 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Debuginfod cache use index cache settings and include real file name. (PR #120814)
https://github.com/GeorgeHuyubo updated
https://github.com/llvm/llvm-project/pull/120814
>From 6923737d728191816567e7874a01c5dfce68bfde Mon Sep 17 00:00:00 2001
From: George Hu
Date: Fri, 20 Dec 2024 15:20:00 -0800
Subject: [PATCH 1/2] [lldb] Change debuginfod cache file name to include
origin file name
---
.../Debuginfod/SymbolLocatorDebuginfod.cpp| 27 +--
1 file changed, 25 insertions(+), 2 deletions(-)
diff --git
a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
index 2cd7244902..c103c98d20ac27 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
@@ -141,6 +141,25 @@ SymbolLocator *SymbolLocatorDebuginfod::CreateInstance() {
return new SymbolLocatorDebuginfod();
}
+static llvm::StringRef getFileName(const ModuleSpec &module_spec,
+ std::string url_path) {
+ // Check if the URL path requests an executable file or a symbol file
+ bool is_executable = url_path.find("debuginfo") == std::string::npos;
+ if (is_executable) {
+return module_spec.GetFileSpec().GetFilename().GetStringRef();
+ }
+ llvm::StringRef symbol_file =
+ module_spec.GetSymbolFileSpec().GetFilename().GetStringRef();
+ // Remove llvmcache- prefix and hash, keep origin file name
+ if (symbol_file.starts_with("llvmcache-")) {
+size_t pos = symbol_file.rfind('-');
+if (pos != llvm::StringRef::npos) {
+ symbol_file = symbol_file.substr(pos + 1);
+}
+ }
+ return symbol_file;
+}
+
static std::optional
GetFileForModule(const ModuleSpec &module_spec,
std::function UrlBuilder)
{
@@ -166,9 +185,13 @@ GetFileForModule(const ModuleSpec &module_spec,
// We're ready to ask the Debuginfod library to find our file.
llvm::object::BuildID build_id(module_uuid.GetBytes());
std::string url_path = UrlBuilder(build_id);
- std::string cache_key = llvm::getDebuginfodCacheKey(url_path);
+ llvm::StringRef file_name = getFileName(module_spec, url_path);
+ std::string cache_file_name = llvm::toHex(build_id, true);
+ if (!file_name.empty()) {
+cache_file_name += "-" + file_name.str();
+ }
llvm::Expected result = llvm::getCachedOrDownloadArtifact(
- cache_key, url_path, cache_path, debuginfod_urls, timeout);
+ cache_file_name, url_path, cache_path, debuginfod_urls, timeout);
if (result)
return FileSpec(*result);
>From 7b808e73aa0193c8a42eae8f2420a803f424bee1 Mon Sep 17 00:00:00 2001
From: George Hu
Date: Fri, 20 Dec 2024 16:37:50 -0800
Subject: [PATCH 2/2] [lldb] Switch debuginfod cache to use lldb index cache
settings
---
.../Debuginfod/SymbolLocatorDebuginfod.cpp| 17 +++
llvm/include/llvm/Debuginfod/Debuginfod.h | 4 ++-
llvm/lib/Debuginfod/Debuginfod.cpp| 29 +--
llvm/unittests/Debuginfod/DebuginfodTests.cpp | 3 +-
4 files changed, 36 insertions(+), 17 deletions(-)
diff --git
a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
index c103c98d20ac27..00b4c9a45b5a76 100644
--- a/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
+++ b/lldb/source/Plugins/SymbolLocator/Debuginfod/SymbolLocatorDebuginfod.cpp
@@ -8,6 +8,7 @@
#include "SymbolLocatorDebuginfod.h"
+#include "lldb/Core/DataFileCache.h"
#include "lldb/Core/PluginManager.h"
#include "lldb/Interpreter/OptionValueString.h"
#include "lldb/Utility/Args.h"
@@ -173,11 +174,14 @@ GetFileForModule(const ModuleSpec &module_spec,
// Grab LLDB's Debuginfod overrides from the
// plugin.symbol-locator.debuginfod.* settings.
PluginProperties &plugin_props = GetGlobalPluginProperties();
- llvm::Expected cache_path_or_err = plugin_props.GetCachePath();
- // A cache location is *required*.
- if (!cache_path_or_err)
-return {};
- std::string cache_path = *cache_path_or_err;
+ // Grab the lldb index cache settings from the global module list properties.
+ ModuleListProperties &properties =
+ ModuleList::GetGlobalModuleListProperties();
+ std::string cache_path = properties.GetLLDBIndexCachePath().GetPath();
+
+ llvm::CachePruningPolicy pruning_policy =
+ DataFileCache::GetLLDBIndexCachePolicy();
+
llvm::SmallVector debuginfod_urls =
llvm::getDefaultDebuginfodUrls();
std::chrono::milliseconds timeout = plugin_props.GetTimeout();
@@ -191,7 +195,8 @@ GetFileForModule(const ModuleSpec &module_spec,
cache_file_name += "-" + file_name.str();
}
llvm::Expected result = llvm::getCachedOrDownloadArtifact(
- cache_file_name, url_path, cache_path, debuginfod_urls, timeout);
+ cache_file_name, url_path, cache_path, debuginfod_urls, timeout,
+ pruning_policy);
if (result)
return FileSpec(*result);
diff --git
[Lldb-commits] [lldb] [llvm] [lldb][telemetry] Implement LLDB Telemetry (part 1) (PR #119716)
https://github.com/labath commented: @JDevlieghere, it looks like the llvm part of this is going to land imminently, so maybe this is a good time to take another look at this. In particular, I expect you'll want to say something about the use of the string "lldb" in identifier names. https://github.com/llvm/llvm-project/pull/119716 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb][telemetry] Implement LLDB Telemetry (part 1) (PR #119716)
@@ -0,0 +1,95 @@
+
+//===-- Telemetry.cpp
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "lldb/Core/Telemetry.h"
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "lldb/Core/Debugger.h"
+#include "lldb/Target/Statistics.h"
+#include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/UUID.h"
+#include "lldb/Version/Version.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/RandomNumberGenerator.h"
+#include "llvm/Telemetry/Telemetry.h"
+
+namespace lldb_private {
+
+using ::llvm::Error;
+using ::llvm::telemetry::Destination;
+using ::llvm::telemetry::TelemetryInfo;
+
+static unsigned long long ToNanosec(const SteadyTimePoint Point) {
+ return nanoseconds(Point.value().time_since_epoch()).count();
+}
+
+void LldbBaseTelemetryInfo::serialize(Serializer &serializer) const {
+ serializer.write("entry_kind", getKind());
+ serializer.write("session_id", SessionId);
+ serializer.write("start_time", ToNanosec(stats.start));
+ if (stats.end.has_value())
+serializer.write("end_time", ToNanosec(stats.end.value()));
+ if (exit_desc.has_value()) {
+serializer.write("exit_code", exit_desc->exit_code);
+serializer.write("exit_msg", exit_desc->description);
+ }
+}
+
+static std::string MakeUUID(lldb_private::Debugger *debugger) {
+ std::string ret;
+ 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.
+ret = std::to_string(
+ std::chrono::steady_clock::now().time_since_epoch().count()) +
+ "_" + std::to_string(debugger->GetID());
+ } else {
+ret = lldb_private::UUID(random_bytes).GetAsString();
+ }
+
+ return ret;
+}
+
+TelemetryManager::TelemetryManager(
+std::unique_ptr config,
+lldb_private::Debugger *debugger)
+: m_config(std::move(config)), m_debugger(debugger),
+ m_session_uuid(MakeUUID(debugger)) {}
+
+llvm::Error TelemetryManager::dispatch(TelemetryInfo *entry) {
+ entry->SessionId = m_session_uuid;
+
+ llvm::Error defferedErrs = llvm::Error::success();
+ for (auto &destination : m_destinations) {
+if (auto err = destination->receiveEntry(entry))
+ deferredErrs = llvm::joinErrors(std::move(deferredErrs), std::move(err));
labath wrote:
```suggestion
deferredErrs = llvm::joinErrors(std::move(deferredErrs),
destination->receiveEntry(entry));
```
https://github.com/llvm/llvm-project/pull/119716
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb][telemetry] Implement LLDB Telemetry (part 1) (PR #119716)
@@ -0,0 +1,95 @@ + labath wrote: delete empty line https://github.com/llvm/llvm-project/pull/119716 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb][telemetry] Implement LLDB Telemetry (part 1) (PR #119716)
@@ -0,0 +1,95 @@
+
+//===-- Telemetry.cpp
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "lldb/Core/Telemetry.h"
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "lldb/Core/Debugger.h"
+#include "lldb/Target/Statistics.h"
+#include "lldb/Utility/ConstString.h"
+#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/UUID.h"
+#include "lldb/Version/Version.h"
+#include "lldb/lldb-enumerations.h"
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/RandomNumberGenerator.h"
+#include "llvm/Telemetry/Telemetry.h"
+
+namespace lldb_private {
+
+using ::llvm::Error;
+using ::llvm::telemetry::Destination;
+using ::llvm::telemetry::TelemetryInfo;
+
+static unsigned long long ToNanosec(const SteadyTimePoint Point) {
labath wrote:
`unsigned long long` is also an unusual type for the number of nanoseconds. The
reason for implementing ~all integral types in the generic implementation was
so that you can use ~any type here, and things will just work. I'd use
`uint64_t` here, but you can also go with `std::chrono::nanoseconds::rep` if
you want to be fancy.
https://github.com/llvm/llvm-project/pull/119716
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb][telemetry] Implement LLDB Telemetry (part 1) (PR #119716)
@@ -0,0 +1,101 @@ +//===-- Telemetry.h --*- C++ +//-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// + +#ifndef LLDB_CORE_TELEMETRY_H +#define LLDB_CORE_TELEMETRY_H + +#include labath wrote: This isn't the order we [use in llvm](https://llvm.org/docs/CodingStandards.html#include-style). Just remove the blank lines and let clang-format order it for you. https://github.com/llvm/llvm-project/pull/119716 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb][telemetry] Implement LLDB Telemetry (part 1) (PR #119716)
@@ -0,0 +1,95 @@ + +//===-- Telemetry.cpp -===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===--===// +#include "lldb/Core/Telemetry.h" labath wrote: fix include order (remove blank lines and reformat) https://github.com/llvm/llvm-project/pull/119716 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb][telemetry] Implement LLDB Telemetry (part 1) (PR #119716)
@@ -0,0 +1,101 @@
+//===-- Telemetry.h --*- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_CORE_TELEMETRY_H
+#define LLDB_CORE_TELEMETRY_H
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "lldb/Core/StructuredDataImpl.h"
+#include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Utility/StructuredData.h"
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Telemetry/Telemetry.h"
+
+namespace lldb_private {
+
+using llvm::telemetry::Destination;
+using llvm::telemetry::KindType;
+using llvm::telemetry::Serializer;
+using llvm::telemetry::TelemetryInfo;
+
+struct LldbEntryKind : public ::llvm::telemetry::EntryKind {
+ static const KindType BaseInfo = 0b11000;
+};
+
+/// Defines a convenient type for timestamp of various events.
+/// This is used by the EventStats below.
+using SteadyTimePoint = std::chrono::time_point;
+
+/// Various time (and possibly memory) statistics of an event.
+struct EventStats {
+ // REQUIRED: Start time of an event
+ SteadyTimePoint start;
+ // OPTIONAL: End time of an event - may be empty if not meaningful.
+ std::optional end;
+ // TBD: could add some memory stats here too?
+
+ EventStats() = default;
+ EventStats(SteadyTimePoint start) : start(start) {}
+ EventStats(SteadyTimePoint start, SteadyTimePoint end)
+ : start(start), end(end) {}
+};
+
+/// Describes the exit signal of an event.
+struct ExitDescription {
+ int exit_code;
+ std::string description;
+};
+
+struct LldbBaseTelemetryInfo : public TelemetryInfo {
+ EventStats stats;
+
+ std::optional exit_desc;
+
+ // For dyn_cast, isa, etc operations.
+ KindType getKind() const override { return LldbEntryKind::BaseInfo; }
+
+ static bool classof(const TelemetryInfo *t) {
+// Subclasses of this is also acceptable.
+return (t->getKind() & LldbEntryKind::BaseInfo) == LldbEntryKind::BaseInfo;
+ }
+
+ void serialize(Serializer &serializer) const override;
+};
+
+/// The base Telemetry manager instance in LLDB
+/// This class declares additional instrumentation points
+/// applicable to LLDB.
+class TelemetryManager : public llvm::telemetry::Manager {
+public:
+ TelemetryManager(std::unique_ptr config,
+ Debugger *debugger);
+
+ llvm::Error dispatch(TelemetryInfo *entry) override;
+
+ void addDestination(std::unique_ptr destination) override;
+
+private:
+ std::unique_ptr m_config;
+ Debugger *m_debugger;
labath wrote:
I thought that we've (in our conversation at the dev meeting) concluded that
the telemetry manager should not be tied to a debugger (because some of the
pieces of code -- e.g. everything inside a Symbol/ObjectFile class -- does not
have access to debugger instance -- *by design*). Has that changed? If not,
then this member shouldn't be here (but it could be attached to event types
which *can* be tied to a specific debugger connection).
In the same breath, I want to make sure you're aware of the effort in [this
RFC](https://discourse.llvm.org/t/rfc-lldb-dap-update-server-mode-to-allow-multiple-connections/82770),
whose result will be that a single process can end up serving multiple DAP
connections (with one Debugger instance for each connection) -- which means
that some events may not be able to be tied to a specific DAP connection either.
I'm not really pushing for any particular solution, but I want to make sure you
make a conscious decision about this, as it can have big implications for the
rest of the implementation. It's basically a tradeoff. If you make the
telemetry instance specific to a debugger object, then you cannot send
telemetry from parts of the code which do not have access to a debugger. Or you
have to do something similar to the Progress events, which send progress
reports to *all* active debuggers. If you don't tie this to a debugger, then
you can send telemetry from ~anywhere, but well.. that telemetry will not be
tied to a debugger..
https://github.com/llvm/llvm-project/pull/119716
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb][telemetry] Implement LLDB Telemetry (part 1) (PR #119716)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/119716 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [lldb][telemetry] Implement LLDB Telemetry (part 1) (PR #119716)
@@ -108,6 +108,9 @@ endfunction(add_lldb_test_dependency) add_lldb_test_dependency(lldb) add_lldb_test_dependency(lldb-test) +# Enable Telemetry for testing. +target_compile_definitions(lldb PRIVATE -DTEST_TELEMETRY) + labath wrote: This doesn't do what you think it does. It builds the *lldb* binary with the macro definition *if* `LLDB_INCLUDE_TESTS` is defined (because that's the condition that guards cmake [entering this directory](https://github.com/llvm/llvm-project/blob/54665f5252695922dd000f311f82af717f1df0c6/lldb/CMakeLists.txt#L154)). (Or, to put it differently `target_compile_definitions` always defines a macro for the target it gets as an argument, regardless of where the macro is called from) https://github.com/llvm/llvm-project/pull/119716 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AIX] Introducing ALL_SOURCE macro into driver CMakeLists (PR #120607)
https://github.com/DavidSpickett approved this pull request. LGTM. Please correct the macro name in the title/description then I'll merge this. https://github.com/llvm/llvm-project/pull/120607 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][LoongArch] Add LSX and LASX register definitions and operations (PR #120664)
https://github.com/DavidSpickett commented: Please note in the PR description that this is for live processes and core files (at least it looks like it is). I would also like to see test cases for both scenarios. Are your vector registers ever absent? Because in AArch64's case we assume we always have Neon (which is these days not 100% true but is pretty safe for any Linux) but we have to check whether SVE exists. I don't know if you'll have to do the same thing here. That said, if the devices without them are unsupported from your point of view, I wouldn't object to you assuming the vector registers exist, or doing so for the time being. As long as you make that clear in the PR's description. The code looks fine, a lot of boilerplate :) I've been looking at reducing this for AArch64, so one day we might have a better, data driven way to add registers. Note: this is my last day at work this year, back second week of January. Once this is in, that completes https://github.com/llvm/llvm-project/issues/112693. So I would like to know what the state of the test suite on LoongArch is at that point. It would be good to release note the major improvements for lldb 20. https://github.com/llvm/llvm-project/pull/120664 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][LoongArch] Add LSX and LASX register definitions and operations (PR #120664)
@@ -48,6 +48,10 @@ bool RegisterContextCorePOSIX_loongarch64::ReadGPR() {
return true; }
bool RegisterContextCorePOSIX_loongarch64::ReadFPR() { return true; }
+bool RegisterContextCorePOSIX_loongarch64::ReadLSX() { return true; }
+
+bool RegisterContextCorePOSIX_loongarch64::ReadLASX() { return true; }
DavidSpickett wrote:
Do core files work with the code like this?
I thought you were just stubbing out the functions but I see that ReadGPR also
does this and I assume that already works. Surprising.
https://github.com/llvm/llvm-project/pull/120664
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][LoongArch] Add LSX and LASX register definitions and operations (PR #120664)
@@ -27,6 +27,14 @@ // struct iovec definition #include +#ifndef NT_LARCH_LSX DavidSpickett wrote: Our usual pattern is: ``` #ifndef THE_MACRO #define THE_MACRO the_value_it_is_in_the_latest_kernel_headers #endif ``` This allows you to build with older kernel headers. So I agree that the name used should match that in the Linux kernel. https://github.com/llvm/llvm-project/pull/120664 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB][LoongArch] Add LSX and LASX register definitions and operations (PR #120664)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/120664 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][SymbolFileDWARF] CompleteType: Lookup type in the declaration DIE's SymbolFile (PR #120569)
Michael137 wrote: > > As an alternative, we might be able to make the die-to-type map shared > > between all symbol files in a debug map (similar to how "unique dwarf ast > > type map" is shared). > > Yea I considered that, but I was concerned with the unforeseen complications > this might have. Particularly I'm not sure how the `TypeSP` ownership should > be managed. IIUC, a `TypeSP` is solely supposed to be owned by a single > `SymbolFile` (`MakeType` kind of binds the two together). But then if we > bundled `TypeSP`s from different modules into a single map on > `SymbolFileDWARFDebugMap`, could we run into issues if one of the > `SymbolFile`'s gets torn down (if that can happen)? Though I'm also not > certain that we _only_ store `TypeSP`s inside the `SymbolFileDWARF`. I don't > think there's anything asserting/enforcing this. On second thought, this does feel more consistent with how all the other bookkeeping structures already work. And re. the ownership, my current patch pretty much achieves the same thing (but possibly in a less apparent manner). I'm still mixing `Type*` into `GetDIEToType` of a different `SymbolFile`. And the `UniqueDWARFASTType` owns a `TypeSP`, and we are sharing those via a debug-map. So I'll go with this suggestion. Confirmed that this does fix the test attached test-case https://github.com/llvm/llvm-project/pull/120569 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AIX] Introducing _ALL_SOURCE macro into driver CMakeLists (PR #120607)
@@ -11,6 +11,11 @@ if(APPLE)
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS}
-Wl,-sectcreate,__TEXT,__info_plist,${CMAKE_CURRENT_BINARY_DIR}/lldb-Info.plist")
endif()
+if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
+ remove_definitions("-D_XOPEN_SOURCE=700")
labath wrote:
> It may be better to do a -U_XOPEN_SOURCE
.. which, at least according to my experiments, isn't possible it seems.
However, also according to my experiments, `remove_definitions` doesn't
actually remove macros defined by `add_compile_definitions` (only
`add_definitions`), which brings up another question: Is this actually needed?
Would it be sufficient to define _ALL_SOURCE as well?
https://github.com/llvm/llvm-project/pull/120607
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)
@@ -173,6 +178,63 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const
lldb::break_id_t bp_id) {
return nullptr;
}
+llvm::Error DAP::ConfigureIO(std::optional overrideOut,
+ std::optional overrideErr) {
+ auto *inull = lldb_private::FileSystem::Instance().Fopen(
+ lldb_private::FileSystem::DEV_NULL, "w");
+ in = lldb::SBFile(inull, true);
+
+ lldb_private::Status status;
+ status = pout.CreateNew(/*child_process_inherit=*/false);
+ if (status.Fail())
+return status.takeError();
+ status = perr.CreateNew(/*child_process_inherit=*/false);
+ if (status.Fail())
+return status.takeError();
+
+ if (overrideOut) {
+if (dup2(pout.GetWriteFileDescriptor(), fileno(*overrideOut)) == -1) {
+ return llvm::make_error(
+ llvm::errnoAsErrorCode(),
+ llvm::formatv("override fd=%d failed", fileno(*overrideOut))
+ .str()
+ .c_str());
+}
+ }
+
+ if (overrideErr) {
+if (dup2(perr.GetWriteFileDescriptor(), fileno(*overrideErr)) == -1) {
+ return llvm::make_error(
+ llvm::errnoAsErrorCode(),
+ llvm::formatv("override fd=%d failed", fileno(*overrideErr))
+ .str()
+ .c_str());
+}
+ }
+
+ auto forwarder = [&](lldb_private::Pipe &pipe, OutputType outputType) {
+char buffer[4098];
+size_t bytes_read;
+while (pipe.CanRead()) {
+ lldb_private::Status error = pipe.ReadWithTimeout(
+ &buffer, sizeof(buffer), std::chrono::seconds(1), bytes_read);
labath wrote:
If this really can be a read-until-EOF loop, then you could just make this a
blocking read (no timeout). *However*, I think this is not a good idea to do in
combination with duping the pipe to stdout/err (TIL that `dup2` is a thing on
windows), because in order to get an EOF you have to close *all* copies of that
FD. It looks like you don't do that, which is probably the cause of the
presubmit test failures, but even if you did (perhaps by restoring the original
FD, or /dev/null), I still wouldn't really like this approach because it's very
easy for stdout/err descriptors to be leaked into other processes (which might
end up outliving all lldb-dap connections).
https://github.com/llvm/llvm-project/pull/120457
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)
@@ -138,15 +140,20 @@ struct SendEventRequestHandler : public
lldb::SBCommandPluginInterface {
struct DAP {
llvm::StringRef debug_adaptor_path;
+ std::optional &log;
labath wrote:
This isn't that big of an issue but I'd still consider using a plain pointer
instead of a reference-to-optional. This way you're forcing the caller to store
the object in an `optional`, which I don't think is your goal here.
https://github.com/llvm/llvm-project/pull/120457
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)
@@ -1099,6 +1098,14 @@ void request_disconnect(DAP &dap, const
llvm::json::Object &request) {
dap.broadcaster.BroadcastEventByType(eBroadcastBitStopProgressThread);
dap.progress_event_thread.join();
}
+ if (dap.stdout_forward_thread.joinable()) {
+dap.pout.Close();
labath wrote:
This may be moot due to due other issue, but you should only close the write
end before joining the thread (the read end can be closed after joining, or
from within the thread). Closing the read descriptor while the thread is
running leads to a race with the `Read` call inside the thread.
https://github.com/llvm/llvm-project/pull/120457
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)
@@ -5027,43 +5019,77 @@ int main(int argc, char *argv[]) {
}
#endif
+ std::optional log = std::nullopt;
+ const char *log_file_path = getenv("LLDBDAP_LOG");
+ if (log_file_path)
+log.emplace(log_file_path);
+
// Initialize LLDB first before we do anything.
lldb::SBDebugger::Initialize();
// Terminate the debugger before the C++ destructor chain kicks in.
auto terminate_debugger =
llvm::make_scope_exit([] { lldb::SBDebugger::Terminate(); });
- DAP dap = DAP(program_path.str(), default_repl_mode);
-
- RegisterRequestCallbacks(dap);
-
- // stdout/stderr redirection to the IDE's console
- int new_stdout_fd = SetupStdoutStderrRedirection(dap);
-
+ StreamDescriptor input;
+ StreamDescriptor output;
+ std::optional redirectOut = std::nullopt;
+ std::optional redirectErr = std::nullopt;
if (portno != -1) {
printf("Listening on port %i...\n", portno);
-SOCKET socket_fd = AcceptConnection(dap, portno);
-if (socket_fd >= 0) {
- dap.input.descriptor = StreamDescriptor::from_socket(socket_fd, true);
- dap.output.descriptor = StreamDescriptor::from_socket(socket_fd, false);
-} else {
+SOCKET socket_fd = AcceptConnection(log, portno);
+if (socket_fd < 0)
return EXIT_FAILURE;
-}
+
+input = StreamDescriptor::from_socket(socket_fd, true);
+output = StreamDescriptor::from_socket(socket_fd, false);
} else {
-dap.input.descriptor = StreamDescriptor::from_file(fileno(stdin), false);
-dap.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, false);
+#if defined(_WIN32)
+// Windows opens stdout and stdin in text mode which converts \n to 13,10
+// while the value is just 10 on Darwin/Linux. Setting the file mode to
+// binary fixes this.
+int result = _setmode(fileno(stdout), _O_BINARY);
+assert(result);
+result = _setmode(fileno(stdin), _O_BINARY);
+UNUSED_IF_ASSERT_DISABLED(result);
+assert(result);
+#endif
+
+int stdout_fd = dup(fileno(stdout));
labath wrote:
```suggestion
int stdout_fd = dup(fileno(stdout));
#else
int stdout_fd = fcntl(fileno(stdout), F_DUPFD_CLOEXEC, 3);
#endif
```
I know this isn't your code, but I think it's a good idea to set this flag,
particularly as we're going to be doing (a lot) more things within the same
process.
https://github.com/llvm/llvm-project/pull/120457
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)
@@ -173,6 +178,63 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const
lldb::break_id_t bp_id) {
return nullptr;
}
+llvm::Error DAP::ConfigureIO(std::optional overrideOut,
+ std::optional overrideErr) {
+ auto *inull = lldb_private::FileSystem::Instance().Fopen(
+ lldb_private::FileSystem::DEV_NULL, "w");
+ in = lldb::SBFile(inull, true);
+
+ lldb_private::Status status;
+ status = pout.CreateNew(/*child_process_inherit=*/false);
+ if (status.Fail())
+return status.takeError();
+ status = perr.CreateNew(/*child_process_inherit=*/false);
+ if (status.Fail())
+return status.takeError();
+
+ if (overrideOut) {
+if (dup2(pout.GetWriteFileDescriptor(), fileno(*overrideOut)) == -1) {
+ return llvm::make_error(
+ llvm::errnoAsErrorCode(),
+ llvm::formatv("override fd=%d failed", fileno(*overrideOut))
+ .str()
+ .c_str());
+}
+ }
+
+ if (overrideErr) {
+if (dup2(perr.GetWriteFileDescriptor(), fileno(*overrideErr)) == -1) {
+ return llvm::make_error(
+ llvm::errnoAsErrorCode(),
+ llvm::formatv("override fd=%d failed", fileno(*overrideErr))
+ .str()
+ .c_str());
+}
+ }
+
+ auto forwarder = [&](lldb_private::Pipe &pipe, OutputType outputType) {
labath wrote:
don't use `[&]` in a lambda which outlives the enclosing function. List the
captures explicitly.
https://github.com/llvm/llvm-project/pull/120457
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)
@@ -5027,43 +5019,77 @@ int main(int argc, char *argv[]) {
}
#endif
+ std::optional log = std::nullopt;
+ const char *log_file_path = getenv("LLDBDAP_LOG");
+ if (log_file_path)
+log.emplace(log_file_path);
+
// Initialize LLDB first before we do anything.
lldb::SBDebugger::Initialize();
// Terminate the debugger before the C++ destructor chain kicks in.
auto terminate_debugger =
llvm::make_scope_exit([] { lldb::SBDebugger::Terminate(); });
- DAP dap = DAP(program_path.str(), default_repl_mode);
-
- RegisterRequestCallbacks(dap);
-
- // stdout/stderr redirection to the IDE's console
- int new_stdout_fd = SetupStdoutStderrRedirection(dap);
-
+ StreamDescriptor input;
+ StreamDescriptor output;
+ std::optional redirectOut = std::nullopt;
+ std::optional redirectErr = std::nullopt;
if (portno != -1) {
printf("Listening on port %i...\n", portno);
-SOCKET socket_fd = AcceptConnection(dap, portno);
-if (socket_fd >= 0) {
- dap.input.descriptor = StreamDescriptor::from_socket(socket_fd, true);
- dap.output.descriptor = StreamDescriptor::from_socket(socket_fd, false);
-} else {
+SOCKET socket_fd = AcceptConnection(log, portno);
+if (socket_fd < 0)
return EXIT_FAILURE;
-}
+
+input = StreamDescriptor::from_socket(socket_fd, true);
+output = StreamDescriptor::from_socket(socket_fd, false);
} else {
-dap.input.descriptor = StreamDescriptor::from_file(fileno(stdin), false);
-dap.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, false);
+#if defined(_WIN32)
+// Windows opens stdout and stdin in text mode which converts \n to 13,10
+// while the value is just 10 on Darwin/Linux. Setting the file mode to
+// binary fixes this.
+int result = _setmode(fileno(stdout), _O_BINARY);
+assert(result);
+result = _setmode(fileno(stdin), _O_BINARY);
+UNUSED_IF_ASSERT_DISABLED(result);
+assert(result);
+#endif
+
+int stdout_fd = dup(fileno(stdout));
+if (stdout_fd == -1) {
+ llvm::errs() << "Failed to configure stdout redirect: "
+ << lldb_private::Status::FromErrno().takeError() << "\n";
+ return EXIT_FAILURE;
+}
+
+redirectOut = stdout;
+redirectErr = stderr;
-/// used only by TestVSCode_redirection_to_console.py
-if (getenv("LLDB_DAP_TEST_STDOUT_STDERR_REDIRECTION") != nullptr)
- redirection_test();
+input = StreamDescriptor::from_file(fileno(stdin), false);
+output = StreamDescriptor::from_file(stdout_fd, false);
}
+ DAP dap = DAP(program_path.str(), log, default_repl_mode, std::move(input),
labath wrote:
If I read this correctly, to get rid of all the references-to-optional, all
you'd need is to change this to `log?&*log:nullptr`
https://github.com/llvm/llvm-project/pull/120457
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)
@@ -52,16 +52,24 @@ struct StreamDescriptor {
struct InputStream {
StreamDescriptor descriptor;
- bool read_full(std::ofstream *log, size_t length, std::string &text);
+ explicit InputStream(StreamDescriptor descriptor)
+ : descriptor(std::move(descriptor)) {};
- bool read_line(std::ofstream *log, std::string &line);
+ bool read_full(std::optional &log, size_t length,
labath wrote:
The same goes for all of these, I don't think reference-to-optional improves
things here.
https://github.com/llvm/llvm-project/pull/120457
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)
@@ -173,6 +178,63 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const
lldb::break_id_t bp_id) {
return nullptr;
}
+llvm::Error DAP::ConfigureIO(std::optional overrideOut,
labath wrote:
optional is overkill in most of the cases (and confusing in the rest). Can
you use a null pointer to denote an empty value?
https://github.com/llvm/llvm-project/pull/120457
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Docs] Add equivalent of GDB's "skip" to command map (PR #120740)
kastiglione wrote: Should this also mention `sif`? When I want to skip one or more functions when stepping, I run `sif desiredFunction`. https://github.com/llvm/llvm-project/pull/120740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][SymbolFileDWARF] CompleteType: Lookup type in the declaration DIE's SymbolFile (PR #120569)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/120569
>From 9aa49efdcde5887e8de6bcd6cfbb08c0a499e24b Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Thu, 19 Dec 2024 12:25:19 +
Subject: [PATCH 1/5] [lldb][SymbolFileDWARF] CompleteType: Lookup type in the
declaration DIE's SymbolFile
The problem here manifests as follows:
1. We are stopped in main.o, so the first `ParseTypeFromDWARF`
on `FooImpl` gets called on `main.o`'s SymbolFile. This
adds a mapping from *declaration die* -> `TypeSP` into `main.o`'s
`GetDIEToType` map.
2. We then `CompleteType(FooImpl)`. Depending on the order of
entries in the debug-map, this might call `CompleteType` on `lib.o`'s
SymbolFile. In which case, `GetDIEToType().lookup(decl_die)` will
return a `nullptr`. This is already a bit iffy because surrounding code
used to assume we don't call `CompleteTypeFromDWARF` with a `nullptr` `Type*`.
This used to be more of an issue when `CompleteRecordType` actually made use of
the `Type*` parameter (see https://github.com/llvm/llvm-project/pull/120456).
3. While in `CompleteTypeFromDWARF`, we call `ParseTypeFromDWARF` again.
This will parse the member function `FooImpl::Create` and its return
type which is a typedef to `FooImpl*`. But now we're inside `lib.o`'s
SymbolFile,
so we call it on the definition DIE. In step (2) we just inserted a `nullptr`
into
`GetDIEToType` for the definition DIE, so we trivially return a `nullptr` from
`ParseTypeFromDWARF`. Instead of reporting back this parse failure to
the user LLDB trucks on and marks `FooImpl::Ref` to be `void*`.
This test-case will trigger an assert in `TypeSystemClang::VerifyDecl`
even if we just `frame var` (but only in debug-builds). In release
builds where this function is a no-op, we'll create an incorrect Clang
AST node for the `Ref` typedef.
---
.../SymbolFile/DWARF/SymbolFileDWARF.cpp | 4 ++-
.../lang/cpp/typedef-to-outer-fwd/Makefile| 3 ++
.../TestTypedefToOuterFwd.py | 32 +++
.../API/lang/cpp/typedef-to-outer-fwd/lib.cpp | 15 +
.../API/lang/cpp/typedef-to-outer-fwd/lib.h | 10 ++
.../lang/cpp/typedef-to-outer-fwd/main.cpp| 8 +
6 files changed, 71 insertions(+), 1 deletion(-)
create mode 100644 lldb/test/API/lang/cpp/typedef-to-outer-fwd/Makefile
create mode 100644
lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py
create mode 100644 lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.cpp
create mode 100644 lldb/test/API/lang/cpp/typedef-to-outer-fwd/lib.h
create mode 100644 lldb/test/API/lang/cpp/typedef-to-outer-fwd/main.cpp
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 68e50902d641a2..936a44865b4a7e 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1593,7 +1593,9 @@ bool SymbolFileDWARF::CompleteType(CompilerType
&compiler_type) {
DWARFASTParser *dwarf_ast = GetDWARFParser(*def_die.GetCU());
if (!dwarf_ast)
return false;
- Type *type = GetDIEToType().lookup(decl_die.GetDIE());
+ Type *type = decl_die.GetDWARF()->GetDIEToType().lookup(decl_die.GetDIE());
+ assert (type);
+
if (decl_die != def_die) {
GetDIEToType()[def_die.GetDIE()] = type;
DWARFASTParserClang *ast_parser =
diff --git a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/Makefile
b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/Makefile
new file mode 100644
index 00..d9db5666f9b6cd
--- /dev/null
+++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := lib.cpp main.cpp
+
+include Makefile.rules
diff --git
a/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py
b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py
new file mode 100644
index 00..ad26d503c545b2
--- /dev/null
+++ b/lldb/test/API/lang/cpp/typedef-to-outer-fwd/TestTypedefToOuterFwd.py
@@ -0,0 +1,32 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestCaseTypedefToOuterFwd(TestBase):
+'''
+We are stopped in main.o, which only sees a forward declaration
+of FooImpl. We then try to get the FooImpl::Ref typedef (whose
+definition is in lib.o). Make sure we correctly resolve this
+typedef.
+'''
+def test(self):
+self.build()
+(_, _, thread, _) = lldbutil.run_to_source_breakpoint(
+self, "return", lldb.SBFileSpec("main.cpp")
+)
+
+foo = thread.frames[0].FindVariable('foo')
+self.assertSuccess(foo.GetError(), "Found foo")
+
+foo_type = foo.GetType()
+self.assertTrue(foo_type)
+
+impl = foo_type.GetPointeeType()
+self.assertTrue(impl)
+
+ref = impl.FindDirectNestedType('Ref')
+self.assertTrue(ref)
+
+self.assertEq
[Lldb-commits] [lldb] [lldb][Docs] Add equivalents of GDB's "skip" to command map (PR #120740)
@@ -235,6 +235,23 @@ Do a source level single step in the currently selected thread (lldb) step (lldb) s +Ignore a function when doing a source level single step in +~~ + +.. code-block:: shell + + (gdb) skip abc + Function abc will be skipped when stepping. + +.. code-block:: shell + + (lldb) settings show target.process.thread.step-avoid-regexp + target.process.thread.step-avoid-regexp (regex) = ^std:: + (lldb) settings set target.process.thread.step-avoid-regexp (^std::)|(^abc) kastiglione wrote: `|` has the lowest precedence, I believe. It's less complex, and still valid, to show `^std::|^abc`. No grouping/capture needed. https://github.com/llvm/llvm-project/pull/120740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Docs] Add equivalents of GDB's "skip" to command map (PR #120740)
https://github.com/kastiglione edited https://github.com/llvm/llvm-project/pull/120740 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Fix address to read segment data (PR #120655)
https://github.com/GeorgeHuyubo edited https://github.com/llvm/llvm-project/pull/120655 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expose structured errors in SBError (PR #120784)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Adrian Prantl (adrian-prantl)
Changes
Building on top of previous work that exposed expression diagnostics via
SBCommandReturnObject, this patch generalizes the support to expose any SBError
as machine-readable structured data. One use-case of this is to allow IDEs to
better visualize expression diagnostics.
rdar://139997604
---
Full diff: https://github.com/llvm/llvm-project/pull/120784.diff
9 Files Affected:
- (modified) lldb/include/lldb/API/SBError.h (+5)
- (modified) lldb/include/lldb/API/SBStructuredData.h (+1)
- (modified) lldb/include/lldb/Utility/DiagnosticsRendering.h (+10-4)
- (modified) lldb/include/lldb/Utility/Status.h (+6)
- (modified) lldb/source/API/SBError.cpp (+14)
- (modified) lldb/source/Interpreter/CommandReturnObject.cpp (+1-51)
- (modified) lldb/source/Utility/DiagnosticsRendering.cpp (+40)
- (modified) lldb/source/Utility/Status.cpp (+24)
- (modified)
lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py (+47-28)
``diff
diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h
index 9f55f92131c06e..58b45ebd793af5 100644
--- a/lldb/include/lldb/API/SBError.h
+++ b/lldb/include/lldb/API/SBError.h
@@ -44,8 +44,13 @@ class LLDB_API SBError {
bool Success() const;
+ /// Get the error code.
uint32_t GetError() const;
+ /// Get the error in machine-readable form. Particularly useful for
+ /// compiler diagnostics.
+ SBStructuredData GetErrorData() const;
+
lldb::ErrorType GetType() const;
void SetError(uint32_t err, lldb::ErrorType type);
diff --git a/lldb/include/lldb/API/SBStructuredData.h
b/lldb/include/lldb/API/SBStructuredData.h
index c0d214a7374c65..f96e169f236edd 100644
--- a/lldb/include/lldb/API/SBStructuredData.h
+++ b/lldb/include/lldb/API/SBStructuredData.h
@@ -115,6 +115,7 @@ class SBStructuredData {
friend class SBLaunchInfo;
friend class SBDebugger;
friend class SBFrame;
+ friend class SBError;
friend class SBTarget;
friend class SBProcess;
friend class SBThread;
diff --git a/lldb/include/lldb/Utility/DiagnosticsRendering.h
b/lldb/include/lldb/Utility/DiagnosticsRendering.h
index 33aded602e3a77..e5f35c4611c20a 100644
--- a/lldb/include/lldb/Utility/DiagnosticsRendering.h
+++ b/lldb/include/lldb/Utility/DiagnosticsRendering.h
@@ -57,6 +57,13 @@ struct DiagnosticDetail {
std::string rendered;
};
+StructuredData::ObjectSP Serialize(llvm::ArrayRef details);
+
+void RenderDiagnosticDetails(Stream &stream,
+ std::optional offset_in_command,
+ bool show_inline,
+ llvm::ArrayRef details);
+
class DiagnosticError
: public llvm::ErrorInfo {
public:
@@ -64,12 +71,11 @@ class DiagnosticError
DiagnosticError(std::error_code ec) : ErrorInfo(ec) {}
lldb::ErrorType GetErrorType() const override;
virtual llvm::ArrayRef GetDetails() const = 0;
+ StructuredData::ObjectSP GetErrorData() const override {
+return Serialize(GetDetails());
+ }
static char ID;
};
-void RenderDiagnosticDetails(Stream &stream,
- std::optional offset_in_command,
- bool show_inline,
- llvm::ArrayRef details);
} // namespace lldb_private
#endif
diff --git a/lldb/include/lldb/Utility/Status.h
b/lldb/include/lldb/Utility/Status.h
index b8993dff3bb18a..681a56b7a155e1 100644
--- a/lldb/include/lldb/Utility/Status.h
+++ b/lldb/include/lldb/Utility/Status.h
@@ -10,6 +10,7 @@
#define LLDB_UTILITY_STATUS_H
#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/StructuredData.h"
#include "lldb/lldb-defines.h"
#include "lldb/lldb-enumerations.h"
#include "llvm/ADT/StringRef.h"
@@ -38,6 +39,7 @@ class CloneableError
CloneableError() : ErrorInfo() {}
virtual std::unique_ptr Clone() const = 0;
virtual lldb::ErrorType GetErrorType() const = 0;
+ virtual StructuredData::ObjectSP GetErrorData() const = 0;
static char ID;
};
@@ -49,6 +51,7 @@ class CloneableECError
std::error_code convertToErrorCode() const override { return EC; }
void log(llvm::raw_ostream &OS) const override { OS << EC.message(); }
lldb::ErrorType GetErrorType() const override;
+ virtual StructuredData::ObjectSP GetErrorData() const override;
static char ID;
protected:
@@ -183,6 +186,9 @@ class Status {
/// NULL otherwise.
const char *AsCString(const char *default_error_str = "unknown error") const;
+ /// Get the error in machine-readable form.
+ StructuredData::ObjectSP GetErrorData() const;
+
/// Clear the object state.
///
/// Reverts the state of this object to contain a generic success value and
diff --git a/lldb/source/API/SBError.cpp b/lldb/source/API/SBError.cpp
index 31964931649db3..8c9e15736d25b1 100644
--- a/lldb/source/API/SBError.cpp
+++ b/lldb/source/API/SBError.cpp
@@ -9,6 +9,8 @@
#include "lldb/API/SBError.h"
#include "U
[Lldb-commits] [lldb] [lldb] Expose structured errors in SBError (PR #120784)
https://github.com/adrian-prantl created
https://github.com/llvm/llvm-project/pull/120784
Building on top of previous work that exposed expression diagnostics via
SBCommandReturnObject, this patch generalizes the support to expose any SBError
as machine-readable structured data. One use-case of this is to allow IDEs to
better visualize expression diagnostics.
rdar://139997604
>From bfc77546ca58b09485951691e4a8d16f9c564d02 Mon Sep 17 00:00:00 2001
From: Adrian Prantl
Date: Fri, 20 Dec 2024 10:35:47 -0800
Subject: [PATCH] [lldb] Expose structured errors in SBError
Building on top of previous work that exposed expression diagnostics
via SBCommandReturnObject, this patch generalizes the support to
expose any SBError as machine-readable structured data. One use-case
of this is to allow IDEs to better visualize expression diagnostics.
rdar://139997604
---
lldb/include/lldb/API/SBError.h | 5 ++
lldb/include/lldb/API/SBStructuredData.h | 1 +
.../lldb/Utility/DiagnosticsRendering.h | 14 +++-
lldb/include/lldb/Utility/Status.h| 6 ++
lldb/source/API/SBError.cpp | 14
.../Interpreter/CommandReturnObject.cpp | 52 +
lldb/source/Utility/DiagnosticsRendering.cpp | 40 ++
lldb/source/Utility/Status.cpp| 24 ++
.../diagnostics/TestExprDiagnostics.py| 75 ---
9 files changed, 148 insertions(+), 83 deletions(-)
diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h
index 9f55f92131c06e..58b45ebd793af5 100644
--- a/lldb/include/lldb/API/SBError.h
+++ b/lldb/include/lldb/API/SBError.h
@@ -44,8 +44,13 @@ class LLDB_API SBError {
bool Success() const;
+ /// Get the error code.
uint32_t GetError() const;
+ /// Get the error in machine-readable form. Particularly useful for
+ /// compiler diagnostics.
+ SBStructuredData GetErrorData() const;
+
lldb::ErrorType GetType() const;
void SetError(uint32_t err, lldb::ErrorType type);
diff --git a/lldb/include/lldb/API/SBStructuredData.h
b/lldb/include/lldb/API/SBStructuredData.h
index c0d214a7374c65..f96e169f236edd 100644
--- a/lldb/include/lldb/API/SBStructuredData.h
+++ b/lldb/include/lldb/API/SBStructuredData.h
@@ -115,6 +115,7 @@ class SBStructuredData {
friend class SBLaunchInfo;
friend class SBDebugger;
friend class SBFrame;
+ friend class SBError;
friend class SBTarget;
friend class SBProcess;
friend class SBThread;
diff --git a/lldb/include/lldb/Utility/DiagnosticsRendering.h
b/lldb/include/lldb/Utility/DiagnosticsRendering.h
index 33aded602e3a77..e5f35c4611c20a 100644
--- a/lldb/include/lldb/Utility/DiagnosticsRendering.h
+++ b/lldb/include/lldb/Utility/DiagnosticsRendering.h
@@ -57,6 +57,13 @@ struct DiagnosticDetail {
std::string rendered;
};
+StructuredData::ObjectSP Serialize(llvm::ArrayRef details);
+
+void RenderDiagnosticDetails(Stream &stream,
+ std::optional offset_in_command,
+ bool show_inline,
+ llvm::ArrayRef details);
+
class DiagnosticError
: public llvm::ErrorInfo {
public:
@@ -64,12 +71,11 @@ class DiagnosticError
DiagnosticError(std::error_code ec) : ErrorInfo(ec) {}
lldb::ErrorType GetErrorType() const override;
virtual llvm::ArrayRef GetDetails() const = 0;
+ StructuredData::ObjectSP GetErrorData() const override {
+return Serialize(GetDetails());
+ }
static char ID;
};
-void RenderDiagnosticDetails(Stream &stream,
- std::optional offset_in_command,
- bool show_inline,
- llvm::ArrayRef details);
} // namespace lldb_private
#endif
diff --git a/lldb/include/lldb/Utility/Status.h
b/lldb/include/lldb/Utility/Status.h
index b8993dff3bb18a..681a56b7a155e1 100644
--- a/lldb/include/lldb/Utility/Status.h
+++ b/lldb/include/lldb/Utility/Status.h
@@ -10,6 +10,7 @@
#define LLDB_UTILITY_STATUS_H
#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/StructuredData.h"
#include "lldb/lldb-defines.h"
#include "lldb/lldb-enumerations.h"
#include "llvm/ADT/StringRef.h"
@@ -38,6 +39,7 @@ class CloneableError
CloneableError() : ErrorInfo() {}
virtual std::unique_ptr Clone() const = 0;
virtual lldb::ErrorType GetErrorType() const = 0;
+ virtual StructuredData::ObjectSP GetErrorData() const = 0;
static char ID;
};
@@ -49,6 +51,7 @@ class CloneableECError
std::error_code convertToErrorCode() const override { return EC; }
void log(llvm::raw_ostream &OS) const override { OS << EC.message(); }
lldb::ErrorType GetErrorType() const override;
+ virtual StructuredData::ObjectSP GetErrorData() const override;
static char ID;
protected:
@@ -183,6 +186,9 @@ class Status {
/// NULL otherwise.
const char *AsCString(const char *default_error_str = "unknown error") const;
+ /// Get the error in machine-readable form.
+ Str
[Lldb-commits] [lldb] [lldb] Expose structured errors in SBError (PR #120784)
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
8c0090030bf89df7e0dbe5827a83d52627b2c87f...88f6c19a87e82f1cc0c589029d8eb288ec53eaba
lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py
lldb/test/API/commands/frame/var/TestFrameVar.py
``
View the diff from darker here.
``diff
--- frame/var/TestFrameVar.py 2024-12-20 18:53:37.00 +
+++ frame/var/TestFrameVar.py 2024-12-20 18:56:55.557137 +
@@ -129,11 +129,10 @@
self.assertEqual(err_ty.GetIntegerValue(), lldb.eErrorTypeGeneric)
message = str(data.GetValueForKey("errors").GetItemAtIndex(0))
for s in error_strings:
self.assertIn(s, message)
-
@skipIfRemote
@skipUnlessDarwin
def test_darwin_dwarf_missing_obj(self):
"""
Test that if we build a binary with DWARF in .o files and we remove
``
https://github.com/llvm/llvm-project/pull/120784
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][SymbolFileDWARF] Share GetDIEToType between SymbolFiles of a SymbolFileDWARFDebugMap (PR #120569)
ZequanWu wrote: > Just tried this. Unfortunately [we remove the CompilerType from the > `GetForwardDeclCompilerTypeToDIE`](https://github.com/llvm/llvm-project/blob/d7ddc976d544528fe7f16882f5bec66c3b2a7884/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp#L1577) > map inside of `CompleteType`. So in the next iteration of the > `SymbolFileDWARFDebugMap::CompleteType` loop, it [won't ever call into > `SymbolFileDWARF::CompleteType` > again](https://github.com/llvm/llvm-project/blob/d7ddc976d544528fe7f16882f5bec66c3b2a7884/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp#L808) > :( > > I guess we could put it back on failure? Not sure if it's worth the potential > trouble We can move the the retrieval and null check of `Type*` before we remove it from `GetForwardDeclCompilerTypeToDIE()`, right? https://github.com/llvm/llvm-project/pull/120569 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][SymbolFileDWARF] Share GetDIEToType between SymbolFiles of a SymbolFileDWARFDebugMap (PR #120569)
ZequanWu wrote: > > > As an alternative, we might be able to make the die-to-type map shared > > > between all symbol files in a debug map (similar to how "unique dwarf ast > > > type map" is shared). > > > > > > Yea I considered that, but I was concerned with the unforeseen > > complications this might have. Particularly I'm not sure how the `TypeSP` > > ownership should be managed. IIUC, a `TypeSP` is solely supposed to be > > owned by a single `SymbolFile` (`MakeType` kind of binds the two together). > > But then if we bundled `TypeSP`s from different modules into a single map > > on `SymbolFileDWARFDebugMap`, could we run into issues if one of the > > `SymbolFile`'s gets torn down (if that can happen)? Though I'm also not > > certain that we _only_ store `TypeSP`s inside the `SymbolFileDWARF`. I > > don't think there's anything asserting/enforcing this. > > On second thought, this does feel more consistent with how all the other > bookkeeping structures already work. And re. the ownership, my current patch > pretty much achieves the same thing (but in a less apparent manner). I'm > still mixing `Type*` into `GetDIEToType` of a different `SymbolFile`. And the > `UniqueDWARFASTType` owns a `TypeSP`, and we are sharing those via a > debug-map already. So I'll go with this suggestion. Confirmed that this does > fix the test attached test-case Good to hear that works and making `TypeSP` shared among different `SymbolFile` make life easier. https://github.com/llvm/llvm-project/pull/120569 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Negate `is_signed` variable for argument `isUnsigned` in TypeSystemClang.cpp (PR #120794)
https://github.com/kuilpd created https://github.com/llvm/llvm-project/pull/120794 None >From b5cb9a262a5e1bdb19eb72e7e357c98e90fa9f4e Mon Sep 17 00:00:00 2001 From: Ilia Kuklin Date: Sat, 21 Dec 2024 02:04:35 +0500 Subject: [PATCH] Negate is_signed variable for argument isUnsigned --- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index 3ec25a2e8aa2df..06c04c992efc09 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -8534,7 +8534,7 @@ clang::EnumConstantDecl *TypeSystemClang::AddEnumerationValueToEnumerationType( bool is_signed = false; underlying_type.IsIntegerType(is_signed); - llvm::APSInt value(enum_value_bit_size, is_signed); + llvm::APSInt value(enum_value_bit_size, !is_signed); value = enum_value; return AddEnumerationValueToEnumerationType(enum_type, decl, name, value); ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Negate `is_signed` variable for argument `isUnsigned` in TypeSystemClang.cpp (PR #120794)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Ilia Kuklin (kuilpd) Changes --- Full diff: https://github.com/llvm/llvm-project/pull/120794.diff 1 Files Affected: - (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+1-1) ``diff diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp index 3ec25a2e8aa2df..06c04c992efc09 100644 --- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -8534,7 +8534,7 @@ clang::EnumConstantDecl *TypeSystemClang::AddEnumerationValueToEnumerationType( bool is_signed = false; underlying_type.IsIntegerType(is_signed); - llvm::APSInt value(enum_value_bit_size, is_signed); + llvm::APSInt value(enum_value_bit_size, !is_signed); value = enum_value; return AddEnumerationValueToEnumerationType(enum_type, decl, name, value); `` https://github.com/llvm/llvm-project/pull/120794 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Negate `is_signed` variable for argument `isUnsigned` in TypeSystemClang.cpp (PR #120794)
kuilpd wrote: This change doesn't really affect anything in lldb yet, but will be used and tested afterwards in #115005 https://github.com/llvm/llvm-project/pull/120794 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [clang] [lldb] [lldb] Analyze enum promotion type during parsing (PR #115005)
@@ -8544,7 +8524,8 @@ clang::EnumConstantDecl *TypeSystemClang::AddEnumerationValueToEnumerationType( bool is_signed = false; underlying_type.IsIntegerType(is_signed); - llvm::APSInt value(enum_value_bit_size, is_signed); + // APSInt constructor's sign argument is isUnsigned + llvm::APSInt value(enum_value_bit_size, !is_signed); kuilpd wrote: Made a PR #120794, once it's merged I will rebase this branch on main and run tests again. https://github.com/llvm/llvm-project/pull/115005 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] Revert "[LLDB] Add a target.launch-working-dir setting" (PR #114973)
keith wrote: did this one reland somewhere? https://github.com/llvm/llvm-project/pull/114973 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Make workaround for the Dynamic loader issue (PR #120166)
https://github.com/Jlalond updated
https://github.com/llvm/llvm-project/pull/120166
>From d5dfb15e1ae90ade9b25374eefc605cc36685a43 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Mon, 16 Dec 2024 16:04:01 -0800
Subject: [PATCH 1/2] Make workaround for the Dynamic loader issue
---
.../Minidump/MinidumpFileBuilder.cpp | 4 +-
.../Process/minidump/MinidumpParser.cpp | 25 ++-
.../Plugins/Process/minidump/MinidumpParser.h | 8 +-
.../Process/minidump/ProcessMinidump.cpp | 205 +-
.../Process/minidump/ProcessMinidump.h| 7 +-
llvm/include/llvm/BinaryFormat/Minidump.h | 4 +
6 files changed, 137 insertions(+), 116 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index bcac5edbc1a793..a4541f8bddf1a2 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -918,8 +918,8 @@ Status MinidumpFileBuilder::DumpHeader() const {
0u), // not used in most of the writers
header.TimeDateStamp =
static_cast(std::time(nullptr));
- header.Flags =
- static_cast(0u); // minidump normal flag
+ header.Flags = static_cast(
+ llvm::minidump::Header::LLDB_HEADER_FLAG);
Status error;
size_t bytes_written;
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index afc095ddbb2f91..6a328b3b841ed0 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -20,8 +20,8 @@
#include
#include
#include
-#include
#include
+#include
using namespace lldb_private;
using namespace minidump;
@@ -45,6 +45,10 @@ llvm::ArrayRef MinidumpParser::GetData() {
m_data_sp->GetByteSize());
}
+const llvm::minidump::Header *MinidumpParser::GetHeader() const {
+ return reinterpret_cast(m_file.get());
+}
+
llvm::ArrayRef MinidumpParser::GetStream(StreamType stream_type) {
return m_file->getRawStream(stream_type).value_or(llvm::ArrayRef());
}
@@ -70,8 +74,7 @@ UUID MinidumpParser::GetModuleUUID(const minidump::Module
*module) {
if (GetArchitecture().GetTriple().isOSBinFormatELF()) {
if (pdb70_uuid->Age != 0)
return UUID(pdb70_uuid, sizeof(*pdb70_uuid));
- return UUID(&pdb70_uuid->Uuid,
-sizeof(pdb70_uuid->Uuid));
+ return UUID(&pdb70_uuid->Uuid, sizeof(pdb70_uuid->Uuid));
}
return UUID(*pdb70_uuid);
} else if (cv_signature == CvSignature::ElfBuildId)
@@ -453,10 +456,12 @@ MinidumpParser::FindMemoryRange(lldb::addr_t addr) {
if (!GetStream(StreamType::Memory64List).empty()) {
llvm::Error err = llvm::Error::success();
-for (const auto &memory_desc : GetMinidumpFile().getMemory64List(err)) {
- if (memory_desc.first.StartOfMemoryRange <= addr
- && addr < memory_desc.first.StartOfMemoryRange +
memory_desc.first.DataSize) {
-return minidump::Range(memory_desc.first.StartOfMemoryRange,
memory_desc.second);
+for (const auto &memory_desc : GetMinidumpFile().getMemory64List(err)) {
+ if (memory_desc.first.StartOfMemoryRange <= addr &&
+ addr < memory_desc.first.StartOfMemoryRange +
+ memory_desc.first.DataSize) {
+return minidump::Range(memory_desc.first.StartOfMemoryRange,
+ memory_desc.second);
}
}
@@ -490,7 +495,8 @@ llvm::ArrayRef
MinidumpParser::GetMemory(lldb::addr_t addr,
return range->range_ref.slice(offset, overlap);
}
-llvm::iterator_range
MinidumpParser::GetMemory64Iterator(llvm::Error &err) {
+llvm::iterator_range
+MinidumpParser::GetMemory64Iterator(llvm::Error &err) {
llvm::ErrorAsOutParameter ErrAsOutParam(&err);
return m_file->getMemory64List(err);
}
@@ -602,8 +608,7 @@ std::pair
MinidumpParser::BuildMemoryRegions() {
case StreamType::ST:
\
return #ST
-llvm::StringRef
-MinidumpParser::GetStreamTypeAsString(StreamType stream_type) {
+llvm::StringRef MinidumpParser::GetStreamTypeAsString(StreamType stream_type) {
switch (stream_type) {
ENUM_TO_CSTR(Unused);
ENUM_TO_CSTR(ThreadList);
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.h
b/lldb/source/Plugins/Process/minidump/MinidumpParser.h
index f0b6e6027c52f0..e13065264668c2 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.h
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.h
@@ -47,7 +47,8 @@ struct Range {
}
};
-using FallibleMemory64Iterator =
llvm::object::MinidumpFile::FallibleMemory64Iterator;
+using FallibleMemory64Iterator =
+llvm::object::MinidumpFile::FallibleMemory64Iterator;
using ExceptionStreamsIterator =
llvm::object::MinidumpFile::
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Make workaround for the Dynamic loader issue (PR #120166)
Jlalond wrote: @labath I went with your suggestion as the better idea. I ended up just writing 'LLDB' to the stream, because the current API behavior for MinidumpParser is to return an empty ArrayRef. So there was no direct way to detect an existing but empty stream. For now I just set the check if it's `<= 4 bytes`. So in the future we can add extra LLDB centric information, and we just want to keep the Magic bytes to keep compatibility. https://github.com/llvm/llvm-project/pull/120166 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [LLDB][Minidump] Make workaround for the Dynamic loader issue (PR #120166)
https://github.com/Jlalond updated
https://github.com/llvm/llvm-project/pull/120166
>From d5dfb15e1ae90ade9b25374eefc605cc36685a43 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Mon, 16 Dec 2024 16:04:01 -0800
Subject: [PATCH 1/3] Make workaround for the Dynamic loader issue
---
.../Minidump/MinidumpFileBuilder.cpp | 4 +-
.../Process/minidump/MinidumpParser.cpp | 25 ++-
.../Plugins/Process/minidump/MinidumpParser.h | 8 +-
.../Process/minidump/ProcessMinidump.cpp | 205 +-
.../Process/minidump/ProcessMinidump.h| 7 +-
llvm/include/llvm/BinaryFormat/Minidump.h | 4 +
6 files changed, 137 insertions(+), 116 deletions(-)
diff --git a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
index bcac5edbc1a793..a4541f8bddf1a2 100644
--- a/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
+++ b/lldb/source/Plugins/ObjectFile/Minidump/MinidumpFileBuilder.cpp
@@ -918,8 +918,8 @@ Status MinidumpFileBuilder::DumpHeader() const {
0u), // not used in most of the writers
header.TimeDateStamp =
static_cast(std::time(nullptr));
- header.Flags =
- static_cast(0u); // minidump normal flag
+ header.Flags = static_cast(
+ llvm::minidump::Header::LLDB_HEADER_FLAG);
Status error;
size_t bytes_written;
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
index afc095ddbb2f91..6a328b3b841ed0 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.cpp
@@ -20,8 +20,8 @@
#include
#include
#include
-#include
#include
+#include
using namespace lldb_private;
using namespace minidump;
@@ -45,6 +45,10 @@ llvm::ArrayRef MinidumpParser::GetData() {
m_data_sp->GetByteSize());
}
+const llvm::minidump::Header *MinidumpParser::GetHeader() const {
+ return reinterpret_cast(m_file.get());
+}
+
llvm::ArrayRef MinidumpParser::GetStream(StreamType stream_type) {
return m_file->getRawStream(stream_type).value_or(llvm::ArrayRef());
}
@@ -70,8 +74,7 @@ UUID MinidumpParser::GetModuleUUID(const minidump::Module
*module) {
if (GetArchitecture().GetTriple().isOSBinFormatELF()) {
if (pdb70_uuid->Age != 0)
return UUID(pdb70_uuid, sizeof(*pdb70_uuid));
- return UUID(&pdb70_uuid->Uuid,
-sizeof(pdb70_uuid->Uuid));
+ return UUID(&pdb70_uuid->Uuid, sizeof(pdb70_uuid->Uuid));
}
return UUID(*pdb70_uuid);
} else if (cv_signature == CvSignature::ElfBuildId)
@@ -453,10 +456,12 @@ MinidumpParser::FindMemoryRange(lldb::addr_t addr) {
if (!GetStream(StreamType::Memory64List).empty()) {
llvm::Error err = llvm::Error::success();
-for (const auto &memory_desc : GetMinidumpFile().getMemory64List(err)) {
- if (memory_desc.first.StartOfMemoryRange <= addr
- && addr < memory_desc.first.StartOfMemoryRange +
memory_desc.first.DataSize) {
-return minidump::Range(memory_desc.first.StartOfMemoryRange,
memory_desc.second);
+for (const auto &memory_desc : GetMinidumpFile().getMemory64List(err)) {
+ if (memory_desc.first.StartOfMemoryRange <= addr &&
+ addr < memory_desc.first.StartOfMemoryRange +
+ memory_desc.first.DataSize) {
+return minidump::Range(memory_desc.first.StartOfMemoryRange,
+ memory_desc.second);
}
}
@@ -490,7 +495,8 @@ llvm::ArrayRef
MinidumpParser::GetMemory(lldb::addr_t addr,
return range->range_ref.slice(offset, overlap);
}
-llvm::iterator_range
MinidumpParser::GetMemory64Iterator(llvm::Error &err) {
+llvm::iterator_range
+MinidumpParser::GetMemory64Iterator(llvm::Error &err) {
llvm::ErrorAsOutParameter ErrAsOutParam(&err);
return m_file->getMemory64List(err);
}
@@ -602,8 +608,7 @@ std::pair
MinidumpParser::BuildMemoryRegions() {
case StreamType::ST:
\
return #ST
-llvm::StringRef
-MinidumpParser::GetStreamTypeAsString(StreamType stream_type) {
+llvm::StringRef MinidumpParser::GetStreamTypeAsString(StreamType stream_type) {
switch (stream_type) {
ENUM_TO_CSTR(Unused);
ENUM_TO_CSTR(ThreadList);
diff --git a/lldb/source/Plugins/Process/minidump/MinidumpParser.h
b/lldb/source/Plugins/Process/minidump/MinidumpParser.h
index f0b6e6027c52f0..e13065264668c2 100644
--- a/lldb/source/Plugins/Process/minidump/MinidumpParser.h
+++ b/lldb/source/Plugins/Process/minidump/MinidumpParser.h
@@ -47,7 +47,8 @@ struct Range {
}
};
-using FallibleMemory64Iterator =
llvm::object::MinidumpFile::FallibleMemory64Iterator;
+using FallibleMemory64Iterator =
+llvm::object::MinidumpFile::FallibleMemory64Iterator;
using ExceptionStreamsIterator =
llvm::object::MinidumpFile::
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)
https://github.com/ashgti updated
https://github.com/llvm/llvm-project/pull/120457
>From 4131b8c5af83a219efb2513a1ac90e8b9877d2ef Mon Sep 17 00:00:00 2001
From: John Harrison
Date: Tue, 17 Dec 2024 17:45:34 -0800
Subject: [PATCH 1/4] [lldb-dap] Ensure the IO forwarding threads are managed
by the DAP object lifecycle.
This moves the ownership of the threads that forward stdout/stderr to the DAP
object itself to ensure that the threads are joined and that the forwarding is
cleaned up when the DAP connection is disconnected.
This is part of a larger refactor to allow lldb-dap to run in a listening mode
and accept multiple connections.
---
lldb/tools/lldb-dap/CMakeLists.txt | 6 +-
lldb/tools/lldb-dap/DAP.cpp | 114 ++
lldb/tools/lldb-dap/DAP.h| 67 +++
lldb/tools/lldb-dap/IOStream.cpp | 8 +-
lldb/tools/lldb-dap/IOStream.h | 14 ++-
lldb/tools/lldb-dap/OutputRedirector.cpp | 63 --
lldb/tools/lldb-dap/OutputRedirector.h | 26
lldb/tools/lldb-dap/lldb-dap.cpp | 146 +--
8 files changed, 231 insertions(+), 213 deletions(-)
delete mode 100644 lldb/tools/lldb-dap/OutputRedirector.cpp
delete mode 100644 lldb/tools/lldb-dap/OutputRedirector.h
diff --git a/lldb/tools/lldb-dap/CMakeLists.txt
b/lldb/tools/lldb-dap/CMakeLists.txt
index d68098bf7b3266..00906e8ac10904 100644
--- a/lldb/tools/lldb-dap/CMakeLists.txt
+++ b/lldb/tools/lldb-dap/CMakeLists.txt
@@ -1,7 +1,3 @@
-if ( CMAKE_SYSTEM_NAME MATCHES "Windows" OR CMAKE_SYSTEM_NAME MATCHES "NetBSD"
)
- list(APPEND extra_libs lldbHost)
-endif ()
-
if (HAVE_LIBPTHREAD)
list(APPEND extra_libs pthread)
endif ()
@@ -32,7 +28,6 @@ add_lldb_tool(lldb-dap
IOStream.cpp
JSONUtils.cpp
LLDBUtils.cpp
- OutputRedirector.cpp
ProgressEvent.cpp
RunInTerminal.cpp
SourceBreakpoint.cpp
@@ -42,6 +37,7 @@ add_lldb_tool(lldb-dap
LINK_LIBS
liblldb
+lldbHost
${extra_libs}
LINK_COMPONENTS
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 35250d9eef608a..4e0de6fa3a4e90 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -6,34 +6,53 @@
//
//===--===//
-#include
-#include
-#include
-#include
-
#include "DAP.h"
#include "JSONUtils.h"
#include "LLDBUtils.h"
+#include "lldb/API/SBBreakpoint.h"
#include "lldb/API/SBCommandInterpreter.h"
#include "lldb/API/SBLanguageRuntime.h"
#include "lldb/API/SBListener.h"
#include "lldb/API/SBStream.h"
+#include "lldb/Host/FileSystem.h"
+#include "lldb/API/SBCommandReturnObject.h"
+#include "lldb/API/SBProcess.h"
+#include "lldb/Utility/Status.h"
+#include "lldb/lldb-defines.h"
+#include "lldb/lldb-enumerations.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Twine.h"
#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/FormatVariadic.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/raw_ostream.h"
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
#if defined(_WIN32)
#define NOMINMAX
#include
#include
#include
+#else
+#include
#endif
using namespace lldb_dap;
namespace lldb_dap {
-DAP::DAP(llvm::StringRef path, ReplMode repl_mode)
-: debug_adaptor_path(path), broadcaster("lldb-dap"),
+DAP::DAP(llvm::StringRef path, std::optional &log,
+ ReplMode repl_mode, StreamDescriptor input, StreamDescriptor output)
+: debug_adaptor_path(path), log(log), input(std::move(input)),
+ output(std::move(output)), broadcaster("lldb-dap"),
exception_breakpoints(), focus_tid(LLDB_INVALID_THREAD_ID),
stop_at_entry(false), is_attach(false),
enable_auto_variable_summaries(false),
@@ -43,21 +62,7 @@ DAP::DAP(llvm::StringRef path, ReplMode repl_mode)
configuration_done_sent(false), waiting_for_run_in_terminal(false),
progress_event_reporter(
[&](const ProgressEvent &event) { SendJSON(event.ToJSON()); }),
- reverse_request_seq(0), repl_mode(repl_mode) {
- const char *log_file_path = getenv("LLDBDAP_LOG");
-#if defined(_WIN32)
- // Windows opens stdout and stdin in text mode which converts \n to 13,10
- // while the value is just 10 on Darwin/Linux. Setting the file mode to
binary
- // fixes this.
- int result = _setmode(fileno(stdout), _O_BINARY);
- assert(result);
- result = _setmode(fileno(stdin), _O_BINARY);
- UNUSED_IF_ASSERT_DISABLED(result);
- assert(result);
-#endif
- if (log_file_path)
-log.reset(new std::ofstream(log_file_path));
-}
+ reverse_request_seq(0), repl_mode(repl_mode) {}
DAP::~DAP() = default;
@@ -173,6 +178,63 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const
lldb::break_id_t bp_id) {
return nullptr;
}
+llvm::Error DAP::ConfigureIO(std::optional overrideOut,
+ std::optional overrideErr) {
+
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)
@@ -52,16 +52,24 @@ struct StreamDescriptor {
struct InputStream {
StreamDescriptor descriptor;
- bool read_full(std::ofstream *log, size_t length, std::string &text);
+ explicit InputStream(StreamDescriptor descriptor)
+ : descriptor(std::move(descriptor)) {};
- bool read_line(std::ofstream *log, std::string &line);
+ bool read_full(std::optional &log, size_t length,
ashgti wrote:
Reverted these changes.
https://github.com/llvm/llvm-project/pull/120457
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)
@@ -138,15 +140,20 @@ struct SendEventRequestHandler : public
lldb::SBCommandPluginInterface {
struct DAP {
llvm::StringRef debug_adaptor_path;
+ std::optional &log;
ashgti wrote:
Removed the optional and just passed a plain pointer instead.
https://github.com/llvm/llvm-project/pull/120457
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add external progress bit category (PR #120171)
https://github.com/Jlalond updated
https://github.com/llvm/llvm-project/pull/120171
>From fcad0a35ec2e10ec90591079d05c3b1726d22967 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Mon, 16 Dec 2024 17:57:44 -0800
Subject: [PATCH 1/6] Add external progress bit category
---
lldb/include/lldb/Core/Progress.h | 12 +++-
lldb/include/lldb/lldb-enumerations.h | 13 +++--
lldb/source/Core/Progress.cpp | 13 ++---
3 files changed, 28 insertions(+), 10 deletions(-)
diff --git a/lldb/include/lldb/Core/Progress.h
b/lldb/include/lldb/Core/Progress.h
index f6cea282842e1c..1d56703a9f586a 100644
--- a/lldb/include/lldb/Core/Progress.h
+++ b/lldb/include/lldb/Core/Progress.h
@@ -21,6 +21,12 @@
namespace lldb_private {
+/// Enum to indicate the origin of a progress event, internal or external.
+enum class ProgressOrigin : uint8_t {
+ eLLDBInternal = 0,
+ eExternal = 1,
+};
+
/// A Progress indicator helper class.
///
/// Any potentially long running sections of code in LLDB should report
@@ -83,7 +89,8 @@ class Progress {
Progress(std::string title, std::string details = {},
std::optional total = std::nullopt,
lldb_private::Debugger *debugger = nullptr,
- Timeout minimum_report_time = std::nullopt);
+ Timeout minimum_report_time = std::nullopt,
+ ProgressOrigin origin = ProgressOrigin::eLLDBInternal);
/// Destroy the progress object.
///
@@ -149,6 +156,9 @@ class Progress {
/// The "completed" value of the last reported event.
std::optional m_prev_completed;
+
+ /// The origin of this progress event.
+ ProgressOrigin m_origin;
};
/// A class used to group progress reports by category. This is done by using a
diff --git a/lldb/include/lldb/lldb-enumerations.h
b/lldb/include/lldb/lldb-enumerations.h
index 0094fcd596fdf7..aa9673feb1a586 100644
--- a/lldb/include/lldb/lldb-enumerations.h
+++ b/lldb/include/lldb/lldb-enumerations.h
@@ -195,10 +195,10 @@ enum Format {
///< character arrays that can contain non printable
///< characters
eFormatAddressInfo,///< Describe what an address points to (func + offset
- ///< with file/line, symbol + offset, data, etc)
- eFormatHexFloat,///< ISO C99 hex float string
- eFormatInstruction, ///< Disassemble an opcode
- eFormatVoid,///< Do not print this
+ ///< with file/line, symbol + offset, data, etc)
+ eFormatHexFloat, ///< ISO C99 hex float string
+ eFormatInstruction,///< Disassemble an opcode
+ eFormatVoid, ///< Do not print this
eFormatUnicode8,
kNumFormats
};
@@ -302,7 +302,7 @@ enum ConnectionStatus {
eConnectionStatusNoConnection, ///< No connection
eConnectionStatusLostConnection, ///< Lost connection while connected to a
///< valid connection
- eConnectionStatusInterrupted ///< Interrupted read
+ eConnectionStatusInterrupted ///< Interrupted read
};
enum ErrorType {
@@ -1094,7 +1094,7 @@ enum PathType {
ePathTypeGlobalLLDBTempSystemDir, ///< The LLDB temp directory for this
///< system, NOT cleaned up on a process
///< exit.
- ePathTypeClangDir ///< Find path to Clang builtin headers
+ ePathTypeClangDir ///< Find path to Clang builtin headers
};
/// Kind of member function.
@@ -1357,6 +1357,7 @@ enum DebuggerBroadcastBit {
eBroadcastBitError = (1 << 2),
eBroadcastSymbolChange = (1 << 3),
eBroadcastBitProgressCategory = (1 << 4),
+ eBroadcastBitExternalProgressCategory = (1 << 5),
};
/// Used for expressing severity in logs and diagnostics.
diff --git a/lldb/source/Core/Progress.cpp b/lldb/source/Core/Progress.cpp
index ed8dfb85639b71..e3161d79275693 100644
--- a/lldb/source/Core/Progress.cpp
+++ b/lldb/source/Core/Progress.cpp
@@ -28,7 +28,8 @@ static llvm::ManagedStatic
g_progress_signposts;
Progress::Progress(std::string title, std::string details,
std::optional total,
lldb_private::Debugger *debugger,
- Timeout minimum_report_time)
+ Timeout minimum_report_time,
+ ProgressOrigin origin)
: m_total(total.value_or(Progress::kNonDeterministicTotal)),
m_minimum_report_time(minimum_report_time),
m_progress_data{title, ++g_id,
@@ -38,7 +39,7 @@ Progress::Progress(std::string title, std::string details,
std::chrono::nanoseconds(
std::chrono::steady_clock::now().time_since_epoch())
.count()),
- m_details(std::move(details)) {
+ m_details(std::move(details)), m_origin(origin) {
std::lock_guard guard(m_mutex);
ReportProgress();
@@ -106,9 +107,15 @@ void Progress::ReportProgress() {
if (completed < m_prev_completed)
return; // An overflow in the m_completed counter. Ju
[Lldb-commits] [lldb] [lldb] Expose structured errors in SBError (PR #120784)
https://github.com/adrian-prantl updated
https://github.com/llvm/llvm-project/pull/120784
>From 59e2d804d49b549f0d3679caf72ed29493e8fd01 Mon Sep 17 00:00:00 2001
From: Adrian Prantl
Date: Fri, 20 Dec 2024 10:35:47 -0800
Subject: [PATCH] [lldb] Expose structured errors in SBError
Building on top of previous work that exposed expression diagnostics
via SBCommandReturnObject, this patch generalizes the support to
expose any SBError as machine-readable structured data. One use-case
of this is to allow IDEs to better visualize expression diagnostics.
rdar://139997604
---
lldb/include/lldb/API/SBError.h | 5 ++
lldb/include/lldb/API/SBStructuredData.h | 1 +
.../lldb/Utility/DiagnosticsRendering.h | 14 +++-
lldb/include/lldb/Utility/Status.h| 6 ++
lldb/source/API/SBError.cpp | 14
.../Interpreter/CommandReturnObject.cpp | 52 +
lldb/source/Utility/DiagnosticsRendering.cpp | 40 ++
lldb/source/Utility/Status.cpp| 24 ++
.../diagnostics/TestExprDiagnostics.py| 75 ---
.../API/commands/frame/var/TestFrameVar.py| 15 +++-
10 files changed, 161 insertions(+), 85 deletions(-)
diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h
index 9f55f92131c06e..58b45ebd793af5 100644
--- a/lldb/include/lldb/API/SBError.h
+++ b/lldb/include/lldb/API/SBError.h
@@ -44,8 +44,13 @@ class LLDB_API SBError {
bool Success() const;
+ /// Get the error code.
uint32_t GetError() const;
+ /// Get the error in machine-readable form. Particularly useful for
+ /// compiler diagnostics.
+ SBStructuredData GetErrorData() const;
+
lldb::ErrorType GetType() const;
void SetError(uint32_t err, lldb::ErrorType type);
diff --git a/lldb/include/lldb/API/SBStructuredData.h
b/lldb/include/lldb/API/SBStructuredData.h
index c0d214a7374c65..f96e169f236edd 100644
--- a/lldb/include/lldb/API/SBStructuredData.h
+++ b/lldb/include/lldb/API/SBStructuredData.h
@@ -115,6 +115,7 @@ class SBStructuredData {
friend class SBLaunchInfo;
friend class SBDebugger;
friend class SBFrame;
+ friend class SBError;
friend class SBTarget;
friend class SBProcess;
friend class SBThread;
diff --git a/lldb/include/lldb/Utility/DiagnosticsRendering.h
b/lldb/include/lldb/Utility/DiagnosticsRendering.h
index 33aded602e3a77..e5f35c4611c20a 100644
--- a/lldb/include/lldb/Utility/DiagnosticsRendering.h
+++ b/lldb/include/lldb/Utility/DiagnosticsRendering.h
@@ -57,6 +57,13 @@ struct DiagnosticDetail {
std::string rendered;
};
+StructuredData::ObjectSP Serialize(llvm::ArrayRef details);
+
+void RenderDiagnosticDetails(Stream &stream,
+ std::optional offset_in_command,
+ bool show_inline,
+ llvm::ArrayRef details);
+
class DiagnosticError
: public llvm::ErrorInfo {
public:
@@ -64,12 +71,11 @@ class DiagnosticError
DiagnosticError(std::error_code ec) : ErrorInfo(ec) {}
lldb::ErrorType GetErrorType() const override;
virtual llvm::ArrayRef GetDetails() const = 0;
+ StructuredData::ObjectSP GetErrorData() const override {
+return Serialize(GetDetails());
+ }
static char ID;
};
-void RenderDiagnosticDetails(Stream &stream,
- std::optional offset_in_command,
- bool show_inline,
- llvm::ArrayRef details);
} // namespace lldb_private
#endif
diff --git a/lldb/include/lldb/Utility/Status.h
b/lldb/include/lldb/Utility/Status.h
index b8993dff3bb18a..681a56b7a155e1 100644
--- a/lldb/include/lldb/Utility/Status.h
+++ b/lldb/include/lldb/Utility/Status.h
@@ -10,6 +10,7 @@
#define LLDB_UTILITY_STATUS_H
#include "lldb/Utility/FileSpec.h"
+#include "lldb/Utility/StructuredData.h"
#include "lldb/lldb-defines.h"
#include "lldb/lldb-enumerations.h"
#include "llvm/ADT/StringRef.h"
@@ -38,6 +39,7 @@ class CloneableError
CloneableError() : ErrorInfo() {}
virtual std::unique_ptr Clone() const = 0;
virtual lldb::ErrorType GetErrorType() const = 0;
+ virtual StructuredData::ObjectSP GetErrorData() const = 0;
static char ID;
};
@@ -49,6 +51,7 @@ class CloneableECError
std::error_code convertToErrorCode() const override { return EC; }
void log(llvm::raw_ostream &OS) const override { OS << EC.message(); }
lldb::ErrorType GetErrorType() const override;
+ virtual StructuredData::ObjectSP GetErrorData() const override;
static char ID;
protected:
@@ -183,6 +186,9 @@ class Status {
/// NULL otherwise.
const char *AsCString(const char *default_error_str = "unknown error") const;
+ /// Get the error in machine-readable form.
+ StructuredData::ObjectSP GetErrorData() const;
+
/// Clear the object state.
///
/// Reverts the state of this object to contain a generic success value and
diff --git a/lldb/source/API/SBError.cpp b/lldb/source/API/SBError.cpp
in
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)
@@ -173,6 +178,63 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const
lldb::break_id_t bp_id) {
return nullptr;
}
+llvm::Error DAP::ConfigureIO(std::optional overrideOut,
ashgti wrote:
Removed the optional and just passed a `std::FILE *` instead.
https://github.com/llvm/llvm-project/pull/120457
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)
@@ -173,6 +178,63 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const
lldb::break_id_t bp_id) {
return nullptr;
}
+llvm::Error DAP::ConfigureIO(std::optional overrideOut,
+ std::optional overrideErr) {
+ auto *inull = lldb_private::FileSystem::Instance().Fopen(
+ lldb_private::FileSystem::DEV_NULL, "w");
+ in = lldb::SBFile(inull, true);
+
+ lldb_private::Status status;
+ status = pout.CreateNew(/*child_process_inherit=*/false);
+ if (status.Fail())
+return status.takeError();
+ status = perr.CreateNew(/*child_process_inherit=*/false);
+ if (status.Fail())
+return status.takeError();
+
+ if (overrideOut) {
+if (dup2(pout.GetWriteFileDescriptor(), fileno(*overrideOut)) == -1) {
+ return llvm::make_error(
+ llvm::errnoAsErrorCode(),
+ llvm::formatv("override fd=%d failed", fileno(*overrideOut))
+ .str()
+ .c_str());
+}
+ }
+
+ if (overrideErr) {
+if (dup2(perr.GetWriteFileDescriptor(), fileno(*overrideErr)) == -1) {
+ return llvm::make_error(
+ llvm::errnoAsErrorCode(),
+ llvm::formatv("override fd=%d failed", fileno(*overrideErr))
+ .str()
+ .c_str());
+}
+ }
+
+ auto forwarder = [&](lldb_private::Pipe &pipe, OutputType outputType) {
+char buffer[4098];
+size_t bytes_read;
+while (pipe.CanRead()) {
+ lldb_private::Status error = pipe.ReadWithTimeout(
+ &buffer, sizeof(buffer), std::chrono::seconds(1), bytes_read);
ashgti wrote:
Updated per this suggestion and I moved this logic into a `OutputRedirection`
helper in the `OutputRedirection.{h,cpp}` file. I also included a comment to
help explain why we need to have a write to ensure the thread stops.
https://github.com/llvm/llvm-project/pull/120457
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)
@@ -173,6 +178,63 @@ ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const
lldb::break_id_t bp_id) {
return nullptr;
}
+llvm::Error DAP::ConfigureIO(std::optional overrideOut,
+ std::optional overrideErr) {
+ auto *inull = lldb_private::FileSystem::Instance().Fopen(
+ lldb_private::FileSystem::DEV_NULL, "w");
+ in = lldb::SBFile(inull, true);
+
+ lldb_private::Status status;
+ status = pout.CreateNew(/*child_process_inherit=*/false);
+ if (status.Fail())
+return status.takeError();
+ status = perr.CreateNew(/*child_process_inherit=*/false);
+ if (status.Fail())
+return status.takeError();
+
+ if (overrideOut) {
+if (dup2(pout.GetWriteFileDescriptor(), fileno(*overrideOut)) == -1) {
+ return llvm::make_error(
+ llvm::errnoAsErrorCode(),
+ llvm::formatv("override fd=%d failed", fileno(*overrideOut))
+ .str()
+ .c_str());
+}
+ }
+
+ if (overrideErr) {
+if (dup2(perr.GetWriteFileDescriptor(), fileno(*overrideErr)) == -1) {
+ return llvm::make_error(
+ llvm::errnoAsErrorCode(),
+ llvm::formatv("override fd=%d failed", fileno(*overrideErr))
+ .str()
+ .c_str());
+}
+ }
+
+ auto forwarder = [&](lldb_private::Pipe &pipe, OutputType outputType) {
ashgti wrote:
Done.
https://github.com/llvm/llvm-project/pull/120457
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)
@@ -1099,6 +1098,14 @@ void request_disconnect(DAP &dap, const
llvm::json::Object &request) {
dap.broadcaster.BroadcastEventByType(eBroadcastBitStopProgressThread);
dap.progress_event_thread.join();
}
+ if (dap.stdout_forward_thread.joinable()) {
+dap.pout.Close();
ashgti wrote:
Done.
https://github.com/llvm/llvm-project/pull/120457
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)
@@ -5027,43 +5019,77 @@ int main(int argc, char *argv[]) {
}
#endif
+ std::optional log = std::nullopt;
+ const char *log_file_path = getenv("LLDBDAP_LOG");
+ if (log_file_path)
+log.emplace(log_file_path);
+
// Initialize LLDB first before we do anything.
lldb::SBDebugger::Initialize();
// Terminate the debugger before the C++ destructor chain kicks in.
auto terminate_debugger =
llvm::make_scope_exit([] { lldb::SBDebugger::Terminate(); });
- DAP dap = DAP(program_path.str(), default_repl_mode);
-
- RegisterRequestCallbacks(dap);
-
- // stdout/stderr redirection to the IDE's console
- int new_stdout_fd = SetupStdoutStderrRedirection(dap);
-
+ StreamDescriptor input;
+ StreamDescriptor output;
+ std::optional redirectOut = std::nullopt;
+ std::optional redirectErr = std::nullopt;
if (portno != -1) {
printf("Listening on port %i...\n", portno);
-SOCKET socket_fd = AcceptConnection(dap, portno);
-if (socket_fd >= 0) {
- dap.input.descriptor = StreamDescriptor::from_socket(socket_fd, true);
- dap.output.descriptor = StreamDescriptor::from_socket(socket_fd, false);
-} else {
+SOCKET socket_fd = AcceptConnection(log, portno);
+if (socket_fd < 0)
return EXIT_FAILURE;
-}
+
+input = StreamDescriptor::from_socket(socket_fd, true);
+output = StreamDescriptor::from_socket(socket_fd, false);
} else {
-dap.input.descriptor = StreamDescriptor::from_file(fileno(stdin), false);
-dap.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, false);
+#if defined(_WIN32)
+// Windows opens stdout and stdin in text mode which converts \n to 13,10
+// while the value is just 10 on Darwin/Linux. Setting the file mode to
+// binary fixes this.
+int result = _setmode(fileno(stdout), _O_BINARY);
+assert(result);
+result = _setmode(fileno(stdin), _O_BINARY);
+UNUSED_IF_ASSERT_DISABLED(result);
+assert(result);
+#endif
+
+int stdout_fd = dup(fileno(stdout));
ashgti wrote:
I created a help for duplicating the FD and added a `#ifdef` around this like I
found in a few other places in lldb like
https://github.com/llvm/llvm-project/blob/81831ef3e7804b9637aff119c1adde661d10fb78/lldb/source/Host/posix/PipePosix.cpp#L43-L50
https://github.com/llvm/llvm-project/pull/120457
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #120457)
@@ -5027,43 +5019,77 @@ int main(int argc, char *argv[]) {
}
#endif
+ std::optional log = std::nullopt;
+ const char *log_file_path = getenv("LLDBDAP_LOG");
+ if (log_file_path)
+log.emplace(log_file_path);
+
// Initialize LLDB first before we do anything.
lldb::SBDebugger::Initialize();
// Terminate the debugger before the C++ destructor chain kicks in.
auto terminate_debugger =
llvm::make_scope_exit([] { lldb::SBDebugger::Terminate(); });
- DAP dap = DAP(program_path.str(), default_repl_mode);
-
- RegisterRequestCallbacks(dap);
-
- // stdout/stderr redirection to the IDE's console
- int new_stdout_fd = SetupStdoutStderrRedirection(dap);
-
+ StreamDescriptor input;
+ StreamDescriptor output;
+ std::optional redirectOut = std::nullopt;
+ std::optional redirectErr = std::nullopt;
if (portno != -1) {
printf("Listening on port %i...\n", portno);
-SOCKET socket_fd = AcceptConnection(dap, portno);
-if (socket_fd >= 0) {
- dap.input.descriptor = StreamDescriptor::from_socket(socket_fd, true);
- dap.output.descriptor = StreamDescriptor::from_socket(socket_fd, false);
-} else {
+SOCKET socket_fd = AcceptConnection(log, portno);
+if (socket_fd < 0)
return EXIT_FAILURE;
-}
+
+input = StreamDescriptor::from_socket(socket_fd, true);
+output = StreamDescriptor::from_socket(socket_fd, false);
} else {
-dap.input.descriptor = StreamDescriptor::from_file(fileno(stdin), false);
-dap.output.descriptor = StreamDescriptor::from_file(new_stdout_fd, false);
+#if defined(_WIN32)
+// Windows opens stdout and stdin in text mode which converts \n to 13,10
+// while the value is just 10 on Darwin/Linux. Setting the file mode to
+// binary fixes this.
+int result = _setmode(fileno(stdout), _O_BINARY);
+assert(result);
+result = _setmode(fileno(stdin), _O_BINARY);
+UNUSED_IF_ASSERT_DISABLED(result);
+assert(result);
+#endif
+
+int stdout_fd = dup(fileno(stdout));
+if (stdout_fd == -1) {
+ llvm::errs() << "Failed to configure stdout redirect: "
+ << lldb_private::Status::FromErrno().takeError() << "\n";
+ return EXIT_FAILURE;
+}
+
+redirectOut = stdout;
+redirectErr = stderr;
-/// used only by TestVSCode_redirection_to_console.py
-if (getenv("LLDB_DAP_TEST_STDOUT_STDERR_REDIRECTION") != nullptr)
- redirection_test();
+input = StreamDescriptor::from_file(fileno(stdin), false);
+output = StreamDescriptor::from_file(stdout_fd, false);
}
+ DAP dap = DAP(program_path.str(), log, default_repl_mode, std::move(input),
ashgti wrote:
Done.
https://github.com/llvm/llvm-project/pull/120457
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Implement basic support for reverse-continue (PR #112079)
rocallahan wrote: > Could you create a separate PR (or two, but probably not eight) with the > refactoring changes, and we can rebase this PR on top of that when they land? Ok. I assume "Require pull request reviews before merging" is enabled so you can't push a group of commits manually. https://github.com/llvm/llvm-project/pull/112079 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Expose structured errors in SBError (PR #120784)
https://github.com/medismailben approved this pull request. https://github.com/llvm/llvm-project/pull/120784 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
