[Lldb-commits] [lldb] [lldb] Do not produce field information for registers known not to exist (PR #95125)
https://github.com/DavidSpickett closed https://github.com/llvm/llvm-project/pull/95125 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Do not produce field information for registers known not to exist (PR #95125)
https://github.com/medismailben approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/95125 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Do not produce field information for registers known not to exist (PR #95125)
DavidSpickett wrote: https://github.com/llvm/llvm-project/pull/85058 is the FreeBSD change, it'll get rebased onto this later. https://github.com/llvm/llvm-project/pull/95125 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Do not produce field information for registers known not to exist (PR #95125)
llvmbot wrote:
@llvm/pr-subscribers-lldb
Author: David Spickett (DavidSpickett)
Changes
Currently the logic is generate field information for all registers in
LinuxArm64RegisterFlags and then as we walk the existing register info, only
those that are in that existing info will get the new fields patched in.
This works fine but on a review for FreeBSD support it was pointed out that
this is not obvious from the source code.
So instead I've allowed the construction of empty lists of fields, and field
detection methods can return an empty field list if they think that the
register will never exist.
Then the pre-existing code will see the empty field list, and never look for
that register in the register info.
I think removing the assert is ok because the GDB classes filter out empty
field lists at runtime, and anyone updating the built in field information
would presumably notice if none of the fields they intended to add were
displayed.
mte_ctrl and svcr are the only registers that need this so far.
There is no extra testing here as the behaviour is the same, it doesn't add
field information to regiters that don't exist. The mechanism is just clearer
now.
---
Full diff: https://github.com/llvm/llvm-project/pull/95125.diff
3 Files Affected:
- (modified) lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
(+9-2)
- (modified) lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
(+3-3)
- (modified) lldb/source/Target/RegisterFlags.cpp (-4)
``diff
diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
index 51553817921f3..8ed75d700f225 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
@@ -20,6 +20,7 @@
#define HWCAP2_BTI (1ULL << 17)
#define HWCAP2_MTE (1ULL << 18)
#define HWCAP2_AFP (1ULL << 20)
+#define HWCAP2_SME (1ULL << 23)
#define HWCAP2_EBF16 (1ULL << 32)
using namespace lldb_private;
@@ -27,7 +28,10 @@ using namespace lldb_private;
LinuxArm64RegisterFlags::Fields
LinuxArm64RegisterFlags::DetectSVCRFields(uint64_t hwcap, uint64_t hwcap2) {
(void)hwcap;
- (void)hwcap2;
+
+ if (!(hwcap2 & HWCAP2_SME))
+return {};
+
// Represents the pseudo register that lldb-server builds, which itself
// matches the architectural register SCVR. The fields match SVCR in the Arm
// manual.
@@ -40,7 +44,10 @@ LinuxArm64RegisterFlags::DetectSVCRFields(uint64_t hwcap,
uint64_t hwcap2) {
LinuxArm64RegisterFlags::Fields
LinuxArm64RegisterFlags::DetectMTECtrlFields(uint64_t hwcap, uint64_t hwcap2) {
(void)hwcap;
- (void)hwcap2;
+
+ if (!(hwcap2 & HWCAP2_MTE))
+return {};
+
// Represents the contents of NT_ARM_TAGGED_ADDR_CTRL and the value passed
// to prctl(PR_TAGGED_ADDR_CTRL...). Fields are derived from the defines
// used to build the value.
diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
index 660bef08700f4..49b1d90db64f6 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
@@ -38,8 +38,8 @@ class LinuxArm64RegisterFlags {
/// For the registers listed in this class, detect which fields are
/// present. Must be called before UpdateRegisterInfos.
/// If called more than once, fields will be redetected each time from
- /// scratch. If you do not have access to hwcap, just pass 0 for each one,
you
- /// will only get unconditional fields.
+ /// scratch. If the target would not have this register at all, the list of
+ /// fields will be left empty.
void DetectFields(uint64_t hwcap, uint64_t hwcap2);
/// Add the field information of any registers named in this class,
@@ -63,7 +63,7 @@ class LinuxArm64RegisterFlags {
struct RegisterEntry {
RegisterEntry(llvm::StringRef name, unsigned size, DetectorFn detector)
-: m_name(name), m_flags(std::string(name) + "_flags", size, {{"", 0}}),
+: m_name(name), m_flags(std::string(name) + "_flags", size, {}),
m_detector(detector) {}
llvm::StringRef m_name;
diff --git a/lldb/source/Target/RegisterFlags.cpp
b/lldb/source/Target/RegisterFlags.cpp
index 5274960587bf3..de9f12649e2ec 100644
--- a/lldb/source/Target/RegisterFlags.cpp
+++ b/lldb/source/Target/RegisterFlags.cpp
@@ -54,10 +54,6 @@ unsigned RegisterFlags::Field::PaddingDistance(const Field
) const {
}
void RegisterFlags::SetFields(const std::vector ) {
- // We expect that the XML processor will discard anything describing flags
but
- // with no fields.
- assert(fields.size() && "Some fields must be provided.");
-
// We expect that these are unsorted but do not overlap.
// They could fill the register but may have gaps.
std::vector provided_fields = fields;
``
[Lldb-commits] [lldb] [lldb] Do not produce field information for registers known not to exist (PR #95125)
https://github.com/DavidSpickett ready_for_review https://github.com/llvm/llvm-project/pull/95125 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] [lldb] Do not produce field information for registers known not to exist (PR #95125)
https://github.com/DavidSpickett created
https://github.com/llvm/llvm-project/pull/95125
Currently the logic is generate field information for all registers in
LinuxArm64RegisterFlags and then as we walk the existing register info, only
those that are in that existing info will get the new fields patched in.
This works fine but on a review for FreeBSD support it was pointed out that
this is not obvious from the source code.
So instead I've allowed the construction of empty lists of fields, and field
detection methods can return an empty field list if they think that the
register will never exist.
Then the pre-existing code will see the empty field list, and never look for
that register in the register info.
I think removing the assert is ok because the GDB classes filter out empty
field lists at runtime, and anyone updating the built in field information
would presumably notice if none of the fields they intended to add were
displayed.
mte_ctrl and svcr are the only registers that need this so far.
There is no extra testing here as the behaviour is the same, it doesn't add
field information to regiters that don't exist. The mechanism is just clearer
now.
>From ebfd98a3dcf80d0fd1b680407e49e95762f90741 Mon Sep 17 00:00:00 2001
From: David Spickett
Date: Tue, 11 Jun 2024 13:41:18 +
Subject: [PATCH] [lldb] Do not produce field information for registers known
not to exist
Currently the logic is generate field information for all registers in
LinuxArm64RegisterFlags and then as we walk the existing register info,
only those that are in that existing info will get the new fields patched in.
This works fine but on a review for FreeBSD support it was pointed out
that this is not obvious from the source code.
So instead I've allowed the construction of empty lists of fields,
and field detection methods can return an empty field list if they
think that the register will never exist.
Then the pre-existing code will see the empty field list, and never
look for that register in the register info.
I think removing the assert is ok because the GDB classes filter
out empty field lists at runtime, and anyone updating the built in
field information would presumably notice if none of the fields they
intended to add were displayed.
mte_ctrl and svcr are the only registers that need this so far.
There is no extra testing here as the behaviour is the same, it
doesn't add field information to regiters that don't exist. The
mechanism is just clearer now.
---
.../Process/Utility/RegisterFlagsLinux_arm64.cpp | 11 +--
.../Process/Utility/RegisterFlagsLinux_arm64.h| 6 +++---
lldb/source/Target/RegisterFlags.cpp | 4
3 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
index 51553817921f3..8ed75d700f225 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.cpp
@@ -20,6 +20,7 @@
#define HWCAP2_BTI (1ULL << 17)
#define HWCAP2_MTE (1ULL << 18)
#define HWCAP2_AFP (1ULL << 20)
+#define HWCAP2_SME (1ULL << 23)
#define HWCAP2_EBF16 (1ULL << 32)
using namespace lldb_private;
@@ -27,7 +28,10 @@ using namespace lldb_private;
LinuxArm64RegisterFlags::Fields
LinuxArm64RegisterFlags::DetectSVCRFields(uint64_t hwcap, uint64_t hwcap2) {
(void)hwcap;
- (void)hwcap2;
+
+ if (!(hwcap2 & HWCAP2_SME))
+return {};
+
// Represents the pseudo register that lldb-server builds, which itself
// matches the architectural register SCVR. The fields match SVCR in the Arm
// manual.
@@ -40,7 +44,10 @@ LinuxArm64RegisterFlags::DetectSVCRFields(uint64_t hwcap,
uint64_t hwcap2) {
LinuxArm64RegisterFlags::Fields
LinuxArm64RegisterFlags::DetectMTECtrlFields(uint64_t hwcap, uint64_t hwcap2) {
(void)hwcap;
- (void)hwcap2;
+
+ if (!(hwcap2 & HWCAP2_MTE))
+return {};
+
// Represents the contents of NT_ARM_TAGGED_ADDR_CTRL and the value passed
// to prctl(PR_TAGGED_ADDR_CTRL...). Fields are derived from the defines
// used to build the value.
diff --git a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
index 660bef08700f4..49b1d90db64f6 100644
--- a/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
+++ b/lldb/source/Plugins/Process/Utility/RegisterFlagsLinux_arm64.h
@@ -38,8 +38,8 @@ class LinuxArm64RegisterFlags {
/// For the registers listed in this class, detect which fields are
/// present. Must be called before UpdateRegisterInfos.
/// If called more than once, fields will be redetected each time from
- /// scratch. If you do not have access to hwcap, just pass 0 for each one,
you
- /// will only get unconditional fields.
+ /// scratch. If the target would not have this register at all, the list of
+ ///
