Extend max register size to accommodate AArch64 SVE vector regs

[omjavaid]

Summary

This patch increases maximum register size to 256 bytes to accommodate AArch64 SVE registers maximum possible size of 256 bytes.

[danielkiss]

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

2051

Could we use the kMaxRegisterByteSize here?

[labath]

Sounds fairly noncontroversial. I don't think we have too many of these objects floating around, but if it turns out we do, we could switch to a SmallVector to optimize for the common case of smaller registers.

lldb/include/lldb/Utility/RegisterValue.h

264

how about we stick to uint16_t here ?

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

2051

An excellent idea.

[omjavaid]

In D77044#1954685, @labath wrote:

Sounds fairly noncontroversial. I don't think we have too many of these objects floating around, but if it turns out we do, we could switch to a SmallVector to optimize for the common case of smaller registers.

DataExtractor is used for caching register values on stop by gdb-remote register context. RegisterValue is mostly used for passing around register values temporarily during read/write.

lldb/include/lldb/Utility/RegisterValue.h

264

ACK.

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

2051

ACK.

[omjavaid]

Posting full diff.

lldb/include/lldb/Utility/RegisterValue.h

 
 namespace lldb_private {
 class DataExtractor;
 class Stream;
 struct RegisterInfo;
 
 class RegisterValue {
 public:
-  enum { kMaxRegisterByteSize = 64u };
+  // big enough to support up to 256 byte AArch64 SVE
+  enum { kMaxRegisterByteSize = 256u };
 
   enum Type {
     eTypeInvalid,
     eTypeUInt8,
     eTypeUInt16,
     eTypeUInt32,
     eTypeUInt64,
     eTypeUInt128,
public:
   void Clear();
 
 protected:
   RegisterValue::Type m_type;
   Scalar m_scalar;
 
   struct {
     uint8_t bytes[kMaxRegisterByteSize]; // This must be big enough to hold any
                                          // register for any supported target.
-    uint8_t length;
+    uint16_t length;
     lldb::ByteOrder byte_order;
   } buffer;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_UTILITY_REGISTERVALUE_H

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

GDBRemoteCommunicationServerLLGS::Handle_P(StringExtractorGDBRemote &packet) {
   }
 
   // Note debugserver would send an E30 here.
   if ((packet.GetBytesLeft() < 1) || (packet.GetChar() != '='))
     return SendIllFormedResponse(
         packet, "P packet missing '=' char after register number");
 
   // Parse out the value.
-  uint8_t reg_bytes[32]; // big enough to support up to 256 bit ymmN register
+  uint8_t reg_bytes[RegisterValue::kMaxRegisterByteSize];
   size_t reg_size = packet.GetHexBytesAvail(reg_bytes);
 
   // Get the thread to use.
   NativeThreadProtocol *thread = GetThreadFromSuffix(packet);
   if (!thread) {
     LLDB_LOGF(log,
               "GDBRemoteCommunicationServerLLGS::%s failed, no thread "
               "available (thread index 0)",
               __FUNCTION__);
     return SendErrorResponse(0x28);
   }
 
   // Get the thread's register context.
   NativeRegisterContext &reg_context = thread->GetRegisterContext();
   const RegisterInfo *reg_info = reg_context.GetRegisterInfoAtIndex(reg_index);
   if (!reg_info) {

lldb/source/Utility/RegisterValue.cpp

if (m_type == rhs.m_type) {
     case eTypeFloat:
     case eTypeDouble:
     case eTypeLongDouble:
       return m_scalar == rhs.m_scalar;
     case eTypeBytes:
       if (buffer.length != rhs.buffer.length)
         return false;
       else {
-        uint8_t length = buffer.length;
+        uint16_t length = buffer.length;
         if (length > kMaxRegisterByteSize)
           length = kMaxRegisterByteSize;
         return memcmp(buffer.bytes, rhs.buffer.bytes, length) == 0;
       }
       break;
     }
   }
   return false;