[lldb] Extract more fields from NSException values

[kubamracek]

Summary

This patch teaches LLDB about more fields on NSException Obj-C objects, specifically we can now retrieve the "name" and "reason" of an NSException. The goal is to eventually be able to have SB API that can provide details about the currently thrown/caught/processed exception.

[davide]

I like the way this patch is structured, some comments inline.

packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py

2–5

This looks like it doesn't need any interactivity whatsoever, so you can probably convert this to a lit test. See the examples in lit/ (and please let me know if you have any questions.

source/Plugins/Language/ObjC/NSArray.cpp

221–228 ↗

(On Diff #136332)

Do you mind to add comments to explain what the structure fields represent (or what this structure is for)?

source/Plugins/Language/ObjC/NSException.cpp

64–71

The fact we have to calculate this by hand is slightly annoying, but I guess that's a bug to fix another day.

[kubamracek]

packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py

2–5

Will do! I didn't realize we already have the lit infrastructure ready. Quick question: Can I test the SB layer from a lit test?

source/Plugins/Language/ObjC/NSArray.cpp

221–228 ↗

(On Diff #136332)

There's many definitions of array layout throughout this file, so I don't see a comment to be necessary here. Basically, this just defines another layout of an array, see above for other instances.

source/Plugins/Language/ObjC/NSException.cpp

64–71

I'll be happy to send follow-up patches. Do you have suggestions how to avoid these manual pointer calculations?

[davide]

LG modulo the test. Update that, I'll take another look and approve it. Thanks for your contribution!

packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py

2–5

No not really. The lit tests are a super recent addition :)

source/Plugins/Language/ObjC/NSArray.cpp

221–228 ↗

(On Diff #136332)

Fair enough.

source/Plugins/Language/ObjC/NSException.cpp

64–71

The way we do that in swift (or well, we're trying to do) is that of using remote mirrors. I'm not really aware of an equivalent mechanism in Obj-C, FWIW, but we should definitely think about it and have a chat (if you want, I don't think you should be signed up for cleaning up this).

[jingham]

For tests that run processes I don't think lit has any advantages over the dotest tests. For tests of a facility that's explicitly going to provide SB API's, testing them with dotest.py which is set up to be convenient to do SB API testing seems the correct thing to do. I think this test is fine as it, except for the inline comment about using run_to_source_breakpoint.

This seems like it is two patches, one fixing the NSCallStackArray data formatter, and one extracting fields from NSException. Is that right. I'm more asking to make sure I'm not missing something about this patch.

This looks fine to me, you might explore getting the offsets from the NSException type - that should be generally available since we can read it from the ObjC runtime even if we don't get it from debug info.

packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py

30–39

All this (and the self.line calculation) can be replaced by:

lldbutils.run_to_source_breakpoint("// Set break point at this line.", lldb.SBFileSpec("main.m"))

source/Plugins/Language/ObjC/NSException.cpp

64–71

It looks like we generally have a type for NSException - it's an ObjC class so we'll read it out of the runtime if we can't get it from debug information. You should be able to get the offsets from that rather than having to hand code them here.

[kubamracek]

This seems like it is two patches, one fixing the NSCallStackArray data formatter, and one extracting fields from NSException. Is that right. I'm more asking to make sure I'm not missing something about this patch.

I'll split the patch. There's actually probably 3 more-or-less independent parts: 1) NFC refactoring of NSException.cpp, 2) extracting fields from NSException, 3) NSCallStackArray data formatter.

source/Plugins/Language/ObjC/NSException.cpp

64–71

Does reading the offsets need to run the target or is it just inspecting memory?

[jingham]

source/Plugins/Language/ObjC/NSException.cpp

64–71

Reading the whole runtime was too slow if done from lldb, since we're chasing pointers around, so we do this with some injected code. So if you happen to be the first guy to ever ask about an ObjC type and its offsets, you might trigger this code. If you aren't the first guy, then it's just inspecting memory.

[kubamracek]

The NFC refactoring part of this is at https://reviews.llvm.org/D44073.

[kubamracek]

I simplified what this patch is doing by removing the other independent parts. I extended the test to be testing both CLI text output and SB APIs (which also means it cannot be moved to a lit test).

[kubamracek]

The changes for _NSCallStackArray are at https://reviews.llvm.org/D44081.

[kubamracek]

Ping.

[davide]

source/Plugins/Language/ObjC/NSException.cpp

161–168

can you clang-format this and add a comment? Thanks.

[kubamracek]

source/Plugins/Language/ObjC/NSException.cpp

161–168

This is already clang-formatted. Added comment.

[davide]

LGTM. Thanks.

[kubamracek]

Thanks for the review!

packages/Python/lldbsuite/test/lang/objc/exceptions/Makefile

+LEVEL = ../../../make
+
+OBJC_SOURCES := main.m
+
+CFLAGS_EXTRAS += -w
+
+include $(LEVEL)/Makefile.rules
+
+LDFLAGS += -framework Foundation

packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py

+# encoding: utf-8
+"""
+Test lldb Obj-C exception support.
+"""
+
+from __future__ import print_function
+
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ObjCExceptionsTestCase(TestBase):
+
+    mydir = TestBase.compute_mydir(__file__)
+
+    @skipUnlessDarwin
+    def test_objc_exceptions_1(self):
+        self.build()
+
+        target = self.dbg.CreateTarget(self.getBuildArtifact("a.out"))
+        self.assertTrue(target, VALID_TARGET)
+
+        lldbutil.run_to_source_breakpoint(self, "// Set break point at this line.", lldb.SBFileSpec("main.m"))
+
+        self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+                    substrs=['stopped', 'stop reason = breakpoint'])
+
+        thread = self.dbg.GetSelectedTarget().GetProcess().GetSelectedThread()
+        frame = thread.GetSelectedFrame()
+
+        self.expect(
+            'frame variable e1',
+            substrs=[
+                '(NSException *) e1 = ',
+                'name: @"ExceptionName" - reason: @"SomeReason"'
+            ])
+
+        self.expect(
+            'frame variable --dynamic-type no-run-target *e1',
+            substrs=[
+                '(NSException) *e1 = ',
+                'name = ', '@"ExceptionName"',
+                'reason = ', '@"SomeReason"',
+                'userInfo = ', '1 key/value pair',
+                'reserved = ', 'nil',
+            ])
+
+        e1 = frame.FindVariable("e1")
+        self.assertTrue(e1)
+        self.assertEqual(e1.type.name, "NSException *")
+        self.assertEqual(e1.GetSummary(), 'name: @"ExceptionName" - reason: @"SomeReason"')
+        self.assertEqual(e1.GetChildMemberWithName("name").description, "ExceptionName")
+        self.assertEqual(e1.GetChildMemberWithName("reason").description, "SomeReason")
+        userInfo = e1.GetChildMemberWithName("userInfo").dynamic
+        self.assertEqual(userInfo.summary, "1 key/value pair")
+        self.assertEqual(userInfo.GetChildAtIndex(0).GetChildAtIndex(0).description, "some_key")
+        self.assertEqual(userInfo.GetChildAtIndex(0).GetChildAtIndex(1).description, "some_value")
+        self.assertEqual(e1.GetChildMemberWithName("reserved").description, "<nil>")
+
+        self.expect(
+            'frame variable e2',
+            substrs=[
+                '(NSException *) e2 = ',
+                'name: @"ThrownException" - reason: @"SomeReason"'
+            ])
+
+        self.expect(
+            'frame variable --dynamic-type no-run-target *e2',
+            substrs=[
+                '(NSException) *e2 = ',
+                'name = ', '@"ThrownException"',
+                'reason = ', '@"SomeReason"',
+                'userInfo = ', '1 key/value pair',
+                'reserved = ',
+            ])
+
+        e2 = frame.FindVariable("e2")
+        self.assertTrue(e2)
+        self.assertEqual(e2.type.name, "NSException *")
+        self.assertEqual(e2.GetSummary(), 'name: @"ThrownException" - reason: @"SomeReason"')
+        self.assertEqual(e2.GetChildMemberWithName("name").description, "ThrownException")
+        self.assertEqual(e2.GetChildMemberWithName("reason").description, "SomeReason")
+        userInfo = e2.GetChildMemberWithName("userInfo").dynamic
+        self.assertEqual(userInfo.summary, "1 key/value pair")
+        self.assertEqual(userInfo.GetChildAtIndex(0).GetChildAtIndex(0).description, "some_key")
+        self.assertEqual(userInfo.GetChildAtIndex(0).GetChildAtIndex(1).description, "some_value")
+        self.assertGreater(e2.GetChildMemberWithName("reserved").dynamic.num_children, 0)

packages/Python/lldbsuite/test/lang/objc/exceptions/main.m

+//===-- main.m ------------------------------------------------*- ObjC -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#import <Foundation/Foundation.h>
+
+void foo()
+{
+    NSDictionary *info = [NSDictionary dictionaryWithObjectsAndKeys:@"some_value", @"some_key", nil];
+    @throw [[NSException alloc] initWithName:@"ThrownException" reason:@"SomeReason" userInfo:info];
+}
+
+int main (int argc, const char * argv[])
+{
+    NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
+
+    NSDictionary *info = [NSDictionary dictionaryWithObjectsAndKeys:@"some_value", @"some_key", nil];
+    NSException *e1 = [[NSException alloc] initWithName:@"ExceptionName" reason:@"SomeReason" userInfo:info];
+    NSException *e2;
+
+    @try {
+        foo();
+    } @catch(NSException *e) {
+        e2 = e;
+    }
+
+    NSLog(@"1"); // Set break point at this line.
+    [pool drain];
+    return 0;
+}
+

source/Plugins/Language/ObjC/NSException.cpp

 
 #include "Plugins/Language/ObjC/NSString.h"
 
 using namespace lldb;
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
 static bool ExtractFields(ValueObject &valobj, ValueObjectSP *name_sp,
-                          ValueObjectSP *reason_sp,
-                          ValueObjectSP *userinfo_sp) {
+                          ValueObjectSP *reason_sp, ValueObjectSP *userinfo_sp,
+                          ValueObjectSP *reserved_sp) {
   ProcessSP process_sp(valobj.GetProcessSP());
   if (!process_sp)
     return false;
 
   lldb::addr_t ptr = LLDB_INVALID_ADDRESS;
 
   CompilerType valobj_type(valobj.GetCompilerType());
   Flags type_flags(valobj_type.GetTypeInfo());
static bool ExtractFields(ValueObject &valobj, ValueObjectSP *name_sp,
   size_t ptr_size = process_sp->GetAddressByteSize();
 
   Status error;
   auto name = process_sp->ReadPointerFromMemory(ptr + 1 * ptr_size, error);
   if (error.Fail() || name == LLDB_INVALID_ADDRESS)
     return false;
   auto reason = process_sp->ReadPointerFromMemory(ptr + 2 * ptr_size, error);
   if (error.Fail() || reason == LLDB_INVALID_ADDRESS)
     return false;
   auto userinfo = process_sp->ReadPointerFromMemory(ptr + 3 * ptr_size, error);
   if (error.Fail() || userinfo == LLDB_INVALID_ADDRESS)
     return false;
+  auto reserved = process_sp->ReadPointerFromMemory(ptr + 4 * ptr_size, error);
+  if (error.Fail() || reserved == LLDB_INVALID_ADDRESS)
+    return false;
 
   InferiorSizedWord name_isw(name, *process_sp);
   InferiorSizedWord reason_isw(reason, *process_sp);
   InferiorSizedWord userinfo_isw(userinfo, *process_sp);
+  InferiorSizedWord reserved_isw(reserved, *process_sp);
 
   CompilerType voidstar = process_sp->GetTarget()
                               .GetScratchClangASTContext()
                               ->GetBasicType(lldb::eBasicTypeVoid)
                               .GetPointerType();
 
   if (name_sp)
     *name_sp = ValueObject::CreateValueObjectFromData(
         "name", name_isw.GetAsData(process_sp->GetByteOrder()),
         valobj.GetExecutionContextRef(), voidstar);
   if (reason_sp)
     *reason_sp = ValueObject::CreateValueObjectFromData(
         "reason", reason_isw.GetAsData(process_sp->GetByteOrder()),
         valobj.GetExecutionContextRef(), voidstar);
   if (userinfo_sp)
     *userinfo_sp = ValueObject::CreateValueObjectFromData(
         "userInfo", userinfo_isw.GetAsData(process_sp->GetByteOrder()),
         valobj.GetExecutionContextRef(), voidstar);
+  if (reserved_sp)
+    *reserved_sp = ValueObject::CreateValueObjectFromData(
+        "reserved", reserved_isw.GetAsData(process_sp->GetByteOrder()),
+        valobj.GetExecutionContextRef(), voidstar);
 
   return true;
 }
 
 bool lldb_private::formatters::NSException_SummaryProvider(
     ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
   lldb::ValueObjectSP name_sp;
   lldb::ValueObjectSP reason_sp;
-  if (!ExtractFields(valobj, &name_sp, &reason_sp, nullptr))
+  if (!ExtractFields(valobj, &name_sp, &reason_sp, nullptr, nullptr))
     return false;
 
   if (!name_sp || !reason_sp)
     return false;
 
   StreamString name_str_summary;
   StreamString reason_str_summary;
   if (NSStringSummaryProvider(*name_sp, name_str_summary, options) &&

 class NSExceptionSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
 public:
   NSExceptionSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
       : SyntheticChildrenFrontEnd(*valobj_sp) {}
 
   ~NSExceptionSyntheticFrontEnd() override = default;
 
   size_t CalculateNumChildren() override {
-    return 1;
+    return 4;
   }
 
   lldb::ValueObjectSP GetChildAtIndex(size_t idx) override {
     switch (idx) {
-      case 0: return m_userinfo_sp;
+      case 0: return m_name_sp;
+      case 1: return m_reason_sp;
+      case 2: return m_userinfo_sp;
+      case 3: return m_reserved_sp;
     }
     return lldb::ValueObjectSP();
   }
 
   bool Update() override {
+    m_name_sp.reset();
+    m_reason_sp.reset();
     m_userinfo_sp.reset();
-    if (!ExtractFields(m_backend, nullptr, nullptr, &m_userinfo_sp)) {
+    m_reserved_sp.reset();
+
+    if (!ExtractFields(m_backend, &m_name_sp, &m_reason_sp, &m_userinfo_sp,
+                       &m_reserved_sp)) {
       return false;
     }
     return true;
   }
 
   bool MightHaveChildren() override { return true; }
 
   size_t GetIndexOfChildWithName(const ConstString &name) override {
+    static ConstString g___name("name");
+    static ConstString g___reason("reason");
     static ConstString g___userInfo("userInfo");
-    if (name == g___userInfo)
-      return 0;
+    static ConstString g___reserved("reserved");
+    if (name == g___name) return 0;
+    if (name == g___reason) return 1;
+    if (name == g___userInfo) return 2;
+    if (name == g___reserved) return 3;
     return UINT32_MAX;
   }
 
 private:
+  ValueObjectSP m_name_sp;
+  ValueObjectSP m_reason_sp;
   ValueObjectSP m_userinfo_sp;
+  ValueObjectSP m_reserved_sp;
 };
 
 SyntheticChildrenFrontEnd *
 lldb_private::formatters::NSExceptionSyntheticFrontEndCreator(
     CXXSyntheticChildren *, lldb::ValueObjectSP valobj_sp) {
   lldb::ProcessSP process_sp(valobj_sp->GetProcessSP());
   if (!process_sp)
     return nullptr;