[Lldb-commits] [lldb] [LLDB] Add an assert to verify sign_bit_pos is within the valid range (NFC) (PR #95678)

[Shivam Gupta via lldb-commits]

https://github.com/xgupta updated https://github.com/llvm/llvm-project/pull/95678

>From ef5b9cefb408ca3a721c95d8f6702abba77a602b Mon Sep 17 00:00:00 2001
From: Shivam Gupta <shivam98.tkg at gmail.com>
Date: Sun, 16 Jun 2024 00:21:51 +0530
Subject: [PATCH 1/4] [LLDB] Add an assert to verify sign_bit_pos is within the
 valid range

---
 lldb/source/Utility/Scalar.cpp | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index c680101aa9efa..6e2f1ca4c1613 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -745,26 +745,25 @@ Status Scalar::SetValueFromData(const DataExtractor &data,
 
 bool Scalar::SignExtend(uint32_t sign_bit_pos) {
   const uint32_t max_bit_pos = GetByteSize() * 8;
+  assert(sign_bit_pos < max_bit_pos);
 
-  if (sign_bit_pos < max_bit_pos) {
-    switch (m_type) {
-    case Scalar::e_void:
-    case Scalar::e_float:
-      return false;
+  switch (m_type) {
+  case Scalar::e_void:
+  case Scalar::e_float:
+    return false;
 
-    case Scalar::e_int:
-      if (sign_bit_pos < (max_bit_pos - 1)) {
-        llvm::APInt sign_bit = llvm::APInt::getSignMask(sign_bit_pos + 1);
-        llvm::APInt bitwize_and = m_integer & sign_bit;
-        if (bitwize_and.getBoolValue()) {
-          llvm::APInt mask =
-              ~(sign_bit) + llvm::APInt(m_integer.getBitWidth(), 1);
-          m_integer |= APSInt(std::move(mask), m_integer.isUnsigned());
-        }
-        return true;
+  case Scalar::e_int:
+    if (sign_bit_pos < (max_bit_pos - 1)) {
+      llvm::APInt sign_bit = llvm::APInt::getSignMask(sign_bit_pos + 1);
+      llvm::APInt bitwize_and = m_integer & sign_bit;
+      if (bitwize_and.getBoolValue()) {
+        llvm::APInt mask =
+            ~(sign_bit) + llvm::APInt(m_integer.getBitWidth(), 1);
+        m_integer |= APSInt(std::move(mask), m_integer.isUnsigned());
       }
-      break;
+      return true;
     }
+    break;
   }
   return false;
 }

>From 98a183742e3dc6393aa55000fff1931263ff165a Mon Sep 17 00:00:00 2001
From: Shivam Gupta <shivam98.tkg at gmail.com>
Date: Wed, 19 Jun 2024 13:57:55 +0530
Subject: [PATCH 2/4] address review suggestion

---
 lldb/source/Utility/Scalar.cpp | 24 ++++++++----------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index 6e2f1ca4c1613..496f402a74114 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -747,25 +747,17 @@ bool Scalar::SignExtend(uint32_t sign_bit_pos) {
   const uint32_t max_bit_pos = GetByteSize() * 8;
   assert(sign_bit_pos < max_bit_pos);
 
-  switch (m_type) {
-  case Scalar::e_void:
-  case Scalar::e_float:
+  if (m_type != Scalar::e_int || sign_bit_pos >= (max_bit_pos - 1)) {
     return false;
+  }
 
-  case Scalar::e_int:
-    if (sign_bit_pos < (max_bit_pos - 1)) {
-      llvm::APInt sign_bit = llvm::APInt::getSignMask(sign_bit_pos + 1);
-      llvm::APInt bitwize_and = m_integer & sign_bit;
-      if (bitwize_and.getBoolValue()) {
-        llvm::APInt mask =
-            ~(sign_bit) + llvm::APInt(m_integer.getBitWidth(), 1);
-        m_integer |= APSInt(std::move(mask), m_integer.isUnsigned());
-      }
-      return true;
-    }
-    break;
+  llvm::APInt sign_bit = llvm::APInt::getSignMask(sign_bit_pos + 1);
+  llvm::APInt bitwize_and = m_integer & sign_bit;
+  if (bitwize_and.getBoolValue()) {
+    llvm::APInt mask = ~(sign_bit) + llvm::APInt(m_integer.getBitWidth(), 1);
+    m_integer |= APSInt(std::move(mask), m_integer.isUnsigned());
   }
-  return false;
+  return true;
 }
 
 size_t Scalar::GetAsMemoryData(void *dst, size_t dst_len,

>From 5fbf491d0781c1dcce6bdc727844e75c527f6473 Mon Sep 17 00:00:00 2001
From: Shivam Gupta <shivma98.tkg at gmail.com>
Date: Wed, 24 Jul 2024 14:36:15 +0200
Subject: [PATCH 3/4] adjust assert to fix
 SymbolFile/DWARF/debug-types-expressions.test

---
 lldb/source/Utility/Scalar.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index 496f402a74114..42247f658da13 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -745,8 +745,8 @@ Status Scalar::SetValueFromData(const DataExtractor &data,
 
 bool Scalar::SignExtend(uint32_t sign_bit_pos) {
   const uint32_t max_bit_pos = GetByteSize() * 8;
-  assert(sign_bit_pos < max_bit_pos);
 
+  assert(sign_bit_pos <= max_bit_pos);
   if (m_type != Scalar::e_int || sign_bit_pos >= (max_bit_pos - 1)) {
     return false;
   }

>From 2a7cbea5c38b57ed412c1415254b2308b2bd1e16 Mon Sep 17 00:00:00 2001
From: xgupta <shivma98.tkg at gmail.com>
Date: Fri, 26 Jul 2024 11:03:32 +0200
Subject: [PATCH 4/4] address review comment

---
 lldb/source/Utility/Scalar.cpp | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index 42247f658da13..facb3e5a6a1a9 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -747,9 +747,8 @@ bool Scalar::SignExtend(uint32_t sign_bit_pos) {
   const uint32_t max_bit_pos = GetByteSize() * 8;
 
   assert(sign_bit_pos <= max_bit_pos);
-  if (m_type != Scalar::e_int || sign_bit_pos >= (max_bit_pos - 1)) {
+  if (m_type != Scalar::e_int || sign_bit_pos >= (max_bit_pos - 1))
     return false;
-  }
 
   llvm::APInt sign_bit = llvm::APInt::getSignMask(sign_bit_pos + 1);
   llvm::APInt bitwize_and = m_integer & sign_bit;