[symbolizer] Fix a dangling pointer in LLVMSymbolizer::getOrCreateObjects

[kubamracek]

Summary

LLVMSymbolizer in getOrCreateObjects() takes ownership of the Binary object it creates, so it's not deallocated early (we're storing pointers to Binary objects in ObjectFileForArch). However, when getObjectFileFromBinary fails (e.g. when the specified architecture is not found), we currently fail to take ownership of Binary. Instead we end up deallocating the object too early and storing a dangling pointer in ObjectFileForArch. This fix calls addOwningBinary even when an error is returned from getObjectFileFromBinary.

[samsonov]

This is wrong for several reasons: first of all, there are many calls to getObjectFileFromBinary, and you likely need to do smth. similar for all of them. But then it somewhat defeats the purpose of caching: we essentially keep object files in memory even if they don't have object file for appropriate architecture. Looks like this code needs to be reworked, and at the very least ObjectFileForArch should not have raw pointers as keys. I'll look into that.

[samsonov]

Let me know if r256041 fixes a problem for you.

[kubamracek]

Yes, r256041 seems to have fixed this problem. Thanks!

lib/DebugInfo/Symbolize/Symbolize.cpp

if (auto EC = BinaryOrErr.getError()) {
     return EC;
   }
   OwningBinary<Binary> &B = BinaryOrErr.get();
 
   auto ObjOrErr = getObjectFileFromBinary(B.getBinary(), ArchName);
   if (auto EC = ObjOrErr.getError()) {
     ObjectPairForPathArch.insert(
         std::make_pair(std::make_pair(Path, ArchName), EC));
+    addOwningBinary(std::move(B));
     return EC;
   }
   addOwningBinary(std::move(B));
 
   ObjectFile *Obj = ObjOrErr.get();
   assert(Obj != nullptr);
   ObjectFile *DbgObj = nullptr;