Download symbol file for .oat files on android

[tberghammer]

Summary

Download symbol file for .oat files on android

On android .oat files (compiled java code) don't have symbol
information but on SDK 23+ it can be generated by the oatdump tool
(based on the dex information).

This CL adds logic to download this information and store it in the
module cache.

[ovyalov]

Please see my comments.

source/Plugins/Platform/Android/PlatformAndroid.cpp

306 ↗

(On Diff #31791)

indentation.

325 ↗

(On Diff #31791)

Please add constant for timeout.

329 ↗

(On Diff #31791)

Please check that tmpdir isn't empty.

331 ↗

(On Diff #31791)

s/directroy/directory

337 ↗

(On Diff #31791)

Please log an error if rm fails.

342 ↗

(On Diff #31791)

Can we have a bigger timeout? 1 sec might not be enough.

352 ↗

(On Diff #31791)

GetCString(false)?

353 ↗

(On Diff #31791)

ditto - please verify that it works as expected from Windows.

source/Utility/ModuleCache.cpp

137 ↗

(On Diff #31791)

Please add ".sym" constant

197 ↗

(On Diff #31791)

s/modul/module

207 ↗

(On Diff #31791)

Do we really want to have a link for sym file in sys root?
I'd rather moved code for sym file download from GetAndPut into Put method and skip link creation for symbol file.

[tberghammer]

Address comments

[tberghammer]

See responds inline

source/Plugins/Platform/Android/PlatformAndroid.cpp

325 ↗

(On Diff #31791)

Added a comment next to the value instead. I prefer that style but I can use a constant if you want.

342 ↗

(On Diff #31791)

I think 1 second should be enough in almost every case but increase it to 5 seconds just in case the device is very busy.

353 ↗

(On Diff #31791)

At the moment I don't have Windows environment but I will try to find a way to test it.

source/Utility/ModuleCache.cpp

207 ↗

(On Diff #31791)

At the moment I think we don't use the sys root at all, but I might be wrong. If anywhere we use the sys root then we will need the symbol file next to the actual file to find it. The reason I put the file download here because we need a Module object to do it (find out if we have symtab) what means we have to do it after the Get. I think moving it to Put will make the implementation more complicated.

[ovyalov]

LGTM

source/Utility/ModuleCache.cpp

207 ↗

(On Diff #31826)

s/.sym/kSymFileExtension
Or maybe introduce a function that returns symbol file spec by given module file spec in order to re-use it here and in ModuleCache::Get

lldb/trunk/include/lldb/Target/Platform.h

protected:
                              const FileSpecList *module_search_paths_ptr,
                              Platform &remote_platform);
 
         virtual Error
         DownloadModuleSlice (const FileSpec& src_file_spec,
                              const uint64_t src_offset,
                              const uint64_t src_size,
                              const FileSpec& dst_file_spec);
         
+        virtual Error
+        DownloadSymbolFile (const lldb::ModuleSP& module_sp,
+                            const FileSpec& dst_file_spec);
+
         virtual const char *
         GetCacheHostname ();
 
     private:
         typedef std::function<Error (const ModuleSpec &)> ModuleResolver;
 
         Error
         GetRemoteSharedModule (const ModuleSpec &module_spec,

lldb/trunk/source/Plugins/Platform/Android/PlatformAndroid.h

protected:
         GetCacheHostname () override;
 
         Error
         DownloadModuleSlice (const FileSpec &src_file_spec,
                              const uint64_t src_offset,
                              const uint64_t src_size,
                              const FileSpec &dst_file_spec) override;
 
+        Error
+        DownloadSymbolFile (const lldb::ModuleSP& module_sp,
+                            const FileSpec& dst_file_spec) override;
+
     private:
         std::string m_device_id;
         uint32_t m_sdk_version;
 
         DISALLOW_COPY_AND_ASSIGN (PlatformAndroid);
     };
 
 } // namespace platofor_android
 } // namespace lldb_private
 
 #endif  // liblldb_PlatformAndroid_h_

lldb/trunk/source/Plugins/Platform/Android/PlatformAndroid.cpp

 //===-- PlatformAndroid.cpp -------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 // C Includes
 // C++ Includes
 // Other libraries and framework includes
 #include "lldb/Core/Log.h"
+#include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Section.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Host/StringConvert.h"
 #include "Utility/UriParser.h"
 
 // Project includes
 #include "AdbClient.h"
 #include "PlatformAndroid.h"
 #include "PlatformAndroidRemoteGDBServer.h"
if (error.Fail() || status != 0 || version_string.empty())
                         status, error.AsCString(), version_string.c_str());
         return 0;
     }
     version_string.erase(version_string.size() - 1); // Remove trailing new line
 
     m_sdk_version = StringConvert::ToUInt32(version_string.c_str());
     return m_sdk_version;
 }
+
+Error
+PlatformAndroid::DownloadSymbolFile (const lldb::ModuleSP& module_sp,
+                                     const FileSpec& dst_file_spec)
+{
+    // For oat file we can try to fetch additional debug info from the device
+    if (module_sp->GetFileSpec().GetFileNameExtension() != ConstString("oat"))
+        return Error("Symbol file downloading only supported for oat files");
+
+    // If we have no information about the platform file we can't execute oatdump
+    if (!module_sp->GetPlatformFileSpec())
+        return Error("No platform file specified");
+
+    // Symbolizer isn't available before SDK version 23
+    if (GetSdkVersion() < 23)
+        return Error("Symbol file generation only supported on SDK 23+");
+
+    // If we already have symtab then we don't have to try and generate one
+    if (module_sp->GetSectionList()->FindSectionByName(ConstString(".symtab")) != nullptr)
+        return Error("Symtab already available in the module");
+
+    int status = 0;
+    std::string tmpdir;
+    StreamString command;
+    command.Printf("mktemp --directory --tmpdir %s", GetWorkingDirectory().GetCString());
+    Error error = RunShellCommand(command.GetData(),
+                                  GetWorkingDirectory(),
+                                  &status,
+                                  nullptr,
+                                  &tmpdir,
+                                  5 /* timeout (s) */);
+
+    if (error.Fail() || status != 0 || tmpdir.empty())
+        return Error("Failed to generate temporary directory on the device (%s)", error.AsCString());
+    tmpdir.erase(tmpdir.size() - 1); // Remove trailing new line
+
+    // Create file remover for the temporary directory created on the device
+    std::unique_ptr<std::string, std::function<void(std::string*)>> tmpdir_remover(
+        &tmpdir,
+        [this](std::string* s) {
+            StreamString command;
+            command.Printf("rm -rf %s", s->c_str());
+            Error error = this->RunShellCommand(command.GetData(),
+                                                GetWorkingDirectory(),
+                                                nullptr,
+                                                nullptr,
+                                                nullptr,
+                                                5 /* timeout (s) */);
+
+            Log *log(GetLogIfAllCategoriesSet (LIBLLDB_LOG_PLATFORM));
+            if (error.Fail())
+                log->Printf("Failed to remove temp directory: %s", error.AsCString());
+        }
+    );
+
+    FileSpec symfile_platform_filespec(tmpdir.c_str(), false);
+    symfile_platform_filespec.AppendPathComponent("symbolized.oat");
+
+    // Execute oatdump on the remote device to generate a file with symtab
+    command.Clear();
+    command.Printf("oatdump --symbolize=%s --output=%s",
+                   module_sp->GetPlatformFileSpec().GetCString(false),
+                   symfile_platform_filespec.GetCString(false));
+    error = RunShellCommand(command.GetData(),
+                            GetWorkingDirectory(),
+                            &status,
+                            nullptr,
+                            nullptr,
+                            60 /* timeout (s) */);
+    if (error.Fail() || status != 0)
+        return Error("Oatdump failed: %s", error.AsCString());
+
+    // Download the symbolfile from the remote device
+    return GetFile(symfile_platform_filespec, dst_file_spec);
+}

lldb/trunk/source/Target/Platform.cpp

 {
     // Get module information from a target.
     ModuleSpec resolved_module_spec;
     bool got_module_spec = false;
     if (process)
     {
         // Try to get module information from the process
         if (process->GetModuleSpec (module_spec.GetFileSpec (), module_spec.GetArchitecture (), resolved_module_spec))
             got_module_spec = true;
     }
 
     if (!got_module_spec)
     {
         // Get module information from a target.
         if (!GetModuleSpec (module_spec.GetFileSpec (), module_spec.GetArchitecture (), resolved_module_spec))
             return module_resolver (module_spec);
     }
Platform::GetCachedSharedModule (const ModuleSpec &module_spec,
 
     Log *log = GetLogIfAnyCategoriesSet (LIBLLDB_LOG_PLATFORM);
 
     // Check local cache for a module.
     auto error = m_module_cache->GetAndPut (
         GetModuleCacheRoot (),
         GetCacheHostname (),
         module_spec,
-        [=](const ModuleSpec &module_spec, const FileSpec &tmp_download_file_spec)
+        [this](const ModuleSpec &module_spec, const FileSpec &tmp_download_file_spec)
         {
             return DownloadModuleSlice (module_spec.GetFileSpec (),
                                         module_spec.GetObjectOffset (),
                                         module_spec.GetObjectSize (),
                                         tmp_download_file_spec);
 
         },
+        [this](const ModuleSP& module_sp, const FileSpec& tmp_download_file_spec)
+        {
+            return DownloadSymbolFile (module_sp, tmp_download_file_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__, module_spec.GetUUID ().GetAsString ().c_str (), error.AsCString ());
if (error.Fail ())
     }
 
     Error close_error;
     CloseFile (src_fd, close_error);  // Ignoring close error.
 
     return error;
 }
 
+Error
+Platform::DownloadSymbolFile (const lldb::ModuleSP& module_sp, const FileSpec& dst_file_spec)
+{
+    return Error ("Symbol file downloading not supported by the default platform.");
+}
+
 FileSpec
 Platform::GetModuleCacheRoot ()
 {
     auto dir_spec = GetGlobalPlatformProperties ()->GetModuleCacheDirectory ();
     dir_spec.AppendPathComponent (GetName ().AsCString ());
     return dir_spec;
 }
 

lldb/trunk/source/Utility/ModuleCache.h

 /// Example:
 /// UUID view   : /tmp/lldb/remote-linux/.cache/30C94DC6-6A1F-E951-80C3-D68D2B89E576-D5AE213C/libc.so.6
 /// Sysroot view: /tmp/lldb/remote-linux/ubuntu/lib/x86_64-linux-gnu/libc.so.6
 //----------------------------------------------------------------------
 
 class ModuleCache
 {
 public:
-    using Downloader = std::function<Error (const ModuleSpec&, const FileSpec&)>;
+    using ModuleDownloader = std::function<Error (const ModuleSpec&, const FileSpec&)>;
+    using SymfileDownloader = std::function<Error (const lldb::ModuleSP&, const FileSpec&)>;
 
     Error
     GetAndPut(const FileSpec &root_dir_spec,
               const char *hostname,
               const ModuleSpec &module_spec,
-              const Downloader &downloader,
+              const ModuleDownloader &module_downloader,
+              const SymfileDownloader &symfile_downloader,
               lldb::ModuleSP &cached_module_sp,
               bool *did_create_ptr);
 
 private:
     Error
     Put (const FileSpec &root_dir_spec,
          const char *hostname,
          const ModuleSpec &module_spec,
-         const FileSpec &tmp_file);
+         const FileSpec &tmp_file,
+         const FileSpec &target_file);
 
     Error
     Get (const FileSpec &root_dir_spec,
          const char *hostname,
          const ModuleSpec &module_spec,
          lldb::ModuleSP &cached_module_sp,
          bool *did_create_ptr);
 
     std::unordered_map<std::string, lldb::ModuleWP> m_loaded_modules;
 };
 
 } // namespace lldb_private
 
 #endif  // utility_ModuleCache_h_

lldb/trunk/source/Utility/ModuleCache.cpp

 using namespace lldb;
 using namespace lldb_private;
 
 namespace {
 
 const char* kModulesSubdir = ".cache";
 const char* kLockFileName = ".lock";
 const char* kTempFileName = ".temp";
+const char* kTempSymFileName = ".symtemp";
+const char* kSymFileExtension = ".sym";
 
 FileSpec
 JoinPath (const FileSpec &path1, const char* path2)
 {
     FileSpec result_spec (path1);
     result_spec.AppendPathComponent (path2);
     return result_spec;
 }
CreateHostSysRootModuleLink (const FileSpec &root_dir_spec, const char *hostname, const FileSpec &platform_module_spec, const FileSpec &local_module_spec)
 
     const auto error = MakeDirectory (FileSpec (sysroot_module_path_spec.GetDirectory ().AsCString (), false));
     if (error.Fail ())
         return error;
 
     return FileSystem::Hardlink(sysroot_module_path_spec, local_module_spec);
 }
 
+FileSpec
+GetSymbolFileSpec(const FileSpec& module_file_spec)
+{
+    return FileSpec((module_file_spec.GetPath() + kSymFileExtension).c_str(), false);
+}
+
 }  // namespace
 
 Error
 ModuleCache::Put (const FileSpec &root_dir_spec,
                   const char *hostname,
                   const ModuleSpec &module_spec,
-                  const FileSpec &tmp_file)
+                  const FileSpec &tmp_file,
+                  const FileSpec &target_file)
 {
     const auto module_spec_dir = GetModuleDirectory (root_dir_spec, module_spec.GetUUID ());
-    const auto module_file_path = JoinPath (module_spec_dir, module_spec.GetFileSpec ().GetFilename ().AsCString ());
+    const auto module_file_path = JoinPath (module_spec_dir, target_file.GetFilename ().AsCString ());
 
     const auto tmp_file_path = tmp_file.GetPath ();
     const auto err_code = llvm::sys::fs::rename (tmp_file_path.c_str (), module_file_path.GetPath ().c_str ());
     if (err_code)
         return Error ("Failed to rename file %s to %s: %s",
                       tmp_file_path.c_str (), module_file_path.GetPath ().c_str (), err_code.message ().c_str ());
 
-    const auto error = CreateHostSysRootModuleLink(root_dir_spec, hostname, module_spec.GetFileSpec(), module_file_path);
+    const auto error = CreateHostSysRootModuleLink(root_dir_spec, hostname, target_file, module_file_path);
     if (error.Fail ())
         return Error ("Failed to create link to %s: %s", module_file_path.GetPath ().c_str (), error.AsCString ());
     return Error ();
 }
 
 Error
 ModuleCache::Get (const FileSpec &root_dir_spec,
                   const char *hostname,
ModuleCache::Get (const FileSpec &root_dir_spec,
     if (error.Fail ())
         return Error ("Failed to create link to %s: %s", module_file_path.GetPath().c_str(), error.AsCString());
 
     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));
+
+    FileSpec symfile_spec = GetSymbolFileSpec(cached_module_sp->GetFileSpec ());
+    if (symfile_spec.Exists ())
+        cached_module_sp->SetSymbolFileFileSpec (symfile_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 ();
 }
 
 Error
 ModuleCache::GetAndPut (const FileSpec &root_dir_spec,
                         const char *hostname,
                         const ModuleSpec &module_spec,
-                        const Downloader &downloader,
+                        const ModuleDownloader &module_downloader,
+                        const SymfileDownloader &symfile_downloader,
                         lldb::ModuleSP &cached_module_sp,
                         bool *did_create_ptr)
 {
     const auto module_spec_dir = GetModuleDirectory (root_dir_spec, module_spec.GetUUID ());
     auto error = MakeDirectory (module_spec_dir);
     if (error.Fail ())
         return error;
 
if (error.Fail ())
         return Error("Failed to lock file %s:%s", lock_file_spec.GetPath ().c_str (), error.AsCString ());
 
     // Check local cache for a module.
     error = Get (root_dir_spec, hostname, module_spec, cached_module_sp, did_create_ptr);
     if (error.Success ())
         return error;
 
     const auto tmp_download_file_spec = JoinPath (module_spec_dir, kTempFileName);
-    error = downloader (module_spec, tmp_download_file_spec);
+    error = module_downloader (module_spec, tmp_download_file_spec);
     llvm::FileRemover tmp_file_remover (tmp_download_file_spec.GetPath ().c_str ());
     if (error.Fail ())
         return Error("Failed to download module: %s", error.AsCString ());
 
     // Put downloaded file into local module cache.
-    error = Put (root_dir_spec, hostname, module_spec, tmp_download_file_spec);
+    error = Put (root_dir_spec, hostname, module_spec, tmp_download_file_spec, module_spec.GetFileSpec ());
     if (error.Fail ())
         return Error ("Failed to put module into cache: %s", error.AsCString ());
 
     tmp_file_remover.releaseFile ();
-    return Get (root_dir_spec, hostname, module_spec, cached_module_sp, did_create_ptr);
+    error = Get (root_dir_spec, hostname, module_spec, cached_module_sp, did_create_ptr);
+    if (error.Fail ())
+        return error;
+
+    // Fetching a symbol file for the module
+    const auto tmp_download_sym_file_spec = JoinPath (module_spec_dir, kTempSymFileName);
+    error = symfile_downloader (cached_module_sp, tmp_download_sym_file_spec);
+    llvm::FileRemover tmp_symfile_remover (tmp_download_sym_file_spec.GetPath ().c_str ());
+    if (error.Fail ())
+        // Failed to download a symfile but fetching the module was successful. The module might
+        // contain the neccessary symbols and the debugging is also possible without a symfile.
+        return Error ();
+
+    FileSpec symfile_spec = GetSymbolFileSpec (cached_module_sp->GetFileSpec ());
+    error = Put (root_dir_spec, hostname, module_spec, tmp_download_sym_file_spec, symfile_spec);
+    if (error.Fail ())
+        return Error ("Failed to put symbol file into cache: %s", error.AsCString ());
+    
+    tmp_symfile_remover.releaseFile();
+
+    cached_module_sp->SetSymbolFileFileSpec (symfile_spec);
+    return Error ();
 }