[lldb] Add missing nullptr checks to AppleObjCRuntime::GetBacktraceThreadFromException

[teemperor]

Summary

We got a few crash reports where LLDB crashes while derefencing the frames_value shared_ptr in the AppleObjCRuntime::GetBacktraceThreadFromException. GetChildMemberWithName returns a nullptr when an error occurs, so this seems to be just a missing nullptr check.

This patch adds that nullptr check and the other ones in the similar code directly below.

Fixes rdar://62174039

[teemperor]

I don't know the code well enough to know how to reach a state where we can't find the _frames member, so I haven't found a test for this yet. If anyone has an idea how to reach that then let me know.

[teemperor]

ping

[jingham]

Yes, we shouldn't fall over if we can't get the exception backtrace out of the exception message.

You should be able to make an NSException that has a callStackReturnAddresses dictionary entry that doesn't contain these fields. So I think you can write a test for this, though the patch is also clearly right...

It makes me a little unhappy that all the errors in this function silently return an empty thread, so we're never going to know why this fails... If there was some way to add useful logging to this function, that would be great.

[teemperor]

• Add logging to all the error cases.

Still figuring out the test.

[jingham]

Excellent, thanks!

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

while (descriptor) {
     if (class_name == "NSException")
       return cpp_exception;
     descriptor = descriptor->GetSuperclass();
   }
 
   return ValueObjectSP();
 }
 
+/// Utility method for error handling in GetBacktraceThreadFromException.
+/// \param msg The message to add to the log.
+/// \return An invalid ThreadSP to be returned from
+///         GetBacktraceThreadFromException.
+LLVM_NODISCARD
+static ThreadSP FailExceptionParsing(llvm::StringRef msg) {
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_LANGUAGE));
+  LLDB_LOG(log, "Failed getting backtrace from exception: {0}", msg);
+  return ThreadSP();
+}
+
 ThreadSP AppleObjCRuntime::GetBacktraceThreadFromException(
     lldb::ValueObjectSP exception_sp) {
   ValueObjectSP reserved_dict =
       exception_sp->GetChildMemberWithName(ConstString("reserved"), true);
-  if (!reserved_dict) return ThreadSP();
+  if (!reserved_dict)
+    return FailExceptionParsing("Failed to get 'reserved' member.");
 
   reserved_dict = reserved_dict->GetSyntheticValue();
-  if (!reserved_dict) return ThreadSP();
+  if (!reserved_dict)
+    return FailExceptionParsing("Failed to get synthetic value.");
 
   TypeSystemClang *clang_ast_context =
       TypeSystemClang::GetScratch(*exception_sp->GetTargetSP());
   if (!clang_ast_context)
-    return ThreadSP();
+    return FailExceptionParsing("Failed to get scratch AST.");
   CompilerType objc_id =
       clang_ast_context->GetBasicType(lldb::eBasicTypeObjCID);
   ValueObjectSP return_addresses;
 
   auto objc_object_from_address = [&exception_sp, &objc_id](uint64_t addr,
                                                             const char *name) {
     Value value(addr);
     value.SetCompilerType(objc_id);
if (lldb_private::formatters::NSStringSummaryProvider(
       if (key_summary.GetString() == "\"callStackReturnAddresses\"") {
         return_addresses = objc_object_from_address(dict_entry_value,
                                                     "callStackReturnAddresses");
         break;
       }
     }
   }
 
-  if (!return_addresses) return ThreadSP();
+  if (!return_addresses)
+    return FailExceptionParsing("Failed to get return addresses.");
   auto frames_value =
       return_addresses->GetChildMemberWithName(ConstString("_frames"), true);
+  if (!frames_value)
+    return FailExceptionParsing("Failed to get frames_value.");
   addr_t frames_addr = frames_value->GetValueAsUnsigned(0);
   auto count_value =
       return_addresses->GetChildMemberWithName(ConstString("_cnt"), true);
+  if (!count_value)
+    return FailExceptionParsing("Failed to get count_value.");
   size_t count = count_value->GetValueAsUnsigned(0);
   auto ignore_value =
       return_addresses->GetChildMemberWithName(ConstString("_ignore"), true);
+  if (!ignore_value)
+    return FailExceptionParsing("Failed to get ignore_value.");
   size_t ignore = ignore_value->GetValueAsUnsigned(0);
 
   size_t ptr_size = m_process->GetAddressByteSize();
   std::vector<lldb::addr_t> pcs;
   for (size_t idx = 0; idx < count; idx++) {
     Status error;
     addr_t pc = m_process->ReadPointerFromMemory(
         frames_addr + (ignore + idx) * ptr_size, error);
     pcs.push_back(pc);
   }
 
-  if (pcs.empty()) return ThreadSP();
+  if (pcs.empty())
+    return FailExceptionParsing("Failed to get PC list.");
 
   ThreadSP new_thread_sp(new HistoryThread(*m_process, 0, pcs));
   m_process->GetExtendedThreadList().AddThread(new_thread_sp);
   return new_thread_sp;
 }
 
 std::tuple<FileSpec, ConstString>
 AppleObjCRuntime::GetExceptionThrowLocation() {