Fix ModuleCache usage in Platform - ask remote platform for module's ModuleSpec beforehand so we can look for a module by UUID locally without need to download it.

[ovyalov]

Summary

Previous implementation caused TestGlobalVariables to fail when running remote Linux<->Linux test - test creates a module for shared library in registerSharedLibrariesWithTarget.
The test was failing because a copy of shared library was downloaded into module cache from remote target instead of using existing local module instance - with this patch in we can lookup a module in ModuleList by known UUID and switch to already registered module instance if such exists.

[tberghammer]

Looks good, but please be careful when rebaseing it to TOT because there will be a lot of conflicts.

BTW it also fixes an issue related to TestInferiorChanged.

[clayborg]

lgtm

[ovyalov]

AFFECTED FILES

/lldb/trunk/include/lldb/Target/Platform.h
/lldb/trunk/source/Target/Platform.cpp
/lldb/trunk/source/Utility/ModuleCache.cpp
/lldb/trunk/source/Utility/ModuleCache.h

USERS

ovyalov (Author)

http://reviews.llvm.org/rL233136

include/lldb/Target/Platform.h

 //===-- Platform.h ----------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef liblldb_Platform_h_
 #define liblldb_Platform_h_
 
 // C Includes
 // C++ Includes
+#include <functional>
 #include <map>
 #include <memory>
 #include <string>
 #include <vector>
 
 // Other libraries and framework includes
 // Project includes
 #include "lldb/lldb-private-forward.h"
protected:
             m_gid_map.clear();
         }
 
         Error
         GetCachedExecutable (ModuleSpec &module_spec,
                              lldb::ModuleSP &module_sp,
                              const FileSpecList *module_search_paths_ptr,
                              Platform &remote_platform);
+    private:
+        typedef std::function<Error (const ModuleSpec &)> ModuleResolver;
 
-        bool
-        GetCachedSharedModule (const ModuleSpec &module_spec,
-                               lldb::ModuleSP &module_sp);
+        Error
+        GetRemoteSharedModule (const ModuleSpec &module_spec,
+                               lldb::ModuleSP &module_sp,
+                               const ModuleResolver &module_resolver,
+                               bool *did_create_ptr);
 
         Error
         DownloadModuleSlice (const FileSpec& src_file_spec,
                              const uint64_t src_offset,
                              const uint64_t src_size,
                              const FileSpec& dst_file_spec);
 
         bool
-        GetModuleFromLocalCache (const ModuleSpec& module_spec,
-                                 lldb::ModuleSP &module_sp);
+        GetCachedSharedModule (const ModuleSpec& module_spec,
+                               lldb::ModuleSP &module_sp,
+                               bool *did_create_ptr);
 
-        FileSpec GetModuleCacheRoot ();
+        FileSpec
+        GetModuleCacheRoot ();
 
-    private:
         Error
         LoadCachedExecutable (const ModuleSpec &module_spec,
                               lldb::ModuleSP &module_sp,
                               const FileSpecList *module_search_paths_ptr,
                               Platform &remote_platform);
 
         DISALLOW_COPY_AND_ASSIGN (Platform);
     };

source/Target/Platform.cpp

 
 Error
 Platform::GetSharedModule (const ModuleSpec &module_spec,
                            ModuleSP &module_sp,
                            const FileSpecList *module_search_paths_ptr,
                            ModuleSP *old_module_sp_ptr,
                            bool *did_create_ptr)
 {
-    if (!IsHost () && GetGlobalPlatformProperties ()->GetUseModuleCache ())
-    {
-        // Use caching only when talking to a remote platform.
-        if (GetCachedSharedModule (module_spec, module_sp))
-        {
-            if (did_create_ptr)
-                *did_create_ptr = true;
-
-            return Error ();
-        }
-    }
-    return ModuleList::GetSharedModule (module_spec, 
+    if (IsHost ())
+        return ModuleList::GetSharedModule (module_spec,
                                             module_sp,
                                             module_search_paths_ptr,
                                             old_module_sp_ptr,
                                             did_create_ptr,
                                             false);
+
+    return GetRemoteSharedModule (module_spec,
+                                  module_sp,
+                                  [&](const ModuleSpec &spec)
+                                  {
+                                      return ModuleList::GetSharedModule (
+                                          spec, module_sp, module_search_paths_ptr, old_module_sp_ptr, did_create_ptr, false);
+                                  },
+                                  did_create_ptr);
 }
 
 bool
 Platform::GetModuleSpec (const FileSpec& module_file_spec,
                          const ArchSpec& arch,
                          ModuleSpec &module_spec)
 {
     ModuleSpecList module_specs;

 }
 
 Error
 Platform::LoadCachedExecutable (const ModuleSpec &module_spec,
                                 lldb::ModuleSP &module_sp,
                                 const FileSpecList *module_search_paths_ptr,
                                 Platform &remote_platform)
 {
-    if (GetGlobalPlatformProperties ()->GetUseModuleCache ())
+    return GetRemoteSharedModule (module_spec,
+                                  module_sp,
+                                  [&](const ModuleSpec &spec)
+                                  {
+                                      return remote_platform.ResolveExecutable (
+                                          spec, module_sp, module_search_paths_ptr);
+                                  },
+                                  nullptr);
+}
+
+Error
+Platform::GetRemoteSharedModule (const ModuleSpec &module_spec,
+                                 lldb::ModuleSP &module_sp,
+                                 const ModuleResolver &module_resolver,
+                                 bool *did_create_ptr)
+{
+    // Get module information from a target.
+    ModuleSpec resolved_module_spec;
+    if (!GetModuleSpec (module_spec.GetFileSpec (), module_spec.GetArchitecture (), resolved_module_spec))
+        return module_resolver (module_spec);
+
+    // Trying to find a module by UUID on local file system.
+    const auto error = module_resolver (resolved_module_spec);
+    if (error.Fail ())
     {
-        if (GetCachedSharedModule (module_spec, module_sp))
+        if (GetCachedSharedModule (resolved_module_spec, module_sp, did_create_ptr))
             return Error ();
     }
 
-    return remote_platform.ResolveExecutable (module_spec,
-                                              module_sp,
-                                              module_search_paths_ptr);
+    return error;
 }
 
 bool
-Platform::GetCachedSharedModule (const ModuleSpec &module_spec,
-                                 lldb::ModuleSP &module_sp)
+Platform::GetCachedSharedModule (const ModuleSpec& module_spec,
+                                 lldb::ModuleSP &module_sp,
+                                 bool *did_create_ptr)
 {
-    return (m_module_cache && GetModuleFromLocalCache (module_spec, module_sp));
-}
+    if (IsHost() ||
+        !GetGlobalPlatformProperties ()->GetUseModuleCache ())
+        return false;
 
-bool
-Platform::GetModuleFromLocalCache (const ModuleSpec& module_spec,
-                                   lldb::ModuleSP &module_sp)
-{
     Log *log = GetLogIfAnyCategoriesSet (LIBLLDB_LOG_PLATFORM);
 
-    // Get module information from a target.
-    ModuleSpec resolved_module_spec;
-    if (!GetModuleSpec (module_spec.GetFileSpec (), module_spec.GetArchitecture (), resolved_module_spec))
-        return false;
-
     // Check local cache for a module.
     auto error = m_module_cache->Get (GetModuleCacheRoot (),
                                       GetHostname (),
-                                      resolved_module_spec,
-                                      module_sp);
+                                      module_spec,
+                                      module_sp,
+                                      did_create_ptr);
     if (error.Success ())
         return true;
 
     if (log)
         log->Printf("Platform::%s - module %s not found in local cache: %s",
-                    __FUNCTION__, resolved_module_spec.GetUUID ().GetAsString ().c_str (), error.AsCString ());
+                    __FUNCTION__, module_spec.GetUUID ().GetAsString ().c_str (), error.AsCString ());
 
     // Get temporary file name for a downloaded module.
     llvm::SmallString<PATH_MAX> tmp_download_file_path;
     const auto err_code = llvm::sys::fs::createTemporaryFile (
-        "lldb", resolved_module_spec.GetUUID ().GetAsString ().c_str (), tmp_download_file_path);
+        "lldb", module_spec.GetUUID ().GetAsString ().c_str (), tmp_download_file_path);
     if (err_code)
     {
         if (log)
             log->Printf ("Platform::%s - failed to create unique file: %s",
                          __FUNCTION__, err_code.message ().c_str ());
         return false;
     }
 
     llvm::FileRemover tmp_file_remover (tmp_download_file_path.c_str ());
 
     const FileSpec tmp_download_file_spec (tmp_download_file_path.c_str (), true);
     // Download a module file.
-    error = DownloadModuleSlice (resolved_module_spec.GetFileSpec (),
-                                 resolved_module_spec.GetObjectOffset (),
-                                 resolved_module_spec.GetObjectSize (),
+    error = DownloadModuleSlice (module_spec.GetFileSpec (),
+                                 module_spec.GetObjectOffset (),
+                                 module_spec.GetObjectSize (),
                                  tmp_download_file_spec);
     if (error.Fail ())
     {
         if (log)
             log->Printf("Platform::%s - failed to download %s to %s: %s",
                         __FUNCTION__, module_spec.GetFileSpec ().GetPath ().c_str (),
                         tmp_download_file_path.c_str (), error.AsCString ());
         return false;
     }
 
     // Put downloaded file into local module cache.
     error = m_module_cache->Put (GetModuleCacheRoot (),
                                  GetHostname (),
-                                 resolved_module_spec,
+                                 module_spec,
                                  tmp_download_file_spec);
     if (error.Fail ())
     {
         if (log)
             log->Printf("Platform::%s - failed to put module %s into cache: %s",
-                        __FUNCTION__, resolved_module_spec.GetUUID ().GetAsString ().c_str (),
+                        __FUNCTION__, module_spec.GetUUID ().GetAsString ().c_str (),
                         error.AsCString ());
         return false;
     }
 
     error = m_module_cache->Get (GetModuleCacheRoot (),
                                  GetHostname (),
-                                 resolved_module_spec,
-                                 module_sp);
+                                 module_spec,
+                                 module_sp,
+                                 did_create_ptr);
     return error.Success ();
 }
 
 Error
 Platform::DownloadModuleSlice (const FileSpec& src_file_spec,
                                const uint64_t src_offset,
                                const uint64_t src_size,
                                const FileSpec& dst_file_spec)

source/Utility/ModuleCache.h

Put (const FileSpec &root_dir_spec,
          const char *hostname,
          const ModuleSpec &module_spec,
          const FileSpec &tmp_file);
 
     Error
     Get (const FileSpec &root_dir_spec,
          const char *hostname,
          const ModuleSpec &module_spec,
-         lldb::ModuleSP &cached_module_sp);
+         lldb::ModuleSP &cached_module_sp,
+         bool *did_create_ptr);
 
 private:
     static FileSpec
     GetModuleDirectory (const FileSpec &root_dir_spec, const UUID &uuid);
 
     static FileSpec
     GetHostSysRootModulePath (const FileSpec &root_dir_spec, const char *hostname, const FileSpec &platform_module_spec);
 

source/Utility/ModuleCache.cpp

ModuleCache::Put (const FileSpec &root_dir_spec,
     const auto sysroot_module_path_spec = GetHostSysRootModulePath (root_dir_spec, hostname, module_spec.GetFileSpec ());
     return CreateHostSysRootModuleSymLink (sysroot_module_path_spec, module_file_path);
 }
 
 Error
 ModuleCache::Get (const FileSpec &root_dir_spec,
                   const char *hostname,
                   const ModuleSpec &module_spec,
-                  ModuleSP &cached_module_sp)
+                  ModuleSP &cached_module_sp,
+                  bool *did_create_ptr)
 {
     const auto find_it = m_loaded_modules.find (module_spec.GetUUID ().GetAsString());
     if (find_it != m_loaded_modules.end ())
     {
         cached_module_sp = (*find_it).second.lock ();
         if (cached_module_sp)
             return Error ();
         m_loaded_modules.erase (find_it);
ModuleCache::Get (const FileSpec &root_dir_spec,
     if (!sysroot_module_path_spec.Exists ())
         CreateHostSysRootModuleSymLink (sysroot_module_path_spec, module_spec.GetFileSpec ());
 
     auto cached_module_spec (module_spec);
     cached_module_spec.GetUUID ().Clear ();  // Clear UUID since it may contain md5 content hash instead of real UUID.
     cached_module_spec.GetFileSpec () = module_file_path;
     cached_module_spec.GetPlatformFileSpec () = module_spec.GetFileSpec ();
     cached_module_sp.reset (new Module (cached_module_spec));
+    if (did_create_ptr)
+        *did_create_ptr = true;
 
     m_loaded_modules.insert (std::make_pair (module_spec.GetUUID ().GetAsString (), cached_module_sp));
 
     return Error ();
 }
 
 FileSpec
 ModuleCache::GetModuleDirectory (const FileSpec &root_dir_spec, const UUID &uuid)