This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Delete register info definitions in the x86_64 ABI classes
ClosedPublic

Authored by labath on Feb 7 2020, 11:12 AM.

Details

Summary

These definitions are used to "augment" information received from the remote
target with eh/debug frame and "generic" register numbers.

Besides being verbose, this information was also incomplete (new registers like
xmm16-31 were missing) and sometimes even downright wrong (ymm register
numbers).

Most of this information is available via llvm's MCRegisterInfo. This patch
creates a new class, MCBasedABI, which retrieves the eh and debug frame register
numbers this way. The tricky part here is that the llvm class uses all-caps
register names, whereas lldb register are lowercase, and sometimes called
slightly differently. Therefore this class introduces some hooks to allow a
subclass to customize the MC lookup. The subclass also needs to suply the
"generic" register numbers, as this is an lldb invention.

This patch ports the x86_64 ABI classes to use the new register info mechanism.
It also creates a new "ABIx86_64" class which can be used to house code common
to x86_64 both ABIs. Right now, this just consists of a single function, but
there are plenty of other things that could be moved here too.

Diff Detail

Event Timeline

labath created this revision.Feb 7 2020, 11:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 7 2020, 11:12 AM
Herald added a subscriber: mgorny. · View Herald Transcript
tatyana-krasnukha added inline comments.
lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
940

Typo? It should be uppercase, I think.

jasonmolenda accepted this revision.Feb 10 2020, 2:24 PM

Nice elimination of duplication.

lldb/source/Target/ABI.cpp
249

Would it be simpler to assign these to LLDB_INVALID_REGNUM at the beginning, instead of -1 and setting it later?

267

will to_integer bit work with an all-letter reg like cpsr?

This revision is now accepted and ready to land.Feb 10 2020, 2:24 PM
labath marked 6 inline comments as done.Feb 14 2020, 4:43 AM
labath added inline comments.
lldb/source/Plugins/ABI/X86/ABISysV_x86_64.cpp
940

Thanks for catching that. After some consideration, I've decided it would be less confusing if this function (and GetMCName) worked on lowercase register names like everything else lldb. In the new version of this patch I've moved the uppercase transformation closer to the point where we interface with the MC layer.

lldb/source/Target/ABI.cpp
249

Not really, because the MC interface uses -1 to denote registers which don't have a dwarf number. (In fact some of the things in the MC list are not even really registers, but more like combinations of registers that are sometimes handy when grouped (Q0_Q1, etc.)

267

We didn't need this functionality yet because we weren't renaming any singular registers, but I've now added code to handle this too.

labath updated this revision to Diff 244624.Feb 14 2020, 4:44 AM
labath marked 3 inline comments as done.
  • make functions work on lowercase names
  • handle singular registers in the remapping function
This revision was automatically updated to reflect the committed changes.