[Lldb-commits] [lldb] [lldb] Fix printing of unsigned enums bitfields when they contain the max value (PR #96202)

[David Spickett via lldb-commits]

https://github.com/DavidSpickett created https://github.com/llvm/llvm-project/pull/96202

While testing register fields I found that if you put the max value into a bitfield with an underlying type that is an unsigned enum, lldb would not print the enum name.

This is because the code to match values to names wasn't checking whether the enum's type was signed, it just assumed it was.

So for example a 2 bit field with value 3 got signed extended to -1, which didn't match the enumerator value of 3. So lldb just printed the number instead of the name.

For a value of 1, the top bit was 0 so the sign extend became a zero extend, and lldb did print the name of the enumerator.

I added a new test because I needed to use C++ to get typed enums. It checks min, max and an in between value for signed and unsigned enums applied to a bitfield.

>From 49113d1f126d4bf048fba6ece0e28463b9ee3884 Mon Sep 17 00:00:00 2001
From: David Spickett <david.spickett at linaro.org>
Date: Thu, 20 Jun 2024 14:37:24 +0000
Subject: [PATCH] [lldb] Fix printing of unsigned enums used as bitfield types

While testing register fields I found that if you put the max
value into a bitfield with an underlying type that is an unsigned
enum, lldb would not print the enum name.

This is because the code to match values to names wasn't checking
whether the enum's type was signed, it just assumed it was.

So for example a 2 bit field with value 3 got signed extended
to -1, which didn't match the enumerator value of 3. So lldb
just printed the number instead of the name.

For a value of 1, the top bit was 0 so the sign extend became
a zero extend, and lldb did print the name of the enumerator.

I added a new test because I needed to use C++ to get typed
enums. It checks min, max and an in between value for signed
and unsigned enums applied to a bitfield.
---
 .../TypeSystem/Clang/TypeSystemClang.cpp      | 18 ++++++++---
 .../expression/bitfield_enums/Makefile        |  3 ++
 .../bitfield_enums/TestBitfieldEnums.py       | 31 +++++++++++++++++++
 .../expression/bitfield_enums/main.cpp        | 24 ++++++++++++++
 4 files changed, 71 insertions(+), 5 deletions(-)
 create mode 100644 lldb/test/API/commands/expression/bitfield_enums/Makefile
 create mode 100644 lldb/test/API/commands/expression/bitfield_enums/TestBitfieldEnums.py
 create mode 100644 lldb/test/API/commands/expression/bitfield_enums/main.cpp

diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index 0270c67366d1d..0fdc0ff150d30 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -8639,8 +8639,13 @@ static bool DumpEnumValue(const clang::QualType &qual_type, Stream &s,
   const clang::EnumDecl *enum_decl = enutype->getDecl();
   assert(enum_decl);
   lldb::offset_t offset = byte_offset;
-  const uint64_t enum_svalue = data.GetMaxS64Bitfield(
-      &offset, byte_size, bitfield_bit_size, bitfield_bit_offset);
+  bool qual_type_is_signed = qual_type->isSignedIntegerOrEnumerationType();
+  const uint64_t enum_svalue =
+      qual_type_is_signed
+          ? data.GetMaxS64Bitfield(&offset, byte_size, bitfield_bit_size,
+                                   bitfield_bit_offset)
+          : data.GetMaxU64Bitfield(&offset, byte_size, bitfield_bit_size,
+                                   bitfield_bit_offset);
   bool can_be_bitfield = true;
   uint64_t covered_bits = 0;
   int num_enumerators = 0;
@@ -8652,8 +8657,11 @@ static bool DumpEnumValue(const clang::QualType &qual_type, Stream &s,
   // enumerators. Also 0 doesn't make sense when the enumerators are used as
   // flags.
   for (auto *enumerator : enum_decl->enumerators()) {
-    uint64_t val = enumerator->getInitVal().getSExtValue();
-    val = llvm::SignExtend64(val, 8*byte_size);
+    llvm::APSInt init_val = enumerator->getInitVal();
+    uint64_t val =
+        qual_type_is_signed ? init_val.getSExtValue() : init_val.getZExtValue();
+    if (qual_type_is_signed)
+      val = llvm::SignExtend64(val, 8 * byte_size);
     if (llvm::popcount(val) != 1 && (val & ~covered_bits) != 0)
       can_be_bitfield = false;
     covered_bits |= val;
@@ -8673,7 +8681,7 @@ static bool DumpEnumValue(const clang::QualType &qual_type, Stream &s,
   // No exact match, but we don't think this is a bitfield. Print the value as
   // decimal.
   if (!can_be_bitfield) {
-    if (qual_type->isSignedIntegerOrEnumerationType())
+    if (qual_type_is_signed)
       s.Printf("%" PRIi64, enum_svalue);
     else
       s.Printf("%" PRIu64, enum_uvalue);
diff --git a/lldb/test/API/commands/expression/bitfield_enums/Makefile b/lldb/test/API/commands/expression/bitfield_enums/Makefile
new file mode 100644
index 0000000000000..99998b20bcb05
--- /dev/null
+++ b/lldb/test/API/commands/expression/bitfield_enums/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/commands/expression/bitfield_enums/TestBitfieldEnums.py b/lldb/test/API/commands/expression/bitfield_enums/TestBitfieldEnums.py
new file mode 100644
index 0000000000000..a484b69300e7b
--- /dev/null
+++ b/lldb/test/API/commands/expression/bitfield_enums/TestBitfieldEnums.py
@@ -0,0 +1,31 @@
+"""
+Test that the expression parser accounts for the underlying type of bitfield
+enums when looking for matching values.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestBitfieldEnum(TestBase):
+    def test_bitfield_enums(self):
+        self.build()
+
+        lldbutil.run_to_source_breakpoint(
+            self, "// break here", lldb.SBFileSpec("main.cpp", False)
+        )
+
+        self.expect_expr(
+            "bfs",
+            result_type="BitfieldStruct",
+            result_children=[
+                ValueCheck(name="signed_min", value="min"),
+                ValueCheck(name="signed_other", value="-1"),
+                ValueCheck(name="signed_max", value="max"),
+                ValueCheck(name="unsigned_min", value="min"),
+                ValueCheck(name="unsigned_other", value="1"),
+                ValueCheck(name="unsigned_max", value="max"),
+            ],
+        )
diff --git a/lldb/test/API/commands/expression/bitfield_enums/main.cpp b/lldb/test/API/commands/expression/bitfield_enums/main.cpp
new file mode 100644
index 0000000000000..f6c53b3100b93
--- /dev/null
+++ b/lldb/test/API/commands/expression/bitfield_enums/main.cpp
@@ -0,0 +1,24 @@
+enum class SignedEnum : int { min = -2, max = 1 };
+enum class UnsignedEnum : unsigned { min = 0, max = 3 };
+
+struct BitfieldStruct {
+  SignedEnum signed_min : 2;
+  SignedEnum signed_other : 2;
+  SignedEnum signed_max : 2;
+  UnsignedEnum unsigned_min : 2;
+  UnsignedEnum unsigned_other : 2;
+  UnsignedEnum unsigned_max : 2;
+};
+
+int main() {
+  BitfieldStruct bfs;
+  bfs.signed_min = SignedEnum::min;
+  bfs.signed_other = static_cast<SignedEnum>(-1);
+  bfs.signed_max = SignedEnum::max;
+
+  bfs.unsigned_min = UnsignedEnum::min;
+  bfs.unsigned_other = static_cast<UnsignedEnum>(1);
+  bfs.unsigned_max = UnsignedEnum::max;
+
+  return 0; // break here
+}