[lldb][NativePDB] Fix width and signedness of enum constants

[Nerixyz]

Enumerator values aren't always encoded in the correct bit width and signedness. This happens with both MSVC and Clang.

On MSVC, unsigned 64bit enumerators can be encoded as signed. For example ULONGLONG_MAX will be encoded as -1.
Clang/LLVM will always encode the values as unsigned.

Example: https://godbolt.org/z/96YjGW48W.

I fixed this by setting the expected width and signedness when creating the enumerator constant. We can't use a shell test like for the other PDB tests, because no output shows the enum values.

[llvmorg-github-actions]

@llvm/pr-subscribers-lldb

Author: Nerixyz (Nerixyz)

Changes

Enumerator values aren't always encoded in the correct bit width and signedness. This happens with both MSVC and Clang.

On MSVC, unsigned 64bit enumerators can be encoded as signed. For example ULONGLONG_MAX will be encoded as -1.
Clang/LLVM will always encode the values as unsigned.

Example: https://godbolt.org/z/96YjGW48W.

I fixed this by setting the expected width and signedness when creating the enumerator constant. We can't use a shell test like for the other PDB tests, because no output shows the enum values.

---

Full diff: https://github.com/llvm/llvm-project/pull/210338.diff

4 Files Affected:

• (modified) lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp (+17-1)
• (added) lldb/test/API/lang/cpp/enum-limits/Makefile (+3)
• (added) lldb/test/API/lang/cpp/enum-limits/TestCPPEnumLimits.py (+91)
• (added) lldb/test/API/lang/cpp/enum-limits/main.cpp (+48)

diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
index 9840b976d2713..871b053151c61 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp
@@ -331,8 +331,24 @@ Error UdtRecordCompleter::visitKnownMember(CVMemberRecord &cvr,
   Declaration decl;
   llvm::StringRef name = DropNameScope(enumerator.getName());
 
+  clang::EnumDecl *enum_decl = TypeSystemClang::GetAsEnumDecl(m_derived_ct);
+  if (!enum_decl)
+    return Error::success();
+
+  llvm::APSInt val = enumerator.Value;
+  clang::QualType int_ty = enum_decl->getIntegerType();
+  uint64_t n_bits = m_ast_builder.clang().getASTContext().getTypeSize(int_ty);
+  if (n_bits == 0)
+    return Error::success();
+
+  // MSVC encodes 64 Bit unsigned enum values as signed integers. For example,
+  // ULONGLONG_MAX will be encoded as -1. LLVM encodes all values as unsigned.
+  // Fix this by explicitly setting the bit width and signedness.
+  val = val.extOrTrunc(n_bits);
+  val.setIsSigned(int_ty->isSignedIntegerType());
+
   m_ast_builder.clang().AddEnumerationValueToEnumerationType(
-      m_derived_ct, decl, name.str().c_str(), enumerator.Value);
+      m_derived_ct, decl, name.str().c_str(), val);
   return Error::success();
 }
 
diff --git a/lldb/test/API/lang/cpp/enum-limits/Makefile b/lldb/test/API/lang/cpp/enum-limits/Makefile
new file mode 100644
index 0000000000000..99998b20bcb05
--- /dev/null
+++ b/lldb/test/API/lang/cpp/enum-limits/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/lang/cpp/enum-limits/TestCPPEnumLimits.py b/lldb/test/API/lang/cpp/enum-limits/TestCPPEnumLimits.py
new file mode 100644
index 0000000000000..4e0c0a088c909
--- /dev/null
+++ b/lldb/test/API/lang/cpp/enum-limits/TestCPPEnumLimits.py
@@ -0,0 +1,91 @@
+"""Check that enumerator values are correct when they're near the limits of the underlying type."""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class CPPEnumLimitsTestCase(TestBase):
+    SHARED_BUILD_TESTCASE = False
+    TEST_WITH_PDB_DEBUG_INFO = True
+
+    def check_signed(self, ty: lldb.SBType, expected):
+        self.assertEqual(
+            {i.GetName(): i.GetValueAsSigned() for i in ty.GetEnumMembers()}, expected
+        )
+
+    def check_unsigned(self, ty: lldb.SBType, expected):
+        self.assertEqual(
+            {i.GetName(): i.GetValueAsUnsigned() for i in ty.GetEnumMembers()}, expected
+        )
+
+    def check_all(self, target: lldb.SBTarget):
+        self.check_unsigned(target.FindFirstType("U8Enum"), {"Min": 0, "Max": 255})
+        self.check_signed(target.FindFirstType("I8Enum"), {"Min": -128, "Max": 127})
+        self.check_unsigned(target.FindFirstType("U16Enum"), {"Min": 0, "Max": 65535})
+        self.check_signed(
+            target.FindFirstType("I16Enum"), {"Min": -32768, "Max": 32767}
+        )
+        self.check_unsigned(
+            target.FindFirstType("U32Enum"),
+            {"Min": 0, "MaxMinusOne": 4294967294, "Max": 4294967295},
+        )
+        self.check_signed(
+            target.FindFirstType("I32Enum"), {"Min": -2147483648, "Max": 2147483647}
+        )
+        self.check_unsigned(
+            target.FindFirstType("U64Enum"),
+            {
+                "Min": 0,
+                "MaxMinusOne": 18446744073709551614,
+                "Max": 18446744073709551615,
+            },
+        )
+        self.check_signed(
+            target.FindFirstType("I64Enum"),
+            {
+                "Min": -9223372036854775808,
+                "MinPlusOne": -9223372036854775807,
+                "MaxMinusOne": 9223372036854775806,
+                "Max": 9223372036854775807,
+            },
+        )
+
+    def test(self):
+        self.build()
+        self.check_all(self.dbg.CreateTarget(self.getBuildArtifact("a.out")))
+
+    @skipUnlessPlatform(["windows"])
+    @skipUnlessMSVC
+    @no_debug_info_test  # We only test MSVC
+    def test_msvc(self):
+        """Test that the limits work on MSVC."""
+
+        src = os.path.join(self.getSourceDir(), "main.cpp")
+        exe = os.path.join(self.getBuildDir(), "a.exe")
+
+        # FIXME: Allow MSVC to be used with the Makefile.
+        result = subprocess.run(
+            [
+                "cl.exe",
+                "/nologo",
+                "/Od",
+                "/Zi",
+                "/Fe" + exe,
+                src,
+                "/link",
+                "/nodefaultlib",
+                "/entry:main",
+            ],
+            cwd=self.getBuildDir(),
+            capture_output=True,
+            text=True,
+        )
+        self.assertEqual(
+            result.returncode,
+            0,
+            "Compilation failed:\n" + result.stdout + result.stderr,
+        )
+
+        self.check_all(self.dbg.CreateTarget(exe))
diff --git a/lldb/test/API/lang/cpp/enum-limits/main.cpp b/lldb/test/API/lang/cpp/enum-limits/main.cpp
new file mode 100644
index 0000000000000..3aeef3fd03e01
--- /dev/null
+++ b/lldb/test/API/lang/cpp/enum-limits/main.cpp
@@ -0,0 +1,48 @@
+enum class U8Enum : unsigned char {
+  Min = 0,
+  Max = 255,
+};
+enum class I8Enum : char {
+  Min = -128,
+  Max = 127,
+};
+enum class U16Enum : unsigned short {
+  Min = 0,
+  Max = 65535,
+};
+enum class I16Enum : short {
+  Min = -32768,
+  Max = 32767,
+};
+enum class U32Enum : unsigned {
+  Min = 0,
+  MaxMinusOne = 4294967294,
+  Max = 4294967295,
+};
+enum class I32Enum : int {
+  Min = -2147483648,
+  Max = 2147483647,
+};
+enum class U64Enum : unsigned long long {
+  Min = 0,
+  MaxMinusOne = 18446744073709551614ULL,
+  Max = 18446744073709551615ULL,
+};
+enum class I64Enum : long long {
+  Min = -9223372036854775807LL - 1,
+  MinPlusOne = -9223372036854775807LL,
+  MaxMinusOne = 9223372036854775806LL,
+  Max = 9223372036854775807LL,
+};
+
+int main(){
+    auto u8 = U8Enum::Max;
+    auto i8 = I8Enum::Max;
+    auto u16 = U16Enum::Max;
+    auto i16 = I16Enum::Max;
+    auto u32 = U32Enum::Max;
+    auto i32 = I32Enum::Max;
+    auto u64 = U64Enum::Max;
+    auto i64 = I64Enum::Max;
+    return 0;
+}

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.