Add qModuleInfo request in order to get module information (uuid, triple,..) by module path from remote platform.

[ovyalov]

Summary

Taking into account comments on http://reviews.llvm.org/D7574, I decided to reconsider the solution - manual modules download from a target isn't reliable solution and may lead to some subtle errors.
The idea behind the alternative solution is to use module's UUID as unique identifier and download module from remote target if such module isn't presented in local cache.
This CL is first of a few steps - add new qModuleInfo request, so platform by given path can return some module context - UUID, triple, file size,...

The ultimate solution will include extending PlatformRemoteGDBServer::ResolveExecutable and GetSharedModule methods with such logic flow:

• For each given module get its UUID using qModuleInfo or fallback to content MD5 if module doesn't have UUID.
• Check local cache for module with such UUID. If module exists in cache - use it, otherwise download a module from a target and put into cache.
• Introduce new bool property to allow conditional modules' download.

Please let me know whether it sounds reasonable to you.

[vharron]

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

1131

Maybe return an error code if there is no UUID in the module? Or add timestamp or timestamp+date as UUID?

[clayborg]

We need to deal with files that contains more than 1 architecture slice and either pick the right one and return it, or return multiple matches. See inlined comments.

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

1126

The module_path can represent a fat file with multiple architectures (or it could be a .a file with many many .o files inside of it).

You actually want to use:

FileSpec module_spec(module_path.c_str(), true);
ModuleSpecList module_specs;
if (ObjectFile::GetModuleSpecifications(module_spec, 0, 0, module_specs))
{
}

and you probably want to return all values that it comes up with. (triple + UUID for each slice). If you want to limit it down, you will need to match the architecture against the current process' architecture and return just that one. This is of course only if "module_specs" has more than 1 module specs inside it.

If you choose to return multiple items, you will need to convert your answer over to using JSON like some of the packets Jason Molenda has added to debugserver. If you limit it down to 1 item, then you can just figure out which one you want from the list if there is more than one and just return that.

1131–1134

Check uuid_str to see if it is empty and if not, do the above "uuid:" code, else maybe get the MD5 checksum and return a "md5:XXXXXXXXX" value instead?

[ovyalov]

• Added md5:.. output field if module doesn't have UUID.
• qModuleInfo takes additional arch triple request param that should match module's architecture.

Please take another look.
Thank you.

[vharron]

Hi all,

In this instance, we are using this uuid to determine if the module is already cached on the host, right?

If a fat binary has multiple architectures in a single file and each arch has it's own uuid, wouldn't we just combine (add/xor/whatever) the uuids to get a single uuid for the entire file?

[ovyalov]

Hi Vince,

In D7709#127748, @vharron wrote:

Hi all,

In this instance, we are using this uuid to determine if the module is already cached on the host, right?

yes, the plan to have local cache on host side with UUID used a key.

If a fat binary has multiple architectures in a single file and each arch has it's own uuid, wouldn't we just combine (add/xor/whatever) the uuids to get a single uuid for the entire file?

My understanding that in case of a fat binary with multiple architectures there is still a single UUID that represents a whole module's binary file, i.e. UUID shouldn't be tied to an architecture.

[ovyalov]

• Added file_offset, file_size response fields for qModuleInfo request
• Made MD5 computation functions to take offset, length parameters to get MD% from a file's slice.

Please take another look.
Thank you.

[clayborg]

A few changes only for the MD5 computation calls: don't require people to pass magic values of 0 for offset and 0 for length when they want the whole file. See inlined comments.

source/Host/common/FileSystem.cpp

57–68

Can we make one version where offset and length are supplied and one that doesn't require it? I hate to have magic values like 0 and 0 for offset and length unless we can make them default argument values.

81–91

Can we make one version where offset and length are supplied and one that doesn't require it? I hate to have magic values like 0 and 0 for offset and length unless we can make them default argument values.

source/Plugins/Platform/MacOSX/PlatformDarwin.cpp

395 ↗

(On Diff #20525)

Call the version that doesn't require file offset and size.

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

828

Call the version that doesn't require file offset and size.

source/Target/Platform.cpp

1470 ↗

(On Diff #20525)

Call the version that doesn't require file offset and size.

[ovyalov]

Overloaded CalculateMD5 and CalculateMD5AsString methods in order to have versions with and without offset/length.

Thank you.

[ovyalov]

Please let me know whether this CL looks good to go.

[clayborg]

Looks good.

[ovyalov]

AFFECTED FILES

/lldb/trunk/docs/lldb-gdb-remote.txt
/lldb/trunk/include/lldb/Host/FileSystem.h
/lldb/trunk/source/Host/common/FileSystem.cpp
/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.h
/lldb/trunk/source/Utility/StringExtractorGDBRemote.cpp
/lldb/trunk/source/Utility/StringExtractorGDBRemote.h

USERS

ovyalov (Author)

http://reviews.llvm.org/rL230556

docs/lldb-gdb-remote.txt

 //
 // PRIORITY TO IMPLEMENT
 //  Low, this is mostly a convenience packet to avoid having to send all
 //  registers via a g packet. It should only be implemented if support
 //  for the QSaveRegisterState is added.
 //----------------------------------------------------------------------
 
 //----------------------------------------------------------------------
+// qModuleInfo:<module_path>;<arch triple>
+//
+// BRIEF
+//  Get information for a module by given module path and architecture.
+//
+// RESPONSE
+//  "(uuid|md5):...;triple:...;file_offset:...;file_size...;"
+//  "EXX" - for any errors
+//
+// PRIORITY TO IMPLEMENT
+//  Optional, required if dynamic loader cannot fetch module's information like
+//  UUID directly from inferior's memory.
+//----------------------------------------------------------------------
+
+//----------------------------------------------------------------------
 // Stop reply packet extensions
 //
 // BRIEF
 //  This section describes some of the additional information you can
 //  specify in stop reply packets that help LLDB to know more detailed
 //  information about your threads.
 //
 // DESCRIPTION

include/lldb/Host/FileSystem.h

public:
     static Error SetFilePermissions(const char *path, uint32_t file_permissions);
     static lldb::user_id_t GetFileSize(const FileSpec &file_spec);
     static bool GetFileExists(const FileSpec &file_spec);
 
     static Error Symlink(const char *src, const char *dst);
     static Error Readlink(const char *path, char *buf, size_t buf_len);
     static Error Unlink(const char *path);
 
+
     static bool CalculateMD5(const FileSpec &file_spec, uint64_t &low, uint64_t &high);
+    static bool CalculateMD5(const FileSpec &file_spec,
+                             uint64_t offset,
+                             uint64_t length,
+                             uint64_t &low,
+                             uint64_t &high);
+
     static bool CalculateMD5AsString(const FileSpec &file_spec, std::string& digest_str);
+    static bool CalculateMD5AsString(const FileSpec &file_spec,
+                                     uint64_t offset,
+                                     uint64_t length,
+                                     std::string& digest_str);
 };
 }
 
 #endif

source/Host/common/FileSystem.cpp

 #include <vector>
 
 using namespace lldb;
 using namespace lldb_private;
 
 namespace {
 
 bool
-CalcMD5(const FileSpec &file_spec, llvm::MD5::MD5Result &md5_result)
+CalcMD5(const FileSpec &file_spec, uint64_t offset, uint64_t length, llvm::MD5::MD5Result &md5_result)
 {
     llvm::MD5 md5_hash;
     std::ifstream file(file_spec.GetPath(), std::ios::binary);
     if (!file.is_open())
         return false;
 
+    if (offset > 0)
+        file.seekg(offset, file.beg);
+
     std::vector<char> read_buf(4096);
+    uint64_t total_read_bytes = 0;
     while (!file.eof())
     {
-        file.read(&read_buf[0], read_buf.size());
+        const uint64_t to_read = (length > 0) ? std::min(read_buf.size(), length - total_read_bytes) : read_buf.size();
+        if (to_read == 0)
+            break;
+
+        file.read(&read_buf[0], to_read);
         const auto read_bytes = file.gcount();
         if (read_bytes == 0)
             break;
 
         md5_hash.update(llvm::StringRef(&read_buf[0], read_bytes));
+        total_read_bytes += read_bytes;
     }
 
     md5_hash.final(md5_result);
     return true;
 }
 
 }  // namespace
 
 bool
 FileSystem::CalculateMD5(const FileSpec &file_spec, uint64_t &low, uint64_t &high)
 {
+    return CalculateMD5(file_spec, 0, 0, low, high);
+}
+
+bool
+FileSystem::CalculateMD5(const FileSpec &file_spec,
+                         uint64_t offset,
+                         uint64_t length,
+                         uint64_t &low,
+                         uint64_t &high)
+{
     llvm::MD5::MD5Result md5_result;
-    if (!CalcMD5(file_spec, md5_result))
+    if (!CalcMD5(file_spec, offset, length, md5_result))
         return false;
 
     const auto uint64_res = reinterpret_cast<const uint64_t*>(md5_result);
     high = uint64_res[0];
     low = uint64_res[1];
 
     return true;
 }
 
 bool
 FileSystem::CalculateMD5AsString(const FileSpec &file_spec, std::string& digest_str)
 {
+    return CalculateMD5AsString(file_spec, 0, 0, digest_str);
+}
+
+bool
+FileSystem::CalculateMD5AsString(const FileSpec &file_spec,
+                                 uint64_t offset,
+                                 uint64_t length,
+                                 std::string& digest_str)
+{
     llvm::MD5::MD5Result md5_result;
-    if (!CalcMD5(file_spec, md5_result))
+    if (!CalcMD5(file_spec, offset, length, md5_result))
         return false;
 
     llvm::SmallString<32> result_str;
     llvm::MD5::stringifyResult(md5_result, result_str);
     digest_str = result_str.c_str();
     return true;
 }

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

public:
     GetGDBServerProgramVersion();
 
     bool
     AvoidGPackets(ProcessGDBRemote *process);
 
     bool
     GetThreadExtendedInfoSupported();
 
+    bool
+    GetModuleInfo (const char* module_path,
+                   const lldb_private::ArchSpec& arch_spec,
+                   StringExtractorGDBRemote &response);
+
 protected:
 
     PacketResult
     SendPacketAndWaitForResponseNoLock (const char *payload,
                                         size_t payload_length,
                                         StringExtractorGDBRemote &response);
 
     bool

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

if (GetSequenceMutex (locker, "Didn't get sequence mutex for QRestoreRegisterState."))
                     // QSaveRegisterState again...
                     m_supports_QSaveRegisterState = eLazyBoolNo;
                 }
             }
         }
     }
     return false;
 }
+
+bool
+GDBRemoteCommunicationClient::GetModuleInfo (const char* module_path,
+                                             const lldb_private::ArchSpec& arch_spec,
+                                             StringExtractorGDBRemote &response)
+{
+    if (!(module_path && module_path[0]))
+        return false;
+
+    StreamString packet;
+    packet.PutCString("qModuleInfo:");
+    packet.PutBytesAsRawHex8(module_path, strlen(module_path));
+    packet.PutCString(";");
+    const auto& tripple = arch_spec.GetTriple().getTriple();
+    packet.PutBytesAsRawHex8(tripple.c_str(), tripple.size());
+
+    return SendPacketAndWaitForResponse (packet.GetData(), packet.GetSize(), response, false) == PacketResult::Success;
+}

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

protected:
 
     PacketResult
     Handle_vFile_Stat (StringExtractorGDBRemote &packet);
 
     PacketResult
     Handle_vFile_MD5 (StringExtractorGDBRemote &packet);
 
     PacketResult
+    Handle_qModuleInfo (StringExtractorGDBRemote &packet);
+
+    PacketResult
     Handle_qPlatform_shell (StringExtractorGDBRemote &packet);
 
     PacketResult
     Handle_qPlatform_mkdir (StringExtractorGDBRemote &packet);
 
     PacketResult
     Handle_qPlatform_chmod (StringExtractorGDBRemote &packet);
 

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

 // C Includes
 // C++ Includes
 #include <cstring>
 #include <chrono>
 
 // Other libraries and framework includes
 #include "llvm/ADT/Triple.h"
 #include "lldb/Core/Log.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/StreamGDBRemote.h"
 #include "lldb/Core/StreamString.h"
 #include "lldb/Host/Config.h"
 #include "lldb/Host/Endian.h"
 #include "lldb/Host/File.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/StringConvert.h"
 #include "lldb/Interpreter/Args.h"
+#include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Target/FileAction.h"
 #include "lldb/Target/Platform.h"
 #include "lldb/Target/Process.h"
 
 // Project includes
 #include "ProcessGDBRemoteLog.h"
 #include "Utility/StringExtractorGDBRemote.h"
 
GDBRemoteCommunicationServerCommon::GDBRemoteCommunicationServerCommon(const char *comm_name, const char *listener_name) :
     RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qKillSpawnedProcess,
                                   &GDBRemoteCommunicationServerCommon::Handle_qKillSpawnedProcess);
     RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_QLaunchArch,
                                   &GDBRemoteCommunicationServerCommon::Handle_QLaunchArch);
     RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qLaunchSuccess,
                                   &GDBRemoteCommunicationServerCommon::Handle_qLaunchSuccess);
     RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_QListThreadsInStopReply,
                                   &GDBRemoteCommunicationServerCommon::Handle_QListThreadsInStopReply);
+    RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qModuleInfo,
+                                  &GDBRemoteCommunicationServerCommon::Handle_qModuleInfo);
     RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qPlatform_chmod,
                                   &GDBRemoteCommunicationServerCommon::Handle_qPlatform_chmod);
     RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qPlatform_mkdir,
                                   &GDBRemoteCommunicationServerCommon::Handle_qPlatform_mkdir);
     RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qPlatform_shell,
                                   &GDBRemoteCommunicationServerCommon::Handle_qPlatform_shell);
     RegisterMemberFunctionHandler(StringExtractorGDBRemote::eServerPacketType_qProcessInfoPID,
                                   &GDBRemoteCommunicationServerCommon::Handle_qProcessInfoPID);

 {
     packet.SetFilePos(::strlen("vFile:MD5:"));
     std::string path;
     packet.GetHexByteString(path);
     if (!path.empty())
     {
         uint64_t a,b;
         StreamGDBRemote response;
-        if (FileSystem::CalculateMD5(FileSpec(path.c_str(), false), a, b) == false)
+        if (!FileSystem::CalculateMD5(FileSpec(path.c_str(), false), a, b))
         {
             response.PutCString("F,");
             response.PutCString("x");
         }
         else
         {
             response.PutCString("F,");
             response.PutHex64(a);
if (success)
                         __FUNCTION__,
                         m_process_launch_error.AsCString());
 
         }
     }
     return SendErrorResponse (8);
 }
 
+GDBRemoteCommunication::PacketResult
+GDBRemoteCommunicationServerCommon::Handle_qModuleInfo (StringExtractorGDBRemote &packet)
+{
+    packet.SetFilePos(::strlen ("qModuleInfo:"));
+
+    std::string module_path;
+    packet.GetHexByteStringTerminatedBy(module_path, ';');
+    if (module_path.empty())
+        return SendErrorResponse (1);
+    const FileSpec module_path_spec(module_path.c_str(), true);
+
+    if (packet.GetChar() != ';')
+        return SendErrorResponse (2);
+
+    std::string triple;
+    packet.GetHexByteString(triple);
+    const ModuleSpec module_spec(module_path_spec, ArchSpec(triple.c_str()));
+
+    ModuleSpecList module_specs;
+    if (!ObjectFile::GetModuleSpecifications(module_path_spec, 0, 0, module_specs))
+        return SendErrorResponse (3);
+
+    ModuleSpec matched_module_spec;
+    if (!module_specs.FindMatchingModuleSpec(module_spec, matched_module_spec))
+        return SendErrorResponse (4);
+
+    const ModuleSP module(new Module(matched_module_spec));
+
+    const auto obj_file(module->GetObjectFile());
+    const auto file_offset = obj_file->GetFileOffset();
+    const auto file_size = obj_file->GetByteSize();
+
+    StreamGDBRemote response;
+
+    const auto uuid_str = module->GetUUID().GetAsString();
+    if (uuid_str.empty())
+    {
+        std::string md5_hash;
+        if (!FileSystem::CalculateMD5AsString(module_path_spec, file_offset, file_size, md5_hash))
+            return SendErrorResponse (5);
+        response.PutCString ("md5:");
+        response.PutCStringAsRawHex8(md5_hash.c_str());
+    }
+    else{
+        response.PutCString ("uuid:");
+        response.PutCStringAsRawHex8(uuid_str.c_str());
+    }
+    response.PutChar(';');
+
+    const auto &module_arch = matched_module_spec.GetArchitecture();
+    response.PutCString("triple:");
+    response.PutCStringAsRawHex8( module_arch.GetTriple().getTriple().c_str());
+    response.PutChar(';');
+
+    response.PutCString("file_offset:");
+    response.PutHex64(file_offset);
+    response.PutChar(';');
+    response.PutCString("file_size:");
+    response.PutHex64(file_size);
+    response.PutChar(';');
+
+    return SendPacketNoLock(response.GetData(), response.GetSize());
+}
+
 void
 GDBRemoteCommunicationServerCommon::CreateProcessInfoResponse (const ProcessInstanceInfo &proc_info,
                                                     StreamString &response)
 {
     response.Printf ("pid:%" PRIu64 ";ppid:%" PRIu64 ";uid:%i;gid:%i;euid:%i;egid:%i;",
                      proc_info.GetProcessID(),
                      proc_info.GetParentProcessID(),
                      proc_info.GetUserID(),

source/Utility/StringExtractorGDBRemote.h

enum ServerPacketType
         eServerPacketType_qfProcessInfo,
         eServerPacketType_qsProcessInfo,
         eServerPacketType_qC,
         eServerPacketType_qGroupName,
         eServerPacketType_qHostInfo,
         eServerPacketType_qLaunchGDBServer,
         eServerPacketType_qKillSpawnedProcess,
         eServerPacketType_qLaunchSuccess,
+        eServerPacketType_qModuleInfo,
         eServerPacketType_qProcessInfoPID,
         eServerPacketType_qSpeedTest,
         eServerPacketType_qUserName,
         eServerPacketType_qGetWorkingDir,
         eServerPacketType_QEnvironment,
         eServerPacketType_QLaunchArch,
         eServerPacketType_QSetDisableASLR,
         eServerPacketType_QSetDetachOnError,

source/Utility/StringExtractorGDBRemote.cpp

case 'q':
         case 'L':
             if (PACKET_STARTS_WITH ("qLaunchGDBServer"))        return eServerPacketType_qLaunchGDBServer;
             if (PACKET_MATCHES ("qLaunchSuccess"))              return eServerPacketType_qLaunchSuccess;
             break;
             
         case 'M':
             if (PACKET_STARTS_WITH ("qMemoryRegionInfo:"))      return eServerPacketType_qMemoryRegionInfo;
             if (PACKET_MATCHES ("qMemoryRegionInfo"))           return eServerPacketType_qMemoryRegionInfoSupported;
+            if (PACKET_STARTS_WITH ("qModuleInfo:"))             return eServerPacketType_qModuleInfo;
             break;
 
         case 'P':
             if (PACKET_STARTS_WITH ("qProcessInfoPID:"))        return eServerPacketType_qProcessInfoPID;
             if (PACKET_STARTS_WITH ("qPlatform_shell:"))        return eServerPacketType_qPlatform_shell;
             if (PACKET_STARTS_WITH ("qPlatform_mkdir:"))        return eServerPacketType_qPlatform_mkdir;
             if (PACKET_STARTS_WITH ("qPlatform_chmod:"))        return eServerPacketType_qPlatform_chmod;
             if (PACKET_MATCHES ("qProcessInfo"))                return eServerPacketType_qProcessInfo;