[Lldb-commits] [PATCH] D112945: [lldb] Improve error reporting in `lang objc tagged-pointer info`
This revision was automatically updated to reflect the committed changes.
Closed by commit rG50b40b051890: [lldb] Improve error reporting in `lang objc
tagged-pointer info` (authored by JDevlieghere).
Herald added a project: LLDB.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112945/new/
https://reviews.llvm.org/D112945
Files:
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
lldb/test/API/lang/objc/tagged-pointer/Makefile
lldb/test/API/lang/objc/tagged-pointer/TestTaggedPointerCmd.py
lldb/test/API/lang/objc/tagged-pointer/main.m
Index: lldb/test/API/lang/objc/tagged-pointer/main.m
===
--- /dev/null
+++ lldb/test/API/lang/objc/tagged-pointer/main.m
@@ -0,0 +1,6 @@
+#import
+int main() {
+ id n1 = [NSNumber numberWithInt:1];
+ printf("%x", n1); // break here
+ return 0;
+}
Index: lldb/test/API/lang/objc/tagged-pointer/TestTaggedPointerCmd.py
===
--- /dev/null
+++ lldb/test/API/lang/objc/tagged-pointer/TestTaggedPointerCmd.py
@@ -0,0 +1,20 @@
+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"])
+
Index: lldb/test/API/lang/objc/tagged-pointer/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/objc/tagged-pointer/Makefile
@@ -0,0 +1,4 @@
+OBJC_SOURCES := main.m
+LD_EXTRAS := -lobjc -framework Foundation
+
+include Makefile.rules
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -951,50 +951,65 @@
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(
- _ctx, arg_str, LLDB_INVALID_ADDRESS, );
- 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(_bits, _bits,
- )) {
-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(""));
- } else {
-result.GetOutputStream().Printf("0x%" PRIx64 " is not tagged.\n",
-(uint64_t)arg_addr);
- }
-}
- } else {
-result.AppendError("current process has no tagged pointer support");
+if (!objc_runtime) {
+ result.AppendError("current process has no Objective-C runtime loaded");
+ result.SetStatus(lldb::eReturnStatusFailed);
+ return false;
+}
+
+ObjCLanguageRuntime::TaggedPointerVendor *tagged_ptr_vendor =
+objc_runtime->GetTaggedPointerVendor();
+if (!tagged_ptr_vendor) {
+ result.AppendError("current process has no tagged pointer support");
+ result.SetStatus(lldb::eReturnStatusFailed);
+ return false;
+}
+
+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 =
[Lldb-commits] [PATCH] D112945: [lldb] Improve error reporting in `lang objc tagged-pointer info`
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Excellent, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112945/new/ https://reviews.llvm.org/D112945 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D112945: [lldb] Improve error reporting in `lang objc tagged-pointer info`
JDevlieghere updated this revision to Diff 384193.
JDevlieghere added a comment.
Address code review feedback
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112945/new/
https://reviews.llvm.org/D112945
Files:
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
lldb/test/API/lang/objc/tagged-pointer/Makefile
lldb/test/API/lang/objc/tagged-pointer/TestTaggedPointerCmd.py
lldb/test/API/lang/objc/tagged-pointer/main.m
Index: lldb/test/API/lang/objc/tagged-pointer/main.m
===
--- /dev/null
+++ lldb/test/API/lang/objc/tagged-pointer/main.m
@@ -0,0 +1,6 @@
+#import
+int main() {
+ id n1 = [NSNumber numberWithInt:1];
+ printf("%x", n1); // break here
+ return 0;
+}
Index: lldb/test/API/lang/objc/tagged-pointer/TestTaggedPointerCmd.py
===
--- /dev/null
+++ lldb/test/API/lang/objc/tagged-pointer/TestTaggedPointerCmd.py
@@ -0,0 +1,20 @@
+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"])
+
Index: lldb/test/API/lang/objc/tagged-pointer/Makefile
===
--- /dev/null
+++ lldb/test/API/lang/objc/tagged-pointer/Makefile
@@ -0,0 +1,4 @@
+OBJC_SOURCES := main.m
+LD_EXTRAS := -lobjc -framework Foundation
+
+include Makefile.rules
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -951,50 +951,65 @@
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(
- _ctx, arg_str, LLDB_INVALID_ADDRESS, );
- 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(_bits, _bits,
- )) {
-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(""));
- } else {
-result.GetOutputStream().Printf("0x%" PRIx64 " is not tagged.\n",
-(uint64_t)arg_addr);
- }
-}
- } else {
-result.AppendError("current process has no tagged pointer support");
+if (!objc_runtime) {
+ result.AppendError("current process has no Objective-C runtime loaded");
+ result.SetStatus(lldb::eReturnStatusFailed);
+ return false;
+}
+
+ObjCLanguageRuntime::TaggedPointerVendor *tagged_ptr_vendor =
+objc_runtime->GetTaggedPointerVendor();
+if (!tagged_ptr_vendor) {
+ result.AppendError("current process has no tagged pointer support");
+ result.SetStatus(lldb::eReturnStatusFailed);
+ return false;
+}
+
+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(
+ _ctx, arg_str, LLDB_INVALID_ADDRESS, );
+ if (arg_addr == 0 || arg_addr == LLDB_INVALID_ADDRESS || error.Fail()) {
+
[Lldb-commits] [PATCH] D112945: [lldb] Improve error reporting in `lang objc tagged-pointer info`
aprantl added inline comments. Comment at: lldb/test/Shell/ObjC/tagged_pointer_info.test:3 + +RUN: %clang_host -g -framework Foundation -o %t.out %S/Inputs/test.m + personal opinion without through reasoning behind it: This feels more like an API test to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112945/new/ https://reviews.llvm.org/D112945 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D112945: [lldb] Improve error reporting in `lang objc tagged-pointer info`
aprantl added a comment. This is great, thanks! (one question inline) Comment at: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp:1008 } - result.SetStatus(lldb::eReturnStatusSuccessFinishResult); - return true; + success = true; } I don't think `success` really works. Shouldn't the function only return `true` if none of the `arg_str`s failed? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112945/new/ https://reviews.llvm.org/D112945 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D112945: [lldb] Improve error reporting in `lang objc tagged-pointer info`
JDevlieghere updated this revision to Diff 383851.
JDevlieghere added a comment.
Add test
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112945/new/
https://reviews.llvm.org/D112945
Files:
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
lldb/test/Shell/ObjC/Inputs/test.m
lldb/test/Shell/ObjC/tagged_pointer_info.test
Index: lldb/test/Shell/ObjC/tagged_pointer_info.test
===
--- /dev/null
+++ lldb/test/Shell/ObjC/tagged_pointer_info.test
@@ -0,0 +1,9 @@
+REQUIRES: system-darwin
+
+RUN: %clang_host -g -framework Foundation -o %t.out %S/Inputs/test.m
+
+RUN: %lldb %t.out -o "b main" -o "r" -o "n" -o "lang objc tagged-pointer info bogus" 2>&1 | FileCheck %s -check-prefix ADDRESS
+ADDRESS: could not convert 'bogus' to a valid address
+
+RUN: %lldb %t.out -o "b main" -o "r" -o "n" -o "lang objc tagged-pointer info 0x1" 2>&1 | FileCheck %s --check-prefix DESCRIPTOR
+DESCRIPTOR: could not get class descriptor for 0x1
Index: lldb/test/Shell/ObjC/Inputs/test.m
===
--- /dev/null
+++ lldb/test/Shell/ObjC/Inputs/test.m
@@ -0,0 +1,6 @@
+#import
+int main() {
+ id n1 = [NSNumber numberWithInt:1];
+ printf("%x", n1);
+ return 0;
+}
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -951,50 +951,70 @@
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(
- _ctx, arg_str, LLDB_INVALID_ADDRESS, );
- 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(_bits, _bits,
- )) {
-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(""));
- } else {
-result.GetOutputStream().Printf("0x%" PRIx64 " is not tagged.\n",
-(uint64_t)arg_addr);
- }
-}
+if (!objc_runtime) {
+ result.AppendError("current process has no Objective-C runtime loaded");
+ result.SetStatus(lldb::eReturnStatusFailed);
+ return false;
+}
+
+ObjCLanguageRuntime::TaggedPointerVendor *tagged_ptr_vendor =
+objc_runtime->GetTaggedPointerVendor();
+if (!tagged_ptr_vendor) {
+ result.AppendError("current process has no tagged pointer support");
+ result.SetStatus(lldb::eReturnStatusFailed);
+ return false;
+}
+
+bool success = false;
+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(
+ _ctx, arg_str, LLDB_INVALID_ADDRESS, );
+ if (arg_addr == 0 || arg_addr == LLDB_INVALID_ADDRESS || error.Fail()) {
+result.GetErrorStream().Printf(
+"could not convert '%s' to a valid address\n", arg_str);
+continue;
+ }
+
+ auto descriptor_sp = tagged_ptr_vendor->GetClassDescriptor(arg_addr);
+ if (!descriptor_sp) {
+result.GetErrorStream().Printf(
+"could not get class descriptor for 0x%" PRIx64 "\n",
+(uint64_t)arg_addr);
+continue;
+ }
+
+ uint64_t info_bits = 0;
+ uint64_t value_bits = 0;
+ uint64_t payload = 0;
+ if (descriptor_sp->GetTaggedPointerInfo(_bits, _bits,
+ )) {
+result.GetOutputStream().Printf(
+"0x%" PRIx64 " is tagged.\n\tpayload = 0x%" PRIx64
+"\n\tvalue =
[Lldb-commits] [PATCH] D112945: [lldb] Improve error reporting in `lang objc tagged-pointer info`
JDevlieghere created this revision.
JDevlieghere added a reviewer: aprantl.
JDevlieghere requested review of this revision.
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
https://reviews.llvm.org/D112945
Files:
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -951,50 +951,70 @@
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(
- _ctx, arg_str, LLDB_INVALID_ADDRESS, );
- 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(_bits, _bits,
- )) {
-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(""));
- } else {
-result.GetOutputStream().Printf("0x%" PRIx64 " is not tagged.\n",
-(uint64_t)arg_addr);
- }
-}
+if (!objc_runtime) {
+ result.AppendError("current process has no Objective-C runtime loaded");
+ result.SetStatus(lldb::eReturnStatusFailed);
+ return false;
+}
+
+ObjCLanguageRuntime::TaggedPointerVendor *tagged_ptr_vendor =
+objc_runtime->GetTaggedPointerVendor();
+if (!tagged_ptr_vendor) {
+ result.AppendError("current process has no tagged pointer support");
+ result.SetStatus(lldb::eReturnStatusFailed);
+ return false;
+}
+
+bool success = false;
+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(
+ _ctx, arg_str, LLDB_INVALID_ADDRESS, );
+ if (arg_addr == 0 || arg_addr == LLDB_INVALID_ADDRESS || error.Fail()) {
+result.GetErrorStream().Printf(
+"could not convert '%s' to a valid address\n", arg_str);
+continue;
+ }
+
+ auto descriptor_sp = tagged_ptr_vendor->GetClassDescriptor(arg_addr);
+ if (!descriptor_sp) {
+result.GetErrorStream().Printf(
+"could not get class descriptor for 0x%" PRIx64 "\n",
+(uint64_t)arg_addr);
+continue;
+ }
+
+ uint64_t info_bits = 0;
+ uint64_t value_bits = 0;
+ uint64_t payload = 0;
+ if (descriptor_sp->GetTaggedPointerInfo(_bits, _bits,
+ )) {
+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(""));
} else {
-result.AppendError("current process has no tagged pointer support");
-result.SetStatus(lldb::eReturnStatusFailed);
-return false;
+result.GetOutputStream().Printf("0x%" PRIx64 " is not tagged.\n",
+(uint64_t)arg_addr);
}
- result.SetStatus(lldb::eReturnStatusSuccessFinishResult);
- return true;
+ success = true;
}
-result.AppendError("current process
