Add nullptr check in FindLibCppStdFunctionCallableInfo()

[shafik]

Summary

We track down a crash in FindLibCppStdFunctionCallableInfo() to a missing nullptr check for the symbol variable.

[friss]

Do you have a testcase triggering this?

[davide]

What's the scenario that's causing this? Adding a nullptr check is an obviously safe thing to do, but it would be excellent if we could add a comment explaining why the symbol could be nullptr.
I also do believe that the logic for checking whether the symbol is nullptr can be hoisted to the beginning of the function, see comment inline.

source/Target/CPPLanguageRuntime.cpp

219–220

This should probably be nullptr, anyway, my general comment is that this check is scattered all around the function and could be centralized in a single place.

264–277

Here in the if branch you check whether the symbol is nullptr or not, but later you dereference it unconditionally. Are you always guaranteed that you're not dereferencing nullptr ?

[aprantl]

source/Target/CPPLanguageRuntime.cpp

219–220

if (symbol && symbol->GetName() ....

265

this should probably be factored out into a bool variable.

[shafik]

Simplified the checking of symbol being a nullptr

[shafik]

@friss we have several bugs, once of which I can reproduce but I have not been able to reduce it to a minimal case yet and the nullptr check is obviously the right to do.

source/Target/CPPLanguageRuntime.cpp

CPPLanguageRuntime::FindLibCppStdFunctionCallableInfo(
   // and obtain the data we need.
   if (target.GetSectionLoadList().ResolveLoadAddress(
           possible_function_address, function_address_resolved)) {
     target.GetImages().ResolveSymbolContextForAddress(
         function_address_resolved, eSymbolContextEverything, sc);
     symbol = sc.symbol;
   }
 
+  if (symbol == nullptr)
+    return optional_info;
+    
   auto get_name = [&first_template_parameter, &symbol]() {
     // Given case 1:
     //
     //    main::$_0
     //
     // we want to append ::operator()()
     if (first_template_parameter.contains("$_"))
       return llvm::Regex::escape(first_template_parameter.str()) +
              R"(::operator\(\)\(.*\))";
 
-    if (symbol != NULL &&
-        symbol->GetName().GetStringRef().contains("__invoke")) {
+    if (symbol->GetName().GetStringRef().contains("__invoke")) {
 
       llvm::StringRef symbol_name = symbol->GetName().GetStringRef();
       size_t pos2 = symbol_name.find_last_of(':');
 
       // Given case 2:
       //
       //    main::$_1::__invoke(...)
       //
if (scl.GetSize() >= 1) {
     Address address = range.GetBaseAddress();
 
     Address addr;
     if (target.ResolveLoadAddress(address.GetCallableLoadAddress(&target),
                                   addr)) {
       LineEntry line_entry;
       addr.CalculateSymbolContextLineEntry(line_entry);
 
       if (first_template_parameter.contains("$_") ||
-          (symbol != nullptr &&
-           symbol->GetName().GetStringRef().contains("__invoke"))) {
+          symbol->GetName().GetStringRef().contains("__invoke")) {
         // Case 1 and 2
         optional_info.callable_case = LibCppStdFunctionCallableCase::Lambda;
       } else {
         // Case 3
         optional_info.callable_case =
             LibCppStdFunctionCallableCase::CallableObject;
       }
 
       optional_info.callable_symbol = *symbol;
       optional_info.callable_line_entry = line_entry;
       optional_info.callable_address = addr;
       return optional_info;
     }
   }
 
   // Case 4 or 5
   if (!symbol->GetName().GetStringRef().startswith("vtable for")) {
     optional_info.callable_case =
         LibCppStdFunctionCallableCase::FreeOrMemberFunction;
     optional_info.callable_address = function_address_resolved;