Page MenuHomePhabricator

SymbolVendor: Introduce Module::GetSymbolFile
ClosedPublic

Authored by labath on Jul 30 2019, 2:25 AM.

Details

Summary

This is the next step in avoiding funneling all SymbolFile calls through
the SymbolVendor. Right now, it is just a convenience function, but it
allows us to update all calls to SymbolVendor functions to access the
SymbolFile directly. Once all call sites have been updated, we can
remove the GetSymbolVendor member function.

This patch just updates the calls to GetSymbolVendor, which were calling
it just so they could fetch the underlying symbol file. Other calls will
be done in follow-ups.

Event Timeline

labath created this revision.Jul 30 2019, 2:25 AM
JDevlieghere accepted this revision.Jul 30 2019, 9:11 AM
This revision is now accepted and ready to land.Jul 30 2019, 9:11 AM
clayborg requested changes to this revision.Jul 31 2019, 6:27 AM

Just add a few null checks and this will be good to go.

source/Commands/CommandObjectTarget.cpp
2240

check m->GetSymbolFile() for NULL

2265

check m->GetSymbolFile() for NULL

This revision now requires changes to proceed.Jul 31 2019, 6:27 AM

Alternatively we can switch to using a reference for:

SymbolFile &Module::GetSymbolFile(bool can_create Stream *feedback_strm);

As I believe we always fall back to SymbolFileSymtab don't we?

Alternatively we can switch to using a reference for:

SymbolFile &Module::GetSymbolFile(bool can_create Stream *feedback_strm);

As I believe we always fall back to SymbolFileSymtab don't we?

That is mostly true, but not 100% true in all circumstances. If we manage to create an object file for the module, then yes, I believe we should also always be able to construct a symbol file. However, we currently can run into situations where we fail to create an object file for a module (because the constructor does not check that). Instead what people usually do is that after constructing a module object, they call GetObjectFile() to check whether the module is really valid.

One of my ideas for the future is to change how modules are constructed, so that once we get a module object, we can assume that it will always contain a valid ObjectFile. At that point we would be able to assume the existence of SymbolFile. Until then, I can add the null checks you requested, though I don't think they can be really triggered now as those objectfile-less modules should not end up in the module list of a target.

labath updated this revision to Diff 212764.Aug 1 2019, 2:32 AM
  • add two null checks
  • replace GetSymbolVendor with GetSymbolFile in a couple of more places, where it is easy to do
clayborg accepted this revision.Aug 1 2019, 11:04 AM
This revision is now accepted and ready to land.Aug 1 2019, 11:04 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 2 2019, 1:16 AM