[clang-tools-extra] [clangd] Allow hover over 128-bit variable without crashing (PR #71415)

Björn Pettersson via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 7 14:40:04 PST 2023


https://github.com/bjope updated https://github.com/llvm/llvm-project/pull/71415

>From 2b1af2bff6432a50eb725adf86e6cfcc52cc020b Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
Date: Mon, 6 Nov 2023 17:27:28 +0100
Subject: [PATCH 1/2] [clangd] Allow hover over 128-bit variable without
 crashing

When hovering over variables larger than 64 bits, with more than
64 active bits, there were assertion failures since Hover is trying
to print the value as a 64-bit hex value.

There is already protection avoiding to call printHex if there is
more than 64 significant bits. And we already truncate and print
negative values using only 32 bits, when possible. So we can simply
truncate values with more than 64 bits to avoid the assert when using
getZExtValue. The result will be that for example a negative 128 bit
variable is printed using 64 bits, when possible.

There is still no support for printing more than 64 bits. That would
involve more changes since for example llvm::FormatterNumber is
limited to 64 bits.
---
 clang-tools-extra/clangd/Hover.cpp        |  4 +++-
 clang-tools-extra/clangd/test/hover2.test | 29 +++++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100644 clang-tools-extra/clangd/test/hover2.test

diff --git a/clang-tools-extra/clangd/Hover.cpp b/clang-tools-extra/clangd/Hover.cpp
index 7f7b5513dff6fee..a868d3bb4e3fa1d 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -408,7 +408,9 @@ void fillFunctionTypeAndParams(HoverInfo &HI, const Decl *D,
 // -2    => 0xfffffffe
 // -2^32 => 0xffffffff00000000
 static llvm::FormattedNumber printHex(const llvm::APSInt &V) {
-  uint64_t Bits = V.getZExtValue();
+  assert(V.getSignificantBits() <= 64 && "Can't print more than 64 bits.");
+  uint64_t Bits =
+      V.getBitWidth() > 64 ? V.trunc(64).getZExtValue() : V.getZExtValue();
   if (V.isNegative() && V.getSignificantBits() <= 32)
     return llvm::format_hex(uint32_t(Bits), 0);
   return llvm::format_hex(Bits, 0);
diff --git a/clang-tools-extra/clangd/test/hover2.test b/clang-tools-extra/clangd/test/hover2.test
new file mode 100644
index 000000000000000..24d82bde20a7823
--- /dev/null
+++ b/clang-tools-extra/clangd/test/hover2.test
@@ -0,0 +1,29 @@
+# RUN: clangd -lit-test < %s | FileCheck %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"__int128_t bar = -4;\n"}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":13}}}
+#      CHECK:  "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": {
+# CHECK-NEXT:    "contents": {
+# CHECK-NEXT:      "kind": "plaintext",
+# CHECK-NEXT:       "value": "variable bar\n\nType: __int128_t (aka __int128)\nValue = -4 (0xfffffffc)\n\n__int128_t bar = -4"
+# CHECK-NEXT:     },
+# CHECK-NEXT:     "range": {
+# CHECK-NEXT:       "end": {
+# CHECK-NEXT:         "character": 14,
+# CHECK-NEXT:         "line": 0
+# CHECK-NEXT:       },
+# CHECK-NEXT:       "start": {
+# CHECK-NEXT:         "character": 11,
+# CHECK-NEXT:         "line": 0
+# CHECK-NEXT:       }
+# CHECK-NEXT:     }
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}

>From 6e2347e28284e0b7e582fc2d51180b4340f15910 Mon Sep 17 00:00:00 2001
From: Bjorn Pettersson <bjorn.a.pettersson at ericsson.com>
Date: Tue, 7 Nov 2023 23:38:19 +0100
Subject: [PATCH 2/2] Fixup (should be squashed)

Replace the test case a with a unittest.
---
 clang-tools-extra/clangd/test/hover2.test     | 29 -------------------
 .../clangd/unittests/HoverTests.cpp           | 11 +++++++
 2 files changed, 11 insertions(+), 29 deletions(-)
 delete mode 100644 clang-tools-extra/clangd/test/hover2.test

diff --git a/clang-tools-extra/clangd/test/hover2.test b/clang-tools-extra/clangd/test/hover2.test
deleted file mode 100644
index 24d82bde20a7823..000000000000000
--- a/clang-tools-extra/clangd/test/hover2.test
+++ /dev/null
@@ -1,29 +0,0 @@
-# RUN: clangd -lit-test < %s | FileCheck %s
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
----
-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"__int128_t bar = -4;\n"}}}
----
-{"jsonrpc":"2.0","id":1,"method":"textDocument/hover","params":{"textDocument":{"uri":"test:///main.cpp"},"position":{"line":0,"character":13}}}
-#      CHECK:  "id": 1,
-# CHECK-NEXT:  "jsonrpc": "2.0",
-# CHECK-NEXT:  "result": {
-# CHECK-NEXT:    "contents": {
-# CHECK-NEXT:      "kind": "plaintext",
-# CHECK-NEXT:       "value": "variable bar\n\nType: __int128_t (aka __int128)\nValue = -4 (0xfffffffc)\n\n__int128_t bar = -4"
-# CHECK-NEXT:     },
-# CHECK-NEXT:     "range": {
-# CHECK-NEXT:       "end": {
-# CHECK-NEXT:         "character": 14,
-# CHECK-NEXT:         "line": 0
-# CHECK-NEXT:       },
-# CHECK-NEXT:       "start": {
-# CHECK-NEXT:         "character": 11,
-# CHECK-NEXT:         "line": 0
-# CHECK-NEXT:       }
-# CHECK-NEXT:     }
-# CHECK-NEXT:   }
-# CHECK-NEXT: }
----
-{"jsonrpc":"2.0","id":3,"method":"shutdown"}
----
-{"jsonrpc":"2.0","method":"exit"}
diff --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 063a60db044060e..847f141f521caa0 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -3349,6 +3349,17 @@ TEST(Hover, NoCrashAPInt64) {
   getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
 }
 
+TEST(Hover, NoCrashInt128) {
+  Annotations T(R"cpp(
+    constexpr __int128_t value = -4;
+    void foo() { va^lue; }
+  )cpp");
+  auto AST = TestTU::withCode(T.code()).build();
+  auto H = getHover(AST, T.point(), format::getLLVMStyle(), nullptr);
+  ASSERT_TRUE(H);
+  EXPECT_EQ(H->Value, "-4 (0xfffffffc)");
+}
+
 TEST(Hover, DocsFromMostSpecial) {
   Annotations T(R"cpp(
   // doc1



More information about the cfe-commits mailing list