[lldb] DynamicRegisterInfo: fix wrong regnos in Dump()

[mgorny]

Summary

Fix the DynamicRegisterInfo::Dump() method to correctly assume that
the register numbers in value_regs and invalidate_regs arrays are
remote (eRegisterKindProcessPlugin) rather than local
(eRegisterKindLLDB). This fixes invalid memory references when the two
register numbers are not in sync.

[mgorny]

TestRemoteRegNums suggests that there's also code relying on remote regnums being used there. What a mess :-(.

[mgorny]

Ok, I think it might just be Dump() being broken. I've found another bug with my code, and things seem to work after fixing it, so I suppose the problem is not really worth my effort right now.

[mgorny]

So I've given it some thought and found a simple fix.

[mgorny]

Also updated the comments in struct RegisterInfo to indicate what kind of numbers go in these arrays.

[labath]

I'm not sure if this is correct, or if I want it to be correct. I think that a lot of the confusion comes from the fact that this structure is used both in lldb-server and lldb, and I'm not sure that the fields have the same meaning in both cases (specifically, the "process plugin" concept is not really a thing in lldb-server). I think it would be better to standardize on the "lldb" numbers, as those should be available everywhere, but I may be forgetting some important issues. Omair may remember this in more detail.

[omjavaid]

This is complicated mainly because we are trying to merge two different register numbering scheme. On LLDB (host) side we try to construct register number from parsing register xml assigning register numbers in the increasing order. But we also have target supplied register numbers in regnum field. However registers on LLDB server side have fixed numbering although it may look like assigned in increasing order but this may not always be the case. Presence of optional register sets also doesnt help so we have the mess on managing target vs host register numbers not always in sync.

For invalidate_regs and value_regs register numbers should always be the ones assigned by remote i-e eRegisterKindProcessPlugin. So I would say this patch is ok as it corrects that info.

Broadly speaking we should do something about integrating user typed registers which may be unions of various other registers but should not require numbering.

[mgorny]

Replaced by D110027.

lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp

if (m_regs[i].kinds[eRegisterKindEHFrame] != LLDB_INVALID_REGNUM)
       s.Printf(", ehframe = %3u", m_regs[i].kinds[eRegisterKindEHFrame]);
     if (m_regs[i].kinds[eRegisterKindGeneric] != LLDB_INVALID_REGNUM)
       s.Printf(", generic = %3u", m_regs[i].kinds[eRegisterKindGeneric]);
     if (m_regs[i].alt_name)
       s.Printf(", alt-name = %s", m_regs[i].alt_name);
     if (m_regs[i].value_regs) {
       s.Printf(", value_regs = [ ");
       for (size_t j = 0; m_regs[i].value_regs[j] != LLDB_INVALID_REGNUM; ++j) {
-        s.Printf("%s ", m_regs[m_regs[i].value_regs[j]].name);
+        const RegisterInfo *value_reg = GetRegisterInfo(
+            eRegisterKindProcessPlugin, m_regs[i].value_regs[j]);
+        if (value_reg)
+          s.Printf("%s ", value_reg->name);
+        else
+          s.Printf("(%d) ", m_regs[i].value_regs[j]);
       }
       s.Printf("]");
     }
     if (m_regs[i].invalidate_regs) {
       s.Printf(", invalidate_regs = [ ");
       for (size_t j = 0; m_regs[i].invalidate_regs[j] != LLDB_INVALID_REGNUM;
            ++j) {
-        s.Printf("%s ", m_regs[m_regs[i].invalidate_regs[j]].name);
+        const RegisterInfo *invalidate_reg = GetRegisterInfo(
+            eRegisterKindProcessPlugin, m_regs[i].invalidate_regs[j]);
+        if (invalidate_reg)
+          s.Printf("%s ", invalidate_reg->name);
+        else
+          s.Printf("(%d) ", m_regs[i].invalidate_regs[j]);
       }
       s.Printf("]");
     }
     s.EOL();
   }
 
   const size_t num_sets = m_sets.size();
   s.Printf("%p: DynamicRegisterInfo contains %" PRIu64 " register sets:\n",