Page MenuHomePhabricator

Complete register kind naming cleanups in lldb -- required touching all register table definitions
ClosedPublic

Authored by jasonmolenda on Sep 10 2015, 8:42 PM.

Details

Reviewers
jasonmolenda
Summary

This change removes the last vestiges of where we refer to eh_frame register numbers as "gcc", calling them "eh_frame" consistently across the lldb source base.

This fixes the incorrect partial rename I did recently of "gdb" into "stabs". I wasn't sure what "gdb" was meant to indicate -- it is meant to indicate the register numbering scheme used by the gdb Remote Protocol stub program to refer to registers. It's not correct to call this the "gdb" register numbering scheme though - we could be talking over kdp or it could be a core file debug session. So I've renamed all of these to "Plugin Process" register numbers.

The Plugin Process register numbers in lldb's register tables are generally undefined. When we start the Process communication, we will query what register numbers should be used (e.g. by qRegisterInfo packets, or the qXfer:features:read:target.xml packet) to refer to different registers when communicating with that remote stub. If we were going to set the Process Plugin register numbers to anything inside lldb, it would be the index numbers of the registers in the table.

We had several definitions of "gdb register numbers" in different tables across lldb. I removed all of these - they are meaningless.

The one other type of register number we use are "lldb" register numbers. These numbers should start at 0 and increase through to the last register that is defined for a given architecture/target.

I'll be making another change after this is committed to allow the Process Plugin register numbers to be noncontiguous. I'm trying to make lldb work with a JTAG device that advertises these noncontiguous register numbers are we've currently got some assumptions in lldb that Process Plugin regnums are contiguous. lldb registers can be assumed to be contiguous, but not Process Plugin regnums.

This is a bit of a large patch but the changes are general mechanical in nature. It may break the build for Windows or another OS - I've only been testing the build on macosx. If anyone can give the patch a build over the next few days and let me know if there are problems, I'd appreciate it.

Diff Detail

Event Timeline

jasonmolenda retitled this revision from to Complete register kind naming cleanups in lldb -- required touching all register table definitions.
jasonmolenda updated this object.
jasonmolenda set the repository for this revision to rL LLVM.
jasonmolenda added a subscriber: lldb-commits.

I tried to apply your patch but couldn't manage to do it because source/Utility/ARM_ehframe_Registers.[h,cpp] is not available in TOT. I think you uploaded only a part of your patch.

jasonmolenda removed rL LLVM as the repository for this revision.

Ah, sorry, I did

svn mv ARM_Stabs_Registers.h ARM_ehframe_Registers.h
svn mv ARM64_Stabs_Registers.h ARM64_ehframe_Registers.h

and the patch didn't represent that correctly. I cobbled up a new patch by hand, did a clean checkout, applied the patch and built it with xcodebuild.

Thank you, the naming conventions were always a huge source of confusion.

I tried out this change and it compiles both on Linux and on arm, but it causes 2 test failure:
TestLldbGdbServer.LldbGdbServerTestCase.test_qRegisterInfo_returns_all_valid_results_llgs_dwarf
TestLldbGdbServer.LldbGdbServerTestCase.test_qRegisterInfo_returns_one_valid_result_llgs_dwarf

Thanks Tamas. I'll figure out what's happening there before I commit.

Those tests are communicating on the gdb remote protocol directly and do a pattern matching on the response. I think they have the register set names hard coded in one of those patters, but haven't looked into it (I expect that they will fail on OSX too).

Thanks Tamas. I have access to a linux machine, I had a little trouble getting TOT set up correctly yesterday but I'll get that figured out today and fix whatever the issue is.

Ah, those failures were due to

Index: gdbremote_testcase.py

  • gdbremote_testcase.py (revision 247726)

+++ gdbremote_testcase.py (working copy)
@@ -548,7 +548,7 @@

"encoding",
"format",
"set",
  • "gcc",

+ "ehframe",

"dwarf",
"generic",
"container-regs",

I'll update the patch and commit.

jasonmolenda accepted this revision.Sep 15 2015, 4:22 PM
jasonmolenda added a reviewer: jasonmolenda.
This revision is now accepted and ready to land.Sep 15 2015, 4:22 PM
jasonmolenda closed this revision.Sep 15 2015, 4:22 PM

Landed in r247741.