[LLDB][MIPS] Fix qProcessInfo to return correct pointer size based on ELF ABI

[nitesh.jain]

Summary

In case of MIPS64, the pointer size depends on ELF abi. The MIPS64 currently support following abi's

1. N32 : - The pointer size is 4 byte
2. N64 :- The pointer size is 8 byte

This patch add one more key (eflags) in qProcessInfo which will be use to get correct pointer size based on abi.

[labath]

Looks good as far as I am concerned. Just a couple of nits here and there.

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

1887 ↗

(On Diff #72823)

Why you chose this name? eflags is the name of a i386 register. How about we use a different field name? archflags maybe ?

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp

1218 ↗

(On Diff #72823)

Why do we have the compilcated switch here. Can't we replace that with:
response.Printf("ptrsize:%d;", proc_arch.GetAddressByteSize()); for all sizes?

[clayborg]

We need to send each flag individually. See inlined comments.

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

1887 ↗

(On Diff #72823)

We should avoid trying pass ArchSpec specific flags over the wire. Seems like we should be abstracting each flag we want to send as a new key. How is some code that isn't based on LLDB going to fill in these flags? Please add a key/value pair for each flag you want to send. You can add a function to ArchSpec that gets a StructuredData::Dictionary from the ArchSpec and then uses that to populate the key/value pairs that you send back. Each architecture defines a set of unique flags. Another issue with sending e_flags directly is if someone changes the bit definitions of the flags and we have a new LLDB hook up to an older lldb-server, we will get the flags wrong.

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp

1218 ↗

(On Diff #72823)

labath: the default ptr size for MIPS64 is 8. We would need to modify ArchSpec.cpp to look at the flags for MIPS64 and change the pointer size to 4 if the N32 is being used.

[nitesh.jain]

source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp

1218 ↗

(On Diff #72823)

Greg: ArchSpec::GetAddressByteSize() will always return pointer size based on abi. So for N64 (default for MIPS64) abi it will return pointer size as 8
labath: We can replace switch with ArchSpec::GetAddressByteSize().

[nitesh.jain]

We just require ABI information so that auxv vector is parse when lldb try to attach a process via "attach -p pid"

[nitesh.jain]

Hi Greg,

Please could you find some time to review this ?

Thanks

lldb/trunk/include/lldb/Core/ArchSpec.h

public:
   /// for tools like compiler, disassembler etc.
   ///
   /// @return A string representing target CPU for the current
   ///         architecture.
   //------------------------------------------------------------------
   std::string GetClangTargetCPU();
 
   //------------------------------------------------------------------
+  /// Return a string representing target application ABI.
+  ///
+  /// @return A string representing target application ABI.
+  //------------------------------------------------------------------
+  std::string GetTargetABI() const;
+
+  //------------------------------------------------------------------
   /// Clears the object state.
   ///
   /// Clears the object state back to a default invalid state.
   //------------------------------------------------------------------
   void Clear();
 
   //------------------------------------------------------------------
   /// Returns the size in bytes of an address of the current
public:
   ///         instructions
   //------------------------------------------------------------------
   bool IsAlwaysThumbInstructions() const;
 
   uint32_t GetFlags() const { return m_flags; }
 
   void SetFlags(uint32_t flags) { m_flags = flags; }
 
+  void SetFlags(std::string elf_abi);
+
 protected:
   bool IsEqualTo(const ArchSpec &rhs, bool exact_match) const;
 
   llvm::Triple m_triple;
   Core m_core = kCore_invalid;
   lldb::ByteOrder m_byte_order = lldb::eByteOrderInvalid;
 
   // Additional arch flags which we cannot get from triple and core

lldb/trunk/source/Core/ArchSpec.cpp

 bool ArchSpec::IsMIPS() const {
   const llvm::Triple::ArchType machine = GetMachine();
   if (machine == llvm::Triple::mips || machine == llvm::Triple::mipsel ||
       machine == llvm::Triple::mips64 || machine == llvm::Triple::mips64el)
     return true;
   return false;
 }
 
+std::string ArchSpec::GetTargetABI() const {
+
+  std::string abi;
+
+  if (IsMIPS()) {
+    switch (GetFlags() & ArchSpec::eMIPSABI_mask) {
+    case ArchSpec::eMIPSABI_N64:
+      abi = "n64";
+      return abi;
+    case ArchSpec::eMIPSABI_N32:
+      abi = "n32";
+      return abi;
+    case ArchSpec::eMIPSABI_O32:
+      abi = "o32";
+      return abi;
+    default:
+      return abi;
+    }
+  }
+  return abi;
+}
+
+void ArchSpec::SetFlags(std::string elf_abi) {
+
+  uint32_t flag = GetFlags();
+  if (IsMIPS()) {
+    if (elf_abi == "n64")
+      flag |= ArchSpec::eMIPSABI_N64;
+    else if (elf_abi == "n32")
+      flag |= ArchSpec::eMIPSABI_N32;
+    else if (elf_abi == "o32")
+      flag |= ArchSpec::eMIPSABI_O32;
+  }
+  SetFlags(flag);
+}
+
 std::string ArchSpec::GetClangTargetCPU() {
   std::string cpu;
   const llvm::Triple::ArchType machine = GetMachine();
 
   if (machine == llvm::Triple::mips || machine == llvm::Triple::mipsel ||
       machine == llvm::Triple::mips64 || machine == llvm::Triple::mips64el) {
     switch (m_core) {
     case ArchSpec::eCore_mips32:
if (core_def) {
     m_byte_order = core_def->default_byte_order;
   } else {
     Clear();
   }
 
   return IsValid();
 }
 
-bool lldb_private::ParseMachCPUDashSubtypeTriple(llvm::StringRef triple_str, ArchSpec &arch) {
+bool lldb_private::ParseMachCPUDashSubtypeTriple(llvm::StringRef triple_str,
+                                                 ArchSpec &arch) {
   // Accept "12-10" or "12.10" as cpu type/subtype
   if (triple_str.empty())
     return false;
 
   size_t pos = triple_str.find_first_of("-.");
   if (pos == llvm::StringRef::npos)
     return false;
 

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

if (response.IsNormalResponse()) {
       llvm::StringRef name;
       llvm::StringRef value;
       uint32_t cpu = LLDB_INVALID_CPUTYPE;
       uint32_t sub = 0;
       std::string arch_name;
       std::string os_name;
       std::string vendor_name;
       std::string triple;
+      std::string elf_abi;
       uint32_t pointer_byte_size = 0;
       StringExtractor extractor;
       ByteOrder byte_order = eByteOrderInvalid;
       uint32_t num_keys_decoded = 0;
       lldb::pid_t pid = LLDB_INVALID_PROCESS_ID;
       while (response.GetNameColonValue(name, value)) {
         if (name.equals("cputype")) {
           if (!value.getAsInteger(16, cpu))
if (response.IsNormalResponse()) {
           if (byte_order != eByteOrderInvalid)
             ++num_keys_decoded;
         } else if (name.equals("ptrsize")) {
           if (!value.getAsInteger(16, pointer_byte_size))
             ++num_keys_decoded;
         } else if (name.equals("pid")) {
           if (!value.getAsInteger(16, pid))
             ++num_keys_decoded;
+        } else if (name.equals("elf_abi")) {
+          elf_abi = value;
+          ++num_keys_decoded;
         }
       }
       if (num_keys_decoded > 0)
         m_qProcessInfo_is_valid = eLazyBoolYes;
       if (pid != LLDB_INVALID_PROCESS_ID) {
         m_curr_pid_is_valid = eLazyBoolYes;
         m_curr_pid = pid;
       }
 
       // Set the ArchSpec from the triple if we have it.
       if (!triple.empty()) {
         m_process_arch.SetTriple(triple.c_str());
+        m_process_arch.SetFlags(elf_abi);
         if (pointer_byte_size) {
           assert(pointer_byte_size == m_process_arch.GetAddressByteSize());
         }
       } else if (cpu != LLDB_INVALID_CPUTYPE && !os_name.empty() &&
                  !vendor_name.empty()) {
         llvm::Triple triple(llvm::Twine("-") + vendor_name + "-" + os_name);
 
         assert(triple.getObjectFormat() != llvm::Triple::UnknownObjectFormat);

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp

case lldb::eByteOrderBig:
       break;
     case lldb::eByteOrderPDP:
       response.PutCString("endian:pdp;");
       break;
     default:
       // Nothing.
       break;
     }
-
-    if (proc_triple.isArch64Bit())
-      response.PutCString("ptrsize:8;");
-    else if (proc_triple.isArch32Bit())
-      response.PutCString("ptrsize:4;");
-    else if (proc_triple.isArch16Bit())
-      response.PutCString("ptrsize:2;");
+    // In case of MIPS64, pointer size is depend on ELF ABI
+    // For N32 the pointer size is 4 and for N64 it is 8
+    std::string abi = proc_arch.GetTargetABI();
+    if (!abi.empty())
+      response.Printf("elf_abi:%s;", abi.c_str());
+    response.Printf("ptrsize:%d;", proc_arch.GetAddressByteSize());
   }
 }
 
 FileSpec GDBRemoteCommunicationServerCommon::FindModuleFile(
     const std::string &module_path, const ArchSpec &arch) {
 #ifdef __ANDROID__
   return HostInfoAndroid::ResolveLibraryPath(module_path, arch);
 #else