[Lldb-commits] [lldb] be0ced0 - Revert "Revert "Add support for fetching signed values from tagged pointers.""

Jim Ingham via lldb-commits lldb-commits at lists.llvm.org
Mon Apr 5 18:21:18 PDT 2021


Author: Jim Ingham
Date: 2021-04-05T18:18:26-07:00
New Revision: be0ced03ba9bfab6fcb1fd2c263a33bc6a359cd8

URL: https://github.com/llvm/llvm-project/commit/be0ced03ba9bfab6fcb1fd2c263a33bc6a359cd8
DIFF: https://github.com/llvm/llvm-project/commit/be0ced03ba9bfab6fcb1fd2c263a33bc6a359cd8.diff

LOG: Revert "Revert "Add support for fetching signed values from tagged pointers.""

This reverts commit 602ab188a7e18b97d9af95e17271e8fbee129081.

The patch replicated an lldbassert for a certain type of NSNumber for tagged
pointers.  This really shouldn't be an assert since we don't do anything wrong
with these numbers, we just don't print a summary.  So this patch changed the
lldbassert to a log message in reverting the revert.

Added: 
    

Modified: 
    lldb/source/Plugins/Language/ObjC/Cocoa.cpp
    lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
    lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.h
    lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
    lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
    lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCCF.py

Removed: 
    


################################################################################
diff  --git a/lldb/source/Plugins/Language/ObjC/Cocoa.cpp b/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
index d871d3470e70..e2367adaa3b1 100644
--- a/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
+++ b/lldb/source/Plugins/Language/ObjC/Cocoa.cpp
@@ -351,7 +351,7 @@ static void NSNumber_FormatInt(ValueObject &valobj, Stream &stream, int value,
 }
 
 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;
@@ -426,6 +426,8 @@ bool lldb_private::formatters::NSNumberSummaryProvider(
   if (!process_sp)
     return false;
 
+  Log * log 
+      = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_DATAFORMATTERS);
   ObjCLanguageRuntime *runtime = ObjCLanguageRuntime::Get(*process_sp);
 
   if (!runtime)
@@ -456,9 +458,17 @@ bool lldb_private::formatters::NSNumberSummaryProvider(
     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) {
+        if (log) 
+          log->Printf("Unsupported (preserved) NSNumber tagged pointer 0x%"
+              PRIu64, valobj_addr);
+        return false;
+      }
+
       switch (i_bits) {
       case 0:
         NSNumber_FormatChar(valobj, stream, (char)value, options.GetLanguage());
@@ -512,7 +522,9 @@ bool lldb_private::formatters::NSNumberSummaryProvider(
 
         bool is_preserved_number = cfinfoa & 0x8;
         if (is_preserved_number) {
-          lldbassert(!static_cast<bool>("We should handle preserved numbers!"));
+        if (log) 
+          log->Printf("Unsupported preserved NSNumber tagged pointer 0x%" 
+              PRIu64, valobj_addr);
           return false;
         }
 

diff  --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
index 9ef21c6e7208..1bea314f63fc 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
@@ -41,6 +41,12 @@ class ClassDescriptorV2 : public ObjCLanguageRuntime::ClassDescriptor {
     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; }
@@ -253,7 +259,7 @@ class ClassDescriptorV2Tagged : public ObjCLanguageRuntime::ClassDescriptor {
 
   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;
@@ -264,9 +270,10 @@ class ClassDescriptorV2Tagged : public ObjCLanguageRuntime::ClassDescriptor {
       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;
@@ -308,6 +315,18 @@ class ClassDescriptorV2Tagged : public ObjCLanguageRuntime::ClassDescriptor {
     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);
   }
@@ -319,6 +338,10 @@ class ClassDescriptorV2Tagged : public ObjCLanguageRuntime::ClassDescriptor {
   // 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); }
@@ -329,6 +352,7 @@ class ClassDescriptorV2Tagged : public ObjCLanguageRuntime::ClassDescriptor {
   bool m_valid;
   uint64_t m_info_bits;
   uint64_t m_value_bits;
+  int64_t m_value_bits_signed;
   uint64_t m_payload;
 };
 

diff  --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.h b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.h
index 4eb7d979394b..0164f83a0600 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.h
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV1.h
@@ -64,6 +64,12 @@ class AppleObjCRuntimeV1 : public AppleObjCRuntime {
       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; }

diff  --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
index d7a7cda1c772..debdfeaf909a 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -2460,7 +2460,6 @@ 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))
@@ -2488,12 +2487,15 @@ AppleObjCRuntimeV2::TaggedPointerVendorRuntimeAssisted::GetClassDescriptor(
     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(
@@ -2545,7 +2547,6 @@ 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))
@@ -2576,12 +2577,16 @@ AppleObjCRuntimeV2::TaggedPointerVendorExtended::GetClassDescriptor(
     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
+                 << m_objc_debug_taggedpointer_ext_payload_lshift) >>
+       m_objc_debug_taggedpointer_ext_payload_rshift);
 
-  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(

diff  --git a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
index 683eff777728..9fc858648bcd 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h
@@ -87,10 +87,20 @@ class ObjCLanguageRuntime : public LanguageRuntime {
 
     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

diff  --git a/lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCCF.py b/lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCCF.py
index 63f4b71e1ec1..63ba59808880 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCCF.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjCCF.py
@@ -45,7 +45,6 @@ def test_coreframeworks_and_run_command(self):
             '(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']:
@@ -56,3 +55,28 @@ def test_coreframeworks_and_run_command(self):
                 '(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)
+
+            


        


More information about the lldb-commits mailing list