Add support for getting signed ObjC tagged pointer values

[jingham]

Summary

The ObjC runtime offers both signed & unsigned tagged pointer value
accessors to tagged pointer providers, but lldb's tagged pointer
code only implemented the unsigned one. This patch adds an
emulation of the signed one.

The motivation for doing this is that NSNumbers use the signed
accessor (they are always signed) and we need to follow that in our
summary provider or we will get incorrect values for negative
NSNumbers.

The data-formatter-objc test file had NSNumber examples (along with lots of other
goodies) but the NSNumber values weren't tested. So I also added
checks for those values to the test.

I also did a quick audit of the other types in that main.m file, and
it looks like pretty much all the other values are either intermediates
or are tested.

[JDevlieghere]

LGTM with a little nit. Please clang-format the patch before landing.

lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h

89–104

Please make this a Doxygen comment and include both methods in a group.

[shafik]

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

2494

note we can use make_shared.

2585

We can use make_shared here.

[stella.stamenova]

The latest change (commit be0ced03) issues the following warning:

/usr/bin/clang++ -DGTEST_HAS_RTTI=0 -DHAVE_ROUND -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/lldb/source/Plugins/Language/ObjC -I/mnt/vss/_work/2/s/lldb/source/Plugins/Language/ObjC -Itools/lldb/source -I/mnt/vss/_work/2/s/lldb/include -Itools/lldb/include -Iinclude -I/mnt/vss/_work/2/s/llvm/include -I/usr/include/python3.8 -I/mnt/vss/_work/2/s/llvm/../clang/include -Itools/lldb/../clang/include -I/usr/include/libxml2 -I/mnt/vss/_work/2/s/lldb/source/. -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -Wno-deprecated-declarations -Wno-unknown-pragmas -Wno-strict-aliasing -Wno-deprecated-register -Wno-vla-extension -O3   -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT tools/lldb/source/Plugins/Language/ObjC/CMakeFiles/lldbPluginObjCLanguage.dir/Cocoa.cpp.o -MF tools/lldb/source/Plugins/Language/ObjC/CMakeFiles/lldbPluginObjCLanguage.dir/Cocoa.cpp.o.d -o tools/lldb/source/Plugins/Language/ObjC/CMakeFiles/lldbPluginObjCLanguage.dir/Cocoa.cpp.o -c /mnt/vss/_work/2/s/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
/mnt/vss/_work/2/s/lldb/source/Plugins/Language/ObjC/Cocoa.cpp:528:11: error: misleading indentation; statement is not part of the previous 'if' [-Werror,-Wmisleading-indentation]
          return false;
          ^
/mnt/vss/_work/2/s/lldb/source/Plugins/Language/ObjC/Cocoa.cpp:525:9: note: previous statement is here
        if (log)

For this badly indented code:

if (is_preserved_number) {
if (log) 
  log->Printf("Unsupported preserved NSNumber tagged pointer 0x%" 
      PRIu64, valobj_addr);
  return false;
}

[stella.stamenova]

It looks like @teemperor fixed it already!

In D99694#2671640, @stella.stamenova wrote:

The latest change (commit be0ced03) issues the following warning:

/usr/bin/clang++ -DGTEST_HAS_RTTI=0 -DHAVE_ROUND -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/lldb/source/Plugins/Language/ObjC -I/mnt/vss/_work/2/s/lldb/source/Plugins/Language/ObjC -Itools/lldb/source -I/mnt/vss/_work/2/s/lldb/include -Itools/lldb/include -Iinclude -I/mnt/vss/_work/2/s/llvm/include -I/usr/include/python3.8 -I/mnt/vss/_work/2/s/llvm/../clang/include -Itools/lldb/../clang/include -I/usr/include/libxml2 -I/mnt/vss/_work/2/s/lldb/source/. -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -Wno-deprecated-declarations -Wno-unknown-pragmas -Wno-strict-aliasing -Wno-deprecated-register -Wno-vla-extension -O3   -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT tools/lldb/source/Plugins/Language/ObjC/CMakeFiles/lldbPluginObjCLanguage.dir/Cocoa.cpp.o -MF tools/lldb/source/Plugins/Language/ObjC/CMakeFiles/lldbPluginObjCLanguage.dir/Cocoa.cpp.o.d -o tools/lldb/source/Plugins/Language/ObjC/CMakeFiles/lldbPluginObjCLanguage.dir/Cocoa.cpp.o -c /mnt/vss/_work/2/s/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
/mnt/vss/_work/2/s/lldb/source/Plugins/Language/ObjC/Cocoa.cpp:528:11: error: misleading indentation; statement is not part of the previous 'if' [-Werror,-Wmisleading-indentation]
          return false;
          ^
/mnt/vss/_work/2/s/lldb/source/Plugins/Language/ObjC/Cocoa.cpp:525:9: note: previous statement is here
        if (log)

For this badly indented code:

if (is_preserved_number) {
if (log) 
  log->Printf("Unsupported preserved NSNumber tagged pointer 0x%" 
      PRIu64, valobj_addr);
  return false;
}

lldb/source/Plugins/Language/ObjC/Cocoa.cpp

if (!language->GetFormatterPrefixSuffix(valobj, g_TypeHint, prefix,
       suffix.clear();
     }
   }
 
   stream.Printf("%s%d%s", prefix.c_str(), value, suffix.c_str());
 }
 
 static void NSNumber_FormatLong(ValueObject &valobj, Stream &stream,
-                                uint64_t value, lldb::LanguageType lang) {
+                                int64_t value, lldb::LanguageType lang) {
   static ConstString g_TypeHint("NSNumber:long");
 
   std::string prefix, suffix;
   if (Language *language = Language::FindPlugin(lang)) {
     if (!language->GetFormatterPrefixSuffix(valobj, g_TypeHint, prefix,
                                             suffix)) {
       prefix.clear();
       suffix.clear();
bool lldb_private::formatters::NSNumberSummaryProvider(
 
   if (class_name == "__NSCFBoolean")
     return ObjCBooleanSummaryProvider(valobj, stream, options);
 
   if (class_name == "NSDecimalNumber")
     return NSDecimalNumberSummaryProvider(valobj, stream, options);
 
   if (class_name == "NSNumber" || class_name == "__NSCFNumber") {
-    uint64_t value = 0;
+    int64_t value = 0;
     uint64_t i_bits = 0;
-    if (descriptor->GetTaggedPointerInfo(&i_bits, &value)) {
+    if (descriptor->GetTaggedPointerInfoSigned(&i_bits, &value)) {
+      // Check for "preserved" numbers.  We still don't support them yet.
+      if (i_bits & 0x8) {
+        lldbassert(!static_cast<bool>("We should handle preserved numbers!"));
+        return false;
+      }
+
       switch (i_bits) {
       case 0:
         NSNumber_FormatChar(valobj, stream, (char)value, options.GetLanguage());
         break;
       case 1:
       case 4:
         NSNumber_FormatShort(valobj, stream, (short)value,
                              options.GetLanguage());

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h

public:
 
   // a custom descriptor is used for tagged pointers
   bool GetTaggedPointerInfo(uint64_t *info_bits = nullptr,
                             uint64_t *value_bits = nullptr,
                             uint64_t *payload = nullptr) override {
     return false;
   }
 
+  bool GetTaggedPointerInfoSigned(uint64_t *info_bits = nullptr,
+                                  int64_t *value_bits = nullptr,
+                                  uint64_t *payload = nullptr) override {
+    return false;
+  }
+
   uint64_t GetInstanceSize() override;
 
   ObjCLanguageRuntime::ObjCISA GetISA() override { return m_objc_class_ptr; }
 
   bool Describe(
       std::function<void(ObjCLanguageRuntime::ObjCISA)> const &superclass_func,
       std::function<bool(const char *, const char *)> const
           &instance_method_func,
ClassDescriptorV2Tagged(ConstString class_name, uint64_t payload) {
     m_valid = true;
     m_payload = payload;
     m_info_bits = (m_payload & 0xF0ULL) >> 4;
     m_value_bits = (m_payload & ~0x0000000000000000FFULL) >> 8;
   }
 
   ClassDescriptorV2Tagged(
       ObjCLanguageRuntime::ClassDescriptorSP actual_class_sp,
-      uint64_t payload) {
+      uint64_t u_payload, int64_t s_payload) {
     if (!actual_class_sp) {
       m_valid = false;
       return;
     }
     m_name = actual_class_sp->GetClassName();
     if (!m_name) {
       m_valid = false;
       return;
     }
     m_valid = true;
-    m_payload = payload;
+    m_payload = u_payload;
     m_info_bits = (m_payload & 0x0FULL);
     m_value_bits = (m_payload & ~0x0FULL) >> 4;
+    m_value_bits_signed = (s_payload & ~0x0FLL) >> 4;
   }
 
   ~ClassDescriptorV2Tagged() override = default;
 
   ConstString GetClassName() override { return m_name; }
 
   ObjCLanguageRuntime::ClassDescriptorSP GetSuperclass() override {
     // tagged pointers can represent a class that has a superclass, but since
if (info_bits)
       *info_bits = GetInfoBits();
     if (value_bits)
       *value_bits = GetValueBits();
     if (payload)
       *payload = GetPayload();
     return true;
   }
 
+  bool GetTaggedPointerInfoSigned(uint64_t *info_bits = nullptr,
+                                  int64_t *value_bits = nullptr,
+                                  uint64_t *payload = nullptr) override {
+    if (info_bits)
+      *info_bits = GetInfoBits();
+    if (value_bits)
+      *value_bits = GetValueBitsSigned();
+    if (payload)
+      *payload = GetPayload();
+    return true;
+  }
+
   uint64_t GetInstanceSize() override {
     return (IsValid() ? m_pointer_size : 0);
   }
 
   ObjCLanguageRuntime::ObjCISA GetISA() override {
     return 0; // tagged pointers have no ISA
   }
 
   // these calls are not part of any formal tagged pointers specification
   virtual uint64_t GetValueBits() { return (IsValid() ? m_value_bits : 0); }
 
+  virtual int64_t GetValueBitsSigned() {
+    return (IsValid() ? m_value_bits_signed : 0);
+  }
+
   virtual uint64_t GetInfoBits() { return (IsValid() ? m_info_bits : 0); }
 
   virtual uint64_t GetPayload() { return (IsValid() ? m_payload : 0); }
 
 private:
   ConstString m_name;
   uint8_t m_pointer_size;
   bool m_valid;
   uint64_t m_info_bits;
   uint64_t m_value_bits;
+  int64_t m_value_bits_signed;
   uint64_t m_payload;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_SOURCE_PLUGINS_LANGUAGERUNTIME_OBJC_APPLEOBJCRUNTIME_APPLEOBJCCLASSDESCRIPTORV2_H

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.h

public:
 
     // v1 does not support tagged pointers
     bool GetTaggedPointerInfo(uint64_t *info_bits = nullptr,
                               uint64_t *value_bits = nullptr,
                               uint64_t *payload = nullptr) override {
       return false;
     }
 
+    bool GetTaggedPointerInfoSigned(uint64_t *info_bits = nullptr,
+                                    int64_t *value_bits = nullptr,
+                                    uint64_t *payload = nullptr) override {
+      return false;
+    }
+
     uint64_t GetInstanceSize() override { return m_instance_size; }
 
     ObjCISA GetISA() override { return m_isa; }
 
     bool
     Describe(std::function<void(ObjCLanguageRuntime::ObjCISA)> const
                  &superclass_func,
              std::function<bool(const char *, const char *)> const

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

bool AppleObjCRuntimeV2::TaggedPointerVendorRuntimeAssisted::
     IsPossibleTaggedPointer(lldb::addr_t ptr) {
   return (ptr & m_objc_debug_taggedpointer_mask) != 0;
 }
 
 ObjCLanguageRuntime::ClassDescriptorSP
 AppleObjCRuntimeV2::TaggedPointerVendorRuntimeAssisted::GetClassDescriptor(
     lldb::addr_t ptr) {
   ClassDescriptorSP actual_class_descriptor_sp;
-  uint64_t data_payload;
   uint64_t unobfuscated = (ptr) ^ m_runtime.GetTaggedPointerObfuscator();
 
   if (!IsPossibleTaggedPointer(unobfuscated))
     return ObjCLanguageRuntime::ClassDescriptorSP();
 
   uintptr_t slot = (ptr >> m_objc_debug_taggedpointer_slot_shift) &
                    m_objc_debug_taggedpointer_slot_mask;
 
if (error.Fail() || slot_data == 0 ||
       return nullptr;
     actual_class_descriptor_sp =
         m_runtime.GetClassDescriptorFromISA((ObjCISA)slot_data);
     if (!actual_class_descriptor_sp)
       return ObjCLanguageRuntime::ClassDescriptorSP();
     m_cache[slot] = actual_class_descriptor_sp;
   }
 
-  data_payload =
+  uint64_t data_payload =
       (((uint64_t)unobfuscated << m_objc_debug_taggedpointer_payload_lshift) >>
        m_objc_debug_taggedpointer_payload_rshift);
-
-  return ClassDescriptorSP(
-      new ClassDescriptorV2Tagged(actual_class_descriptor_sp, data_payload));
+  int64_t data_payload_signed =
+      ((int64_t)((int64_t)unobfuscated
+                 << m_objc_debug_taggedpointer_payload_lshift) >>
+       m_objc_debug_taggedpointer_payload_rshift);
+  return ClassDescriptorSP(new ClassDescriptorV2Tagged(
+      actual_class_descriptor_sp, data_payload, data_payload_signed));
 }
 
 AppleObjCRuntimeV2::TaggedPointerVendorExtended::TaggedPointerVendorExtended(
     AppleObjCRuntimeV2 &runtime, uint64_t objc_debug_taggedpointer_mask,
     uint64_t objc_debug_taggedpointer_ext_mask,
     uint32_t objc_debug_taggedpointer_slot_shift,
     uint32_t objc_debug_taggedpointer_ext_slot_shift,
     uint32_t objc_debug_taggedpointer_slot_mask,
bool AppleObjCRuntimeV2::TaggedPointerVendorExtended::
   return ((ptr & m_objc_debug_taggedpointer_ext_mask) ==
           m_objc_debug_taggedpointer_ext_mask);
 }
 
 ObjCLanguageRuntime::ClassDescriptorSP
 AppleObjCRuntimeV2::TaggedPointerVendorExtended::GetClassDescriptor(
     lldb::addr_t ptr) {
   ClassDescriptorSP actual_class_descriptor_sp;
-  uint64_t data_payload;
   uint64_t unobfuscated = (ptr) ^ m_runtime.GetTaggedPointerObfuscator();
 
   if (!IsPossibleTaggedPointer(unobfuscated))
     return ObjCLanguageRuntime::ClassDescriptorSP();
 
   if (!IsPossibleExtendedTaggedPointer(unobfuscated))
     return this->TaggedPointerVendorRuntimeAssisted::GetClassDescriptor(ptr);
 
if (error.Fail() || slot_data == 0 ||
       return nullptr;
     actual_class_descriptor_sp =
         m_runtime.GetClassDescriptorFromISA((ObjCISA)slot_data);
     if (!actual_class_descriptor_sp)
       return ObjCLanguageRuntime::ClassDescriptorSP();
     m_ext_cache[slot] = actual_class_descriptor_sp;
   }
 
-  data_payload = (((uint64_t)unobfuscated
-                   << m_objc_debug_taggedpointer_ext_payload_lshift) >>
-                  m_objc_debug_taggedpointer_ext_payload_rshift);
+  uint64_t data_payload = (((uint64_t)unobfuscated
+                            << m_objc_debug_taggedpointer_ext_payload_lshift) >>
+                           m_objc_debug_taggedpointer_ext_payload_rshift);
+  int64_t data_payload_signed =
+      ((int64_t)((int64_t)unobfuscated
 
-  return ClassDescriptorSP(
-      new ClassDescriptorV2Tagged(actual_class_descriptor_sp, data_payload));
+  return ClassDescriptorSP(new ClassDescriptorV2Tagged(
+      actual_class_descriptor_sp, data_payload, data_payload_signed));
 }
 
 AppleObjCRuntimeV2::NonPointerISACache::NonPointerISACache(
     AppleObjCRuntimeV2 &runtime, const ModuleSP &objc_module_sp,
     uint64_t objc_debug_isa_class_mask, uint64_t objc_debug_isa_magic_mask,
     uint64_t objc_debug_isa_magic_value,
     uint64_t objc_debug_indexed_isa_magic_mask,
     uint64_t objc_debug_indexed_isa_magic_value,

lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h

virtual bool IsCFType() {
         if (class_name && *class_name)
           m_is_cf = (LazyBool)(strcmp(class_name, "__NSCFType") == 0 ||
                                strcmp(class_name, "NSCFType") == 0);
       }
       return (m_is_cf == eLazyBoolYes);
     }
 
     virtual bool IsValid() = 0;
 
+    /// There are two routines in the ObjC runtime that tagged pointer clients
+    /// can call to get the value from their tagged pointer, one that retrieves
+    /// it as an unsigned value and one a signed value.  These two
+    /// GetTaggedPointerInfo methods mirror those two ObjC runtime calls.
+    /// @{
     virtual bool GetTaggedPointerInfo(uint64_t *info_bits = nullptr,
                                       uint64_t *value_bits = nullptr,
                                       uint64_t *payload = nullptr) = 0;
 
+    virtual bool GetTaggedPointerInfoSigned(uint64_t *info_bits = nullptr,
+                                            int64_t *value_bits = nullptr,
+                                            uint64_t *payload = nullptr) = 0;
+    /// @}
+ 
     virtual uint64_t GetInstanceSize() = 0;
 
     // use to implement version-specific additional constraints on pointers
     virtual bool CheckPointer(lldb::addr_t value, uint32_t ptr_size) const {
       return true;
     }
 
     virtual ObjCISA GetISA() = 0;
 

lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCCF.py

def test_coreframeworks_and_run_command(self):
             '(CGSize) cg_size = (width = 1, height = 6)',
             '(CGPoint) cg_point = (x = 2, y = 7)',
             '(CGRect) cg_rect = (origin = (x = 1, y = 2), size = (width = 7, height = 7))',
             '(Rect) rect = (t=4, l=8, b=4, r=7)',
             '(Rect *) rect_ptr = (t=4, l=8, b=4, r=7)',
             '(Point) point = (v=7, h=12)', '(Point *) point_ptr = (v=7, h=12)',
             '(SEL) foo_selector = "foo_selector_impl"'
         ]
-
         self.expect("frame variable", substrs=expect_strings)
 
         if self.getArchitecture() in ['i386', 'x86_64']:
             extra_string = [
                 '(RGBColor) rgb_color = red=3 green=56 blue=35',
                 '(RGBColor *) rgb_color_ptr = red=3 green=56 blue=35',
                 '(HIPoint) hi_point = (x=7, y=12)',
                 '(HIRect) hi_rect = origin=(x = 3, y = 5) size=(width = 4, height = 6)',
             ]
             self.expect("frame variable", substrs=extra_string)
+
+        # The original tests left out testing the NSNumber values, so do that here.
+        # This set is all pointers, with summaries, so we only check the summary.
+        var_list_pointer = [
+            ['NSNumber *', 'num1',    '(int)5'],
+            ['NSNumber *', 'num2',    '(float)3.140000'],
+            ['NSNumber *', 'num3',    '(double)3.14'],
+            ['NSNumber *', 'num4',    '(int128_t)18446744073709551614'],
+            ['NSNumber *', 'num5',    '(char)65'],
+            ['NSNumber *', 'num6',    '(long)255'],
+            ['NSNumber *', 'num7',    '(long)2000000'],
+            ['NSNumber *', 'num8_Y',  'YES'],
+            ['NSNumber *', 'num8_N',  'NO'],
+            ['NSNumber *', 'num9',    '(short)-31616'],
+            ['NSNumber *', 'num_at1', '(int)12'],
+            ['NSNumber *', 'num_at2', '(int)-12'],
+            ['NSNumber *', 'num_at3', '(double)12.5'],
+            ['NSNumber *', 'num_at4', '(double)-12.5'],
+            ['NSDecimalNumber *', 'decimal_number', '123456 x 10^-10'],
+            ['NSDecimalNumber *', 'decimal_number_neg', '-123456 x 10^10']
+        ]
+        for type, var_path, summary in var_list_pointer:
+            self.expect_var_path(var_path, summary, None, type)
+
+