[lldb] Support querying registers via generic names without alt_names

[mgorny]

Summary

Update GetRegisterInfoByName() methods to support getting registers
by a generic name independently of alt_name entries in the register
context. This makes it possible to use generic names when interacting
with gdbserver (that does not supply alt_names). It also makes it
possible to remove some of the duplicated information from register
context declarations and/or use alt_names for another purpose.

[mgorny]

Preserve the prior (probably accidental) behavior of generic names taking precedence over later registers with the same name (e.g. generic sp alias taking precedence over 16-bit sp pseudo-register).

[mgorny]

The test requires building with X86 target.

[labath]

I'm not sure we should be preserving this behavior for the native register context. User-provided strings should not make it there, and code should really be retrieving the registers in some other way (like via the LLDB_REGNUM_GENERIC_*** constants).

It would also be nice to follow this up by actually removing the generic altnames from register definitions, for cases where they don't match architecturally defined names.

[mgorny]

In D108554#2993766, @labath wrote:

I'm not sure we should be preserving this behavior for the native register context. User-provided strings should not make it there, and code should really be retrieving the registers in some other way (like via the LLDB_REGNUM_GENERIC_*** constants).

Could you rephrase this comment to indicate the requested change, if any? ;-)

It would also be nice to follow this up by actually removing the generic altnames from register definitions, for cases where they don't match architecturally defined names.

That was roughly my plan, presuming you like the idea. Should I include this in the same commit or separately for each platform?

[labath]

In D108554#2993830, @mgorny wrote:

In D108554#2993766, @labath wrote:

I'm not sure we should be preserving this behavior for the native register context. User-provided strings should not make it there, and code should really be retrieving the registers in some other way (like via the LLDB_REGNUM_GENERIC_*** constants).

Could you rephrase this comment to indicate the requested change, if any? ;-)

Well.. the comment was open ended, because my thoughts on this are quite open ended as well.. :) But if I wanted to be more precise, I suppose it boils down to this:

• Is the NativeRC change there for a particular reason (like, does anything break if you leave it out)?
• Do _you_ agree with my assessment of its necessity?

If there's no reason for its presence, and you don't see it as necessary, then I'd like to remove it. If not, let's talk.

It would also be nice to follow this up by actually removing the generic altnames from register definitions, for cases where they don't match architecturally defined names.

That was roughly my plan, presuming you like the idea. Should I include this in the same commit or separately for each platform?

Sounds good. I think we should do it as a separate patch. Whether it's one or more of them depends on how large they get...

[mgorny]

In D108554#2993993, @labath wrote:

In D108554#2993830, @mgorny wrote:

In D108554#2993766, @labath wrote:

I'm not sure we should be preserving this behavior for the native register context. User-provided strings should not make it there, and code should really be retrieving the registers in some other way (like via the LLDB_REGNUM_GENERIC_*** constants).

Could you rephrase this comment to indicate the requested change, if any? ;-)

Well.. the comment was open ended, because my thoughts on this are quite open ended as well.. :) But if I wanted to be more precise, I suppose it boils down to this:

• Is the NativeRC change there for a particular reason (like, does anything break if you leave it out)?
• Do _you_ agree with my assessment of its necessity?

If there's no reason for its presence, and you don't see it as necessary, then I'd like to remove it. If not, let's talk.

Ah, now I get what you mean. To be honest, I've done that because I wasn't sure whether there isn't any code relying on that. I guess I'll try removing it and see if any tests fail.

[mgorny]

Ok, so I've done some testing and indeed Native* part doesn't seem necessary.

However, after removing the aliases from x86_64 target def, there are two test regressions. The two following snippets no longer seem to work:

eip = frame.FindRegister("pc")
sp_value = frame0.FindValue("sp", lldb.eValueTypeRegister)

This doesn't seem to work anymore. Should I attempt to fix the API to make grabbing these registers by name work again, or should I try to modify the tests to grab these registers via generic reg nums?

[labath]

These are pretty much the SB equivalents of register read, and they should keep working

[mgorny]

Fixed SBFrame stuffs. While at it, changed ValueObjectRegister to take const RegisterInfo * instead of the index, since it's almost always what it really wants.

[labath]

lldb/source/API/SBFrame.cpp

636–638 ↗

(On Diff #371941)

I like this.

lldb/source/Target/RegisterContext.cpp

57–59

This isn't just "consistent with" generic aliases taking precedence. This is actually the implementation of that precedence (at least, after we remove the aliases in the other patch. This should just state as a fact that we want the aliases to take priority. The historic trivia can go into the commit message.

lldb/source/Host/common/NativeRegisterContext.cpp

NativeRegisterContext::GetRegisterInfoByName(llvm::StringRef reg_name,
   const uint32_t num_registers = GetRegisterCount();
   for (uint32_t reg = start_idx; reg < num_registers; ++reg) {
     const RegisterInfo *reg_info = GetRegisterInfoAtIndex(reg);
 
     if (reg_name.equals_insensitive(reg_info->name) ||
         reg_name.equals_insensitive(reg_info->alt_name))
       return reg_info;
   }
+
+  // if none of the register entries matched directly, try matching
+  // using a generic register name.
+  uint32_t generic_reg = Args::StringToGenericRegister(reg_name);
+  if (generic_reg != LLDB_INVALID_REGNUM)
+    return GetRegisterInfo(eRegisterKindGeneric, generic_reg);
+
   return nullptr;
 }
 
 const RegisterInfo *NativeRegisterContext::GetRegisterInfo(uint32_t kind,
                                                            uint32_t num) {
   const uint32_t reg_num = ConvertRegisterKindToRegisterNumber(kind, num);
   if (reg_num == LLDB_INVALID_REGNUM)
     return nullptr;

lldb/source/Target/RegisterContext.cpp

 }
 
 const RegisterInfo *
 RegisterContext::GetRegisterInfoByName(llvm::StringRef reg_name,
                                        uint32_t start_idx) {
   if (reg_name.empty())
     return nullptr;
 
   const uint32_t num_registers = GetRegisterCount();
   for (uint32_t reg = start_idx; reg < num_registers; ++reg) {
     const RegisterInfo *reg_info = GetRegisterInfoAtIndex(reg);
 
     if (reg_name.equals_insensitive(reg_info->name) ||
         reg_name.equals_insensitive(reg_info->alt_name))
       return reg_info;
   }
+
+  // if none of the register entries matched directly, try matching
+  // using a generic register name.
+  uint32_t generic_reg = Args::StringToGenericRegister(reg_name);
+  if (generic_reg != LLDB_INVALID_REGNUM)
+    return GetRegisterInfo(eRegisterKindGeneric, generic_reg);
+
   return nullptr;
 }
 
 uint32_t
 RegisterContext::UpdateDynamicRegisterSize(const lldb_private::ArchSpec &arch,
                                            RegisterInfo *reg_info) {
   ExecutionContext exe_ctx(CalculateThread());
 

lldb/test/API/functionalities/gdb_remote_client/TestGDBServerTargetXML.py

def test_x86_64_vec_regs(self):
             def qXferRead(self, obj, annex, offset, length):
                 if annex == "target.xml":
                     return """<?xml version="1.0"?>
                         <!DOCTYPE feature SYSTEM "gdb-target.dtd">
                         <target>
                           <architecture>i386:x86-64</architecture>
                           <osabi>GNU/Linux</osabi>
                           <feature name="org.gnu.gdb.i386.core">
+                            <reg name="rcx" bitsize="64" type="int64" regnum="2"/>
+                            <reg name="rdx" bitsize="64" type="int64" regnum="3"/>
+                            <reg name="rsi" bitsize="64" type="int64" regnum="4"/>
+                            <reg name="rdi" bitsize="64" type="int64" regnum="5"/>
+                            <reg name="rbp" bitsize="64" type="data_ptr" regnum="6"/>
                             <reg name="rsp" bitsize="64" type="data_ptr" regnum="7"/>
+                            <reg name="r8" bitsize="64" type="int64" regnum="8"/>
+                            <reg name="r9" bitsize="64" type="int64" regnum="9"/>
                             <reg name="rip" bitsize="64" type="code_ptr" regnum="16"/>
+                            <reg name="eflags" bitsize="32" type="i386_eflags" regnum="17"/>
                             <reg name="st0" bitsize="80" type="i387_ext" regnum="24"/>
                             <reg name="st1" bitsize="80" type="i387_ext" regnum="25"/>
                             <reg name="st2" bitsize="80" type="i387_ext" regnum="26"/>
                             <reg name="st3" bitsize="80" type="i387_ext" regnum="27"/>
                             <reg name="st4" bitsize="80" type="i387_ext" regnum="28"/>
                             <reg name="st5" bitsize="80" type="i387_ext" regnum="29"/>
                             <reg name="st6" bitsize="80" type="i387_ext" regnum="30"/>
                             <reg name="st7" bitsize="80" type="i387_ext" regnum="31"/>
def test_x86_64_vec_regs(self):
                 else:
                     return None, False
 
             def readRegister(self, regnum):
                 return ""
 
             def readRegisters(self):
                 return (
-                    "0102030405060708"  # rsp
-                    "1112131415161718"  # rip
+                    "0102030405060708"  # rcx
+                    "1112131415161718"  # rdx
+                    "2122232425262728"  # rsi
+                    "3132333435363738"  # rdi
+                    "4142434445464748"  # rbp
+                    "5152535455565758"  # rsp
+                    "6162636465666768"  # r8
+                    "7172737475767778"  # r9
+                    "8182838485868788"  # rip
+                    "91929394"  # eflags
                     "0102030405060708090a"  # st0
                     "1112131415161718191a" +  # st1
                     "2122232425262728292a" * 6 +  # st2..st7
                     "8182838485868788898a8b8c8d8e8f90"  # xmm0
                     "9192939495969798999a9b9c9d9e9fa0"  # xmm1
                     "a1a2a3a4a5a6a7a8a9aaabacadaeafb0"  # xmm2..xmm7
                 )
 
             def haltReason(self):
                 return "T02thread:1ff0d;threads:1ff0d;thread-pcs:000000010001bc00;07:0102030405060708;10:1112131415161718;"
 
         self.server.responder = MyResponder()
 
         target = self.createTarget("basic_eh_frame.yaml")
         process = self.connect(target)
         lldbutil.expect_state_changes(self, self.dbg.GetListener(), process,
                                       [lldb.eStateStopped])
 
         # rsp and rip should be displayed as uints
         self.match("register read rsp",
-                   ["rsp = 0x0807060504030201"])
+                   ["rsp = 0x5857565554535251"])
         self.match("register read rip",
-                   ["rip = 0x1817161514131211"])
+                   ["rip = 0x8887868584838281"])
+
+        # test generic aliases
+        self.match("register read arg4",
+                   ["rcx = 0x0807060504030201"])
+        self.match("register read arg3",
+                   ["rdx = 0x1817161514131211"])
+        self.match("register read arg2",
+                   ["rsi = 0x2827262524232221"])
+        self.match("register read arg1",
+                   ["rdi = 0x3837363534333231"])
+        self.match("register read fp",
+                   ["rbp = 0x4847464544434241"])
+        self.match("register read sp",
+                   ["rsp = 0x5857565554535251"])
+        self.match("register read arg5",
+                   ["r8 = 0x6867666564636261"])
+        self.match("register read arg6",
+                   ["r9 = 0x7877767574737271"])
+        self.match("register read pc",
+                   ["rip = 0x8887868584838281"])
+        self.match("register read flags",
+                   ["eflags = 0x94939291"])
 
         # both stX and xmmX should be displayed as vectors
         self.match("register read st0",
                    ["st0 = {0x01 0x02 0x03 0x04 0x05 0x06 0x07 0x08 0x09 0x0a}"])
         self.match("register read st1",
                    ["st1 = {0x11 0x12 0x13 0x14 0x15 0x16 0x17 0x18 0x19 0x1a}"])
         self.match("register read xmm0",
                    ["xmm0 = {0x81 0x82 0x83 0x84 0x85 0x86 0x87 0x88 "
                     "0x89 0x8a 0x8b 0x8c 0x8d 0x8e 0x8f 0x90}"])
         self.match("register read xmm1",
                    ["xmm1 = {0x91 0x92 0x93 0x94 0x95 0x96 0x97 0x98 "
                     "0x99 0x9a 0x9b 0x9c 0x9d 0x9e 0x9f 0xa0}"])