[Lldb-commits] [PATCH] D43884: [lldb] Extract more fields from NSException values

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

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



================
Comment at: packages/Python/lldbsuite/test/lang/objc/exceptions/TestObjCExceptions.py:1-4
+# encoding: utf-8
+"""
+Test lldb Obj-C exception support.
+"""
----------------
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.


================
Comment at: source/Plugins/Language/ObjC/NSArray.cpp:221-228
+namespace CallStackArray {
+  struct DataDescriptor_32 {
+    uint32_t _data;
+    uint32_t _used;
+    uint32_t _offset;
+    const uint32_t _size = 0;
+  };
----------------
Do you mind to add comments to explain what the structure fields represent (or what this structure is for)?


================
Comment at: source/Plugins/Language/ObjC/NSException.cpp:57-64
+  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() || reason == LLDB_INVALID_ADDRESS) return false;
+  auto reserved = process_sp->ReadPointerFromMemory(ptr + 4 * ptr_size, error);
----------------
The fact we have to calculate this by hand is slightly annoying, but I guess that's a bug to fix another day.


https://reviews.llvm.org/D43884