Skip to content

Conversation

@kuilpd
Copy link
Contributor

@kuilpd kuilpd commented Sep 22, 2025

Right shift operator in Scalar didn't check if the value is unsigned to perform a logical right shift. Use the right shift operator from APSInt that does this check.

@kuilpd kuilpd requested a review from Michael137 September 22, 2025 16:44
@kuilpd kuilpd added the lldb label Sep 22, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 22, 2025

@llvm/pr-subscribers-lldb

Author: Ilia Kuklin (kuilpd)

Changes

Right shift operator in Scalar didn't check if the value is unsigned to perform a logical right shift. Use the right shift operator from APSInt that does this check.


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

2 Files Affected:

  • (modified) lldb/source/Utility/Scalar.cpp (+3-17)
  • (modified) lldb/unittests/Utility/ScalarTest.cpp (+3)
diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index c8766bdf2aee7..f2c18cdd896da 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -471,24 +471,10 @@ bool Scalar::ShiftRightLogical(const Scalar &rhs) {
 }
 
 Scalar &Scalar::operator>>=(const Scalar &rhs) {
-  switch (m_type) {
-  case e_void:
-  case e_float:
+  if (m_type == e_int && rhs.m_type == e_int)
+    m_integer >>= rhs.m_integer.getZExtValue();
+  else
     m_type = e_void;
-    break;
-
-  case e_int:
-    switch (rhs.m_type) {
-    case e_void:
-    case e_float:
-      m_type = e_void;
-      break;
-    case e_int:
-      m_integer = m_integer.ashr(rhs.m_integer);
-      break;
-    }
-    break;
-  }
   return *this;
 }
 
diff --git a/lldb/unittests/Utility/ScalarTest.cpp b/lldb/unittests/Utility/ScalarTest.cpp
index 6d5caef42bee4..e6d7479b59fee 100644
--- a/lldb/unittests/Utility/ScalarTest.cpp
+++ b/lldb/unittests/Utility/ScalarTest.cpp
@@ -118,11 +118,14 @@ TEST(ScalarTest, RightShiftOperator) {
   int a = 0x00001000;
   int b = 0xFFFFFFFF;
   int c = 4;
+  unsigned d = 0xFFFFFFFF;
   Scalar a_scalar(a);
   Scalar b_scalar(b);
   Scalar c_scalar(c);
+  Scalar d_scalar(d);
   ASSERT_EQ(a >> c, a_scalar >> c_scalar);
   ASSERT_EQ(b >> c, b_scalar >> c_scalar);
+  ASSERT_EQ(d >> c, d_scalar >> c_scalar);
 }
 
 TEST(ScalarTest, GetBytes) {

Scalar d_scalar(d);
ASSERT_EQ(a >> c, a_scalar >> c_scalar);
ASSERT_EQ(b >> c, b_scalar >> c_scalar);
ASSERT_EQ(d >> c, d_scalar >> c_scalar);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also add a test for unsigned short?

@kuilpd kuilpd merged commit 154e063 into llvm:main Sep 23, 2025
9 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Sep 23, 2025

LLVM Buildbot has detected a new failure on builder cross-project-tests-sie-ubuntu running on doug-worker-1a while building lldb at step 6 "test-build-unified-tree-check-cross-project".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/181/builds/28439

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-cross-project) failure: test (failure)
******************** TEST 'cross-project-tests :: debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/limit_steps_conditional_hit_count.cpp' FAILED ********************
Exit Code: 2

Command Output (stderr):
--
clang++ -O0 -glldb -std=gnu++11 /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/limit_steps_conditional_hit_count.cpp -o /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/build/projects/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/Output/limit_steps_conditional_hit_count.cpp.tmp # RUN: at line 11
+ clang++ -O0 -glldb -std=gnu++11 /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/limit_steps_conditional_hit_count.cpp -o /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/build/projects/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/Output/limit_steps_conditional_hit_count.cpp.tmp
"/usr/bin/python3.8" "/home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/cross-project-tests/debuginfo-tests/dexter/dexter.py" test --fail-lt 1.0 -w -v --debugger lldb-dap --lldb-executable "/home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/build/bin/lldb-dap" --dap-message-log=-e --binary /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/build/projects/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/Output/limit_steps_conditional_hit_count.cpp.tmp -- /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/limit_steps_conditional_hit_count.cpp | /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/build/bin/FileCheck /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/limit_steps_conditional_hit_count.cpp # RUN: at line 12
+ /usr/bin/python3.8 /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/cross-project-tests/debuginfo-tests/dexter/dexter.py test --fail-lt 1.0 -w -v --debugger lldb-dap --lldb-executable /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/build/bin/lldb-dap --dap-message-log=-e --binary /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/build/projects/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/Output/limit_steps_conditional_hit_count.cpp.tmp -- /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/limit_steps_conditional_hit_count.cpp
+ /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/build/bin/FileCheck /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/llvm-project/cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/dex_finish_test/limit_steps_conditional_hit_count.cpp
note: Opening DAP server: /home/buildbot/buildbot-root/cross-project-tests-sie-ubuntu/build/bin/lldb-dap
-> {
  "type": "request",
  "command": "initialize",
  "arguments": {
    "clientID": "dexter",
    "adapterID": "lldb-dap",
    "pathFormat": "path",
    "linesStartAt1": true,
    "columnsStartAt1": true,
    "supportsVariableType": true,
    "supportsVariablePaging": true,
    "supportsRunInTerminalRequest": false
  },
  "seq": 1
}
<- {
  "body": {
    "$__lldb_version": "lldb version 22.0.0git (https://github.com/llvm/llvm-project.git revision 154e0637c9b60108bda4e73645a3c9b9f62020c4)\n  clang revision 154e0637c9b60108bda4e73645a3c9b9f62020c4\n  llvm revision 154e0637c9b60108bda4e73645a3c9b9f62020c4",
    "completionTriggerCharacters": [
      ".",
      " ",
      "\t"
    ],
    "exceptionBreakpointFilters": [
      {
        "description": "C++ Catch",
        "filter": "cpp_catch",
        "label": "C++ Catch",
        "supportsCondition": true
      },
      {
        "description": "C++ Throw",
        "filter": "cpp_throw",
        "label": "C++ Throw",
        "supportsCondition": true
      },
      {
        "description": "Objective-C Catch",
        "filter": "objc_catch",
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants