[Lldb-commits] [lldb] cccb554 - [lldb] Remove unused posix_openpt function definition for Android (#124257)
Author: Brad Smith Date: 2025-01-24T11:33:05-05:00 New Revision: cccb55491223cd410cb2f83973377dd75757cb60 URL: https://github.com/llvm/llvm-project/commit/cccb55491223cd410cb2f83973377dd75757cb60 DIFF: https://github.com/llvm/llvm-project/commit/cccb55491223cd410cb2f83973377dd75757cb60.diff LOG: [lldb] Remove unused posix_openpt function definition for Android (#124257) This was for the wrapper function that was in source/Host/android/LibcGlue.cpp. Android added support 10+ years ago. Added: Modified: lldb/source/Host/common/PseudoTerminal.cpp Removed: diff --git a/lldb/source/Host/common/PseudoTerminal.cpp b/lldb/source/Host/common/PseudoTerminal.cpp index d53327973eb270..53e91aff212a4a 100644 --- a/lldb/source/Host/common/PseudoTerminal.cpp +++ b/lldb/source/Host/common/PseudoTerminal.cpp @@ -27,10 +27,6 @@ #include #endif -#if defined(__ANDROID__) -int posix_openpt(int flags); -#endif - using namespace lldb_private; // PseudoTerminal constructor ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove support and workarounds for Android 4 and older (PR #124047)
emrekultursay wrote: On the Android Studio side, we don't run tests on old API levels. We only run automated on latest API levels (and some best-effort manual tests on recent-ish API levels). Different features in Android Studio have different min-API requirements. To me, it seems OK to add "API-21 or newer required for native debugging" to future versions of Android Studio; so LGTM. https://github.com/llvm/llvm-project/pull/124047 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Formatters] Do not recursively dereference pointer type when creating formatter candicates list. (PR #124048)
jimingham wrote: I think that argument has some force for synthetic child providers, but less so for summaries. I'd like to see the size of a std::vector regardless of whether I'm holding it as a *, **, etc. https://github.com/llvm/llvm-project/pull/124048 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/124279
>From 23c7f4a3e067739a050964b3f1c2140787b0c31f Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Fri, 24 Jan 2025 13:33:07 +
Subject: [PATCH 1/4] [lldb][TypeSystem] Ensure that ParmVarDecls have the
correct DeclContext
While sifting through this part of the code I noticed that when we parse C++
methods, `DWARFASTParserClang` creates two sets of `ParmVarDecls`, one in
`ParseChildParameters` and once in `AddMethodToCXXRecordType`. The former is
unused when we're dealing with methods. Moreover, the `ParmVarDecls` we created
in `ParseChildParameters` were created with an incorrect `clang::DeclContext`
(namely the DeclContext of the function, and not the function itself). In
Clang, there's `ParmVarDecl::setOwningFunction` to adjust the DeclContext of a
parameter if the parameter was created before the FunctionDecl. But we never
used it.
This patch removes the `ParmVarDecl` creation from `ParseChildParameters` and
instead creates a `TypeSystemClang::CreateParameterDeclarations` that ensures
we set the DeclContext correctly.
This wasn't causing any concrete issues (that I know of), but was quite
surprising. And this way of setting the parameters seems easier to reason about
(in my opinion).
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 31 ++--
.../SymbolFile/DWARF/DWARFASTParserClang.h| 3 +-
.../SymbolFile/NativePDB/PdbAstBuilder.cpp| 2 +-
.../Plugins/SymbolFile/PDB/PDBASTParser.cpp | 4 +-
.../TypeSystem/Clang/TypeSystemClang.cpp | 43 ++---
.../TypeSystem/Clang/TypeSystemClang.h| 16 +-
lldb/unittests/Symbol/TestTypeSystemClang.cpp | 42 +
.../DWARF/DWARFASTParserClangTests.cpp| 154 ++
8 files changed, 247 insertions(+), 48 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index e77188bfbd2e4a..30dec4bbf91c10 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1272,7 +1272,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
return_clang_type = m_ast.GetBasicType(eBasicTypeVoid);
std::vector function_param_types;
- std::vector function_param_decls;
// Parse the function children for the parameters
@@ -1283,8 +1282,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
if (die.HasChildren()) {
ParseChildParameters(containing_decl_ctx, die, is_variadic,
- has_template_params, function_param_types,
- function_param_decls);
+ has_template_params, function_param_types);
}
bool is_cxx_method = DeclKindIsCXXClass(containing_decl_ctx->getDeclKind());
@@ -1414,11 +1412,15 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE
&die,
LinkDeclContextToDIE(function_decl, die);
- if (!function_param_decls.empty()) {
-m_ast.SetFunctionParameters(function_decl, function_param_decls);
+ const clang::FunctionProtoType *function_prototype(
+ llvm::cast(
+ ClangUtil::GetQualType(clang_type).getTypePtr()));
+ auto params = m_ast.CreateParameterDeclarations(function_decl,
+ *function_prototype);
+ if (!params.empty()) {
+function_decl->setParams(params);
if (template_function_decl)
- m_ast.SetFunctionParameters(template_function_decl,
- function_param_decls);
+ template_function_decl->setParams(params);
}
ClangASTMetadata metadata;
@@ -2380,7 +2382,6 @@
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
bool is_variadic = false;
bool has_template_params = false;
std::vector param_types;
- std::vector param_decls;
StreamString sstr;
DWARFDeclContext decl_ctx = die.GetDWARFDeclContext();
@@ -2394,7 +2395,7 @@
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
die, GetCXXObjectParameter(die, *containing_decl_ctx));
ParseChildParameters(containing_decl_ctx, die, is_variadic,
- has_template_params, param_types, param_decls);
+ has_template_params, param_types);
sstr << "(";
for (size_t i = 0; i < param_types.size(); i++) {
if (i > 0)
@@ -3156,8 +3157,7 @@ bool DWARFASTParserClang::ParseChildMembers(
void DWARFASTParserClang::ParseChildParameters(
clang::DeclContext *containing_decl_ctx, const DWARFDIE &parent_die,
bool &is_variadic, bool &has_template_params,
-std::vector &function_param_types,
-std::vector &function_param_decls) {
+std::vector &function_param_types) {
if (!parent_die)
return;
@@ -3168,7 +3168,6 @
[Lldb-commits] [lldb] [lldb] Remove support and workarounds for Android 4 and older (PR #124047)
enh-google wrote: > > that's especially weird because execve() doesn't use $PATH ... it's the > > members of the exec family that have a 'p' in the name that do. > > It's been a long time, but I'm pretty sure this was due to a test which ran > something with an empty environment (some of our tests do that, sometimes > deliberately, mostly by accident). It probably wasn't the binary itself that > failed to launch, but that it then couldn't find the executable that _it_ was > trying to launch. The comment could be incorrect. Maybe it's (ba)sh who > inserts `PATH=/bin:/usr/bin` into the environment if the variable is missing? it is, but mksh (Android's shell) does that too. > Bottom line: I'm pretty sure that the only reason this needs to be there is > to make the test suite happy (and maybe not even that, if things have changed > since that time). I don't think anyone is running the lldb test suite on > android. If you're not planning to start doing that, I think we could delete > it. even if anyone is, it might be better to delete this hack and then someone should come and hassle me if/when there's a test failure so we can try to work out what (if anything) is Android-specific here. if nothing else, we'll have a better code comment to add next time round :-) @emrekultursay fyi, because i thought he _did_ run the lldb tests? but maybe i'm confused about which tests exactly... > > i don't know what lldb's deprecation policy is (or Android Studio's, as > > "deliverers of the lldb"), but the ndk at least still supports api >= 21 > > I was asking this because I thought `android-4` is newer than `api-21`. It > looks like that's not the case (though I'm pretty sure we did not call it > "android 4" back in those days), so I think it's fine to keep it. It's pretty > well encapsulated, and I don't think it has gotten in the way of anything. heh, i've worked on Android since 2009 and i still struggle. https://apilevels.com/ is a godsend for this kind of thing, being a rosetta stone to translate between "whatever you have" and "whatever you need". (though personally i've given up on codenames and marketing names and only use api levels any more, and have even been retroactively changing documentation too. especially because [marketing name] "Android 16" is getting awfully close to something that sounds like an api level!) > > is there a workaround in android n? > > As I recall, we were cherry-picking the fix into all supported kernel > branches (and maybe even adding conformance tests -- I'm not sure), so that > at least new devices (starting from a new kernel branch) should have it. But > yes, it's possible that claim is too optimistic... ah, yes, it's coming back to me now --- it was you who wrote most of the excellent tests in https://cs.android.com/android/platform/superproject/main/+/main:bionic/tests/sys_ptrace_test.cpp wasn't it? i _think_ that the relevant tests weren't in CTS before api 28 though? certainly the copyright date for that whole file was 2016, so it can't have been much earlier than that, and certainly wasn't api 21. (fyi, these tests found a linux kernel virtualization bug just recently: https://lore.kernel.org/lkml/z5fvfe9rwvnr2...@google.com/ --- so thank you again for those!) https://github.com/llvm/llvm-project/pull/124047 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SBThread.selected_frame property (PR #123981)
llvm-ci wrote: LLVM Buildbot has detected a new failure on builder `lldb-remote-linux-win` running on `as-builder-10` while building `lldb` at step 17 "test-check-lldb-api". Full details are available at: https://lab.llvm.org/buildbot/#/builders/197/builds/ Here is the relevant piece of the build log for the reference ``` Step 17 (test-check-lldb-api) failure: Test just built components: check-lldb-api completed (failure) ... PASS: lldb-api :: functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py (365 of 1223) PASS: lldb-api :: functionalities/data-formatter/data-formatter-stl/libcxx-simulators/optional/TestDataFormatterLibcxxOptionalSimulator.py (366 of 1223) PASS: lldb-api :: commands/command/script_alias/TestCommandScriptAlias.py (367 of 1223) PASS: lldb-api :: functionalities/data-formatter/data-formatter-stl/generic/optional/TestDataFormatterGenericOptional.py (368 of 1223) PASS: lldb-api :: functionalities/data-formatter/data-formatter-stl/libcxx-simulators/unique_ptr/TestDataFormatterLibcxxUniquePtrSimulator.py (369 of 1223) PASS: lldb-api :: functionalities/data-formatter/data-formatter-stl/generic/multiset/TestDataFormatterGenericMultiSet.py (370 of 1223) UNSUPPORTED: lldb-api :: functionalities/data-formatter/data-formatter-stl/libcxx/atomic/TestLibCxxAtomic.py (371 of 1223) PASS: lldb-api :: functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py (372 of 1223) UNSUPPORTED: lldb-api :: functionalities/data-formatter/data-formatter-stl/libcxx/initializerlist/TestInitializerList.py (373 of 1223) UNRESOLVED: lldb-api :: functionalities/data-formatter/data-formatter-stl/libcxx/chrono/TestDataFormatterLibcxxChrono.py (374 of 1223) TEST 'lldb-api :: functionalities/data-formatter/data-formatter-stl/libcxx/chrono/TestDataFormatterLibcxxChrono.py' FAILED Script: -- C:/Python312/python.exe C:/buildbot/as-builder-10/lldb-x-aarch64/llvm-project/lldb\test\API\dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=C:/buildbot/as-builder-10/lldb-x-aarch64/build/./lib --env LLVM_INCLUDE_DIR=C:/buildbot/as-builder-10/lldb-x-aarch64/build/include --env LLVM_TOOLS_DIR=C:/buildbot/as-builder-10/lldb-x-aarch64/build/./bin --arch aarch64 --build-dir C:/buildbot/as-builder-10/lldb-x-aarch64/build/lldb-test-build.noindex --lldb-module-cache-dir C:/buildbot/as-builder-10/lldb-x-aarch64/build/lldb-test-build.noindex/module-cache-lldb\lldb-api --clang-module-cache-dir C:/buildbot/as-builder-10/lldb-x-aarch64/build/lldb-test-build.noindex/module-cache-clang\lldb-api --executable C:/buildbot/as-builder-10/lldb-x-aarch64/build/./bin/lldb.exe --compiler C:/buildbot/as-builder-10/lldb-x-aarch64/build/./bin/clang.exe --dsymutil C:/buildbot/as-builder-10/lldb-x-aarch64/build/./bin/dsymutil.exe --make C:/ninja/make.exe --llvm-tools-dir C:/buildbot/as-builder-10/lldb-x-aarch64/build/./bin --lldb-obj-root C:/buildbot/as-builder-10/lldb-x-aarch64/build/tools/lldb --lldb-libs-dir C:/buildbot/as-builder-10/lldb-x-aarch64/build/./lib --platform-url connect://jetson-agx-0086.lab.llvm.org:1234 --platform-working-dir /home/ubuntu/lldb-tests --sysroot c:/buildbot/fs/jetson-agx-ubuntu --env ARCH_CFLAGS=-mcpu=cortex-a78 --platform-name remote-linux --skip-category=lldb-server C:\buildbot\as-builder-10\lldb-x-aarch64\llvm-project\lldb\test\API\functionalities\data-formatter\data-formatter-stl\libcxx\chrono -p TestDataFormatterLibcxxChrono.py -- Exit Code: 1 Command Output (stdout): -- lldb version 20.0.0git (https://github.com/llvm/llvm-project.git revision 7293455cf292cfaa263ea04fc1bc2aee4ceab6a6) clang revision 7293455cf292cfaa263ea04fc1bc2aee4ceab6a6 llvm revision 7293455cf292cfaa263ea04fc1bc2aee4ceab6a6 Setting up remote platform 'remote-linux' Connecting to remote platform 'remote-linux' at 'connect://jetson-agx-0086.lab.llvm.org:1234'... Connected. Setting remote platform working directory to '/home/ubuntu/lldb-tests'... Skipping the following test categories: ['lldb-server', 'dsym', 'gmodules', 'debugserver', 'objc', 'lldb-dap'] -- Command Output (stderr): -- UNSUPPORTED: LLDB (C:\buildbot\as-builder-10\lldb-x-aarch64\build\bin\clang.exe-aarch64) :: test_with_run_command_dsym (TestDataFormatterLibcxxChrono.LibcxxChronoDataFormatterTestCase.test_with_run_command_dsym) (test case does not fall in any category of interest for this run) FAIL: LLDB (C:\buildbot\as-builder-10\lldb-x-aarch64\build\bin\clang.exe-aarch64) :: test_with_run_command_dwarf (TestDataFormatterLibcxxChrono.LibcxxChronoDataFormatterTestCase.test_with_run_command_dwarf) FAIL: LLDB (C:\buildbot\as-builder-10\lldb-x-aarch64\build\bin\clang.exe-aarch64) :: test_with_run_command_dwo (TestDataFormatterLibcxxChrono.LibcxxChronoDataFormatterTestCase.test_with_run_command_dwo) == ERR
[Lldb-commits] [lldb] 7293455 - [lldb] Add SBThread.selected_frame property (#123981)
Author: Dave Lee
Date: 2025-01-24T10:02:15-08:00
New Revision: 7293455cf292cfaa263ea04fc1bc2aee4ceab6a6
URL:
https://github.com/llvm/llvm-project/commit/7293455cf292cfaa263ea04fc1bc2aee4ceab6a6
DIFF:
https://github.com/llvm/llvm-project/commit/7293455cf292cfaa263ea04fc1bc2aee4ceab6a6.diff
LOG: [lldb] Add SBThread.selected_frame property (#123981)
Adds a `selected_frame` property to `SBThread`. The setter accepts either a
frame index (like `SetSelectedFrame`), or a frame object.
Updates a few tests to make use of the new `selected_frame`. While doing so I
noticed some of the usage could be cleaned up, so I did that too.
Added:
Modified:
lldb/bindings/interface/SBThreadExtensions.i
lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
lldb/test/API/lang/cpp/std-function-recognizer/TestStdFunctionRecognizer.py
lldb/test/API/lang/objc/print-obj/TestPrintObj.py
Removed:
diff --git a/lldb/bindings/interface/SBThreadExtensions.i
b/lldb/bindings/interface/SBThreadExtensions.i
index 860a2d765a6695..267faad9d651fb 100644
--- a/lldb/bindings/interface/SBThreadExtensions.i
+++ b/lldb/bindings/interface/SBThreadExtensions.i
@@ -51,6 +51,14 @@ STRING_EXTENSION_OUTSIDE(SBThread)
for idx in range(self.GetStopReasonDataCount())
]
+def set_selected_frame(self, frame):
+if isinstance(frame, SBFrame):
+if frame.thread != self:
+raise ValueError("cannot select frame from
diff erent thread")
+self.SetSelectedFrame(frame.idx)
+else:
+self.SetSelectedFrame(frame)
+
id = property(GetThreadID, None, doc='''A read only property that
returns the thread ID as an integer.''')
idx = property(GetIndexID, None, doc='''A read only property that
returns the thread index ID as an integer. Thread index ID values start at 1
and increment as threads come and go and can be used to uniquely identify
threads.''')
return_value = property(GetStopReturnValue, None, doc='''A read only
property that returns an lldb object that represents the return value from the
last stop (lldb.SBValue) if we just stopped due to stepping out of a
function.''')
@@ -65,6 +73,7 @@ STRING_EXTENSION_OUTSIDE(SBThread)
stop_reason_data = property(get_stop_reason_data, None, doc='''A read
only property that returns the stop reason data as a list.''')
is_suspended = property(IsSuspended, None, doc='''A read only property
that returns a boolean value that indicates if this thread is suspended.''')
is_stopped = property(IsStopped, None, doc='''A read only property
that returns a boolean value that indicates if this thread is stopped but not
exited.''')
+selected_frame = property(GetSelectedFrame, set_selected_frame,
doc='''A read/write property that gets and sets the selected frame of this
SBThread.''')
%}
#endif
}
diff --git a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
index aa2a448087431e..3e9dbfe6d85552 100644
--- a/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
+++ b/lldb/test/API/commands/frame/recognizer/TestFrameRecognizer.py
@@ -20,7 +20,7 @@ def test_frame_recognizer_1(self):
target, process, thread, _ = lldbutil.run_to_name_breakpoint(
self, "foo", exe_name=exe
)
-frame = thread.GetSelectedFrame()
+frame = thread.selected_frame
# Clear internal & plugins recognizers that get initialized at launch
self.runCmd("frame recognizer clear")
@@ -166,7 +166,7 @@ def test_frame_recognizer_hiding(self):
self.build()
target, process, thread, _ = lldbutil.run_to_name_breakpoint(self,
"nested")
-frame = thread.GetSelectedFrame()
+frame = thread.selected_frame
# Sanity check.
self.expect(
@@ -229,7 +229,6 @@ def test_frame_recognizer_multi_symbol(self):
target, process, thread, _ = lldbutil.run_to_name_breakpoint(
self, "foo", exe_name=exe
)
-frame = thread.GetSelectedFrame()
self.expect(
"frame recognizer info 0",
@@ -239,7 +238,6 @@ def test_frame_recognizer_multi_symbol(self):
target, process, thread, _ = lldbutil.run_to_name_breakpoint(
self, "bar", exe_name=exe
)
-frame = thread.GetSelectedFrame()
self.expect(
"frame recognizer info 0",
@@ -374,7 +372,7 @@ def test_frame_recognizer_not_only_first_instruction(self):
opts = lldb.SBVariablesOptions()
opts.SetIncludeRecognizedArguments(True)
-frame = thread.GetSelectedFrame()
+frame = thread.selected_frame
variables =
[Lldb-commits] [lldb] [lldb][AArch64] Add Guarded Control Stack registers (PR #123720)
https://github.com/DavidSpickett updated
https://github.com/llvm/llvm-project/pull/123720
>From 951a38910e49f3e93dc6c9a084e955ab01ad4c49 Mon Sep 17 00:00:00 2001
From: David Spickett
Date: Fri, 9 Aug 2024 11:56:29 +0100
Subject: [PATCH 1/3] [lldb][AArch64] Add Guarded Control Stack registers
The Guarded Control Stack extension implements a shadow stack and
the Linux kernel provides access to 3 registers for it via ptrace.
struct user_gcs {
__u64 features_enabled;
__u64 features_locked;
__u64 gcspr_el0;
};
This commit adds support for reading those from a live process.
The first 2 are pseudo registers based on the real control register
and the 3rd is a real register. This is the stack pointer for the
guarded stack.
I have added a "gcs_" prefix to the "features" registers so that
they have a clear name when shown individually. Also this means
they will tab complete from "gcs", and be next to gcspr_el0 in
any sorted lists of registers.
Guarded Control Stack Registers:
gcs_features_enabled = 0x
gcs_features_locked = 0x
gcspr_el0 = 0x
Testing is more of the usual, where possible I'm writing a register
then doing something in the program to confirm the value was actually
sent to ptrace.
---
.../NativeRegisterContextLinux_arm64.cpp | 86 +++
.../Linux/NativeRegisterContextLinux_arm64.h | 16 +++
.../Utility/RegisterContextPOSIX_arm64.cpp| 4 +
.../Utility/RegisterContextPOSIX_arm64.h | 1 +
.../Utility/RegisterInfoPOSIX_arm64.cpp | 39 -
.../Process/Utility/RegisterInfoPOSIX_arm64.h | 7 +
.../linux/aarch64/gcs/TestAArch64LinuxGCS.py | 134 ++
lldb/test/API/linux/aarch64/gcs/main.c| 23 ++-
8 files changed, 305 insertions(+), 5 deletions(-)
diff --git
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 6056f3001fed6e..efd3385c46e92f 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -64,8 +64,14 @@
#define NT_ARM_FPMR 0x40e /* Floating point mode register */
#endif
+#ifndef NT_ARM_GCS
+#define NT_ARM_GCS 0x410 /* Guarded Control Stack control registers */
+#endif
+
#define HWCAP_PACA (1 << 30)
+#define HWCAP_GCS (1UL << 32)
+
#define HWCAP2_MTE (1 << 18)
#define HWCAP2_FPMR (1UL << 48)
@@ -150,6 +156,8 @@
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskMTE);
if (*auxv_at_hwcap2 & HWCAP2_FPMR)
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskFPMR);
+ if (*auxv_at_hwcap & HWCAP_GCS)
+opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskGCS);
}
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskTLS);
@@ -193,6 +201,7 @@
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
::memset(&m_pac_mask, 0, sizeof(m_pac_mask));
::memset(&m_tls_regs, 0, sizeof(m_tls_regs));
::memset(&m_sme_pseudo_regs, 0, sizeof(m_sme_pseudo_regs));
+ ::memset(&m_gcs_regs, 0, sizeof(m_gcs_regs));
std::fill(m_zt_reg.begin(), m_zt_reg.end(), 0);
m_mte_ctrl_reg = 0;
@@ -213,6 +222,7 @@
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
m_tls_is_valid = false;
m_zt_buffer_is_valid = false;
m_fpmr_is_valid = false;
+ m_gcs_is_valid = false;
// SME adds the tpidr2 register
m_tls_size = GetRegisterInfo().IsSSVEPresent() ? sizeof(m_tls_regs)
@@ -433,6 +443,14 @@ NativeRegisterContextLinux_arm64::ReadRegister(const
RegisterInfo *reg_info,
offset = reg_info->byte_offset - GetRegisterInfo().GetFPMROffset();
assert(offset < GetFPMRBufferSize());
src = (uint8_t *)GetFPMRBuffer() + offset;
+ } else if (IsGCS(reg)) {
+error = ReadGCS();
+if (error.Fail())
+ return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetGCSOffset();
+assert(offset < GetGCSBufferSize());
+src = (uint8_t *)GetGCSBuffer() + offset;
} else
return Status::FromErrorString(
"failed - register wasn't recognized to be a GPR or an FPR, "
@@ -657,6 +675,17 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
return WriteFPMR();
+ } else if (IsGCS(reg)) {
+error = ReadGCS();
+if (error.Fail())
+ return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetGCSOffset();
+assert(offset < GetGCSBufferSize());
+dst = (uint8_t *)GetGCSBuffer() + offset;
+::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
+
+return WriteGCS();
}
return Status::FromErrorString("Failed to write register value");
@@ -672,6 +701,7 @@ enum RegisterSetType : uint32_t {
SME, // ZA only, because SVCR and SVG are pseudo registers.
SME2, // ZT only.
[Lldb-commits] [lldb] 57b4898 - [lldb] Use the first address range as the function address (#122440)
Author: Pavel Labath
Date: 2025-01-24T12:50:06+01:00
New Revision: 57b48987f6c21e369e7bb1626dc79ca74aa34fdb
URL:
https://github.com/llvm/llvm-project/commit/57b48987f6c21e369e7bb1626dc79ca74aa34fdb
DIFF:
https://github.com/llvm/llvm-project/commit/57b48987f6c21e369e7bb1626dc79ca74aa34fdb.diff
LOG: [lldb] Use the first address range as the function address (#122440)
This is the behavior expected by DWARF. It also requires some fixups to
algorithms which were storing the addresses of some objects (Blocks and
Variables) relative to the beginning of the function.
There are plenty of things that still don't work in this setups, but
this change is sufficient for the expression evaluator to correctly
recognize the entry point of a function in this case.
Added:
Modified:
lldb/include/lldb/Symbol/Function.h
lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp
lldb/source/Symbol/Function.cpp
lldb/test/Shell/SymbolFile/DWARF/x86/discontinuous-function.s
Removed:
diff --git a/lldb/include/lldb/Symbol/Function.h
b/lldb/include/lldb/Symbol/Function.h
index d0b27269568b04..f3b956139f3c5b 100644
--- a/lldb/include/lldb/Symbol/Function.h
+++ b/lldb/include/lldb/Symbol/Function.h
@@ -428,7 +428,7 @@ class Function : public UserID, public SymbolContextScope {
/// The section offset based address for this function.
Function(CompileUnit *comp_unit, lldb::user_id_t func_uid,
lldb::user_id_t func_type_uid, const Mangled &mangled,
- Type *func_type, AddressRanges ranges);
+ Type *func_type, Address address, AddressRanges ranges);
/// Destructor.
~Function() override;
diff --git a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
index 45c8f121db2bc4..c7229568e1a0cd 100644
--- a/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
+++ b/lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
@@ -251,11 +251,11 @@ FunctionSP
SymbolFileBreakpad::GetOrCreateFunction(CompileUnit &comp_unit) {
addr_t address = record->Address + base;
SectionSP section_sp = list->FindSectionContainingFileAddress(address);
if (section_sp) {
- AddressRange func_range(
- section_sp, address - section_sp->GetFileAddress(), record->Size);
+ Address func_addr(section_sp, address - section_sp->GetFileAddress());
// Use the CU's id because every CU has only one function inside.
- func_sp = std::make_shared(&comp_unit, id, 0, func_name,
- nullptr, AddressRanges{func_range});
+ func_sp = std::make_shared(
+ &comp_unit, id, 0, func_name, nullptr, func_addr,
+ AddressRanges{AddressRange(func_addr, record->Size)});
comp_unit.AddFunction(func_sp);
}
}
diff --git a/lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
b/lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
index 15e8d38e7f334b..0feb927c5c9488 100644
--- a/lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
+++ b/lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
@@ -829,7 +829,7 @@ size_t SymbolFileCTF::ParseFunctions(CompileUnit &cu) {
lldb::user_id_t func_uid = m_functions.size();
FunctionSP function_sp = std::make_shared(
&cu, func_uid, function_type_uid, symbol->GetMangled(),
type_sp.get(),
- AddressRanges{func_range});
+ symbol->GetAddress(), AddressRanges{func_range});
m_functions.emplace_back(function_sp);
cu.AddFunction(function_sp);
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 682ee6d287bf5c..e77188bfbd2e4a 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2462,10 +2462,29 @@ Function *DWARFASTParserClang::ParseFunctionFromDWARF(
assert(func_type == nullptr || func_type != DIE_IS_BEING_PARSED);
const user_id_t func_user_id = die.GetID();
+
+// The base address of the scope for any of the debugging information
+// entries listed above is given by either the DW_AT_low_pc attribute or
the
+// first address in the first range entry in the list of ranges given by
the
+// DW_AT_ranges attribute.
+// -- DWARFv5, Section 2.17 Code Addresses, Ranges and Base Addresses
+//
[Lldb-commits] [lldb] [lldb] Use the first address range as the function address (PR #122440)
labath wrote: Thank you both. The cases where I'm creating a function with a single range are all in non-DWARF plugins, and the range is the only one we have available. Breakpad definitely doesn't support multiple function ranges. I'm not sure about PDB, but I didn't find anything obvious there either (Propeller also doesn't work on windows, and I don't know if the Microsoft has anything comparable). If it does, or it gains that ability in the future, the lldb infrastructure will be ready for that. I'm going to merge this now. Greg, if you have further comments, we can handle them in followups. https://github.com/llvm/llvm-project/pull/122440 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Use the first address range as the function address (PR #122440)
https://github.com/labath closed https://github.com/llvm/llvm-project/pull/122440 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)
@@ -0,0 +1,156 @@
+//===-- DILLexer.h --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_VALUEOBJECT_DILLEXER_H_
+#define LLDB_VALUEOBJECT_DILLEXER_H_
+
+#include "llvm/ADT/StringRef.h"
+#include
+#include
+#include
+#include
+#include
+
+namespace lldb_private {
+
+namespace dil {
+
+enum class TokenKind {
+ coloncolon,
+ eof,
+ identifier,
+ invalid,
+ kw_namespace,
+ l_paren,
+ none,
+ r_paren,
+ unknown,
+};
+
+/// Class defining the tokens generated by the DIL lexer and used by the
+/// DIL parser.
+class DILToken {
+public:
+ DILToken(dil::TokenKind kind, std::string spelling, uint32_t start)
+ : m_kind(kind), m_spelling(spelling), m_start_pos(start) {}
+
+ DILToken() : m_kind(dil::TokenKind::none), m_spelling(""), m_start_pos(0) {}
+
+ void setKind(dil::TokenKind kind) { m_kind = kind; }
+ dil::TokenKind getKind() const { return m_kind; }
+
+ std::string getSpelling() const { return m_spelling; }
+
+ uint32_t getLength() const { return m_spelling.size(); }
+
+ bool is(dil::TokenKind kind) const { return m_kind == kind; }
+
+ bool isNot(dil::TokenKind kind) const { return m_kind != kind; }
+
+ bool isOneOf(dil::TokenKind kind1, dil::TokenKind kind2) const {
+return is(kind1) || is(kind2);
+ }
+
+ template bool isOneOf(dil::TokenKind kind, Ts... Ks) const {
+return is(kind) || isOneOf(Ks...);
+ }
+
+ uint32_t getLocation() const { return m_start_pos; }
+
+ void setValues(dil::TokenKind kind, std::string spelling, uint32_t start) {
+m_kind = kind;
+m_spelling = spelling;
+m_start_pos = start;
+ }
+
+ static const std::string getTokenName(dil::TokenKind kind);
+
+private:
+ dil::TokenKind m_kind;
+ std::string m_spelling;
+ uint32_t m_start_pos; // within entire expression string
+};
+
+/// Class for doing the simple lexing required by DIL.
+class DILLexer {
+public:
+ DILLexer(llvm::StringRef dil_expr) : m_expr(dil_expr.str()) {
+m_cur_pos = m_expr.begin();
+// Use UINT_MAX to indicate invalid/uninitialized value.
+m_tokens_idx = UINT_MAX;
+ }
+
+ bool Lex(DILToken &result, bool look_ahead = false);
+
+ bool Is_Word(std::string::iterator start, uint32_t &length);
+
+ uint32_t GetLocation() { return m_cur_pos - m_expr.begin(); }
+
+ /// Update 'result' with the other paremeter values, create a
+ /// duplicate token, and push the duplicate token onto the vector of
+ /// lexed tokens.
+ void UpdateLexedTokens(DILToken &result, dil::TokenKind tok_kind,
labath wrote:
There are workarounds, but no magic secrets. If something is called by the
parser (which is the only user of the class anyway), then it should be public
(btw, friends can see even private members, not just the public ones). And unit
tests should generally test using the public APIs (as that's what the users
will use). There are exceptions to that, but they usually involve situations
where the public API requires arguments that would be difficult/impossible to
mock in a test, which shouldn't be the case here.
Separating the public API from the private implementation details would really
help this class, as I really don't know which one of these functions is meant
to be called from the outside.
https://github.com/llvm/llvm-project/pull/123521
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Add Lexer (with tests) for DIL (Data Inspection Language). (PR #123521)
@@ -0,0 +1,156 @@
+//===-- DILLexer.h --*- C++
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLDB_VALUEOBJECT_DILLEXER_H_
+#define LLDB_VALUEOBJECT_DILLEXER_H_
+
+#include "llvm/ADT/StringRef.h"
+#include
+#include
+#include
+#include
+#include
+
+namespace lldb_private {
+
+namespace dil {
+
+enum class TokenKind {
+ coloncolon,
+ eof,
+ identifier,
+ invalid,
+ kw_namespace,
+ l_paren,
+ none,
+ r_paren,
+ unknown,
+};
+
+/// Class defining the tokens generated by the DIL lexer and used by the
+/// DIL parser.
+class DILToken {
+public:
+ DILToken(dil::TokenKind kind, std::string spelling, uint32_t start)
+ : m_kind(kind), m_spelling(spelling), m_start_pos(start) {}
+
+ DILToken() : m_kind(dil::TokenKind::none), m_spelling(""), m_start_pos(0) {}
+
+ void setKind(dil::TokenKind kind) { m_kind = kind; }
+ dil::TokenKind getKind() const { return m_kind; }
+
+ std::string getSpelling() const { return m_spelling; }
+
+ uint32_t getLength() const { return m_spelling.size(); }
+
+ bool is(dil::TokenKind kind) const { return m_kind == kind; }
+
+ bool isNot(dil::TokenKind kind) const { return m_kind != kind; }
+
+ bool isOneOf(dil::TokenKind kind1, dil::TokenKind kind2) const {
+return is(kind1) || is(kind2);
+ }
+
+ template bool isOneOf(dil::TokenKind kind, Ts... Ks) const {
+return is(kind) || isOneOf(Ks...);
+ }
+
+ uint32_t getLocation() const { return m_start_pos; }
+
+ void setValues(dil::TokenKind kind, std::string spelling, uint32_t start) {
+m_kind = kind;
+m_spelling = spelling;
+m_start_pos = start;
+ }
+
+ static const std::string getTokenName(dil::TokenKind kind);
+
+private:
+ dil::TokenKind m_kind;
+ std::string m_spelling;
+ uint32_t m_start_pos; // within entire expression string
+};
+
+/// Class for doing the simple lexing required by DIL.
+class DILLexer {
+public:
+ DILLexer(llvm::StringRef dil_expr) : m_expr(dil_expr.str()) {
+m_cur_pos = m_expr.begin();
+// Use UINT_MAX to indicate invalid/uninitialized value.
+m_tokens_idx = UINT_MAX;
+ }
+
+ bool Lex(DILToken &result, bool look_ahead = false);
+
+ bool Is_Word(std::string::iterator start, uint32_t &length);
+
+ uint32_t GetLocation() { return m_cur_pos - m_expr.begin(); }
+
+ /// Update 'result' with the other paremeter values, create a
+ /// duplicate token, and push the duplicate token onto the vector of
+ /// lexed tokens.
+ void UpdateLexedTokens(DILToken &result, dil::TokenKind tok_kind,
+ std::string tok_str, uint32_t tok_pos);
+
+ /// Return the lexed token N+1 positions ahead of the 'current' token
+ /// being handled by the DIL parser.
+ const DILToken &LookAhead(uint32_t N);
+
+ const DILToken &AcceptLookAhead(uint32_t N);
labath wrote:
Yesterday I also thought of another advantage of early/eager parsing -- we can
report errors (things like invalid tokens and such) early. Then the other
functions ("get next token" and stuff) shouldn't have any failure modes except
running off the end of token stream (which I think we could handle by
pretending the stream ends with an infinite amount of EOF tokens)
https://github.com/llvm/llvm-project/pull/123521
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)
@@ -111,8 +111,10 @@ Error
RunInTerminalLauncherCommChannel::WaitUntilDebugAdaptorAttaches(
return message.takeError();
}
-Error RunInTerminalLauncherCommChannel::NotifyPid() {
- return m_io.SendJSON(RunInTerminalMessagePid(getpid()).ToJSON());
+Error RunInTerminalLauncherCommChannel::NotifyPid(lldb::pid_t pid) {
+ if (pid == 0)
+pid = getpid();
SuibianP wrote:
I used this as kind of a dynamic default argument. The original codepath always
used `getpid()` as the PID to be sent through the pipe, but this does not work
for Windows where the PID of the target differs from that of the launcher.
https://github.com/llvm/llvm-project/pull/121269/files#diff-2de0ddf6c080497da43990b05d21e6229d309f51cf269ba7e3860a677f507905R4903-R4905
Maybe an explicit argument without such implicit default would be more readable?
https://github.com/llvm/llvm-project/pull/121269
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)
https://github.com/SuibianP updated
https://github.com/llvm/llvm-project/pull/121269
>From dc866c2d106292b7fe49f8c1ac45dbd3905b81d6 Mon Sep 17 00:00:00 2001
From: Hu Jialun
Date: Sat, 28 Dec 2024 22:39:33 +0800
Subject: [PATCH] [lldb-dap] Implement runInTerminal for Windows
Currently, the named pipe is passed by name and a transient ofstream is
constructed at each I/O request. This assumes,
- Blocking semantics: FIFO I/O waits for the other side to connect.
- Buffered semantics: Closing one side does not discard existing data.
The former can be replaced by WaitNamedPipe/ConnectNamedPipe on Win32,
but the second cannot be easily worked around. It is also impossible to
have another "keep-alive" pipe server instance, as server-client pairs
are fixed on connection on Win32 and the client may get connected to it
instead of the real one.
Refactor FifoFile[IO] to use an open file handles rather than file name.
---
Win32 provides no way to replace the process image. Under the hood exec*
actually creates a new process with a new PID. DebugActiveProcess also
cannot get notified of process creations.
Create the new process in a suspended state and resume it after attach.
---
lldb/packages/Python/lldbsuite/test/dotest.py | 2 +-
.../runInTerminal/TestDAP_runInTerminal.py| 4 -
.../API/tools/lldb-dap/runInTerminal/main.c | 6 +-
lldb/tools/lldb-dap/FifoFiles.cpp | 120 +++---
lldb/tools/lldb-dap/FifoFiles.h | 27 ++--
lldb/tools/lldb-dap/RunInTerminal.cpp | 36 --
lldb/tools/lldb-dap/RunInTerminal.h | 6 +-
lldb/tools/lldb-dap/lldb-dap.cpp | 50 ++--
8 files changed, 193 insertions(+), 58 deletions(-)
diff --git a/lldb/packages/Python/lldbsuite/test/dotest.py
b/lldb/packages/Python/lldbsuite/test/dotest.py
index 681ea1638f2d6f..538cece8b4913d 100644
--- a/lldb/packages/Python/lldbsuite/test/dotest.py
+++ b/lldb/packages/Python/lldbsuite/test/dotest.py
@@ -545,7 +545,7 @@ def setupSysPath():
lldbDir = os.path.dirname(lldbtest_config.lldbExec)
-lldbDAPExec = os.path.join(lldbDir, "lldb-dap")
+lldbDAPExec = os.path.join(lldbDir, "lldb-dap.exe" if os.name == "nt" else
"lldb-dap")
if is_exe(lldbDAPExec):
os.environ["LLDBDAP_EXEC"] = lldbDAPExec
diff --git
a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py
b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py
index ac96bcc1364a27..e1a75bdcf440d5 100644
--- a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py
+++ b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py
@@ -43,7 +43,6 @@ def isTestSupported(self):
except:
return False
-@skipIfWindows
@skipIf(archs=no_match(["x86_64"]))
def test_runInTerminal(self):
if not self.isTestSupported():
@@ -90,7 +89,6 @@ def test_runInTerminal(self):
env = self.dap_server.request_evaluate("foo")["body"]["result"]
self.assertIn("bar", env)
-@skipIf(archs=no_match(["x86_64"]))
def test_runInTerminalWithObjectEnv(self):
if not self.isTestSupported():
return
@@ -113,7 +111,6 @@ def test_runInTerminalWithObjectEnv(self):
self.assertIn("FOO", request_envs)
self.assertEqual("BAR", request_envs["FOO"])
-@skipIfWindows
@skipIf(archs=no_match(["x86_64"]))
def test_runInTerminalInvalidTarget(self):
if not self.isTestSupported():
@@ -132,7 +129,6 @@ def test_runInTerminalInvalidTarget(self):
response["message"],
)
-@skipIfWindows
@skipIf(archs=no_match(["x86_64"]))
def test_missingArgInRunInTerminalLauncher(self):
if not self.isTestSupported():
diff --git a/lldb/test/API/tools/lldb-dap/runInTerminal/main.c
b/lldb/test/API/tools/lldb-dap/runInTerminal/main.c
index 676bd830e657b4..9cc25b2e45a494 100644
--- a/lldb/test/API/tools/lldb-dap/runInTerminal/main.c
+++ b/lldb/test/API/tools/lldb-dap/runInTerminal/main.c
@@ -1,11 +1,13 @@
#include
#include
-#include
+
+#include
+#include
int main(int argc, char *argv[]) {
const char *foo = getenv("FOO");
for (int counter = 1;; counter++) {
-sleep(1); // breakpoint
+thrd_sleep(&(struct timespec){.tv_sec = 1}, NULL); // breakpoint
}
return 0;
}
diff --git a/lldb/tools/lldb-dap/FifoFiles.cpp
b/lldb/tools/lldb-dap/FifoFiles.cpp
index 1f1bba80bd3b11..e3ed7c5d064135 100644
--- a/lldb/tools/lldb-dap/FifoFiles.cpp
+++ b/lldb/tools/lldb-dap/FifoFiles.cpp
@@ -9,7 +9,13 @@
#include "FifoFiles.h"
#include "JSONUtils.h"
-#if !defined(_WIN32)
+#include "llvm/Support/FileSystem.h"
+
+#if defined(_WIN32)
+#include
+#include
+#include
+#else
#include
#include
#include
@@ -24,27 +30,74 @@ using namespace llvm;
namespace lldb_dap {
-FifoFile::FifoFile(StringRef path) : m_path(path) {}
+std::error_code EC;
+FifoFile::FifoFile(StringRef path)
+: m_path(path), m_file(fopen(path.da
[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)
@@ -158,13 +164,24 @@ std::string
RunInTerminalDebugAdapterCommChannel::GetLauncherError() {
}
Expected> CreateRunInTerminalCommFile() {
+ int comm_fd;
SmallString<256> comm_file;
- if (std::error_code EC = sys::fs::getPotentiallyUniqueTempFileName(
- "lldb-dap-run-in-terminal-comm", "", comm_file))
+ if (std::error_code EC = createNamedPipe("lldb-dap-run-in-terminal-comm", "",
+ comm_fd, comm_file))
return createStringError(EC, "Error making unique file name for "
"runInTerminal communication files");
-
- return CreateFifoFile(comm_file.str());
+ FILE *cf = fdopen(comm_fd, "r+");
+ if (setvbuf(cf, NULL, _IONBF, 0))
+return createStringError(std::error_code(errno, std::generic_category()),
+ "Error setting unbuffered mode on C FILE");
+ // There is no portable way to conjure an ofstream from HANDLE, so use FILE *
+ // llvm::raw_fd_stream does not support getline() and there is no
+ // llvm::buffer_istream
+
+ if (cf == NULL)
+return createStringError(std::error_code(errno, std::generic_category()),
+ "Error converting file descriptor to C FILE");
SuibianP wrote:
Added.
https://github.com/llvm/llvm-project/pull/121269
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Reland "[lldb] Implement basic support for reverse-continue" (#123906)" (PR #123945)
labath wrote: I ran this on an arm mac, and also on x86-via-rosetta mac (which isn't quite the same as native, but it's the best I have). I did find one problem, which is that the packet forwarder misinterprets a response to the `x` packet which begins with an `O` as an "output" packet. I guess it needs to be taught that these can only come while the process is running. I'm still thinking about the best way to achieve that. This change is definitely stress-testing the python packet parsing code as well. Before this, it was just handling a very limited about of traffic (just the packets that are explicitly sent by the lldb-server tests), but now it basically needs to forward entire lldb traffic. https://github.com/llvm/llvm-project/pull/123945 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] 02c6002 - [lldb][AArch64] Add Guarded Control Stack registers (#123720)
Author: David Spickett
Date: 2025-01-24T13:42:06Z
New Revision: 02c6002d1cd2dabe4b98368f91e7b4395e5ab11d
URL:
https://github.com/llvm/llvm-project/commit/02c6002d1cd2dabe4b98368f91e7b4395e5ab11d
DIFF:
https://github.com/llvm/llvm-project/commit/02c6002d1cd2dabe4b98368f91e7b4395e5ab11d.diff
LOG: [lldb][AArch64] Add Guarded Control Stack registers (#123720)
The Guarded Control Stack extension implements a shadow stack and the
Linux kernel provides access to 3 registers for it via ptrace.
struct user_gcs {
__u64 features_enabled;
__u64 features_locked;
__u64 gcspr_el0;
};
This commit adds support for reading those from a live process.
The first 2 are pseudo registers based on the real control register and
the 3rd is a real register. This is the stack pointer for the guarded
stack.
I have added a "gcs_" prefix to the "features" registers so that they
have a clear name when shown individually. Also this means they will tab
complete from "gcs", and be next to gcspr_el0 in any sorted lists of
registers.
Guarded Control Stack Registers:
gcs_features_enabled = 0x
gcs_features_locked = 0x
gcspr_el0 = 0x
Testing is more of the usual, where possible I'm writing a register then
doing something in the program to confirm the value was actually sent to
ptrace.
Added:
Modified:
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py
lldb/test/API/linux/aarch64/gcs/main.c
Removed:
diff --git
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 6056f3001fed6e..efd3385c46e92f 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -64,8 +64,14 @@
#define NT_ARM_FPMR 0x40e /* Floating point mode register */
#endif
+#ifndef NT_ARM_GCS
+#define NT_ARM_GCS 0x410 /* Guarded Control Stack control registers */
+#endif
+
#define HWCAP_PACA (1 << 30)
+#define HWCAP_GCS (1UL << 32)
+
#define HWCAP2_MTE (1 << 18)
#define HWCAP2_FPMR (1UL << 48)
@@ -150,6 +156,8 @@
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskMTE);
if (*auxv_at_hwcap2 & HWCAP2_FPMR)
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskFPMR);
+ if (*auxv_at_hwcap & HWCAP_GCS)
+opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskGCS);
}
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskTLS);
@@ -193,6 +201,7 @@
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
::memset(&m_pac_mask, 0, sizeof(m_pac_mask));
::memset(&m_tls_regs, 0, sizeof(m_tls_regs));
::memset(&m_sme_pseudo_regs, 0, sizeof(m_sme_pseudo_regs));
+ ::memset(&m_gcs_regs, 0, sizeof(m_gcs_regs));
std::fill(m_zt_reg.begin(), m_zt_reg.end(), 0);
m_mte_ctrl_reg = 0;
@@ -213,6 +222,7 @@
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
m_tls_is_valid = false;
m_zt_buffer_is_valid = false;
m_fpmr_is_valid = false;
+ m_gcs_is_valid = false;
// SME adds the tpidr2 register
m_tls_size = GetRegisterInfo().IsSSVEPresent() ? sizeof(m_tls_regs)
@@ -433,6 +443,14 @@ NativeRegisterContextLinux_arm64::ReadRegister(const
RegisterInfo *reg_info,
offset = reg_info->byte_offset - GetRegisterInfo().GetFPMROffset();
assert(offset < GetFPMRBufferSize());
src = (uint8_t *)GetFPMRBuffer() + offset;
+ } else if (IsGCS(reg)) {
+error = ReadGCS();
+if (error.Fail())
+ return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetGCSOffset();
+assert(offset < GetGCSBufferSize());
+src = (uint8_t *)GetGCSBuffer() + offset;
} else
return Status::FromErrorString(
"failed - register wasn't recognized to be a GPR or an FPR, "
@@ -657,6 +675,17 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
return WriteFPMR();
+ } else if (IsGCS(reg)) {
+error = ReadGCS();
+if (error.Fail())
+ return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetGCSOffset();
+assert(offset < GetGCSBufferSize());
+dst = (uint8_t *)GetGCSBuffer() + offset;
+::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
+
+return WriteGCS();
}
return Status::FromE
[Lldb-commits] [lldb] [lldb][AArch64] Add Guarded Control Stack registers (PR #123720)
https://github.com/DavidSpickett closed https://github.com/llvm/llvm-project/pull/123720 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)
https://github.com/DavidSpickett updated
https://github.com/llvm/llvm-project/pull/123918
>From 951a38910e49f3e93dc6c9a084e955ab01ad4c49 Mon Sep 17 00:00:00 2001
From: David Spickett
Date: Fri, 9 Aug 2024 11:56:29 +0100
Subject: [PATCH 1/4] [lldb][AArch64] Add Guarded Control Stack registers
The Guarded Control Stack extension implements a shadow stack and
the Linux kernel provides access to 3 registers for it via ptrace.
struct user_gcs {
__u64 features_enabled;
__u64 features_locked;
__u64 gcspr_el0;
};
This commit adds support for reading those from a live process.
The first 2 are pseudo registers based on the real control register
and the 3rd is a real register. This is the stack pointer for the
guarded stack.
I have added a "gcs_" prefix to the "features" registers so that
they have a clear name when shown individually. Also this means
they will tab complete from "gcs", and be next to gcspr_el0 in
any sorted lists of registers.
Guarded Control Stack Registers:
gcs_features_enabled = 0x
gcs_features_locked = 0x
gcspr_el0 = 0x
Testing is more of the usual, where possible I'm writing a register
then doing something in the program to confirm the value was actually
sent to ptrace.
---
.../NativeRegisterContextLinux_arm64.cpp | 86 +++
.../Linux/NativeRegisterContextLinux_arm64.h | 16 +++
.../Utility/RegisterContextPOSIX_arm64.cpp| 4 +
.../Utility/RegisterContextPOSIX_arm64.h | 1 +
.../Utility/RegisterInfoPOSIX_arm64.cpp | 39 -
.../Process/Utility/RegisterInfoPOSIX_arm64.h | 7 +
.../linux/aarch64/gcs/TestAArch64LinuxGCS.py | 134 ++
lldb/test/API/linux/aarch64/gcs/main.c| 23 ++-
8 files changed, 305 insertions(+), 5 deletions(-)
diff --git
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 6056f3001fed6e..efd3385c46e92f 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -64,8 +64,14 @@
#define NT_ARM_FPMR 0x40e /* Floating point mode register */
#endif
+#ifndef NT_ARM_GCS
+#define NT_ARM_GCS 0x410 /* Guarded Control Stack control registers */
+#endif
+
#define HWCAP_PACA (1 << 30)
+#define HWCAP_GCS (1UL << 32)
+
#define HWCAP2_MTE (1 << 18)
#define HWCAP2_FPMR (1UL << 48)
@@ -150,6 +156,8 @@
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskMTE);
if (*auxv_at_hwcap2 & HWCAP2_FPMR)
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskFPMR);
+ if (*auxv_at_hwcap & HWCAP_GCS)
+opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskGCS);
}
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskTLS);
@@ -193,6 +201,7 @@
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
::memset(&m_pac_mask, 0, sizeof(m_pac_mask));
::memset(&m_tls_regs, 0, sizeof(m_tls_regs));
::memset(&m_sme_pseudo_regs, 0, sizeof(m_sme_pseudo_regs));
+ ::memset(&m_gcs_regs, 0, sizeof(m_gcs_regs));
std::fill(m_zt_reg.begin(), m_zt_reg.end(), 0);
m_mte_ctrl_reg = 0;
@@ -213,6 +222,7 @@
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
m_tls_is_valid = false;
m_zt_buffer_is_valid = false;
m_fpmr_is_valid = false;
+ m_gcs_is_valid = false;
// SME adds the tpidr2 register
m_tls_size = GetRegisterInfo().IsSSVEPresent() ? sizeof(m_tls_regs)
@@ -433,6 +443,14 @@ NativeRegisterContextLinux_arm64::ReadRegister(const
RegisterInfo *reg_info,
offset = reg_info->byte_offset - GetRegisterInfo().GetFPMROffset();
assert(offset < GetFPMRBufferSize());
src = (uint8_t *)GetFPMRBuffer() + offset;
+ } else if (IsGCS(reg)) {
+error = ReadGCS();
+if (error.Fail())
+ return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetGCSOffset();
+assert(offset < GetGCSBufferSize());
+src = (uint8_t *)GetGCSBuffer() + offset;
} else
return Status::FromErrorString(
"failed - register wasn't recognized to be a GPR or an FPR, "
@@ -657,6 +675,17 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
return WriteFPMR();
+ } else if (IsGCS(reg)) {
+error = ReadGCS();
+if (error.Fail())
+ return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetGCSOffset();
+assert(offset < GetGCSBufferSize());
+dst = (uint8_t *)GetGCSBuffer() + offset;
+::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
+
+return WriteGCS();
}
return Status::FromErrorString("Failed to write register value");
@@ -672,6 +701,7 @@ enum RegisterSetType : uint32_t {
SME, // ZA only, because SVCR and SVG are pseudo registers.
SME2, // ZT only.
[Lldb-commits] [lldb] [lldb] Remove unused posix_openpt function definition for Android (PR #124257)
https://github.com/enh-google approved this pull request. https://github.com/llvm/llvm-project/pull/124257 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/123918 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)
@@ -103,6 +156,9 @@ bool ABISysV_arm64::PrepareTrivialCall(Thread &thread, addr_t sp, return_addr)) return false; + if (GetProcessSP()->GetTarget().GetArchitecture().GetTriple().isOSLinux()) +PushToLinuxGuardedControlStack(return_addr, reg_ctx, thread); DavidSpickett wrote: I've addressed this. Mostly. There is a general problem with PrepareTrivalCall but it's a bigger job to fix so I've handled what I need here manually, but opened https://github.com/llvm/llvm-project/issues/124269 to fix it generally later. https://github.com/llvm/llvm-project/pull/123918 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)
DavidSpickett wrote: This conflicts a lot with main but I'm going to deal with that after approval. https://github.com/llvm/llvm-project/pull/123918 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/123918 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)
@@ -60,6 +60,69 @@ ABISysV_arm64::CreateInstance(lldb::ProcessSP process_sp,
const ArchSpec &arch)
return ABISP();
}
+static Status PushToLinuxGuardedControlStack(addr_t return_addr,
+ RegisterContext *reg_ctx,
+ Thread &thread) {
+ Status err;
+
+ // If the Guarded Control Stack extension is present we may need to put the
+ // return address onto that stack.
+ const RegisterInfo *gcs_features_enabled_info =
+ reg_ctx->GetRegisterInfoByName("gcs_features_enabled");
+ if (!gcs_features_enabled_info)
+return err;
+
+ uint64_t gcs_features_enabled = reg_ctx->ReadRegisterAsUnsigned(
+ gcs_features_enabled_info, LLDB_INVALID_ADDRESS);
+ if (gcs_features_enabled == LLDB_INVALID_ADDRESS)
+return Status("Could not read GCS features enabled register.");
+
+ // Only attempt this if GCS is enabled. If it's not enabled then gcspr_el0
+ // may point to unmapped memory.
+ if ((gcs_features_enabled & 1) == 0)
+return err;
+
+ const RegisterInfo *gcspr_el0_info =
+ reg_ctx->GetRegisterInfoByName("gcspr_el0");
+ if (!gcspr_el0_info)
+return Status("Could not get register info for gcspr_el0.");
+
+ uint64_t gcspr_el0 =
+ reg_ctx->ReadRegisterAsUnsigned(gcspr_el0_info, LLDB_INVALID_ADDRESS);
+ if (gcspr_el0 == LLDB_INVALID_ADDRESS)
+return Status("Could not read gcspr_el0.");
+
+ // A link register entry on the GCS is 8 bytes.
+ gcspr_el0 -= 8;
+ if (!reg_ctx->WriteRegisterFromUnsigned(gcspr_el0_info, gcspr_el0))
+return Status(
+"Attempted to decrement gcspr_el0, but could not write to it.");
+
+ Status error;
+ size_t wrote = thread.GetProcess()->WriteMemory(gcspr_el0, &return_addr,
+ sizeof(return_addr), error);
+ if ((wrote != sizeof(return_addr) || error.Fail())) {
+// When PrepareTrivialCall fails, the register context is not restored,
+// unlike when an expression fails to execute. This is arguably a bug,
+// see https://github.com/llvm/llvm-project/issues/124269.
+// For now we are handling this here specifically. We can assume this
+// write will work as the one to decrement the register did.
DavidSpickett wrote:
See this comment.
https://github.com/llvm/llvm-project/pull/123918
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)
@@ -87,6 +150,18 @@ bool ABISysV_arm64::PrepareTrivialCall(Thread &thread,
addr_t sp,
if (args.size() > 8)
return false;
+ // Do this first, as it's got the most chance of failing (though still very
+ // low).
+ if (GetProcessSP()->GetTarget().GetArchitecture().GetTriple().isOSLinux()) {
+Status err = PushToLinuxGuardedControlStack(return_addr, reg_ctx, thread);
+// If we could not manage the GCS, the expression will certainly fail,
+// and if we just carried on, that failure would be a lot more cryptic.
+if (err.Fail()) {
+ LLDB_LOGF(log, "Failed to setup Guarded Call Stack: %s",
err.AsCString());
+ return false;
+}
+ }
DavidSpickett wrote:
Moved this up, before we write any other registers, as a temporary workaround.
https://github.com/llvm/llvm-project/pull/123918
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)
@@ -83,3 +83,278 @@ def test_gcs_fault(self):
"stop reason = signal SIGSEGV: control protection fault",
],
)
+
+def check_gcs_registers(
+self,
+expected_gcs_features_enabled=None,
+expected_gcs_features_locked=None,
+expected_gcspr_el0=None,
+):
+thread = self.dbg.GetSelectedTarget().process.GetThreadAtIndex(0)
+registerSets = thread.GetFrameAtIndex(0).GetRegisters()
+gcs_registers = registerSets.GetFirstValueByName(
+r"Guarded Control Stack Registers"
+)
+
+gcs_features_enabled = gcs_registers.GetChildMemberWithName(
+"gcs_features_enabled"
+).GetValueAsUnsigned()
+if expected_gcs_features_enabled is not None:
+self.assertEqual(expected_gcs_features_enabled,
gcs_features_enabled)
+
+gcs_features_locked = gcs_registers.GetChildMemberWithName(
+"gcs_features_locked"
+).GetValueAsUnsigned()
+if expected_gcs_features_locked is not None:
+self.assertEqual(expected_gcs_features_locked, gcs_features_locked)
+
+gcspr_el0 = gcs_registers.GetChildMemberWithName(
+"gcspr_el0"
+).GetValueAsUnsigned()
+if expected_gcspr_el0 is not None:
+self.assertEqual(expected_gcspr_el0, gcspr_el0)
+
+return gcs_features_enabled, gcs_features_locked, gcspr_el0
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+def test_gcs_registers(self):
+if not self.isAArch64GCS():
+self.skipTest("Target must support GCS.")
+
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"),
CURRENT_EXECUTABLE_SET)
+
+self.runCmd("b test_func")
+self.runCmd("b test_func2")
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect(
+"thread list",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stopped", "stop reason = breakpoint"],
+)
+
+self.expect("register read --all", substrs=["Guarded Control Stack
Registers:"])
+
+enabled, locked, spr_el0 = self.check_gcs_registers()
+
+# Features enabled should have at least the enable bit set, it could
have
+# others depending on what the C library did.
+self.assertTrue(enabled & 1, "Expected GCS enable bit to be set.")
+
+# Features locked we cannot predict, we will just assert that it
remains
+# the same as we continue.
+
+# spr_el0 will point to some memory region that is a shadow stack
region.
+self.expect(f"memory region {spr_el0}", substrs=["shadow stack: yes"])
+
+# Continue into test_func2, where the GCS pointer should have been
+# decremented, and the other registers remain the same.
+self.runCmd("continue")
+
+self.expect(
+"thread list",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stopped", "stop reason = breakpoint"],
+)
+
+_, _, spr_el0 = self.check_gcs_registers(enabled, locked, spr_el0 - 8)
+
+# Modify the control stack pointer to cause a fault.
+spr_el0 += 8
+self.runCmd(f"register write gcspr_el0 {spr_el0}")
+self.expect(
+"register read gcspr_el0", substrs=[f"gcspr_el0 =
0x{spr_el0:016x}"]
+)
+
+# If we wrote it back correctly, we will now fault but don't pass this
+# signal to the application.
+self.runCmd("process handle SIGSEGV --pass false")
+self.runCmd("continue")
+
+self.expect(
+"thread list",
+"Expected stopped by SIGSEGV.",
+substrs=[
+"stopped",
+"stop reason = signal SIGSEGV: control protection fault",
+],
+)
+
+# Any combination of lock bits could be set. Flip then restore one of
them.
+STACK_PUSH = 2
+stack_push = bool((locked >> STACK_PUSH) & 1)
+new_locked = (locked & ~(1 << STACK_PUSH)) | (int(not stack_push) <<
STACK_PUSH)
+self.runCmd(f"register write gcs_features_locked 0x{new_locked:x}")
+self.expect(
+f"register read gcs_features_locked",
+substrs=[f"gcs_features_locked = 0x{new_locked:016x}"],
+)
+
+# We could prove the write made it to hardware by trying to prctl to
change
+# the feature here, but we cannot know if the libc locked it or not.
+# Given that we know the other registers in the set write correctly, we
+# can assume this one does.
+
+self.runCmd(f"register write gcs_features_locked 0x{locked:x}")
+
+# Now to prove we can write gcs_features_enabled, disable GCS and
continue
+# past the fault.
+enabled &= ~1
+self.runCmd(f"register write gcs_feature
[Lldb-commits] [lldb] [lldb] Add SBThread.selected_frame property (PR #123981)
https://github.com/kastiglione closed https://github.com/llvm/llvm-project/pull/123981 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SBThread.selected_frame property (PR #123981)
llvm-ci wrote: LLVM Buildbot has detected a new failure on builder `lldb-remote-linux-ubuntu` running on `as-builder-9` while building `lldb` at step 16 "test-check-lldb-api". Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/3971 Here is the relevant piece of the build log for the reference ``` Step 16 (test-check-lldb-api) failure: Test just built components: check-lldb-api completed (failure) ... PASS: lldb-api :: functionalities/data-formatter/data-formatter-stl/generic/forward_list/TestDataFormatterGenericForwardList.py (364 of 1223) PASS: lldb-api :: functionalities/data-formatter/data-formatter-stl/generic/multimap/TestDataFormatterGenericMultiMap.py (365 of 1223) PASS: lldb-api :: functionalities/data-formatter/data-formatter-stl/libcxx-simulators/optional/TestDataFormatterLibcxxOptionalSimulator.py (366 of 1223) PASS: lldb-api :: functionalities/data-formatter/data-formatter-stl/generic/optional/TestDataFormatterGenericOptional.py (367 of 1223) PASS: lldb-api :: functionalities/data-formatter/data-formatter-stl/libcxx-simulators/unique_ptr/TestDataFormatterLibcxxUniquePtrSimulator.py (368 of 1223) UNSUPPORTED: lldb-api :: functionalities/data-formatter/data-formatter-stl/libcxx/atomic/TestLibCxxAtomic.py (369 of 1223) PASS: lldb-api :: functionalities/completion/TestCompletion.py (370 of 1223) PASS: lldb-api :: functionalities/data-formatter/data-formatter-stl/generic/unordered/TestDataFormatterGenericUnordered.py (371 of 1223) PASS: lldb-api :: functionalities/data-formatter/data-formatter-stl/generic/multiset/TestDataFormatterGenericMultiSet.py (372 of 1223) UNRESOLVED: lldb-api :: functionalities/data-formatter/data-formatter-stl/libcxx/chrono/TestDataFormatterLibcxxChrono.py (373 of 1223) TEST 'lldb-api :: functionalities/data-formatter/data-formatter-stl/libcxx/chrono/TestDataFormatterLibcxxChrono.py' FAILED Script: -- /usr/bin/python3.12 /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin --libcxx-include-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include/c++/v1 --libcxx-include-target-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/include/aarch64-unknown-linux-gnu/c++/v1 --libcxx-library-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib/aarch64-unknown-linux-gnu --arch aarch64 --build-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin/lldb --compiler /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/bin/clang --dsymutil /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./bin --lldb-obj-root /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/tools/lldb --lldb-libs-dir /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/./lib --platform-url connect://jetson-agx-2198.lab.llvm.org:1234 --platform-working-dir /home/ubuntu/lldb-tests --sysroot /mnt/fs/jetson-agx-ubuntu --env ARCH_CFLAGS=-mcpu=cortex-a78 --platform-name remote-linux --skip-category=lldb-server /home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/llvm-project/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/chrono -p TestDataFormatterLibcxxChrono.py -- Exit Code: 1 Command Output (stdout): -- lldb version 20.0.0git (https://github.com/llvm/llvm-project.git revision 7293455cf292cfaa263ea04fc1bc2aee4ceab6a6) clang revision 7293455cf292cfaa263ea04fc1bc2aee4ceab6a6 llvm revision 7293455cf292cfaa263ea04fc1bc2aee4ceab6a6 Setting up remote platform 'remote-linux' Connecting to remote platform 'remote-linux' at 'connect://jetson-agx-2198.lab.llvm.org:1234'... Connected. Setting remote platform working directory to '/home/ubuntu/lldb-tests'... Skipping the following test categories: ['lldb-server', 'dsym', 'gmodules', 'debugserver', 'objc', 'lldb-dap'] -- Command Output (stderr): -- WARNING:root:Custom libc++ is not supported for remote runs: ignoring --libcxx arguments UNSUPPORTED: LLDB (/home/buildbot/worker/as-builder-9/lldb-remote-linux-ubuntu/build/bin/clang-
[Lldb-commits] [lldb] Sbsavecore doc strings reapply (PR #124355)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Jacob Lalonde (Jlalond)
Changes
In my last attempt at this (#123873), I didn't realize we needed semi
colons! Also fixed the bug that the feature summary didn't have a type defined.
CC @JDevlieghere hope you get a laugh at needing to revert doc strings
for breaking the build
---
Full diff: https://github.com/llvm/llvm-project/pull/124355.diff
1 Files Affected:
- (modified) lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i (+71)
``diff
diff --git a/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
b/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
index e69de29bb2d1d6..08bbdf89d68ded 100644
--- a/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
+++ b/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
@@ -0,0 +1,71 @@
+%feature("docstring",
+"A container to specify how to save a core file.
+
+SBSaveCoreOptions includes API's to specify the memory regions and threads to
include
+when generating a core file. It extends the existing SaveCoreStyle option.
+
+* eSaveCoreFull will save off all thread and memory regions, ignoring the
memory regions and threads in
+the options object.
+
+* eSaveCoreDirtyOnly pages will capture all threads and all rw- memory
regions, in addition to the regions specified
+in the options object if they are not already captured.
+
+* eSaveCoreStackOnly will capture all threads, but no memory regions unless
specified.
+
+* eSaveCoreCustomOnly Custom defers entirely to the SBSaveCoreOptions object
and will only save what is specified.
+ Picking custom and specifying nothing will result in an error being returned.
+
+Note that currently ELF Core files are not supported."
+) lldb::SBSaveCoreOptions;
+
+%feature("docstring", "
+Set the plugin name to save a Core file with. Only plugins registered with
Plugin manager will be accepted
+Examples are Minidump and Mach-O."
+) lldb::SBSaveCoreOptions::SetPluginName;
+
+%feature("docstring", "
+Get the specified plugin name, or None if the name is not set."
+) lldb::SBSaveCoreOptions::GetPluginName;
+
+%feature("docstring", "
+Set the lldb.SaveCoreStyle."
+) lldb::SBSaveCoreOptions::SetStyle;
+
+%feature("docstring", "
+Get the specified lldb.SaveCoreStyle, or eSaveCoreUnspecified if not set."
+) lldb::SBSaveCoreOptions::GetStyle;
+
+%feature("docstring", "
+Set the file path to save the Core file at."
+) lldb::SBSaveCoreOptions::SetOutputFile;
+
+%feature("docstring", "
+Get an SBFileSpec corresponding to the specified output path, or none if
not set."
+) lldb::SBSaveCoreOptions::GetOutputFile;
+
+%feature("docstring", "
+Set the process to save, or unset a process by providing a default
SBProcess.
+Resetting will result in the reset of all process specific options, such
as Threads to save."
+) lldb::SBSaveCoreOptions::SetProcess;
+
+%feature("docstring", "
+Add an SBThread to be saved, an error will be returned if an SBThread from
a different process is specified.
+The process is set either by the first SBThread added to the options
container, or explicitly by the SetProcess call."
+) lldb::SBSaveCoreOptions::AddThread;
+
+%feature("docstring", "
+Remove an SBthread if present in the container, returns true if a matching
thread was found and removed."
+) lldb::SBSaveCoreOptions::RemoveThread;
+
+%feature("docstring", "
+Add a memory region to save, an error will be returned in the region is
invalid.
+Ranges that overlap will be unioned into a single region."
+) lldb::SBSaveCoreOptions::AddMemoryRegionToSave;
+
+%feature("docstring", "
+Get an SBThreadCollection of all threads marked to be saved. This
collection is not sorted according to insertion order."
+) lldb::SBSaveCoreOptions::GetThreadsToSave;
+
+%feature("docstring", "
+Unset all options."
+) lldb::SBSaveCoreOptions::Clear;
``
https://github.com/llvm/llvm-project/pull/124355
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Sbsavecore doc strings reapply (PR #124355)
https://github.com/Jlalond created
https://github.com/llvm/llvm-project/pull/124355
In my last attempt at this (#123873), I didn't realize we needed semi colons!
Also fixed the bug that the feature summary didn't have a type defined.
CC @JDevlieghere hope you get a laugh at needing to revert doc strings for
breaking the build
>From 1c335e8dc10225ba9ed9bac513362a6abbee73fb Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Fri, 24 Jan 2025 13:46:51 -0800
Subject: [PATCH 1/2] Reapply "[LLDB] Add draft docstrings for
SBSaveCoreOptions" (#123873)
This reverts commit 18ee7e1792cf4cc9b287ae10063a7c2b7792da8f.
---
.../interface/SBSaveCoreOptionsDocstrings.i | 71 +++
1 file changed, 71 insertions(+)
diff --git a/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
b/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
index e69de29bb2d1d6..3e99101203ee9c 100644
--- a/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
+++ b/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
@@ -0,0 +1,71 @@
+%feature("docstring",
+"A container to specify how to save a core file.
+
+SBSaveCoreOptions includes API's to specify the memory regions and threads to
include
+when generating a core file. It extends the existing SaveCoreStyle option.
+
+* eSaveCoreFull will save off all thread and memory regions, ignoring the
memory regions and threads in
+the options object.
+
+* eSaveCoreDirtyOnly pages will capture all threads and all rw- memory
regions, in addition to the regions specified
+in the options object if they are not already captured.
+
+* eSaveCoreStackOnly will capture all threads, but no memory regions unless
specified.
+
+* eSaveCoreCustomOnly Custom defers entirely to the SBSaveCoreOptions object
and will only save what is specified.
+ Picking custom and specifying nothing will result in an error being returned.
+
+Note that currently ELF Core files are not supported.
+")
+
+%feature("docstring", "
+Set the plugin name to save a Core file with. Only plugins registered with
Plugin manager will be accepted
+Examples are Minidump and Mach-O."
+) lldb::SBSaveCoreOptions::SetPluginName
+
+%feature("docstring", "
+Get the specified plugin name, or None if the name is not set."
+) lldb::SBSaveCoreOptions::GetPluginName
+
+%feature("docstring", "
+Set the lldb.SaveCoreStyle."
+) lldb::SBSaveCoreOptions::SetStyle
+
+%feature("docstring", "
+Get the specified lldb.SaveCoreStyle, or eSaveCoreUnspecified if not set."
+) lldb::SBSaveCoreOptions::GetStyle
+
+%feature("docstring", "
+Set the file path to save the Core file at."
+) lldb::SBSaveCoreOptions::SetOutputFile
+
+%feature("docstring", "
+Get an SBFileSpec corresponding to the specified output path, or none if
not set."
+) lldb::SBSaveCoreOptions::GetOutputFile
+
+%feature("docstring", "
+Set the process to save, or unset a process by providing a default
SBProcess.
+Resetting will result in the reset of all process specific options, such
as Threads to save."
+) lldb::SBSaveCoreOptions::SetProcess
+
+%feature("docstring", "
+Add an SBThread to be saved, an error will be returned if an SBThread from
a different process is specified.
+The process is set either by the first SBThread added to the options
container, or explicitly by the SetProcess call."
+) lldb::SBSaveCoreOptions::AddThread
+
+%feature("docstring", "
+Remove an SBthread if present in the container, returns true if a matching
thread was found and removed."
+) lldb::SBSaveCoreOptions::RemoveThread
+
+%feature("docstring", "
+Add a memory region to save, an error will be returned in the region is
invalid.
+Ranges that overlap will be unioned into a single region."
+) lldb::SBSaveCoreOptions::AddMemoryRegionToSave
+
+%feature("docstring", "
+Get an SBThreadCollection of all threads marked to be saved. This
collection is not sorted according to insertion order."
+) lldb::SBSaveCoreOptions::GetThreadsToSave
+
+%feature("docstring", "
+Unset all options."
+) lldb::SBSaveCoreOptions::Clear
>From 61a78673e7f7fc40b3ccfb006c9e6b93f3ba9539 Mon Sep 17 00:00:00 2001
From: Jacob Lalonde
Date: Fri, 24 Jan 2025 13:52:56 -0800
Subject: [PATCH 2/2] Fix docstrings format
---
.../interface/SBSaveCoreOptionsDocstrings.i | 28 +--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
b/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
index 3e99101203ee9c..08bbdf89d68ded 100644
--- a/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
+++ b/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
@@ -15,57 +15,57 @@ in the options object if they are not already captured.
* eSaveCoreCustomOnly Custom defers entirely to the SBSaveCoreOptions object
and will only save what is specified.
Picking custom and specifying nothing will result in an error being returned.
-Note that currently ELF Core files are not supported.
-")
[Lldb-commits] [lldb] [LLDB] Reapply #123873 SBSaveCore Docstrings (PR #124355)
https://github.com/Jlalond edited https://github.com/llvm/llvm-project/pull/124355 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Reapply #123873 SBSaveCore Docstrings (PR #124355)
https://github.com/JDevlieghere approved this pull request. https://github.com/llvm/llvm-project/pull/124355 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)
@@ -103,6 +156,9 @@ bool ABISysV_arm64::PrepareTrivialCall(Thread &thread, addr_t sp, return_addr)) return false; + if (GetProcessSP()->GetTarget().GetArchitecture().GetTriple().isOSLinux()) +PushToLinuxGuardedControlStack(return_addr, reg_ctx, thread); DavidSpickett wrote: In fact I need to draw a distinction between "did nothing because GCS isn't present" and "needed to do something with GCS but could not". https://github.com/llvm/llvm-project/pull/123918 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Fix expression evaluation with Guarded Control Stacks (PR #123918)
@@ -103,6 +156,9 @@ bool ABISysV_arm64::PrepareTrivialCall(Thread &thread, addr_t sp, return_addr)) return false; + if (GetProcessSP()->GetTarget().GetArchitecture().GetTriple().isOSLinux()) +PushToLinuxGuardedControlStack(return_addr, reg_ctx, thread); DavidSpickett wrote: Yeah not sure why I made it return bool then did nothing with it... You're right, all we can do at that point is error. If we continue without setting it up we will just hit an exception and the expression fails in a cryptic way. https://github.com/llvm/llvm-project/pull/123918 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][LoongArch] Complete register alias name in `AugmentRegisterInfo` (PR #124059)
DavidSpickett wrote: `lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py` - `test_loongarch64_regs` for example. You should be able to read all those registers using the aliases as well as the normal names. If you look through the LoongArch lldb changes you'll probably find the live process equivalent quite quickly. https://github.com/llvm/llvm-project/pull/124059 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Add Guarded Control Stack registers (PR #123720)
@@ -83,3 +83,137 @@ def test_gcs_fault(self):
"stop reason = signal SIGSEGV: control protection fault",
],
)
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+def test_gcs_registers(self):
+if not self.isAArch64GCS():
+self.skipTest("Target must support GCS.")
+
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"),
CURRENT_EXECUTABLE_SET)
+
+self.runCmd("b test_func")
+self.runCmd("b test_func2")
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect(
+"thread list",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stopped", "stop reason = breakpoint"],
+)
+
+self.expect("register read --all", substrs=["Guarded Control Stack
Registers:"])
+
+def check_gcs_registers(
+expected_gcs_features_enabled=None,
+expected_gcs_features_locked=None,
+expected_gcspr_el0=None,
+):
+thread = self.dbg.GetSelectedTarget().process.GetThreadAtIndex(0)
+registerSets = thread.GetFrameAtIndex(0).GetRegisters()
+gcs_registers = registerSets.GetFirstValueByName(
+r"Guarded Control Stack Registers"
+)
+
+gcs_features_enabled = gcs_registers.GetChildMemberWithName(
+"gcs_features_enabled"
+).GetValueAsUnsigned()
+if expected_gcs_features_enabled is not None:
+self.assertEqual(expected_gcs_features_enabled,
gcs_features_enabled)
+
+gcs_features_locked = gcs_registers.GetChildMemberWithName(
+"gcs_features_locked"
+).GetValueAsUnsigned()
+if expected_gcs_features_locked is not None:
+self.assertEqual(expected_gcs_features_locked,
gcs_features_locked)
+
+gcspr_el0 = gcs_registers.GetChildMemberWithName(
+"gcspr_el0"
+).GetValueAsUnsigned()
+if expected_gcspr_el0 is not None:
+self.assertEqual(expected_gcspr_el0, gcspr_el0)
+
+return gcs_features_enabled, gcs_features_locked, gcspr_el0
+
+enabled, locked, spr_el0 = check_gcs_registers()
+
+# Features enabled should have at least the enable bit set, it could
have
+# others depending on what the C library did.
+self.assertTrue(enabled & 1, "Expected GCS enable bit to be set.")
+
+# Features locked we cannot predict, we will just assert that it
remains
+# the same as we continue.
+
+# spr_el0 will point to some memory region that is a shadow stack
region.
+self.expect(f"memory region {spr_el0}", substrs=["shadow stack: yes"])
+
+# Continue into test_func2, where the GCS pointer should have been
+# decremented, and the other registers remain the same.
+self.runCmd("continue")
+
+self.expect(
+"thread list",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stopped", "stop reason = breakpoint"],
+)
+
+_, _, spr_el0 = check_gcs_registers(enabled, locked, spr_el0 - 8)
+
+# Modify the control stack pointer to cause a fault.
+spr_el0 += 8
+self.runCmd(f"register write gcspr_el0 {spr_el0}")
+self.expect(
+"register read gcspr_el0", substrs=[f"gcspr_el0 =
0x{spr_el0:016x}"]
+)
+
+# If we wrote it back correctly, we will now fault but don't pass this
+# signal to the application.
+self.runCmd("process handle SIGSEGV --pass false")
+self.runCmd("continue")
+
+self.expect(
+"thread list",
+"Expected stopped by SIGSEGV.",
+substrs=[
+"stopped",
+"stop reason = signal SIGSEGV: control protection fault",
+],
+)
+
+# Any combination of lock bits could be set. Flip then restore one of
them.
DavidSpickett wrote:
Will add more explanation.
https://github.com/llvm/llvm-project/pull/123720
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Add Guarded Control Stack registers (PR #123720)
@@ -2,8 +2,8 @@ #include #include -#ifndef HWCAP2_GCS -#define HWCAP2_GCS (1UL << 63) +#ifndef HWCAP_GCS +#define HWCAP_GCS (1UL << 32) DavidSpickett wrote: Yes that is strange. Initially it was a HWCAP2 value but it was moved for some reason. I think this is just the documentation being out of sync because the tests use HWCAP - https://github.com/torvalds/linux/blob/master/tools/testing/selftests/arm64/signal/test_signals.c#L30. As does my code. I've asked the person who wrote the GCS support to confirm whether this is the case. https://github.com/llvm/llvm-project/pull/123720 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] WIP: Stop using replicated variable ids (PR #124232)
https://github.com/Anthony-Eid updated
https://github.com/llvm/llvm-project/pull/124232
>From e741fc75ccb6e2a725b3b26dd4c503cdea6c5fbb Mon Sep 17 00:00:00 2001
From: Anthony Eid
Date: Fri, 24 Jan 2025 00:43:39 -0500
Subject: [PATCH 1/2] Stop using replicated variable ids
---
lldb/tools/lldb-dap/DAP.cpp | 28 ---
lldb/tools/lldb-dap/DAP.h | 15 ++--
lldb/tools/lldb-dap/JSONUtils.cpp | 6 ++--
lldb/tools/lldb-dap/JSONUtils.h | 3 +-
lldb/tools/lldb-dap/ProgressEvent.h | 5 +++
lldb/tools/lldb-dap/lldb-dap.cpp| 54 ++---
6 files changed, 80 insertions(+), 31 deletions(-)
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 35250d9eef608a..0836fbf9f32dd6 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -8,6 +8,7 @@
#include
#include
+#include
#include
#include
@@ -137,6 +138,15 @@ void DAP::PopulateExceptionBreakpoints() {
});
}
+std::optional DAP::ScopeKind(const int64_t variablesReference) {
+ auto iter = scope_kinds.find(variablesReference);
+ if (iter != scope_kinds.end()) {
+return iter->second;
+ }
+
+ return std::nullopt;
+}
+
ExceptionBreakpoint *DAP::GetExceptionBreakpoint(const std::string &filter) {
// PopulateExceptionBreakpoints() is called after g_dap.debugger is created
// in a request-initialize.
@@ -476,12 +486,22 @@ lldb::SBFrame DAP::GetLLDBFrame(const llvm::json::Object
&arguments) {
llvm::json::Value DAP::CreateTopLevelScopes() {
llvm::json::Array scopes;
- scopes.emplace_back(
- CreateScope("Locals", VARREF_LOCALS, variables.locals.GetSize(), false));
- scopes.emplace_back(CreateScope("Globals", VARREF_GLOBALS,
+
+ scopes.emplace_back(CreateScope("Locals", variables.next_temporary_var_ref,
+ ScopeKind::Locals,
variables.locals.GetSize(),
+ false));
+ scope_kinds[variables.next_temporary_var_ref++] = ScopeKind::Locals;
+
+ scopes.emplace_back(CreateScope("Globals", variables.next_temporary_var_ref,
+ ScopeKind::Globals,
variables.globals.GetSize(), false));
- scopes.emplace_back(CreateScope("Registers", VARREF_REGS,
+ scope_kinds[variables.next_temporary_var_ref++] = ScopeKind::Globals;
+
+ scopes.emplace_back(CreateScope("Registers",
variables.next_temporary_var_ref,
+ ScopeKind::Registers,
variables.registers.GetSize(), false));
+ scope_kinds[variables.next_temporary_var_ref++] = ScopeKind::Registers;
+
return llvm::json::Value(std::move(scopes));
}
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index ae496236f13369..68399e2e0410c7 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -13,6 +13,7 @@
#include
#include
#include
+#include
#include
#include "llvm/ADT/DenseMap.h"
@@ -39,6 +40,7 @@
#include "InstructionBreakpoint.h"
#include "ProgressEvent.h"
#include "SourceBreakpoint.h"
+#include "lldb/API/SBValueList.h"
#define VARREF_LOCALS (int64_t)1
#define VARREF_GLOBALS (int64_t)2
@@ -86,6 +88,8 @@ struct Variables {
int64_t next_temporary_var_ref{VARREF_FIRST_VAR_IDX};
int64_t next_permanent_var_ref{PermanentVariableStartIndex};
+ std::set added_frames;
+
/// Variables that are alive in this stop state.
/// Will be cleared when debuggee resumes.
llvm::DenseMap referenced_variables;
@@ -117,25 +121,27 @@ struct Variables {
struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface {
DAP &dap;
- explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {};
+ explicit StartDebuggingRequestHandler(DAP &d) : dap(d){};
bool DoExecute(lldb::SBDebugger debugger, char **command,
lldb::SBCommandReturnObject &result) override;
};
struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface {
DAP &dap;
- explicit ReplModeRequestHandler(DAP &d) : dap(d) {};
+ explicit ReplModeRequestHandler(DAP &d) : dap(d){};
bool DoExecute(lldb::SBDebugger debugger, char **command,
lldb::SBCommandReturnObject &result) override;
};
struct SendEventRequestHandler : public lldb::SBCommandPluginInterface {
DAP &dap;
- explicit SendEventRequestHandler(DAP &d) : dap(d) {};
+ explicit SendEventRequestHandler(DAP &d) : dap(d){};
bool DoExecute(lldb::SBDebugger debugger, char **command,
lldb::SBCommandReturnObject &result) override;
};
+enum ScopeKind { Locals, Globals, Registers };
+
struct DAP {
llvm::StringRef debug_adaptor_path;
InputStream input;
@@ -159,6 +165,7 @@ struct DAP {
std::vector exit_commands;
std::vector stop_commands;
std::vector terminate_commands;
+ std::map scope_kinds;
// Map step in target id to list of function targets that user can choose.
llvm::DenseMap step_in_targets;
// A copy of the last
[Lldb-commits] [lldb] [lldb] Remove support and workarounds for Android 4 and older (PR #124047)
labath wrote: All of these workarounds are very old. They date back to the time that API level 21 was brand sparkling new, we were trying to support everything back to level 8, and NDK headers were missing many important functions. This is why you can find references to highly questionable practices like linking to an level 21 libc but running on a leve 8 device. Android support hasn't gotten much attention since those days (though it seems it still works, yay), so nobody went through the trouble of figuring out which one of them is still needed. > that's especially weird because execve() doesn't use $PATH ... it's the > members of the exec family that have a 'p' in the name that do. It's been a long time, but I'm pretty sure this was due to a test which ran something with an empty environment (some of our tests do that, sometimes deliberately, mostly by accident). It probably wasn't the binary itself that failed to launch, but that it then couldn't find the executable that *it* was trying to launch. The comment could be incorrect. Maybe it's (ba)sh who inserts `PATH=/bin:/usr/bin` into the environment if the variable is missing? Bottom line: I'm pretty sure that the only reason this needs to be there is to make the test suite happy (and maybe not even that, if things have changed since that time). I don't think anyone is running the lldb test suite on android. If you're not planning to start doing that, I think we could delete it. > i don't know what lldb's deprecation policy is (or Android Studio's, as > "deliverers of the lldb"), but the ndk at least still supports api >= 21 I was asking this because I thought `android-4` is newer than `api-21`. It looks like that's not the case (though I'm pretty sure we did not call it "android 4" back in those days), so I think it's fine to keep it. It's pretty well encapsulated, and I don't think it has gotten in the way of anything. > is there a workaround in android n? As I recall, we were cherry-picking the fix into all supported kernel branches (and maybe even adding conformance tests -- I'm not sure), so that at least new devices (starting from a new kernel branch should have it). But yes, it's possible that claim is too optimistic... https://github.com/llvm/llvm-project/pull/124047 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Enable the use of dladdr() on Android (PR #124187)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/124187 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Check Android API for existence of getgrgid_r() introduced in 24 (PR #124182)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/124182 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] a6cfde6 - [lldb] Check Android API for existence of getgrgid_r() introduced in 24 (#124182)
Author: Brad Smith
Date: 2025-01-24T03:18:36-05:00
New Revision: a6cfde62bb89e595db2bf7bb8ae810293d8edf26
URL:
https://github.com/llvm/llvm-project/commit/a6cfde62bb89e595db2bf7bb8ae810293d8edf26
DIFF:
https://github.com/llvm/llvm-project/commit/a6cfde62bb89e595db2bf7bb8ae810293d8edf26.diff
LOG: [lldb] Check Android API for existence of getgrgid_r() introduced in 24
(#124182)
Added:
Modified:
lldb/source/Host/posix/HostInfoPosix.cpp
Removed:
diff --git a/lldb/source/Host/posix/HostInfoPosix.cpp
b/lldb/source/Host/posix/HostInfoPosix.cpp
index 23ba3177de317a..879dccfd353be5 100644
--- a/lldb/source/Host/posix/HostInfoPosix.cpp
+++ b/lldb/source/Host/posix/HostInfoPosix.cpp
@@ -119,7 +119,7 @@ std::optional
PosixUserIDResolver::DoGetUserName(id_t uid) {
}
std::optional PosixUserIDResolver::DoGetGroupName(id_t gid) {
-#ifndef __ANDROID__
+#if !defined(__ANDROID__) || __ANDROID_API__ >= 24
char group_buffer[PATH_MAX];
size_t group_buffer_size = sizeof(group_buffer);
struct group group_info;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Check Android API for existence of getgrgid_r() introduced in 24 (PR #124182)
https://github.com/brad0 closed https://github.com/llvm/llvm-project/pull/124182 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] eda1699 - [lldb] Enable the use of dladdr() on Android (#124187)
Author: Brad Smith
Date: 2025-01-24T03:19:34-05:00
New Revision: eda16991adeb078647b2d239fcf666ddece5c30a
URL:
https://github.com/llvm/llvm-project/commit/eda16991adeb078647b2d239fcf666ddece5c30a
DIFF:
https://github.com/llvm/llvm-project/commit/eda16991adeb078647b2d239fcf666ddece5c30a.diff
LOG: [lldb] Enable the use of dladdr() on Android (#124187)
dladdr() was introduced 15 years ago.
Added:
Modified:
lldb/source/Host/common/Host.cpp
Removed:
diff --git a/lldb/source/Host/common/Host.cpp
b/lldb/source/Host/common/Host.cpp
index 7b2bae74e196fe..fdb623667bc251 100644
--- a/lldb/source/Host/common/Host.cpp
+++ b/lldb/source/Host/common/Host.cpp
@@ -352,7 +352,6 @@ bool Host::ResolveExecutableInBundle(FileSpec &file) {
return false; }
FileSpec Host::GetModuleFileSpecForHostAddress(const void *host_addr) {
FileSpec module_filespec;
-#if !defined(__ANDROID__)
Dl_info info;
if (::dladdr(host_addr, &info)) {
if (info.dli_fname) {
@@ -360,7 +359,6 @@ FileSpec Host::GetModuleFileSpecForHostAddress(const void
*host_addr) {
FileSystem::Instance().Resolve(module_filespec);
}
}
-#endif
return module_filespec;
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Enable the use of dladdr() on Android (PR #124187)
https://github.com/brad0 closed https://github.com/llvm/llvm-project/pull/124187 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Use the first address range as the function address (PR #122440)
https://github.com/DavidSpickett approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/122440 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Add Guarded Control Stack registers (PR #123720)
https://github.com/DavidSpickett updated
https://github.com/llvm/llvm-project/pull/123720
>From 951a38910e49f3e93dc6c9a084e955ab01ad4c49 Mon Sep 17 00:00:00 2001
From: David Spickett
Date: Fri, 9 Aug 2024 11:56:29 +0100
Subject: [PATCH 1/2] [lldb][AArch64] Add Guarded Control Stack registers
The Guarded Control Stack extension implements a shadow stack and
the Linux kernel provides access to 3 registers for it via ptrace.
struct user_gcs {
__u64 features_enabled;
__u64 features_locked;
__u64 gcspr_el0;
};
This commit adds support for reading those from a live process.
The first 2 are pseudo registers based on the real control register
and the 3rd is a real register. This is the stack pointer for the
guarded stack.
I have added a "gcs_" prefix to the "features" registers so that
they have a clear name when shown individually. Also this means
they will tab complete from "gcs", and be next to gcspr_el0 in
any sorted lists of registers.
Guarded Control Stack Registers:
gcs_features_enabled = 0x
gcs_features_locked = 0x
gcspr_el0 = 0x
Testing is more of the usual, where possible I'm writing a register
then doing something in the program to confirm the value was actually
sent to ptrace.
---
.../NativeRegisterContextLinux_arm64.cpp | 86 +++
.../Linux/NativeRegisterContextLinux_arm64.h | 16 +++
.../Utility/RegisterContextPOSIX_arm64.cpp| 4 +
.../Utility/RegisterContextPOSIX_arm64.h | 1 +
.../Utility/RegisterInfoPOSIX_arm64.cpp | 39 -
.../Process/Utility/RegisterInfoPOSIX_arm64.h | 7 +
.../linux/aarch64/gcs/TestAArch64LinuxGCS.py | 134 ++
lldb/test/API/linux/aarch64/gcs/main.c| 23 ++-
8 files changed, 305 insertions(+), 5 deletions(-)
diff --git
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 6056f3001fed6e..efd3385c46e92f 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -64,8 +64,14 @@
#define NT_ARM_FPMR 0x40e /* Floating point mode register */
#endif
+#ifndef NT_ARM_GCS
+#define NT_ARM_GCS 0x410 /* Guarded Control Stack control registers */
+#endif
+
#define HWCAP_PACA (1 << 30)
+#define HWCAP_GCS (1UL << 32)
+
#define HWCAP2_MTE (1 << 18)
#define HWCAP2_FPMR (1UL << 48)
@@ -150,6 +156,8 @@
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskMTE);
if (*auxv_at_hwcap2 & HWCAP2_FPMR)
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskFPMR);
+ if (*auxv_at_hwcap & HWCAP_GCS)
+opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskGCS);
}
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskTLS);
@@ -193,6 +201,7 @@
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
::memset(&m_pac_mask, 0, sizeof(m_pac_mask));
::memset(&m_tls_regs, 0, sizeof(m_tls_regs));
::memset(&m_sme_pseudo_regs, 0, sizeof(m_sme_pseudo_regs));
+ ::memset(&m_gcs_regs, 0, sizeof(m_gcs_regs));
std::fill(m_zt_reg.begin(), m_zt_reg.end(), 0);
m_mte_ctrl_reg = 0;
@@ -213,6 +222,7 @@
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
m_tls_is_valid = false;
m_zt_buffer_is_valid = false;
m_fpmr_is_valid = false;
+ m_gcs_is_valid = false;
// SME adds the tpidr2 register
m_tls_size = GetRegisterInfo().IsSSVEPresent() ? sizeof(m_tls_regs)
@@ -433,6 +443,14 @@ NativeRegisterContextLinux_arm64::ReadRegister(const
RegisterInfo *reg_info,
offset = reg_info->byte_offset - GetRegisterInfo().GetFPMROffset();
assert(offset < GetFPMRBufferSize());
src = (uint8_t *)GetFPMRBuffer() + offset;
+ } else if (IsGCS(reg)) {
+error = ReadGCS();
+if (error.Fail())
+ return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetGCSOffset();
+assert(offset < GetGCSBufferSize());
+src = (uint8_t *)GetGCSBuffer() + offset;
} else
return Status::FromErrorString(
"failed - register wasn't recognized to be a GPR or an FPR, "
@@ -657,6 +675,17 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
return WriteFPMR();
+ } else if (IsGCS(reg)) {
+error = ReadGCS();
+if (error.Fail())
+ return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetGCSOffset();
+assert(offset < GetGCSBufferSize());
+dst = (uint8_t *)GetGCSBuffer() + offset;
+::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
+
+return WriteGCS();
}
return Status::FromErrorString("Failed to write register value");
@@ -672,6 +701,7 @@ enum RegisterSetType : uint32_t {
SME, // ZA only, because SVCR and SVG are pseudo registers.
SME2, // ZT only.
[Lldb-commits] [lldb] [lldb][AArch64] Add Guarded Control Stack registers (PR #123720)
@@ -83,3 +83,137 @@ def test_gcs_fault(self):
"stop reason = signal SIGSEGV: control protection fault",
],
)
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])
+def test_gcs_registers(self):
+if not self.isAArch64GCS():
+self.skipTest("Target must support GCS.")
+
+self.build()
+self.runCmd("file " + self.getBuildArtifact("a.out"),
CURRENT_EXECUTABLE_SET)
+
+self.runCmd("b test_func")
+self.runCmd("b test_func2")
+self.runCmd("run", RUN_SUCCEEDED)
+
+if self.process().GetState() == lldb.eStateExited:
+self.fail("Test program failed to run.")
+
+self.expect(
+"thread list",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stopped", "stop reason = breakpoint"],
+)
+
+self.expect("register read --all", substrs=["Guarded Control Stack
Registers:"])
+
+def check_gcs_registers(
+expected_gcs_features_enabled=None,
+expected_gcs_features_locked=None,
+expected_gcspr_el0=None,
+):
+thread = self.dbg.GetSelectedTarget().process.GetThreadAtIndex(0)
+registerSets = thread.GetFrameAtIndex(0).GetRegisters()
+gcs_registers = registerSets.GetFirstValueByName(
+r"Guarded Control Stack Registers"
+)
+
+gcs_features_enabled = gcs_registers.GetChildMemberWithName(
+"gcs_features_enabled"
+).GetValueAsUnsigned()
+if expected_gcs_features_enabled is not None:
+self.assertEqual(expected_gcs_features_enabled,
gcs_features_enabled)
+
+gcs_features_locked = gcs_registers.GetChildMemberWithName(
+"gcs_features_locked"
+).GetValueAsUnsigned()
+if expected_gcs_features_locked is not None:
+self.assertEqual(expected_gcs_features_locked,
gcs_features_locked)
+
+gcspr_el0 = gcs_registers.GetChildMemberWithName(
+"gcspr_el0"
+).GetValueAsUnsigned()
+if expected_gcspr_el0 is not None:
+self.assertEqual(expected_gcspr_el0, gcspr_el0)
+
+return gcs_features_enabled, gcs_features_locked, gcspr_el0
+
+enabled, locked, spr_el0 = check_gcs_registers()
+
+# Features enabled should have at least the enable bit set, it could
have
+# others depending on what the C library did.
+self.assertTrue(enabled & 1, "Expected GCS enable bit to be set.")
+
+# Features locked we cannot predict, we will just assert that it
remains
+# the same as we continue.
+
+# spr_el0 will point to some memory region that is a shadow stack
region.
+self.expect(f"memory region {spr_el0}", substrs=["shadow stack: yes"])
+
+# Continue into test_func2, where the GCS pointer should have been
+# decremented, and the other registers remain the same.
+self.runCmd("continue")
+
+self.expect(
+"thread list",
+STOPPED_DUE_TO_BREAKPOINT,
+substrs=["stopped", "stop reason = breakpoint"],
+)
+
+_, _, spr_el0 = check_gcs_registers(enabled, locked, spr_el0 - 8)
+
+# Modify the control stack pointer to cause a fault.
+spr_el0 += 8
+self.runCmd(f"register write gcspr_el0 {spr_el0}")
+self.expect(
+"register read gcspr_el0", substrs=[f"gcspr_el0 =
0x{spr_el0:016x}"]
+)
+
+# If we wrote it back correctly, we will now fault but don't pass this
+# signal to the application.
+self.runCmd("process handle SIGSEGV --pass false")
+self.runCmd("continue")
+
+self.expect(
+"thread list",
+"Expected stopped by SIGSEGV.",
+substrs=[
+"stopped",
+"stop reason = signal SIGSEGV: control protection fault",
+],
+)
+
+# Any combination of lock bits could be set. Flip then restore one of
them.
DavidSpickett wrote:
I added bunch more explanation and moved the lock bits part so that we do that,
then we cause the fault and handle it. Before they were interleaved for no
particular reason.
https://github.com/llvm/llvm-project/pull/123720
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)
llvmbot wrote:
@llvm/pr-subscribers-backend-nvptx
Author: Jeremy Morse (jmorse)
Changes
As part of the "RemoveDIs" work to eliminate debug intrinsics, we're replacing
methods that use Instruction*'s as positions with iterators. A number of these
(such as getFirstNonPHIOrDbg) are sufficiently infrequently used that we can
just replace the pointer-returning version with an iterator-returning version,
hopefully without much/any disruption.
Thus this patch has getFirstNonPHIOrDbg and getFirstNonPHIOrDbgOrLifetime
return an iterator, and updates all call-sites. There are no concerns about the
iterators returned being converted to Instruction*'s and losing the debug-info
bit: because the methods skip debug intrinsics, the iterator head bit is always
false anyway.
---
Patch is 20.61 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/124287.diff
19 Files Affected:
- (modified) lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp (+3-3)
- (modified) llvm/include/llvm/IR/BasicBlock.h (+8-10)
- (modified) llvm/lib/CodeGen/CodeGenPrepare.cpp (+1-1)
- (modified) llvm/lib/IR/BasicBlock.cpp (+19-9)
- (modified) llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp (+1-1)
- (modified) llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp (+2-2)
- (modified) llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp (+1-1)
- (modified) llvm/lib/Transforms/Coroutines/CoroFrame.cpp (+3-2)
- (modified) llvm/lib/Transforms/Coroutines/CoroSplit.cpp (+1-1)
- (modified) llvm/lib/Transforms/IPO/IROutliner.cpp (+1-1)
- (modified) llvm/lib/Transforms/IPO/SCCP.cpp (+2-2)
- (modified) llvm/lib/Transforms/IPO/SampleProfile.cpp (+1-1)
- (modified) llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
(+2-2)
- (modified) llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp (+1-1)
- (modified) llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp (+1-1)
- (modified) llvm/lib/Transforms/Utils/CodeMoverUtils.cpp (+1-1)
- (modified) llvm/lib/Transforms/Utils/IRNormalizer.cpp (+1-1)
- (modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+2-2)
- (modified) llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp (+16-8)
``diff
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
index 6c728f34474898..a414ad652448e9 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -66,7 +66,7 @@ static llvm::Value *FindEntryInstruction(llvm::Function
*function) {
if (function->empty())
return nullptr;
- return function->getEntryBlock().getFirstNonPHIOrDbg();
+ return &*function->getEntryBlock().getFirstNonPHIOrDbg();
}
IRForTarget::IRForTarget(lldb_private::ClangExpressionDeclMap *decl_map,
@@ -361,7 +361,7 @@ bool IRForTarget::CreateResultVariable(llvm::Function
&llvm_function) {
// there's nothing to put into its equivalent persistent variable.
BasicBlock &entry_block(llvm_function.getEntryBlock());
-Instruction *first_entry_instruction(entry_block.getFirstNonPHIOrDbg());
+Instruction *first_entry_instruction(&*entry_block.getFirstNonPHIOrDbg());
if (!first_entry_instruction)
return false;
@@ -1505,7 +1505,7 @@ bool IRForTarget::ReplaceVariables(Function
&llvm_function) {
LLDB_LOG(log, "Arg: \"{0}\"", PrintValue(argument));
BasicBlock &entry_block(llvm_function.getEntryBlock());
- Instruction *FirstEntryInstruction(entry_block.getFirstNonPHIOrDbg());
+ Instruction *FirstEntryInstruction(&*entry_block.getFirstNonPHIOrDbg());
if (!FirstEntryInstruction) {
m_error_stream.Printf("Internal error [IRForTarget]: Couldn't find the "
diff --git a/llvm/include/llvm/IR/BasicBlock.h
b/llvm/include/llvm/IR/BasicBlock.h
index e22fe1e7e7dc8f..5df4dcecebc878 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -299,22 +299,20 @@ class BasicBlock final : public Value, // Basic blocks
are data objects also
/// Returns a pointer to the first instruction in this block that is not a
/// PHINode or a debug intrinsic, or any pseudo operation if \c SkipPseudoOp
/// is true.
- const Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) const;
- Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
-return const_cast(
-static_cast(this)->getFirstNonPHIOrDbg(
-SkipPseudoOp));
+ InstListType::const_iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true)
const;
+ InstListType::iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
+return static_cast(this)->getFirstNonPHIOrDbg(
+SkipPseudoOp).getNonConst();
}
/// Returns a pointer to the first instruction in this block that is not a
/// PHINode, a debug intrinsic, or a lifetime intrinsic, or any pseudo
/// operation if \c SkipPseudoOp is true.
- const Instruction *
+ InstListType::const_iterator
getFi
[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)
llvmbot wrote:
@llvm/pr-subscribers-backend-aarch64
Author: Jeremy Morse (jmorse)
Changes
As part of the "RemoveDIs" work to eliminate debug intrinsics, we're replacing
methods that use Instruction*'s as positions with iterators. A number of these
(such as getFirstNonPHIOrDbg) are sufficiently infrequently used that we can
just replace the pointer-returning version with an iterator-returning version,
hopefully without much/any disruption.
Thus this patch has getFirstNonPHIOrDbg and getFirstNonPHIOrDbgOrLifetime
return an iterator, and updates all call-sites. There are no concerns about the
iterators returned being converted to Instruction*'s and losing the debug-info
bit: because the methods skip debug intrinsics, the iterator head bit is always
false anyway.
---
Patch is 20.61 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/124287.diff
19 Files Affected:
- (modified) lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp (+3-3)
- (modified) llvm/include/llvm/IR/BasicBlock.h (+8-10)
- (modified) llvm/lib/CodeGen/CodeGenPrepare.cpp (+1-1)
- (modified) llvm/lib/IR/BasicBlock.cpp (+19-9)
- (modified) llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp (+1-1)
- (modified) llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp (+2-2)
- (modified) llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp (+1-1)
- (modified) llvm/lib/Transforms/Coroutines/CoroFrame.cpp (+3-2)
- (modified) llvm/lib/Transforms/Coroutines/CoroSplit.cpp (+1-1)
- (modified) llvm/lib/Transforms/IPO/IROutliner.cpp (+1-1)
- (modified) llvm/lib/Transforms/IPO/SCCP.cpp (+2-2)
- (modified) llvm/lib/Transforms/IPO/SampleProfile.cpp (+1-1)
- (modified) llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
(+2-2)
- (modified) llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp (+1-1)
- (modified) llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp (+1-1)
- (modified) llvm/lib/Transforms/Utils/CodeMoverUtils.cpp (+1-1)
- (modified) llvm/lib/Transforms/Utils/IRNormalizer.cpp (+1-1)
- (modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+2-2)
- (modified) llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp (+16-8)
``diff
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
index 6c728f34474898..a414ad652448e9 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -66,7 +66,7 @@ static llvm::Value *FindEntryInstruction(llvm::Function
*function) {
if (function->empty())
return nullptr;
- return function->getEntryBlock().getFirstNonPHIOrDbg();
+ return &*function->getEntryBlock().getFirstNonPHIOrDbg();
}
IRForTarget::IRForTarget(lldb_private::ClangExpressionDeclMap *decl_map,
@@ -361,7 +361,7 @@ bool IRForTarget::CreateResultVariable(llvm::Function
&llvm_function) {
// there's nothing to put into its equivalent persistent variable.
BasicBlock &entry_block(llvm_function.getEntryBlock());
-Instruction *first_entry_instruction(entry_block.getFirstNonPHIOrDbg());
+Instruction *first_entry_instruction(&*entry_block.getFirstNonPHIOrDbg());
if (!first_entry_instruction)
return false;
@@ -1505,7 +1505,7 @@ bool IRForTarget::ReplaceVariables(Function
&llvm_function) {
LLDB_LOG(log, "Arg: \"{0}\"", PrintValue(argument));
BasicBlock &entry_block(llvm_function.getEntryBlock());
- Instruction *FirstEntryInstruction(entry_block.getFirstNonPHIOrDbg());
+ Instruction *FirstEntryInstruction(&*entry_block.getFirstNonPHIOrDbg());
if (!FirstEntryInstruction) {
m_error_stream.Printf("Internal error [IRForTarget]: Couldn't find the "
diff --git a/llvm/include/llvm/IR/BasicBlock.h
b/llvm/include/llvm/IR/BasicBlock.h
index e22fe1e7e7dc8f..5df4dcecebc878 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -299,22 +299,20 @@ class BasicBlock final : public Value, // Basic blocks
are data objects also
/// Returns a pointer to the first instruction in this block that is not a
/// PHINode or a debug intrinsic, or any pseudo operation if \c SkipPseudoOp
/// is true.
- const Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) const;
- Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
-return const_cast(
-static_cast(this)->getFirstNonPHIOrDbg(
-SkipPseudoOp));
+ InstListType::const_iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true)
const;
+ InstListType::iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
+return static_cast(this)->getFirstNonPHIOrDbg(
+SkipPseudoOp).getNonConst();
}
/// Returns a pointer to the first instruction in this block that is not a
/// PHINode, a debug intrinsic, or a lifetime intrinsic, or any pseudo
/// operation if \c SkipPseudoOp is true.
- const Instruction *
+ InstListType::const_iterator
get
[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)
llvmbot wrote:
@llvm/pr-subscribers-llvm-transforms
@llvm/pr-subscribers-llvm-ir
Author: Jeremy Morse (jmorse)
Changes
As part of the "RemoveDIs" work to eliminate debug intrinsics, we're replacing
methods that use Instruction*'s as positions with iterators. A number of these
(such as getFirstNonPHIOrDbg) are sufficiently infrequently used that we can
just replace the pointer-returning version with an iterator-returning version,
hopefully without much/any disruption.
Thus this patch has getFirstNonPHIOrDbg and getFirstNonPHIOrDbgOrLifetime
return an iterator, and updates all call-sites. There are no concerns about the
iterators returned being converted to Instruction*'s and losing the debug-info
bit: because the methods skip debug intrinsics, the iterator head bit is always
false anyway.
---
Patch is 20.61 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/124287.diff
19 Files Affected:
- (modified) lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp (+3-3)
- (modified) llvm/include/llvm/IR/BasicBlock.h (+8-10)
- (modified) llvm/lib/CodeGen/CodeGenPrepare.cpp (+1-1)
- (modified) llvm/lib/IR/BasicBlock.cpp (+19-9)
- (modified) llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp (+1-1)
- (modified) llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp (+2-2)
- (modified) llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp (+1-1)
- (modified) llvm/lib/Transforms/Coroutines/CoroFrame.cpp (+3-2)
- (modified) llvm/lib/Transforms/Coroutines/CoroSplit.cpp (+1-1)
- (modified) llvm/lib/Transforms/IPO/IROutliner.cpp (+1-1)
- (modified) llvm/lib/Transforms/IPO/SCCP.cpp (+2-2)
- (modified) llvm/lib/Transforms/IPO/SampleProfile.cpp (+1-1)
- (modified) llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
(+2-2)
- (modified) llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp (+1-1)
- (modified) llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp (+1-1)
- (modified) llvm/lib/Transforms/Utils/CodeMoverUtils.cpp (+1-1)
- (modified) llvm/lib/Transforms/Utils/IRNormalizer.cpp (+1-1)
- (modified) llvm/lib/Transforms/Utils/SimplifyCFG.cpp (+2-2)
- (modified) llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp (+16-8)
``diff
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
index 6c728f34474898..a414ad652448e9 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -66,7 +66,7 @@ static llvm::Value *FindEntryInstruction(llvm::Function
*function) {
if (function->empty())
return nullptr;
- return function->getEntryBlock().getFirstNonPHIOrDbg();
+ return &*function->getEntryBlock().getFirstNonPHIOrDbg();
}
IRForTarget::IRForTarget(lldb_private::ClangExpressionDeclMap *decl_map,
@@ -361,7 +361,7 @@ bool IRForTarget::CreateResultVariable(llvm::Function
&llvm_function) {
// there's nothing to put into its equivalent persistent variable.
BasicBlock &entry_block(llvm_function.getEntryBlock());
-Instruction *first_entry_instruction(entry_block.getFirstNonPHIOrDbg());
+Instruction *first_entry_instruction(&*entry_block.getFirstNonPHIOrDbg());
if (!first_entry_instruction)
return false;
@@ -1505,7 +1505,7 @@ bool IRForTarget::ReplaceVariables(Function
&llvm_function) {
LLDB_LOG(log, "Arg: \"{0}\"", PrintValue(argument));
BasicBlock &entry_block(llvm_function.getEntryBlock());
- Instruction *FirstEntryInstruction(entry_block.getFirstNonPHIOrDbg());
+ Instruction *FirstEntryInstruction(&*entry_block.getFirstNonPHIOrDbg());
if (!FirstEntryInstruction) {
m_error_stream.Printf("Internal error [IRForTarget]: Couldn't find the "
diff --git a/llvm/include/llvm/IR/BasicBlock.h
b/llvm/include/llvm/IR/BasicBlock.h
index e22fe1e7e7dc8f..5df4dcecebc878 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -299,22 +299,20 @@ class BasicBlock final : public Value, // Basic blocks
are data objects also
/// Returns a pointer to the first instruction in this block that is not a
/// PHINode or a debug intrinsic, or any pseudo operation if \c SkipPseudoOp
/// is true.
- const Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) const;
- Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
-return const_cast(
-static_cast(this)->getFirstNonPHIOrDbg(
-SkipPseudoOp));
+ InstListType::const_iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true)
const;
+ InstListType::iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
+return static_cast(this)->getFirstNonPHIOrDbg(
+SkipPseudoOp).getNonConst();
}
/// Returns a pointer to the first instruction in this block that is not a
/// PHINode, a debug intrinsic, or a lifetime intrinsic, or any pseudo
/// operation if \c SkipPseudoOp is true.
- const Instruction *
+ InstLi
[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)
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 c546b5317c518987a5f45dd4c4d25321a955c758
3d2aa2734d6cb49c43565e3ac8584ba8130fe302 --extensions cpp,h --
lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
llvm/include/llvm/IR/BasicBlock.h llvm/lib/CodeGen/CodeGenPrepare.cpp
llvm/lib/IR/BasicBlock.cpp
llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
llvm/lib/Target/AMDGPU/SIAnnotateControlFlow.cpp
llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp
llvm/lib/Transforms/Coroutines/CoroFrame.cpp
llvm/lib/Transforms/Coroutines/CoroSplit.cpp
llvm/lib/Transforms/IPO/IROutliner.cpp llvm/lib/Transforms/IPO/SCCP.cpp
llvm/lib/Transforms/IPO/SampleProfile.cpp
llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp
llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp
llvm/lib/Transforms/Utils/CodeMoverUtils.cpp
llvm/lib/Transforms/Utils/IRNormalizer.cpp
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
``
View the diff from clang-format here.
``diff
diff --git a/llvm/include/llvm/IR/BasicBlock.h
b/llvm/include/llvm/IR/BasicBlock.h
index 5df4dceceb..f2e34a7d24 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -299,10 +299,12 @@ public:
/// Returns a pointer to the first instruction in this block that is not a
/// PHINode or a debug intrinsic, or any pseudo operation if \c SkipPseudoOp
/// is true.
- InstListType::const_iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true)
const;
+ InstListType::const_iterator
+ getFirstNonPHIOrDbg(bool SkipPseudoOp = true) const;
InstListType::iterator getFirstNonPHIOrDbg(bool SkipPseudoOp = true) {
-return static_cast(this)->getFirstNonPHIOrDbg(
-SkipPseudoOp).getNonConst();
+return static_cast(this)
+->getFirstNonPHIOrDbg(SkipPseudoOp)
+.getNonConst();
}
/// Returns a pointer to the first instruction in this block that is not a
@@ -310,9 +312,11 @@ public:
/// operation if \c SkipPseudoOp is true.
InstListType::const_iterator
getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp = true) const;
- InstListType::iterator getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp =
true) {
-return static_cast(this)->getFirstNonPHIOrDbgOrLifetime(
-SkipPseudoOp).getNonConst();
+ InstListType::iterator
+ getFirstNonPHIOrDbgOrLifetime(bool SkipPseudoOp = true) {
+return static_cast(this)
+->getFirstNonPHIOrDbgOrLifetime(SkipPseudoOp)
+.getNonConst();
}
/// Returns an iterator to the first instruction in this block that is
diff --git a/llvm/lib/IR/BasicBlock.cpp b/llvm/lib/IR/BasicBlock.cpp
index 62cf9970ad..a078f0afac 100644
--- a/llvm/lib/IR/BasicBlock.cpp
+++ b/llvm/lib/IR/BasicBlock.cpp
@@ -416,7 +416,6 @@ BasicBlock::getFirstNonPHIOrDbgOrLifetime(bool
SkipPseudoOp) const {
// Signal that this comes after any debug records.
It.setHeadBit(false);
return It;
-
}
return end();
}
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index 1195478fdf..2319afb11d 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -1698,7 +1698,8 @@ static void eliminateSwiftErrorAlloca(Function &F,
AllocaInst *Alloca,
static void eliminateSwiftErrorArgument(Function &F, Argument &Arg,
coro::Shape &Shape,
SmallVectorImpl &AllocasToPromote) {
- IRBuilder<> Builder(&F.getEntryBlock(),
F.getEntryBlock().getFirstNonPHIOrDbg());
+ IRBuilder<> Builder(&F.getEntryBlock(),
+ F.getEntryBlock().getFirstNonPHIOrDbg());
auto ArgTy = cast(Arg.getType());
auto ValueTy = PointerType::getUnqual(F.getContext());
diff --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index 3bf412e4e5..58fb6b01e9 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -597,7 +597,8 @@ static void replaceSwiftErrorOps(Function &F, coro::Shape
&Shape,
}
// Create a swifterror alloca.
-IRBuilder<> Builder(&F.getEntryBlock(),
F.getEntryBlock().getFirstNonPHIOrDbg());
+IRBuilder<> Builder(&F.getEntryBlock(),
+F.getEntryBlock().getFirstNonPHIOrDbg());
auto Alloca = Builder.CreateAlloca(ValueTy);
Alloca->setSwiftError(true);
``
https://github.com/llvm/llvm-project/pull/124287
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Add Guarded Control Stack support for Linux core files (PR #124293)
https://github.com/DavidSpickett created
https://github.com/llvm/llvm-project/pull/124293
This allows you to read the same registers as you would for a live process.
As the content of proc/pid/smaps is not included in the core file,
we don't get the "ss" marker. The GCS region is stil in the list
though.
>From 951a38910e49f3e93dc6c9a084e955ab01ad4c49 Mon Sep 17 00:00:00 2001
From: David Spickett
Date: Fri, 9 Aug 2024 11:56:29 +0100
Subject: [PATCH 1/5] [lldb][AArch64] Add Guarded Control Stack registers
The Guarded Control Stack extension implements a shadow stack and
the Linux kernel provides access to 3 registers for it via ptrace.
struct user_gcs {
__u64 features_enabled;
__u64 features_locked;
__u64 gcspr_el0;
};
This commit adds support for reading those from a live process.
The first 2 are pseudo registers based on the real control register
and the 3rd is a real register. This is the stack pointer for the
guarded stack.
I have added a "gcs_" prefix to the "features" registers so that
they have a clear name when shown individually. Also this means
they will tab complete from "gcs", and be next to gcspr_el0 in
any sorted lists of registers.
Guarded Control Stack Registers:
gcs_features_enabled = 0x
gcs_features_locked = 0x
gcspr_el0 = 0x
Testing is more of the usual, where possible I'm writing a register
then doing something in the program to confirm the value was actually
sent to ptrace.
---
.../NativeRegisterContextLinux_arm64.cpp | 86 +++
.../Linux/NativeRegisterContextLinux_arm64.h | 16 +++
.../Utility/RegisterContextPOSIX_arm64.cpp| 4 +
.../Utility/RegisterContextPOSIX_arm64.h | 1 +
.../Utility/RegisterInfoPOSIX_arm64.cpp | 39 -
.../Process/Utility/RegisterInfoPOSIX_arm64.h | 7 +
.../linux/aarch64/gcs/TestAArch64LinuxGCS.py | 134 ++
lldb/test/API/linux/aarch64/gcs/main.c| 23 ++-
8 files changed, 305 insertions(+), 5 deletions(-)
diff --git
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 6056f3001fed6e..efd3385c46e92f 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -64,8 +64,14 @@
#define NT_ARM_FPMR 0x40e /* Floating point mode register */
#endif
+#ifndef NT_ARM_GCS
+#define NT_ARM_GCS 0x410 /* Guarded Control Stack control registers */
+#endif
+
#define HWCAP_PACA (1 << 30)
+#define HWCAP_GCS (1UL << 32)
+
#define HWCAP2_MTE (1 << 18)
#define HWCAP2_FPMR (1UL << 48)
@@ -150,6 +156,8 @@
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskMTE);
if (*auxv_at_hwcap2 & HWCAP2_FPMR)
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskFPMR);
+ if (*auxv_at_hwcap & HWCAP_GCS)
+opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskGCS);
}
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskTLS);
@@ -193,6 +201,7 @@
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
::memset(&m_pac_mask, 0, sizeof(m_pac_mask));
::memset(&m_tls_regs, 0, sizeof(m_tls_regs));
::memset(&m_sme_pseudo_regs, 0, sizeof(m_sme_pseudo_regs));
+ ::memset(&m_gcs_regs, 0, sizeof(m_gcs_regs));
std::fill(m_zt_reg.begin(), m_zt_reg.end(), 0);
m_mte_ctrl_reg = 0;
@@ -213,6 +222,7 @@
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
m_tls_is_valid = false;
m_zt_buffer_is_valid = false;
m_fpmr_is_valid = false;
+ m_gcs_is_valid = false;
// SME adds the tpidr2 register
m_tls_size = GetRegisterInfo().IsSSVEPresent() ? sizeof(m_tls_regs)
@@ -433,6 +443,14 @@ NativeRegisterContextLinux_arm64::ReadRegister(const
RegisterInfo *reg_info,
offset = reg_info->byte_offset - GetRegisterInfo().GetFPMROffset();
assert(offset < GetFPMRBufferSize());
src = (uint8_t *)GetFPMRBuffer() + offset;
+ } else if (IsGCS(reg)) {
+error = ReadGCS();
+if (error.Fail())
+ return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetGCSOffset();
+assert(offset < GetGCSBufferSize());
+src = (uint8_t *)GetGCSBuffer() + offset;
} else
return Status::FromErrorString(
"failed - register wasn't recognized to be a GPR or an FPR, "
@@ -657,6 +675,17 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
return WriteFPMR();
+ } else if (IsGCS(reg)) {
+error = ReadGCS();
+if (error.Fail())
+ return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetGCSOffset();
+assert(offset < GetGCSBufferSize());
+dst = (uint8_t *)GetGCSBuffer() + offset;
+::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
+
+return WriteGCS
[Lldb-commits] [lldb] [lldb][AArch64] Add Guarded Control Stack support for Linux core files (PR #124293)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: David Spickett (DavidSpickett)
Changes
This allows you to read the same registers as you would for a live process.
As the content of proc/pid/smaps is not included in the core file,
we don't get the "ss" marker. The GCS region is stil in the list
though.
---
Patch is 39.19 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/124293.diff
13 Files Affected:
- (modified) lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp (+75)
- (modified)
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp (+104)
- (modified)
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h (+16)
- (modified) lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
(+4)
- (modified) lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
(+1)
- (modified) lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
(+38-1)
- (modified) lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h (+7)
- (modified)
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp (+17)
- (modified)
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h (+1)
- (modified) lldb/source/Plugins/Process/elf-core/RegisterUtilities.h (+4)
- (modified) lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py (+307)
- (added) lldb/test/API/linux/aarch64/gcs/corefile ()
- (modified) lldb/test/API/linux/aarch64/gcs/main.c (+57-5)
``diff
diff --git a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
index 93b8141e97ef86..74047ea65788cf 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
+++ b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
@@ -60,6 +60,69 @@ ABISysV_arm64::CreateInstance(lldb::ProcessSP process_sp,
const ArchSpec &arch)
return ABISP();
}
+static Status PushToLinuxGuardedControlStack(addr_t return_addr,
+ RegisterContext *reg_ctx,
+ Thread &thread) {
+ Status err;
+
+ // If the Guarded Control Stack extension is present we may need to put the
+ // return address onto that stack.
+ const RegisterInfo *gcs_features_enabled_info =
+ reg_ctx->GetRegisterInfoByName("gcs_features_enabled");
+ if (!gcs_features_enabled_info)
+return err;
+
+ uint64_t gcs_features_enabled = reg_ctx->ReadRegisterAsUnsigned(
+ gcs_features_enabled_info, LLDB_INVALID_ADDRESS);
+ if (gcs_features_enabled == LLDB_INVALID_ADDRESS)
+return Status("Could not read GCS features enabled register.");
+
+ // Only attempt this if GCS is enabled. If it's not enabled then gcspr_el0
+ // may point to unmapped memory.
+ if ((gcs_features_enabled & 1) == 0)
+return err;
+
+ const RegisterInfo *gcspr_el0_info =
+ reg_ctx->GetRegisterInfoByName("gcspr_el0");
+ if (!gcspr_el0_info)
+return Status("Could not get register info for gcspr_el0.");
+
+ uint64_t gcspr_el0 =
+ reg_ctx->ReadRegisterAsUnsigned(gcspr_el0_info, LLDB_INVALID_ADDRESS);
+ if (gcspr_el0 == LLDB_INVALID_ADDRESS)
+return Status("Could not read gcspr_el0.");
+
+ // A link register entry on the GCS is 8 bytes.
+ gcspr_el0 -= 8;
+ if (!reg_ctx->WriteRegisterFromUnsigned(gcspr_el0_info, gcspr_el0))
+return Status(
+"Attempted to decrement gcspr_el0, but could not write to it.");
+
+ Status error;
+ size_t wrote = thread.GetProcess()->WriteMemory(gcspr_el0, &return_addr,
+ sizeof(return_addr), error);
+ if ((wrote != sizeof(return_addr) || error.Fail())) {
+// When PrepareTrivialCall fails, the register context is not restored,
+// unlike when an expression fails to execute. This is arguably a bug,
+// see https://github.com/llvm/llvm-project/issues/124269.
+// For now we are handling this here specifically. We can assume this
+// write will work as the one to decrement the register did.
+reg_ctx->WriteRegisterFromUnsigned(gcspr_el0_info, gcspr_el0 + 8);
+return Status("Failed to write new Guarded Control Stack entry.");
+ }
+
+ Log *log = GetLog(LLDBLog::Expressions);
+ LLDB_LOGF(log,
+"Pushed return address 0x%" PRIx64 " to Guarded Control Stack. "
+"gcspr_el0 was 0%" PRIx64 ", is now 0x%" PRIx64 ".",
+return_addr, gcspr_el0 - 8, gcspr_el0);
+
+ // gcspr_el0 will be restored to the original value by lldb-server after
+ // the call has finished, which serves as the "pop".
+
+ return err;
+}
+
bool ABISysV_arm64::PrepareTrivialCall(Thread &thread, addr_t sp,
addr_t func_addr, addr_t return_addr,
llvm::ArrayRef args) const {
@@ -87,6 +150,18 @@ bool ABISysV_arm64::PrepareTrivialCall(Thread &thread,
addr_t sp,
if (args.size() > 8)
return false;
+ // Do this first, as it's got the mo
[Lldb-commits] [lldb] [lldb][AArch64] Add Guarded Control Stack support for Linux core files (PR #124293)
DavidSpickett wrote: Only the latest commit is new, the rest is from https://github.com/llvm/llvm-project/pull/123918. I will rebase and fix conflicts as and when these get landed. https://github.com/llvm/llvm-project/pull/124293 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [llvm] [NFC][DebugInfo] Make some block-start-position methods return iterators (PR #124287)
https://github.com/jmorse created
https://github.com/llvm/llvm-project/pull/124287
As part of the "RemoveDIs" work to eliminate debug intrinsics, we're replacing
methods that use Instruction*'s as positions with iterators. A number of these
(such as getFirstNonPHIOrDbg) are sufficiently infrequently used that we can
just replace the pointer-returning version with an iterator-returning version,
hopefully without much/any disruption.
Thus this patch has getFirstNonPHIOrDbg and getFirstNonPHIOrDbgOrLifetime
return an iterator, and updates all call-sites. There are no concerns about the
iterators returned being converted to Instruction*'s and losing the debug-info
bit: because the methods skip debug intrinsics, the iterator head bit is always
false anyway.
>From 3d2aa2734d6cb49c43565e3ac8584ba8130fe302 Mon Sep 17 00:00:00 2001
From: Jeremy Morse
Date: Thu, 23 Jan 2025 12:24:14 +
Subject: [PATCH] [NFC][DebugInfo] Make some block-start-position methods
return iterators
As part of the "RemoveDIs" work to eliminate debug intrinsics, we're
replacing methods that use Instruction*'s as positions with iterators. A
number of these (such as getFirstNonPHIOrDbg) are sufficiently infrequently
used that we can just replace the pointer-returning version with an
iterator-returning version, hopefully without much/any disruption.
Thus this patch has getFirstNonPHIOrDbg and getFirstNonPHIOrDbgOrLifetime
return an iterator, and updates all call-sites. There are no concerns about
the iterators returned being converted to Instruction*'s and losing the
debug-info bit: because the methods skip debug intrinsics, the iterator
head bit is always false anyway.
---
.../ExpressionParser/Clang/IRForTarget.cpp| 6 ++--
llvm/include/llvm/IR/BasicBlock.h | 18 ++--
llvm/lib/CodeGen/CodeGenPrepare.cpp | 2 +-
llvm/lib/IR/BasicBlock.cpp| 28 +--
.../AArch64/AArch64TargetTransformInfo.cpp| 2 +-
.../Target/AMDGPU/SIAnnotateControlFlow.cpp | 4 +--
llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp | 2 +-
llvm/lib/Transforms/Coroutines/CoroFrame.cpp | 5 ++--
llvm/lib/Transforms/Coroutines/CoroSplit.cpp | 2 +-
llvm/lib/Transforms/IPO/IROutliner.cpp| 2 +-
llvm/lib/Transforms/IPO/SCCP.cpp | 4 +--
llvm/lib/Transforms/IPO/SampleProfile.cpp | 2 +-
.../InstCombineLoadStoreAlloca.cpp| 4 +--
.../Transforms/Scalar/CallSiteSplitting.cpp | 2 +-
.../Transforms/Scalar/SimpleLoopUnswitch.cpp | 2 +-
llvm/lib/Transforms/Utils/CodeMoverUtils.cpp | 2 +-
llvm/lib/Transforms/Utils/IRNormalizer.cpp| 2 +-
llvm/lib/Transforms/Utils/SimplifyCFG.cpp | 4 +--
.../Frontend/OpenMPIRBuilderTest.cpp | 24 ++--
19 files changed, 67 insertions(+), 50 deletions(-)
diff --git a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
index 6c728f34474898..a414ad652448e9 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp
@@ -66,7 +66,7 @@ static llvm::Value *FindEntryInstruction(llvm::Function
*function) {
if (function->empty())
return nullptr;
- return function->getEntryBlock().getFirstNonPHIOrDbg();
+ return &*function->getEntryBlock().getFirstNonPHIOrDbg();
}
IRForTarget::IRForTarget(lldb_private::ClangExpressionDeclMap *decl_map,
@@ -361,7 +361,7 @@ bool IRForTarget::CreateResultVariable(llvm::Function
&llvm_function) {
// there's nothing to put into its equivalent persistent variable.
BasicBlock &entry_block(llvm_function.getEntryBlock());
-Instruction *first_entry_instruction(entry_block.getFirstNonPHIOrDbg());
+Instruction *first_entry_instruction(&*entry_block.getFirstNonPHIOrDbg());
if (!first_entry_instruction)
return false;
@@ -1505,7 +1505,7 @@ bool IRForTarget::ReplaceVariables(Function
&llvm_function) {
LLDB_LOG(log, "Arg: \"{0}\"", PrintValue(argument));
BasicBlock &entry_block(llvm_function.getEntryBlock());
- Instruction *FirstEntryInstruction(entry_block.getFirstNonPHIOrDbg());
+ Instruction *FirstEntryInstruction(&*entry_block.getFirstNonPHIOrDbg());
if (!FirstEntryInstruction) {
m_error_stream.Printf("Internal error [IRForTarget]: Couldn't find the "
diff --git a/llvm/include/llvm/IR/BasicBlock.h
b/llvm/include/llvm/IR/BasicBlock.h
index e22fe1e7e7dc8f..5df4dcecebc878 100644
--- a/llvm/include/llvm/IR/BasicBlock.h
+++ b/llvm/include/llvm/IR/BasicBlock.h
@@ -299,22 +299,20 @@ class BasicBlock final : public Value, // Basic blocks
are data objects also
/// Returns a pointer to the first instruction in this block that is not a
/// PHINode or a debug intrinsic, or any pseudo operation if \c SkipPseudoOp
/// is true.
- const Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = true) const;
- Instruction *getFirstNonPHIOrDbg(bool SkipPseudoOp = tr
[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/124279 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Add Guarded Control Stack support for Linux core files (PR #124293)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/124293 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/124279
>From 23c7f4a3e067739a050964b3f1c2140787b0c31f Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Fri, 24 Jan 2025 13:33:07 +
Subject: [PATCH 1/2] [lldb][TypeSystem] Ensure that ParmVarDecls have the
correct DeclContext
While sifting through this part of the code I noticed that when we parse C++
methods, `DWARFASTParserClang` creates two sets of `ParmVarDecls`, one in
`ParseChildParameters` and once in `AddMethodToCXXRecordType`. The former is
unused when we're dealing with methods. Moreover, the `ParmVarDecls` we created
in `ParseChildParameters` were created with an incorrect `clang::DeclContext`
(namely the DeclContext of the function, and not the function itself). In
Clang, there's `ParmVarDecl::setOwningFunction` to adjust the DeclContext of a
parameter if the parameter was created before the FunctionDecl. But we never
used it.
This patch removes the `ParmVarDecl` creation from `ParseChildParameters` and
instead creates a `TypeSystemClang::CreateParameterDeclarations` that ensures
we set the DeclContext correctly.
This wasn't causing any concrete issues (that I know of), but was quite
surprising. And this way of setting the parameters seems easier to reason about
(in my opinion).
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 31 ++--
.../SymbolFile/DWARF/DWARFASTParserClang.h| 3 +-
.../SymbolFile/NativePDB/PdbAstBuilder.cpp| 2 +-
.../Plugins/SymbolFile/PDB/PDBASTParser.cpp | 4 +-
.../TypeSystem/Clang/TypeSystemClang.cpp | 43 ++---
.../TypeSystem/Clang/TypeSystemClang.h| 16 +-
lldb/unittests/Symbol/TestTypeSystemClang.cpp | 42 +
.../DWARF/DWARFASTParserClangTests.cpp| 154 ++
8 files changed, 247 insertions(+), 48 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index e77188bfbd2e4a..30dec4bbf91c10 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1272,7 +1272,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
return_clang_type = m_ast.GetBasicType(eBasicTypeVoid);
std::vector function_param_types;
- std::vector function_param_decls;
// Parse the function children for the parameters
@@ -1283,8 +1282,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
if (die.HasChildren()) {
ParseChildParameters(containing_decl_ctx, die, is_variadic,
- has_template_params, function_param_types,
- function_param_decls);
+ has_template_params, function_param_types);
}
bool is_cxx_method = DeclKindIsCXXClass(containing_decl_ctx->getDeclKind());
@@ -1414,11 +1412,15 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE
&die,
LinkDeclContextToDIE(function_decl, die);
- if (!function_param_decls.empty()) {
-m_ast.SetFunctionParameters(function_decl, function_param_decls);
+ const clang::FunctionProtoType *function_prototype(
+ llvm::cast(
+ ClangUtil::GetQualType(clang_type).getTypePtr()));
+ auto params = m_ast.CreateParameterDeclarations(function_decl,
+ *function_prototype);
+ if (!params.empty()) {
+function_decl->setParams(params);
if (template_function_decl)
- m_ast.SetFunctionParameters(template_function_decl,
- function_param_decls);
+ template_function_decl->setParams(params);
}
ClangASTMetadata metadata;
@@ -2380,7 +2382,6 @@
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
bool is_variadic = false;
bool has_template_params = false;
std::vector param_types;
- std::vector param_decls;
StreamString sstr;
DWARFDeclContext decl_ctx = die.GetDWARFDeclContext();
@@ -2394,7 +2395,7 @@
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
die, GetCXXObjectParameter(die, *containing_decl_ctx));
ParseChildParameters(containing_decl_ctx, die, is_variadic,
- has_template_params, param_types, param_decls);
+ has_template_params, param_types);
sstr << "(";
for (size_t i = 0; i < param_types.size(); i++) {
if (i > 0)
@@ -3156,8 +3157,7 @@ bool DWARFASTParserClang::ParseChildMembers(
void DWARFASTParserClang::ParseChildParameters(
clang::DeclContext *containing_decl_ctx, const DWARFDIE &parent_die,
bool &is_variadic, bool &has_template_params,
-std::vector &function_param_types,
-std::vector &function_param_decls) {
+std::vector &function_param_types) {
if (!parent_die)
return;
@@ -3168,7 +3168,6 @
[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)
https://github.com/Michael137 updated
https://github.com/llvm/llvm-project/pull/124279
>From 23c7f4a3e067739a050964b3f1c2140787b0c31f Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Fri, 24 Jan 2025 13:33:07 +
Subject: [PATCH 1/3] [lldb][TypeSystem] Ensure that ParmVarDecls have the
correct DeclContext
While sifting through this part of the code I noticed that when we parse C++
methods, `DWARFASTParserClang` creates two sets of `ParmVarDecls`, one in
`ParseChildParameters` and once in `AddMethodToCXXRecordType`. The former is
unused when we're dealing with methods. Moreover, the `ParmVarDecls` we created
in `ParseChildParameters` were created with an incorrect `clang::DeclContext`
(namely the DeclContext of the function, and not the function itself). In
Clang, there's `ParmVarDecl::setOwningFunction` to adjust the DeclContext of a
parameter if the parameter was created before the FunctionDecl. But we never
used it.
This patch removes the `ParmVarDecl` creation from `ParseChildParameters` and
instead creates a `TypeSystemClang::CreateParameterDeclarations` that ensures
we set the DeclContext correctly.
This wasn't causing any concrete issues (that I know of), but was quite
surprising. And this way of setting the parameters seems easier to reason about
(in my opinion).
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 31 ++--
.../SymbolFile/DWARF/DWARFASTParserClang.h| 3 +-
.../SymbolFile/NativePDB/PdbAstBuilder.cpp| 2 +-
.../Plugins/SymbolFile/PDB/PDBASTParser.cpp | 4 +-
.../TypeSystem/Clang/TypeSystemClang.cpp | 43 ++---
.../TypeSystem/Clang/TypeSystemClang.h| 16 +-
lldb/unittests/Symbol/TestTypeSystemClang.cpp | 42 +
.../DWARF/DWARFASTParserClangTests.cpp| 154 ++
8 files changed, 247 insertions(+), 48 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index e77188bfbd2e4a..30dec4bbf91c10 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1272,7 +1272,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
return_clang_type = m_ast.GetBasicType(eBasicTypeVoid);
std::vector function_param_types;
- std::vector function_param_decls;
// Parse the function children for the parameters
@@ -1283,8 +1282,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
if (die.HasChildren()) {
ParseChildParameters(containing_decl_ctx, die, is_variadic,
- has_template_params, function_param_types,
- function_param_decls);
+ has_template_params, function_param_types);
}
bool is_cxx_method = DeclKindIsCXXClass(containing_decl_ctx->getDeclKind());
@@ -1414,11 +1412,15 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE
&die,
LinkDeclContextToDIE(function_decl, die);
- if (!function_param_decls.empty()) {
-m_ast.SetFunctionParameters(function_decl, function_param_decls);
+ const clang::FunctionProtoType *function_prototype(
+ llvm::cast(
+ ClangUtil::GetQualType(clang_type).getTypePtr()));
+ auto params = m_ast.CreateParameterDeclarations(function_decl,
+ *function_prototype);
+ if (!params.empty()) {
+function_decl->setParams(params);
if (template_function_decl)
- m_ast.SetFunctionParameters(template_function_decl,
- function_param_decls);
+ template_function_decl->setParams(params);
}
ClangASTMetadata metadata;
@@ -2380,7 +2382,6 @@
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
bool is_variadic = false;
bool has_template_params = false;
std::vector param_types;
- std::vector param_decls;
StreamString sstr;
DWARFDeclContext decl_ctx = die.GetDWARFDeclContext();
@@ -2394,7 +2395,7 @@
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
die, GetCXXObjectParameter(die, *containing_decl_ctx));
ParseChildParameters(containing_decl_ctx, die, is_variadic,
- has_template_params, param_types, param_decls);
+ has_template_params, param_types);
sstr << "(";
for (size_t i = 0; i < param_types.size(); i++) {
if (i > 0)
@@ -3156,8 +3157,7 @@ bool DWARFASTParserClang::ParseChildMembers(
void DWARFASTParserClang::ParseChildParameters(
clang::DeclContext *containing_decl_ctx, const DWARFDIE &parent_die,
bool &is_variadic, bool &has_template_params,
-std::vector &function_param_types,
-std::vector &function_param_decls) {
+std::vector &function_param_types) {
if (!parent_die)
return;
@@ -3168,7 +3168,6 @
[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)
Michael137 wrote: Hmm one important difference here is that we would now not attach a name to the parameters that we create for free functions (we already don't do it for methods). I don't think having a name on the parameter is important for anything other than diagnostics of the expression evaluator. Hence the one test failure in `TestExprDiagnostics.py`: We would now get: ``` note: candidate function not viable: requires 1 argument, but 2 were provided ``` instead of ``` note: candidate function not viable: requires single argument 'x', but 2 arguments were provided ``` I'm tempted to simply adjust the test-case and not worry about attaching names at all. But if other disagree, I guess we could plumb the parameter names from DWARF into `CreateParameterDeclarations`. https://github.com/llvm/llvm-project/pull/124279 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)
https://github.com/Michael137 edited https://github.com/llvm/llvm-project/pull/124279 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Add register fields for Guarded Control Stack registers (PR #124295)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: David Spickett (DavidSpickett)
Changes
The features and locked registers hold the same bits, the latter
is a lock for the former. Tested with core files and live processes.
I thought about setting a non-zero lock register in the core file,
however:
* We can be pretty sure it's reading correctly because its between
the 2 other GCS registers in the same core file note.
* I can't make the test case modify lock bits because userspace
can't clear them and we don't know what the libc has locked
(probably all feature bits).
---
Patch is 42.40 KiB, truncated to 20.00 KiB below, full version:
https://github.com/llvm/llvm-project/pull/124295.diff
15 Files Affected:
- (modified) lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp (+75)
- (modified)
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp (+104)
- (modified)
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h (+16)
- (modified) lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.cpp
(+4)
- (modified) lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_arm64.h
(+1)
- (modified)
lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.cpp (+16)
- (modified) lldb/source/Plugins/Process/Utility/RegisterFlagsDetector_arm64.h
(+4-1)
- (modified) lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
(+38-1)
- (modified) lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h (+7)
- (modified)
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.cpp (+17)
- (modified)
lldb/source/Plugins/Process/elf-core/RegisterContextPOSIXCore_arm64.h (+1)
- (modified) lldb/source/Plugins/Process/elf-core/RegisterUtilities.h (+4)
- (modified) lldb/test/API/linux/aarch64/gcs/TestAArch64LinuxGCS.py (+323)
- (added) lldb/test/API/linux/aarch64/gcs/corefile ()
- (modified) lldb/test/API/linux/aarch64/gcs/main.c (+57-5)
``diff
diff --git a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
index 93b8141e97ef86..74047ea65788cf 100644
--- a/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
+++ b/lldb/source/Plugins/ABI/AArch64/ABISysV_arm64.cpp
@@ -60,6 +60,69 @@ ABISysV_arm64::CreateInstance(lldb::ProcessSP process_sp,
const ArchSpec &arch)
return ABISP();
}
+static Status PushToLinuxGuardedControlStack(addr_t return_addr,
+ RegisterContext *reg_ctx,
+ Thread &thread) {
+ Status err;
+
+ // If the Guarded Control Stack extension is present we may need to put the
+ // return address onto that stack.
+ const RegisterInfo *gcs_features_enabled_info =
+ reg_ctx->GetRegisterInfoByName("gcs_features_enabled");
+ if (!gcs_features_enabled_info)
+return err;
+
+ uint64_t gcs_features_enabled = reg_ctx->ReadRegisterAsUnsigned(
+ gcs_features_enabled_info, LLDB_INVALID_ADDRESS);
+ if (gcs_features_enabled == LLDB_INVALID_ADDRESS)
+return Status("Could not read GCS features enabled register.");
+
+ // Only attempt this if GCS is enabled. If it's not enabled then gcspr_el0
+ // may point to unmapped memory.
+ if ((gcs_features_enabled & 1) == 0)
+return err;
+
+ const RegisterInfo *gcspr_el0_info =
+ reg_ctx->GetRegisterInfoByName("gcspr_el0");
+ if (!gcspr_el0_info)
+return Status("Could not get register info for gcspr_el0.");
+
+ uint64_t gcspr_el0 =
+ reg_ctx->ReadRegisterAsUnsigned(gcspr_el0_info, LLDB_INVALID_ADDRESS);
+ if (gcspr_el0 == LLDB_INVALID_ADDRESS)
+return Status("Could not read gcspr_el0.");
+
+ // A link register entry on the GCS is 8 bytes.
+ gcspr_el0 -= 8;
+ if (!reg_ctx->WriteRegisterFromUnsigned(gcspr_el0_info, gcspr_el0))
+return Status(
+"Attempted to decrement gcspr_el0, but could not write to it.");
+
+ Status error;
+ size_t wrote = thread.GetProcess()->WriteMemory(gcspr_el0, &return_addr,
+ sizeof(return_addr), error);
+ if ((wrote != sizeof(return_addr) || error.Fail())) {
+// When PrepareTrivialCall fails, the register context is not restored,
+// unlike when an expression fails to execute. This is arguably a bug,
+// see https://github.com/llvm/llvm-project/issues/124269.
+// For now we are handling this here specifically. We can assume this
+// write will work as the one to decrement the register did.
+reg_ctx->WriteRegisterFromUnsigned(gcspr_el0_info, gcspr_el0 + 8);
+return Status("Failed to write new Guarded Control Stack entry.");
+ }
+
+ Log *log = GetLog(LLDBLog::Expressions);
+ LLDB_LOGF(log,
+"Pushed return address 0x%" PRIx64 " to Guarded Control Stack. "
+"gcspr_el0 was 0%" PRIx64 ", is now 0x%" PRIx64 ".",
+return_addr, gcspr_el0 - 8, gcspr_el0);
+
+ // gcspr_el0 will be restored to the original value by lldb-server after
+ // the ca
[Lldb-commits] [lldb] [lldb][AArch64] Add register fields for Guarded Control Stack registers (PR #124295)
https://github.com/DavidSpickett updated
https://github.com/llvm/llvm-project/pull/124295
>From 951a38910e49f3e93dc6c9a084e955ab01ad4c49 Mon Sep 17 00:00:00 2001
From: David Spickett
Date: Fri, 9 Aug 2024 11:56:29 +0100
Subject: [PATCH 1/6] [lldb][AArch64] Add Guarded Control Stack registers
The Guarded Control Stack extension implements a shadow stack and
the Linux kernel provides access to 3 registers for it via ptrace.
struct user_gcs {
__u64 features_enabled;
__u64 features_locked;
__u64 gcspr_el0;
};
This commit adds support for reading those from a live process.
The first 2 are pseudo registers based on the real control register
and the 3rd is a real register. This is the stack pointer for the
guarded stack.
I have added a "gcs_" prefix to the "features" registers so that
they have a clear name when shown individually. Also this means
they will tab complete from "gcs", and be next to gcspr_el0 in
any sorted lists of registers.
Guarded Control Stack Registers:
gcs_features_enabled = 0x
gcs_features_locked = 0x
gcspr_el0 = 0x
Testing is more of the usual, where possible I'm writing a register
then doing something in the program to confirm the value was actually
sent to ptrace.
---
.../NativeRegisterContextLinux_arm64.cpp | 86 +++
.../Linux/NativeRegisterContextLinux_arm64.h | 16 +++
.../Utility/RegisterContextPOSIX_arm64.cpp| 4 +
.../Utility/RegisterContextPOSIX_arm64.h | 1 +
.../Utility/RegisterInfoPOSIX_arm64.cpp | 39 -
.../Process/Utility/RegisterInfoPOSIX_arm64.h | 7 +
.../linux/aarch64/gcs/TestAArch64LinuxGCS.py | 134 ++
lldb/test/API/linux/aarch64/gcs/main.c| 23 ++-
8 files changed, 305 insertions(+), 5 deletions(-)
diff --git
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 6056f3001fed6e..efd3385c46e92f 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -64,8 +64,14 @@
#define NT_ARM_FPMR 0x40e /* Floating point mode register */
#endif
+#ifndef NT_ARM_GCS
+#define NT_ARM_GCS 0x410 /* Guarded Control Stack control registers */
+#endif
+
#define HWCAP_PACA (1 << 30)
+#define HWCAP_GCS (1UL << 32)
+
#define HWCAP2_MTE (1 << 18)
#define HWCAP2_FPMR (1UL << 48)
@@ -150,6 +156,8 @@
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskMTE);
if (*auxv_at_hwcap2 & HWCAP2_FPMR)
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskFPMR);
+ if (*auxv_at_hwcap & HWCAP_GCS)
+opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskGCS);
}
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskTLS);
@@ -193,6 +201,7 @@
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
::memset(&m_pac_mask, 0, sizeof(m_pac_mask));
::memset(&m_tls_regs, 0, sizeof(m_tls_regs));
::memset(&m_sme_pseudo_regs, 0, sizeof(m_sme_pseudo_regs));
+ ::memset(&m_gcs_regs, 0, sizeof(m_gcs_regs));
std::fill(m_zt_reg.begin(), m_zt_reg.end(), 0);
m_mte_ctrl_reg = 0;
@@ -213,6 +222,7 @@
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
m_tls_is_valid = false;
m_zt_buffer_is_valid = false;
m_fpmr_is_valid = false;
+ m_gcs_is_valid = false;
// SME adds the tpidr2 register
m_tls_size = GetRegisterInfo().IsSSVEPresent() ? sizeof(m_tls_regs)
@@ -433,6 +443,14 @@ NativeRegisterContextLinux_arm64::ReadRegister(const
RegisterInfo *reg_info,
offset = reg_info->byte_offset - GetRegisterInfo().GetFPMROffset();
assert(offset < GetFPMRBufferSize());
src = (uint8_t *)GetFPMRBuffer() + offset;
+ } else if (IsGCS(reg)) {
+error = ReadGCS();
+if (error.Fail())
+ return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetGCSOffset();
+assert(offset < GetGCSBufferSize());
+src = (uint8_t *)GetGCSBuffer() + offset;
} else
return Status::FromErrorString(
"failed - register wasn't recognized to be a GPR or an FPR, "
@@ -657,6 +675,17 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
return WriteFPMR();
+ } else if (IsGCS(reg)) {
+error = ReadGCS();
+if (error.Fail())
+ return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetGCSOffset();
+assert(offset < GetGCSBufferSize());
+dst = (uint8_t *)GetGCSBuffer() + offset;
+::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
+
+return WriteGCS();
}
return Status::FromErrorString("Failed to write register value");
@@ -672,6 +701,7 @@ enum RegisterSetType : uint32_t {
SME, // ZA only, because SVCR and SVG are pseudo registers.
SME2, // ZT only.
[Lldb-commits] [lldb] [lldb][AArch64] Add register fields for Guarded Control Stack registers (PR #124295)
https://github.com/DavidSpickett created
https://github.com/llvm/llvm-project/pull/124295
The features and locked registers hold the same bits, the latter
is a lock for the former. Tested with core files and live processes.
I thought about setting a non-zero lock register in the core file,
however:
* We can be pretty sure it's reading correctly because its between
the 2 other GCS registers in the same core file note.
* I can't make the test case modify lock bits because userspace
can't clear them and we don't know what the libc has locked
(probably all feature bits).
>From 951a38910e49f3e93dc6c9a084e955ab01ad4c49 Mon Sep 17 00:00:00 2001
From: David Spickett
Date: Fri, 9 Aug 2024 11:56:29 +0100
Subject: [PATCH 1/6] [lldb][AArch64] Add Guarded Control Stack registers
The Guarded Control Stack extension implements a shadow stack and
the Linux kernel provides access to 3 registers for it via ptrace.
struct user_gcs {
__u64 features_enabled;
__u64 features_locked;
__u64 gcspr_el0;
};
This commit adds support for reading those from a live process.
The first 2 are pseudo registers based on the real control register
and the 3rd is a real register. This is the stack pointer for the
guarded stack.
I have added a "gcs_" prefix to the "features" registers so that
they have a clear name when shown individually. Also this means
they will tab complete from "gcs", and be next to gcspr_el0 in
any sorted lists of registers.
Guarded Control Stack Registers:
gcs_features_enabled = 0x
gcs_features_locked = 0x
gcspr_el0 = 0x
Testing is more of the usual, where possible I'm writing a register
then doing something in the program to confirm the value was actually
sent to ptrace.
---
.../NativeRegisterContextLinux_arm64.cpp | 86 +++
.../Linux/NativeRegisterContextLinux_arm64.h | 16 +++
.../Utility/RegisterContextPOSIX_arm64.cpp| 4 +
.../Utility/RegisterContextPOSIX_arm64.h | 1 +
.../Utility/RegisterInfoPOSIX_arm64.cpp | 39 -
.../Process/Utility/RegisterInfoPOSIX_arm64.h | 7 +
.../linux/aarch64/gcs/TestAArch64LinuxGCS.py | 134 ++
lldb/test/API/linux/aarch64/gcs/main.c| 23 ++-
8 files changed, 305 insertions(+), 5 deletions(-)
diff --git
a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
index 6056f3001fed6e..efd3385c46e92f 100644
--- a/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
@@ -64,8 +64,14 @@
#define NT_ARM_FPMR 0x40e /* Floating point mode register */
#endif
+#ifndef NT_ARM_GCS
+#define NT_ARM_GCS 0x410 /* Guarded Control Stack control registers */
+#endif
+
#define HWCAP_PACA (1 << 30)
+#define HWCAP_GCS (1UL << 32)
+
#define HWCAP2_MTE (1 << 18)
#define HWCAP2_FPMR (1UL << 48)
@@ -150,6 +156,8 @@
NativeRegisterContextLinux::CreateHostNativeRegisterContextLinux(
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskMTE);
if (*auxv_at_hwcap2 & HWCAP2_FPMR)
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskFPMR);
+ if (*auxv_at_hwcap & HWCAP_GCS)
+opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskGCS);
}
opt_regsets.Set(RegisterInfoPOSIX_arm64::eRegsetMaskTLS);
@@ -193,6 +201,7 @@
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
::memset(&m_pac_mask, 0, sizeof(m_pac_mask));
::memset(&m_tls_regs, 0, sizeof(m_tls_regs));
::memset(&m_sme_pseudo_regs, 0, sizeof(m_sme_pseudo_regs));
+ ::memset(&m_gcs_regs, 0, sizeof(m_gcs_regs));
std::fill(m_zt_reg.begin(), m_zt_reg.end(), 0);
m_mte_ctrl_reg = 0;
@@ -213,6 +222,7 @@
NativeRegisterContextLinux_arm64::NativeRegisterContextLinux_arm64(
m_tls_is_valid = false;
m_zt_buffer_is_valid = false;
m_fpmr_is_valid = false;
+ m_gcs_is_valid = false;
// SME adds the tpidr2 register
m_tls_size = GetRegisterInfo().IsSSVEPresent() ? sizeof(m_tls_regs)
@@ -433,6 +443,14 @@ NativeRegisterContextLinux_arm64::ReadRegister(const
RegisterInfo *reg_info,
offset = reg_info->byte_offset - GetRegisterInfo().GetFPMROffset();
assert(offset < GetFPMRBufferSize());
src = (uint8_t *)GetFPMRBuffer() + offset;
+ } else if (IsGCS(reg)) {
+error = ReadGCS();
+if (error.Fail())
+ return error;
+
+offset = reg_info->byte_offset - GetRegisterInfo().GetGCSOffset();
+assert(offset < GetGCSBufferSize());
+src = (uint8_t *)GetGCSBuffer() + offset;
} else
return Status::FromErrorString(
"failed - register wasn't recognized to be a GPR or an FPR, "
@@ -657,6 +675,17 @@ Status NativeRegisterContextLinux_arm64::WriteRegister(
::memcpy(dst, reg_value.GetBytes(), reg_info->byte_size);
return WriteFPMR();
+ } else if (IsGCS(reg)) {
+error = ReadGCS();
+if (error.Fai
[Lldb-commits] [lldb] [lldb][AArch64] Add register fields for Guarded Control Stack registers (PR #124295)
DavidSpickett wrote: All commits before the one named the same as this PR are from https://github.com/llvm/llvm-project/pull/124293. I will handle the rebasing and conflicts as that gets landed. https://github.com/llvm/llvm-project/pull/124295 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Add register fields for Guarded Control Stack registers (PR #124295)
DavidSpickett wrote: This is the last piece of functionality for GCS. Remaining is the documentation (https://github.com/llvm/llvm-project/pull/117860) and a release note that I'll push directly if the 20 branch hasn't happened yet by that time. https://github.com/llvm/llvm-project/pull/124295 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Add register fields for Guarded Control Stack registers (PR #124295)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/124295 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove unused posix_openpt function definition for Android (PR #124257)
https://github.com/brad0 closed https://github.com/llvm/llvm-project/pull/124257 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Michael Buch (Michael137)
Changes
While sifting through this part of the code I noticed that when we parse C++
methods, `DWARFASTParserClang` creates two sets of `ParmVarDecls`, one in
`ParseChildParameters` and once in `AddMethodToCXXRecordType`. The former is
unused when we're dealing with methods. Moreover, the `ParmVarDecls` we created
in `ParseChildParameters` were created with an incorrect `clang::DeclContext`
(namely the DeclContext of the function, and not the function itself). In
Clang, there's `ParmVarDecl::setOwningFunction` to adjust the DeclContext of a
parameter if the parameter was created before the FunctionDecl. But we never
used it.
This patch removes the `ParmVarDecl` creation from `ParseChildParameters` and
instead creates a `TypeSystemClang::CreateParameterDeclarations` that ensures
we set the DeclContext correctly.
This wasn't causing any concrete issues (that I know of), but was quite
surprising. And this way of setting the parameters seems easier to reason about
(in my opinion).
---
Full diff: https://github.com/llvm/llvm-project/pull/124279.diff
8 Files Affected:
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
(+11-20)
- (modified) lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h (+1-2)
- (modified) lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp (+1-1)
- (modified) lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp (+2-2)
- (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+23-20)
- (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h (+13-3)
- (modified) lldb/unittests/Symbol/TestTypeSystemClang.cpp (+42)
- (modified) lldb/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
(+154)
``diff
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index e77188bfbd2e4a..30dec4bbf91c10 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1272,7 +1272,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
return_clang_type = m_ast.GetBasicType(eBasicTypeVoid);
std::vector function_param_types;
- std::vector function_param_decls;
// Parse the function children for the parameters
@@ -1283,8 +1282,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
if (die.HasChildren()) {
ParseChildParameters(containing_decl_ctx, die, is_variadic,
- has_template_params, function_param_types,
- function_param_decls);
+ has_template_params, function_param_types);
}
bool is_cxx_method = DeclKindIsCXXClass(containing_decl_ctx->getDeclKind());
@@ -1414,11 +1412,15 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE
&die,
LinkDeclContextToDIE(function_decl, die);
- if (!function_param_decls.empty()) {
-m_ast.SetFunctionParameters(function_decl, function_param_decls);
+ const clang::FunctionProtoType *function_prototype(
+ llvm::cast(
+ ClangUtil::GetQualType(clang_type).getTypePtr()));
+ auto params = m_ast.CreateParameterDeclarations(function_decl,
+ *function_prototype);
+ if (!params.empty()) {
+function_decl->setParams(params);
if (template_function_decl)
- m_ast.SetFunctionParameters(template_function_decl,
- function_param_decls);
+ template_function_decl->setParams(params);
}
ClangASTMetadata metadata;
@@ -2380,7 +2382,6 @@
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
bool is_variadic = false;
bool has_template_params = false;
std::vector param_types;
- std::vector param_decls;
StreamString sstr;
DWARFDeclContext decl_ctx = die.GetDWARFDeclContext();
@@ -2394,7 +2395,7 @@
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
die, GetCXXObjectParameter(die, *containing_decl_ctx));
ParseChildParameters(containing_decl_ctx, die, is_variadic,
- has_template_params, param_types, param_decls);
+ has_template_params, param_types);
sstr << "(";
for (size_t i = 0; i < param_types.size(); i++) {
if (i > 0)
@@ -3156,8 +3157,7 @@ bool DWARFASTParserClang::ParseChildMembers(
void DWARFASTParserClang::ParseChildParameters(
clang::DeclContext *containing_decl_ctx, const DWARFDIE &parent_die,
bool &is_variadic, bool &has_template_params,
-std::vector &function_param_types,
-std::vector &function_param_decls) {
+std::vector &function_param_types) {
if (!parent_die)
return;
@@ -3168,7 +3168,6 @@ void DWARFASTParserC
[Lldb-commits] [lldb] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct DeclContext (PR #124279)
https://github.com/Michael137 created
https://github.com/llvm/llvm-project/pull/124279
While sifting through this part of the code I noticed that when we parse C++
methods, `DWARFASTParserClang` creates two sets of `ParmVarDecls`, one in
`ParseChildParameters` and once in `AddMethodToCXXRecordType`. The former is
unused when we're dealing with methods. Moreover, the `ParmVarDecls` we created
in `ParseChildParameters` were created with an incorrect `clang::DeclContext`
(namely the DeclContext of the function, and not the function itself). In
Clang, there's `ParmVarDecl::setOwningFunction` to adjust the DeclContext of a
parameter if the parameter was created before the FunctionDecl. But we never
used it.
This patch removes the `ParmVarDecl` creation from `ParseChildParameters` and
instead creates a `TypeSystemClang::CreateParameterDeclarations` that ensures
we set the DeclContext correctly.
This wasn't causing any concrete issues (that I know of), but was quite
surprising. And this way of setting the parameters seems easier to reason about
(in my opinion).
>From 23c7f4a3e067739a050964b3f1c2140787b0c31f Mon Sep 17 00:00:00 2001
From: Michael Buch
Date: Fri, 24 Jan 2025 13:33:07 +
Subject: [PATCH] [lldb][TypeSystem] Ensure that ParmVarDecls have the correct
DeclContext
While sifting through this part of the code I noticed that when we parse C++
methods, `DWARFASTParserClang` creates two sets of `ParmVarDecls`, one in
`ParseChildParameters` and once in `AddMethodToCXXRecordType`. The former is
unused when we're dealing with methods. Moreover, the `ParmVarDecls` we created
in `ParseChildParameters` were created with an incorrect `clang::DeclContext`
(namely the DeclContext of the function, and not the function itself). In
Clang, there's `ParmVarDecl::setOwningFunction` to adjust the DeclContext of a
parameter if the parameter was created before the FunctionDecl. But we never
used it.
This patch removes the `ParmVarDecl` creation from `ParseChildParameters` and
instead creates a `TypeSystemClang::CreateParameterDeclarations` that ensures
we set the DeclContext correctly.
This wasn't causing any concrete issues (that I know of), but was quite
surprising. And this way of setting the parameters seems easier to reason about
(in my opinion).
---
.../SymbolFile/DWARF/DWARFASTParserClang.cpp | 31 ++--
.../SymbolFile/DWARF/DWARFASTParserClang.h| 3 +-
.../SymbolFile/NativePDB/PdbAstBuilder.cpp| 2 +-
.../Plugins/SymbolFile/PDB/PDBASTParser.cpp | 4 +-
.../TypeSystem/Clang/TypeSystemClang.cpp | 43 ++---
.../TypeSystem/Clang/TypeSystemClang.h| 16 +-
lldb/unittests/Symbol/TestTypeSystemClang.cpp | 42 +
.../DWARF/DWARFASTParserClangTests.cpp| 154 ++
8 files changed, 247 insertions(+), 48 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index e77188bfbd2e4a..30dec4bbf91c10 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1272,7 +1272,6 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
return_clang_type = m_ast.GetBasicType(eBasicTypeVoid);
std::vector function_param_types;
- std::vector function_param_decls;
// Parse the function children for the parameters
@@ -1283,8 +1282,7 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE &die,
if (die.HasChildren()) {
ParseChildParameters(containing_decl_ctx, die, is_variadic,
- has_template_params, function_param_types,
- function_param_decls);
+ has_template_params, function_param_types);
}
bool is_cxx_method = DeclKindIsCXXClass(containing_decl_ctx->getDeclKind());
@@ -1414,11 +1412,15 @@ DWARFASTParserClang::ParseSubroutine(const DWARFDIE
&die,
LinkDeclContextToDIE(function_decl, die);
- if (!function_param_decls.empty()) {
-m_ast.SetFunctionParameters(function_decl, function_param_decls);
+ const clang::FunctionProtoType *function_prototype(
+ llvm::cast(
+ ClangUtil::GetQualType(clang_type).getTypePtr()));
+ auto params = m_ast.CreateParameterDeclarations(function_decl,
+ *function_prototype);
+ if (!params.empty()) {
+function_decl->setParams(params);
if (template_function_decl)
- m_ast.SetFunctionParameters(template_function_decl,
- function_param_decls);
+ template_function_decl->setParams(params);
}
ClangASTMetadata metadata;
@@ -2380,7 +2382,6 @@
DWARFASTParserClang::ConstructDemangledNameFromDWARF(const DWARFDIE &die) {
bool is_variadic = false;
bool has_template_params = false;
st
[Lldb-commits] [lldb] [lldb] Remove unused posix_openpt function definition for Android (PR #124257)
https://github.com/brad0 created https://github.com/llvm/llvm-project/pull/124257 This was for the wrapper function that was in source/Host/android/LibcGlue.cpp. Android added support 10+ years ago. >From ed441c18d824e25a184f348a0dba7019cfb01b6f Mon Sep 17 00:00:00 2001 From: Brad Smith Date: Fri, 24 Jan 2025 06:13:17 -0500 Subject: [PATCH] [lldb] Remove unused posix_openpt function definition for Android This was for the wrapper function that was in source/Host/android/LibcGlue.cpp. Android added support 10+ years ago. --- lldb/source/Host/common/PseudoTerminal.cpp | 4 1 file changed, 4 deletions(-) diff --git a/lldb/source/Host/common/PseudoTerminal.cpp b/lldb/source/Host/common/PseudoTerminal.cpp index d53327973eb270..53e91aff212a4a 100644 --- a/lldb/source/Host/common/PseudoTerminal.cpp +++ b/lldb/source/Host/common/PseudoTerminal.cpp @@ -27,10 +27,6 @@ #include #endif -#if defined(__ANDROID__) -int posix_openpt(int flags); -#endif - using namespace lldb_private; // PseudoTerminal constructor ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove unused posix_openpt function definition for Android (PR #124257)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Brad Smith (brad0) Changes This was for the wrapper function that was in source/Host/android/LibcGlue.cpp. Android added support 10+ years ago. --- Full diff: https://github.com/llvm/llvm-project/pull/124257.diff 1 Files Affected: - (modified) lldb/source/Host/common/PseudoTerminal.cpp (-4) ``diff diff --git a/lldb/source/Host/common/PseudoTerminal.cpp b/lldb/source/Host/common/PseudoTerminal.cpp index d53327973eb270..53e91aff212a4a 100644 --- a/lldb/source/Host/common/PseudoTerminal.cpp +++ b/lldb/source/Host/common/PseudoTerminal.cpp @@ -27,10 +27,6 @@ #include #endif -#if defined(__ANDROID__) -int posix_openpt(int flags); -#endif - using namespace lldb_private; // PseudoTerminal constructor `` https://github.com/llvm/llvm-project/pull/124257 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Remove unused posix_openpt function definition for Android (PR #124257)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/124257 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SymbolContext::GetFunctionOrSymbolAddress (PR #123340)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/123340 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SymbolContext::GetFunctionOrSymbolAddress (PR #123340)
@@ -370,6 +370,31 @@ bool SymbolContext::GetAddressRange(uint32_t scope,
uint32_t range_idx,
return false;
}
+Address SymbolContext::GetAddress(uint32_t scope,
+ bool use_inline_block_range) const {
+ if ((scope & eSymbolContextLineEntry) && line_entry.IsValid())
+return line_entry.range.GetBaseAddress();
+
+ if (scope & eSymbolContextBlock) {
+Block *block_to_use = (block && use_inline_block_range)
+ ? block->GetContainingInlinedBlock()
+ : block;
+if (block_to_use) {
+ Address addr;
+ block_to_use->GetStartAddress(addr);
+ return addr;
+}
+ }
labath wrote:
Okay, please take a look at the new version. I had to inline the old
implementation into one place (CommandObjectTarget, for `image lookup`), but
generally, I think this looks better than what I expected.
https://github.com/llvm/llvm-project/pull/123340
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Include api-level.h header for Android (PR #124383)
brad0 wrote: >From the look of the rest of the codebase I should have included this header >when utilizing __ANDROID_API__. https://github.com/llvm/llvm-project/pull/124383 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Include api-level.h header for Android (PR #124383)
https://github.com/brad0 created
https://github.com/llvm/llvm-project/pull/124383
None
>From b0580b8b0f1f50e5fa10d2b872c79b4942faf093 Mon Sep 17 00:00:00 2001
From: Brad Smith
Date: Fri, 24 Jan 2025 21:38:07 -0500
Subject: [PATCH] [lldb] Include api-level.h header for Android
---
lldb/source/Host/posix/HostInfoPosix.cpp | 4
1 file changed, 4 insertions(+)
diff --git a/lldb/source/Host/posix/HostInfoPosix.cpp
b/lldb/source/Host/posix/HostInfoPosix.cpp
index 879dccfd353be5..78116eb161afa6 100644
--- a/lldb/source/Host/posix/HostInfoPosix.cpp
+++ b/lldb/source/Host/posix/HostInfoPosix.cpp
@@ -29,6 +29,10 @@
#include
#include
+#ifdef __ANDROID__
+#include
+#endif
+
using namespace lldb_private;
namespace {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Include api-level.h header for Android (PR #124383)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: Brad Smith (brad0)
Changes
---
Full diff: https://github.com/llvm/llvm-project/pull/124383.diff
1 Files Affected:
- (modified) lldb/source/Host/posix/HostInfoPosix.cpp (+4)
``diff
diff --git a/lldb/source/Host/posix/HostInfoPosix.cpp
b/lldb/source/Host/posix/HostInfoPosix.cpp
index 879dccfd353be5..78116eb161afa6 100644
--- a/lldb/source/Host/posix/HostInfoPosix.cpp
+++ b/lldb/source/Host/posix/HostInfoPosix.cpp
@@ -29,6 +29,10 @@
#include
#include
+#ifdef __ANDROID__
+#include
+#endif
+
using namespace lldb_private;
namespace {
``
https://github.com/llvm/llvm-project/pull/124383
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)
SuibianP wrote: I [enabled `runInTerminal` tests for Windows](https://github.com/llvm/llvm-project/pull/121269/files#diff-38cf10e0a83515e5c6acf5449a3f5de1d63f116d932a32ba169b746826ead1d4) (will fix line 93 later) except for the 4 launcher tests that requires `os.mkfifo` (https://github.com/python/cpython/issues/103510) and they pass on my local computer. This should be sufficient to guarantee the common functionalities. I can do a simple polyfill using `pywin32` for Windows if the ignored test is a blocker for this PR. All LLDB tests still pass on Linux CI, so there seems to be no regression https://buildkite.com/llvm-project/github-pull-requests/builds/140240 https://github.com/llvm/llvm-project/pull/121269 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] e2005d1 - [LLDB] Reapply #123873 SBSaveCore Docstrings (#124355)
Author: Jacob Lalonde
Date: 2025-01-24T14:59:56-08:00
New Revision: e2005d1461942539f7533a518aa78017074f6bf9
URL:
https://github.com/llvm/llvm-project/commit/e2005d1461942539f7533a518aa78017074f6bf9
DIFF:
https://github.com/llvm/llvm-project/commit/e2005d1461942539f7533a518aa78017074f6bf9.diff
LOG: [LLDB] Reapply #123873 SBSaveCore Docstrings (#124355)
In my last attempt at this (#123873), I didn't realize we needed semi
colons! Also fixed the bug that the feature summary didn't have a type
defined.
CC @JDevlieghere hope you get a laugh at needing to revert doc strings
for breaking the build
Added:
Modified:
lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
Removed:
diff --git a/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
b/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
index e69de29bb2d1d6..08bbdf89d68ded 100644
--- a/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
+++ b/lldb/bindings/interface/SBSaveCoreOptionsDocstrings.i
@@ -0,0 +1,71 @@
+%feature("docstring",
+"A container to specify how to save a core file.
+
+SBSaveCoreOptions includes API's to specify the memory regions and threads to
include
+when generating a core file. It extends the existing SaveCoreStyle option.
+
+* eSaveCoreFull will save off all thread and memory regions, ignoring the
memory regions and threads in
+the options object.
+
+* eSaveCoreDirtyOnly pages will capture all threads and all rw- memory
regions, in addition to the regions specified
+in the options object if they are not already captured.
+
+* eSaveCoreStackOnly will capture all threads, but no memory regions unless
specified.
+
+* eSaveCoreCustomOnly Custom defers entirely to the SBSaveCoreOptions object
and will only save what is specified.
+ Picking custom and specifying nothing will result in an error being returned.
+
+Note that currently ELF Core files are not supported."
+) lldb::SBSaveCoreOptions;
+
+%feature("docstring", "
+Set the plugin name to save a Core file with. Only plugins registered with
Plugin manager will be accepted
+Examples are Minidump and Mach-O."
+) lldb::SBSaveCoreOptions::SetPluginName;
+
+%feature("docstring", "
+Get the specified plugin name, or None if the name is not set."
+) lldb::SBSaveCoreOptions::GetPluginName;
+
+%feature("docstring", "
+Set the lldb.SaveCoreStyle."
+) lldb::SBSaveCoreOptions::SetStyle;
+
+%feature("docstring", "
+Get the specified lldb.SaveCoreStyle, or eSaveCoreUnspecified if not set."
+) lldb::SBSaveCoreOptions::GetStyle;
+
+%feature("docstring", "
+Set the file path to save the Core file at."
+) lldb::SBSaveCoreOptions::SetOutputFile;
+
+%feature("docstring", "
+Get an SBFileSpec corresponding to the specified output path, or none if
not set."
+) lldb::SBSaveCoreOptions::GetOutputFile;
+
+%feature("docstring", "
+Set the process to save, or unset a process by providing a default
SBProcess.
+Resetting will result in the reset of all process specific options, such
as Threads to save."
+) lldb::SBSaveCoreOptions::SetProcess;
+
+%feature("docstring", "
+Add an SBThread to be saved, an error will be returned if an SBThread from
a
diff erent process is specified.
+The process is set either by the first SBThread added to the options
container, or explicitly by the SetProcess call."
+) lldb::SBSaveCoreOptions::AddThread;
+
+%feature("docstring", "
+Remove an SBthread if present in the container, returns true if a matching
thread was found and removed."
+) lldb::SBSaveCoreOptions::RemoveThread;
+
+%feature("docstring", "
+Add a memory region to save, an error will be returned in the region is
invalid.
+Ranges that overlap will be unioned into a single region."
+) lldb::SBSaveCoreOptions::AddMemoryRegionToSave;
+
+%feature("docstring", "
+Get an SBThreadCollection of all threads marked to be saved. This
collection is not sorted according to insertion order."
+) lldb::SBSaveCoreOptions::GetThreadsToSave;
+
+%feature("docstring", "
+Unset all options."
+) lldb::SBSaveCoreOptions::Clear;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [LLDB] Reapply #123873 SBSaveCore Docstrings (PR #124355)
https://github.com/Jlalond closed https://github.com/llvm/llvm-project/pull/124355 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SymbolContext::GetFunctionOrSymbolAddress (PR #123340)
https://github.com/labath edited https://github.com/llvm/llvm-project/pull/123340 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Add SymbolContext::GetAddress (PR #123340)
https://github.com/labath updated
https://github.com/llvm/llvm-project/pull/123340
>From 2d8987fbffa462c671f0e8cc40accc6ff63aa625 Mon Sep 17 00:00:00 2001
From: Pavel Labath
Date: Fri, 17 Jan 2025 12:36:36 +0100
Subject: [PATCH] [lldb] Add SymbolContext::GetFunctionOrSymbolAddress
Many uses of SC::GetAddressRange were not interested in the range, but
in the address of the function/symbol contained inside the symbol
context. They were getting that by calling the GetBaseAddress on the
returned range, which worked well enough so far, but isn't compatible
with discontinuous functions, whose address (entry point) may not be the
lowest address in the range.
To resolve this problem, this PR creates a new function whose purpose is
return the address of the function or symbol inside the symbol context.
It also changes all of the callers of GetAddressRange which do not
actually care about the range to call this function instead.
---
lldb/include/lldb/Symbol/SymbolContext.h | 11 ++--
lldb/source/Commands/CommandObjectTarget.cpp | 22
.../Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp | 4 +--
.../MacOSX-DYLD/DynamicLoaderDarwin.cpp | 18 +
.../POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp | 4 +--
.../CPlusPlus/CPPLanguageRuntime.cpp | 5 +---
.../Process/Utility/InferiorCallPOSIX.cpp | 26 ++-
.../MacOSX/SystemRuntimeMacOSX.cpp| 24 ++---
lldb/source/Symbol/SymbolContext.cpp | 10 +++
9 files changed, 55 insertions(+), 69 deletions(-)
diff --git a/lldb/include/lldb/Symbol/SymbolContext.h
b/lldb/include/lldb/Symbol/SymbolContext.h
index 07769cd8dffae7..69fbe544c73cd2 100644
--- a/lldb/include/lldb/Symbol/SymbolContext.h
+++ b/lldb/include/lldb/Symbol/SymbolContext.h
@@ -165,8 +165,8 @@ class SymbolContext {
/// eSymbolContextSymbol is set in \a scope
///
/// \param[in] scope
- /// A mask of symbol context bits telling this function which
- /// address ranges it can use when trying to extract one from
+ /// A mask bits from the \b SymbolContextItem enum telling this function
+ /// which address ranges it can use when trying to extract one from
/// the valid (non-nullptr) symbol context classes.
///
/// \param[in] range_idx
@@ -192,6 +192,13 @@ class SymbolContext {
bool GetAddressRange(uint32_t scope, uint32_t range_idx,
bool use_inline_block_range, AddressRange &range) const;
+ /// Get the address of the function or symbol represented by this symbol
+ /// context.
+ ///
+ /// If both fields are present, the address of the function is returned. If
+ /// both are empty, the result is an invalid address.
+ Address GetFunctionOrSymbolAddress() const;
+
llvm::Error GetAddressRangeFromHereToEndLine(uint32_t end_line,
AddressRange &range);
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp
b/lldb/source/Commands/CommandObjectTarget.cpp
index d8265e41a7384e..d0092c237b4c96 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -1621,12 +1621,15 @@ static void DumpSymbolContextList(
if (!first_module)
strm.EOL();
-AddressRange range;
-
-sc.GetAddressRange(eSymbolContextEverything, 0, true, range);
+Address addr;
+if (sc.line_entry.IsValid())
+ addr = sc.line_entry.range.GetBaseAddress();
+else if (sc.block && sc.block->GetContainingInlinedBlock())
+ sc.block->GetContainingInlinedBlock()->GetStartAddress(addr);
+else
+ addr = sc.GetFunctionOrSymbolAddress();
-DumpAddress(exe_scope, range.GetBaseAddress(), verbose, all_ranges, strm,
-settings);
+DumpAddress(exe_scope, addr, verbose, all_ranges, strm, settings);
first_module = false;
}
strm.IndentLess();
@@ -3570,16 +3573,13 @@ class CommandObjectTargetModulesShowUnwind : public
CommandObjectParsed {
continue;
if (!sc.module_sp || sc.module_sp->GetObjectFile() == nullptr)
continue;
- AddressRange range;
- if (!sc.GetAddressRange(eSymbolContextFunction | eSymbolContextSymbol, 0,
- false, range))
-continue;
- if (!range.GetBaseAddress().IsValid())
+ Address addr = sc.GetFunctionOrSymbolAddress();
+ if (!addr.IsValid())
continue;
ConstString funcname(sc.GetFunctionName());
if (funcname.IsEmpty())
continue;
- addr_t start_addr = range.GetBaseAddress().GetLoadAddress(target);
+ addr_t start_addr = addr.GetLoadAddress(target);
if (abi)
start_addr = abi->FixCodeAddress(start_addr);
diff --git
a/lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
b/lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/DynamicLoaderHexagonDYLD.cpp
index 96c94535c623c7..480500e034bc1b 100644
---
a/lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/
[Lldb-commits] [lldb] [lldb-dap] Ensure the IO forwarding threads are managed by the DAP object lifecycle. (PR #122783)
https://github.com/labath approved this pull request. https://github.com/llvm/llvm-project/pull/122783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][AArch64] Add Guarded Control Stack registers (PR #123720)
@@ -2,8 +2,8 @@ #include #include -#ifndef HWCAP2_GCS -#define HWCAP2_GCS (1UL << 63) +#ifndef HWCAP_GCS +#define HWCAP_GCS (1UL << 32) DavidSpickett wrote: Mark Brown confirmed that the documentation is incorrect. It is in HWCAP as the code shows. https://github.com/llvm/llvm-project/pull/123720 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)
@@ -21,21 +23,22 @@ namespace lldb_dap {
/// The file is destroyed when the destructor is invoked.
struct FifoFile {
FifoFile(llvm::StringRef path);
+ FifoFile(llvm::StringRef path, FILE *f);
+ // FifoFile(llvm::StringRef path, FILE *f);
+ FifoFile(FifoFile &&other);
+
+ FifoFile(const FifoFile &) = delete;
+ FifoFile &operator=(const FifoFile &) = delete;
~FifoFile();
std::string m_path;
+ FILE *m_file;
};
-/// Create a fifo file in the filesystem.
-///
-/// \param[in] path
-/// The path for the fifo file.
-///
-/// \return
-/// A \a std::shared_ptr if the file could be created, or an
-/// \a llvm::Error in case of failures.
-llvm::Expected> CreateFifoFile(llvm::StringRef path);
+std::error_code createNamedPipe(const llvm::Twine &Prefix,
SuibianP wrote:
I was mimicking the `llvm::sys::fs::createTemporaryFile` API with the idea that
this abstraction might be melded in there at some later time.
Is this considered too broad?
https://github.com/llvm/llvm-project/pull/121269
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)
@@ -71,12 +74,14 @@ class FifoFileIO {
/// \return
/// An \a llvm::Error object indicating whether the data was consumed by
/// a reader or not.
- llvm::Error SendJSON(
- const llvm::json::Value &json,
- std::chrono::milliseconds timeout = std::chrono::milliseconds(2));
+ llvm::Error
+ SendJSON(const llvm::json::Value &json,
+ std::chrono::milliseconds timeout =
std::chrono::milliseconds(2000));
SuibianP wrote:
Debug leftover; rectified.
https://github.com/llvm/llvm-project/pull/121269
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb][Formatters] Do not recursively dereference pointer type when creating formatter candicates list. (PR #124048)
labath wrote: > @labath What do you think? The solution above will still use the formatters > for `T` when `T**` is printed. Before I answer that, I want to go back to the question of dereferencing. While I don't think that skipping of arbitrary numbers of points is *wrong* (it's consistent and easy to explain), I also don't find it *useful*. One level -- sure, Even though I don't think it should be the default, I can definitely see reasons for why one would want to see the contents of `T` when one has a variable of type `T*`. Reason: many codebases -- lldb included -- use `T*` for function "return" types or for optional arguments, so if you have a `T*`, it's quite likely you're interested in seeing the `T` object rather than the pointer itself. I don't think this argument holds for more than one level of pointers. If I have a `T**` or `T***` (I've seen those in the wild), what are the chances I want to see the `T` object at the end of the chain? I think they're very low. I think it's more likely, I'm looking at either: - some generic code which works with `U*`, where `U` happens to be a `T*`. In this case I'm more likely to want to see the pointer values that the final object. - or some C-like representation of an array (argc, argv), in which case formatter will just tell me something about the first element of the array (but maybe I want to see some other element, or information about the array itself) Even if the users really does want to see the final object, they can still do that even if we don't dereference all the levels for them. It just means they need to do that themselves. As for how does this fit in with documented behavior, I don't see much of a problem with that. It's true that you could interpret `Don't use this format for pointers-to-type objects.` as skipping any level of pointers, but I think you could also interpret it as skipping just one. It says "pointers to **type**", not "arbitrary level of pointers" or anything like that. For comparison, the `cascade` option (which skips arbitrary levels of typedefs) says this (emphasis mine) "If true, cascade through typedef ***chains***.", which is a lot more explicit. It also makes more sense because typedefs are often chained and the language (the C family at least) treats typedefs (but not pointers) transparently. In addition to that, the `skip-references` option uses the exact same language as for pointers, but it skips only one level of references (although that's because there are no reference chains at the language level, not because we chose to implement it that way). So, even though you can rightfully accuse me of retconning this, I think the result would be fine. Another factor, which I think supports the fact that skipping all levels isn't the right default is that ~all of our pretty printers are currently broken for multiple pointer levels: ``` (std::map *) pm = 0x7fffd798 size=1 (std::optional *) po = Has Value=true (std::tuple *) pt = 0x7fffd780 size=2 (std::vector *) pv = 0x7fffd760 size=2 (std::map **) ppm = 0x7fffd758 size=0 # points to pm (std::optional **) ppo = Has Value=false# points to po (std::tuple **) ppt = 0x7fffd748 size=0 # points to pt (std::vector **) ppv = 0x7fffd740 size=1 # points to pv ``` If this was one incident, you could dismiss it as a buggy formatter, but since it affects all of them, I think this points to a deeper problem. We can fix all of our formatters to dereference all pointers, but that won't change all the pretty printers out there, and I'm sure that most of them have the same bug. We could try to fix all of them by dereferencing the value before handing it off to the formatter, but I don't think that's useful. The fact that we didn't notice this until now tells me users don't have that many double pointers floating around, and so they likely will not complain if we stop printing this output (which was incorrect anyway). TL:DR: I think it would be better to skip just one level of pointers. If someone really wants to format an arbitrary number of pointers, they can always use a regex to register a pretty printer for `T (\*)*` https://github.com/llvm/llvm-project/pull/124048 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb-dap] Implement `runInTerminal` for Windows (PR #121269)
@@ -71,12 +74,14 @@ class FifoFileIO {
/// \return
/// An \a llvm::Error object indicating whether the data was consumed by
/// a reader or not.
- llvm::Error SendJSON(
- const llvm::json::Value &json,
- std::chrono::milliseconds timeout = std::chrono::milliseconds(2));
+ llvm::Error
+ SendJSON(const llvm::json::Value &json,
+ std::chrono::milliseconds timeout =
std::chrono::milliseconds(2000));
+
+ llvm::Error WaitForPeer();
-private:
- std::string m_fifo_file;
+ // private:
SuibianP wrote:
Debug leftover; rectified.
https://github.com/llvm/llvm-project/pull/121269
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
