Page MenuHomePhabricator

PDBFPO: Refactor register reference resolution
ClosedPublic

Authored by labath on Apr 1 2019, 8:04 AM.

Details

Summary

This refactors moves the register name->number resolution out of the
FPOProgramNodeRegisterRef class. Instead I create a special
FPOProgramNodeSymbol class, which holds unresolved symbols, and move the
resolution into the ResolveRegisterRefs visitor.

The background here is that I'd like to use this code for Breakpad
unwind info, which uses similar syntax to describe unwind info. For
example, a simple breakpad unwind program might look like:

.cfa: $esp 8 + $ebp: .cfa 8 - ^

To be able to do this, I need to be able to customize register
resolving, as that is presently hardcoded to use codeview register
names, but breakpad supports a lot more architectures with different
register names. Moving the resolution into a separate class will allow
each user to use a different resolution logic.

Diff Detail

Repository
rLLDB LLDB

Event Timeline

labath created this revision.Apr 1 2019, 8:04 AM
zturner accepted this revision.Apr 1 2019, 1:31 PM
This revision is now accepted and ready to land.Apr 1 2019, 1:31 PM
amccarth accepted this revision.Apr 1 2019, 2:50 PM
amccarth added inline comments.
source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
287

nit: As long as you're correcting this comment, can you fix "look up" to be two words?

labath marked an inline comment as done.Apr 2 2019, 1:31 AM

Thanks.

BTW, do you have any thoughts where this code could live (it can't stay here because I need to access it from SymbolFileBreakpad too). I was thinking of the Symbol library, as both users are SymbolFiles.

I don't think this will have any external dependencies (that I can't get rid of), so another option might be Utility, but that seems a bit too low for this functionality (it will include code for generating dwarf expressions).

source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
287

will do.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptApr 2 2019, 1:43 AM
Herald added a subscriber: abidh. · View Herald Transcript

I think both approaches are acceptable, but I would prefer to include it into the Symbols library because this functionality is more symbols-related than general.