[lldb] Improve error reporting in `lang objc tagged-pointer info`

[JDevlieghere]

Summary

Improve error handling for the lang objc tagged-pointer info. Rather than failing silently, report an error if we couldn't convert an argument to an address or resolve the class descriptor.

(lldb) lang objc tagged-pointer info 0xbb6404c47a587764
could not get class descriptor for 0xbb6404c47a587764
(lldb) lang objc tagged-pointer info n1
could not convert 'n1' to a valid address

[JDevlieghere]

Add test

[aprantl]

This is great, thanks! (one question inline)

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

984–1008

I don't think success really works. Shouldn't the function only return true if none of the arg_strs failed?

[aprantl]

lldb/test/Shell/ObjC/tagged_pointer_info.test

3 ↗

(On Diff #383851)

personal opinion without through reasoning behind it: This feels more like an API test to me.

[JDevlieghere]

Address code review feedback

[aprantl]

Excellent, thanks!

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

bool DoExecute(Args &command, CommandReturnObject &result) override {
     if (command.GetArgumentCount() == 0) {
       result.AppendError("this command requires arguments");
       result.SetStatus(lldb::eReturnStatusFailed);
       return false;
     }
 
     Process *process = m_exe_ctx.GetProcessPtr();
     ExecutionContext exe_ctx(process);
+
     ObjCLanguageRuntime *objc_runtime = ObjCLanguageRuntime::Get(*process);
-    if (objc_runtime) {
-      ObjCLanguageRuntime::TaggedPointerVendor *tagged_ptr_vendor =
-          objc_runtime->GetTaggedPointerVendor();
-      if (tagged_ptr_vendor) {
-        for (size_t i = 0; i < command.GetArgumentCount(); i++) {
-          const char *arg_str = command.GetArgumentAtIndex(i);
-          if (!arg_str)
-            continue;
-          Status error;
-          lldb::addr_t arg_addr = OptionArgParser::ToAddress(
-              &exe_ctx, arg_str, LLDB_INVALID_ADDRESS, &error);
-          if (arg_addr == 0 || arg_addr == LLDB_INVALID_ADDRESS || error.Fail())
-            continue;
-          auto descriptor_sp = tagged_ptr_vendor->GetClassDescriptor(arg_addr);
-          if (!descriptor_sp)
-            continue;
-          uint64_t info_bits = 0;
-          uint64_t value_bits = 0;
-          uint64_t payload = 0;
-          if (descriptor_sp->GetTaggedPointerInfo(&info_bits, &value_bits,
-                                                  &payload)) {
-            result.GetOutputStream().Printf(
-                "0x%" PRIx64 " is tagged.\n\tpayload = 0x%" PRIx64
-                "\n\tvalue = 0x%" PRIx64 "\n\tinfo bits = 0x%" PRIx64
-                "\n\tclass = %s\n",
-                (uint64_t)arg_addr, payload, value_bits, info_bits,
-                descriptor_sp->GetClassName().AsCString("<unknown>"));
-          } else {
-            result.GetOutputStream().Printf("0x%" PRIx64 " is not tagged.\n",
-                                            (uint64_t)arg_addr);
-          }
-        }
-      } else {
-        result.AppendError("current process has no tagged pointer support");
-        result.SetStatus(lldb::eReturnStatusFailed);
-        return false;
-      }
-      result.SetStatus(lldb::eReturnStatusSuccessFinishResult);
-      return true;
-    }
-    result.AppendError("current process has no Objective-C runtime loaded");
-    result.SetStatus(lldb::eReturnStatusFailed);
-    return false;
-  }
+    if (!objc_runtime) {
+      result.AppendError("current process has no Objective-C runtime loaded");
+      result.SetStatus(lldb::eReturnStatusFailed);
+      return false;
+    }
+
+    if (!tagged_ptr_vendor) {
+      result.AppendError("current process has no tagged pointer support");
+      result.SetStatus(lldb::eReturnStatusFailed);
+      return false;
+    }
+
+
+      if (arg_addr == 0 || arg_addr == LLDB_INVALID_ADDRESS || error.Fail()) {
+        result.AppendErrorWithFormat(
+            "could not convert '%s' to a valid address\n", arg_str);
+        result.SetStatus(lldb::eReturnStatusFailed);
+        return false;
+      }
+
+      if (!descriptor_sp) {
+        result.AppendErrorWithFormat(
+            "could not get class descriptor for 0x%" PRIx64 "\n",
+            (uint64_t)arg_addr);
+        result.SetStatus(lldb::eReturnStatusFailed);
+        return false;
+      }
+
+
 };
 
 class CommandObjectMultiwordObjC_ClassTable : public CommandObjectMultiword {
 public:
   CommandObjectMultiwordObjC_ClassTable(CommandInterpreter &interpreter)
       : CommandObjectMultiword(
             interpreter, "class-table",
             "Commands for operating on the Objective-C class table.",

lldb/test/API/lang/objc/tagged-pointer/Makefile

+OBJC_SOURCES := main.m
+LD_EXTRAS := -lobjc -framework Foundation
+
+include Makefile.rules

lldb/test/API/lang/objc/tagged-pointer/TestTaggedPointerCmd.py

+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class TestTaggedPointerCommand(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    def test(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(self,"// break here", lldb.SBFileSpec("main.m"))
+
+        self.expect("lang objc tagged-pointer info bogus", error=True,
+                    patterns=["could not convert 'bogus' to a valid address"])
+
+        self.expect("lang objc tagged-pointer info 0x1", error=True,
+                    patterns=["could not get class descriptor for 0x1"])
+

lldb/test/API/lang/objc/tagged-pointer/main.m

+#import <Foundation/Foundation.h>
+int main() {
+  id n1 = [NSNumber numberWithInt:1];
+  printf("%x", n1); // break here
+  return 0;
+}