[lldb] Fix invalid shift operator overload in Scalar

[mamai]

Summary

This also fixes an infinite recursion between lldb_private::operator>> () and Scalar::operator>>= ().

[labath]

Scalar::operator<<= seems to do a m_integer <<= .... Will it suffer from the same recursion problem?

In any case, I think the implementations of both methods should use the same API (it doesn't matter which one if both work).

Also it would be super great if you could make a tiny test for this, say in unittests/Core/ScalarTest.cpp

[mamai]

Scalar::operator<<= works well as-is because it uses APInt &operator<<=(unsigned shiftAmt), whereas the right shift equivalent is not implemented. Should I add APInt &operator>>=, or should I change Scalar::operator<<= for consistency ?

[labath]

Ah, ok I understand what's going on now. I guess APInt operator >> probably does not exist because it would be ambiguous (arithmetic or bitwise shift?). Let's just leave this as it is now.

Could you add that test though?

[mamai]

Added a small unit test for scalar right shift operator, which invokes the >>= operator.

[labath]

That's perfect. Thanks for adding that test.

Do you have commit access, or shall I put this in?

[mamai]

I don't have commit access. It would be great if you can push it for me. Thanks !

[labath]

Committed as r260239.

source/Core/Scalar.cpp

         case e_uint:
         case e_slong:
         case e_ulong:
         case e_slonglong:
         case e_ulonglong:
              case e_sint128:
              case e_uint128:
              {
-                 m_integer >> *rhs.m_integer.getRawData();
+                 m_integer = m_integer.ashr(*(const uint_t *)rhs.m_integer.getRawData());
                  break;
              }
         }
         break;
     }
     return *this;
 }
 

unittests/CMakeLists.txt

add_unittest(LLDBUnitTests
     ${ARGN}
     )
 
   lldb_link_common_libs(${test_name} EXE)
   target_link_libraries(${test_name} ${CLANG_USED_LIBS} ${LLDB_SYSTEM_LIBS})
   llvm_config(${test_name} ${LLVM_LINK_COMPONENTS})
 endfunction()
 
+add_subdirectory(Core)
 add_subdirectory(Editline)
 add_subdirectory(Expression)
 add_subdirectory(Host)
 add_subdirectory(Interpreter)
 add_subdirectory(ScriptInterpreter)
 add_subdirectory(Utility)

unittests/Core/CMakeLists.txt

+add_lldb_unittest(CoreTests
+  ScalarTest.cpp
+  )

unittests/Core/ScalarTest.cpp

+//===-- ScalarTest.cpp ------------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#if defined(_MSC_VER) && (_HAS_EXCEPTIONS == 0)
+// Workaround for MSVC standard library bug, which fails to include <thread> when
+// exceptions are disabled.
+#include <eh.h>
+#endif
+
+#include "gtest/gtest.h"
+
+#include "lldb/Core/Scalar.h"
+
+using namespace lldb_private;
+
+TEST(ScalarTest, RightShiftOperator)
+{
+    int a = 0x00001000;
+    int b = 0xFFFFFFFF;
+    int c = 4;
+    Scalar a_scalar(a);
+    Scalar b_scalar(b);
+    Scalar c_scalar(c);
+    ASSERT_EQ(a >> c, a_scalar >> c_scalar);
+    ASSERT_EQ(b >> c, b_scalar >> c_scalar);
+}