Page MenuHomePhabricator

SymbolVendor: Remove the object file member variable
ClosedPublic

Authored by labath on Jul 29 2019, 9:08 AM.

Details

Summary

The last responsibility of the SymbolVendor was to hold an owning
reference to the object file (in case symbols are being read from a
different file than the main module). As SymbolFile classes already hold
a non-owning reference to the object file, we can easily remove this
responsibility of the SymbolVendor by making the SymbolFile reference
owning.

Diff Detail

Repository
rL LLVM

Event Timeline

labath created this revision.Jul 29 2019, 9:08 AM

So previously the SymbolVendor would only make a strong reference to the object file if it didn't match the object file of the module. Now it always makes one. Might be ok since the Module owns the SymbolVendor and thus owns the SymbolFile. We should make sure we don't end up keeping an object file alive when a module goes away due to multiple strong references. I am guessing it will be ok, but good to verify.

So previously the SymbolVendor would only make a strong reference to the object file if it didn't match the object file of the module. Now it always makes one. Might be ok since the Module owns the SymbolVendor and thus owns the SymbolFile. We should make sure we don't end up keeping an object file alive when a module goes away due to multiple strong references. I am guessing it will be ok, but good to verify.

I've checked that the ObjectFile goes away when the owning Module is destroyed (e.g., in response to SBDebugger::MemoryPressureDetected), but I don't think this patch actually changes anything in terms of who-owns-who. The SymbolVendor was already storing a pointer to the object file even in case this was the main module object file (it stored the pointer "in case the object file is different", but the actual storing was unconditional).

clayborg accepted this revision.Jul 30 2019, 2:53 PM
This revision is now accepted and ready to land.Jul 30 2019, 2:53 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2019, 1:28 AM