Page MenuHomePhabricator

[LLVMSymbolize] Move ModuleInfo into a separate class (SymbolizableModule).

Authored by samsonov on Oct 26 2015, 5:10 PM.



This is mostly NFC. It is a first step in cleaning up LLVMSymbolize
library. It removes "ModuleInfo" class which bundles together ObjectFile
and its debug info context in favor of:

  • abstract SymbolizableModule in public headers;
  • SymbolizableObjectFile subclass in implementation.

Additionally, SymbolizableObjectFile is now created via factory, so we
can properly detect object parsing error at this stage instead of keeping
the broken half-parsed object. As a next step, we would be able to
propagate the error all the way back to the library user.

Further improvements might include:

  • factoring out the logic of finding appropriate file with debug info for a given object file, and caching all parsed object files into a separate class [A].
  • factoring out DILineInfo rendering [B].

This would make what is now a heavyweight "LLVMSymbolizer" a relatively
straightforward class, that calls into [A] to turn filepath into a
SymbolizableModule, delegates actual symbolization to concrete SymbolizableModule
implementation, and lets [C] render the result.

Feel free to express any concerns regarding this plan, or suggest better

Diff Detail


Event Timeline

samsonov updated this revision to Diff 38485.Oct 26 2015, 5:10 PM
samsonov retitled this revision from to [LLVMSymbolize] Move ModuleInfo into a separate class (SymbolizableModule)..
samsonov added reviewers: dblaikie, echristo, rafael.
samsonov updated this object.
samsonov added a subscriber: llvm-commits.
echristo accepted this revision.Oct 28 2015, 3:18 PM
echristo edited edge metadata.

Meta: Not a big fan of SymbolizableModule as a name. Especially as we're adding actual Modules with debug info it's going to get confusing. SymbolizableUnit maybe? *shrug*

Couple of inline comments, but otherwise it looks fine.


213–220 ↗(On Diff #38485)

Coding style nit.

224–238 ↗(On Diff #38485)

Could probably use some more comments here, also if there's a way to unindent a bit of this that'd be awesome.

This revision is now accepted and ready to land.Oct 28 2015, 3:18 PM
samsonov marked 2 inline comments as done.Oct 29 2015, 3:23 PM

Thanks! What do you mean by "Modules with debug info"? :)

This revision was automatically updated to reflect the committed changes.