[Lldb-commits] [lldb] 8c61c9b - [lldb][LocateModuleCallback] Call locate module callback in Platform too

Kazuki Sakamoto via lldb-commits lldb-commits at lists.llvm.org
Tue Jul 25 10:59:04 PDT 2023


Author: Kazuki Sakamoto
Date: 2023-07-25T10:57:50-07:00
New Revision: 8c61c9b02b492b00179e6e2cd95d2c54dac69a0e

URL: https://github.com/llvm/llvm-project/commit/8c61c9b02b492b00179e6e2cd95d2c54dac69a0e
DIFF: https://github.com/llvm/llvm-project/commit/8c61c9b02b492b00179e6e2cd95d2c54dac69a0e.diff

LOG: [lldb][LocateModuleCallback] Call locate module callback in Platform too

This is an enhancement for the locate module callback.
https://discourse.llvm.org/t/rfc-python-callback-for-target-get-module/71580/6

On Android remote platform, module UUID is resolved by
Platform::GetRemoteSharedModule. Which means the current
Target::CallLocateModuleCallbackIfSet() call undesirably is not able to pass the
module UUID to the locate module callback.

This diff moves the CallLocateModuleCallbackIfSet() implementation from Target
to Platform to allows both Target and Platform can call it. One is from the
current Target call site, and second is from Platform after resolving the module
UUID.

As the result of this change, the locate module callback may be called twice
for a same module on remote platforms. And it should be ok.

- First, without UUID.
  - The locate module callback is allowed to return an error
    if the callback requires UUID.
- Second, with UUID, if the first callback call did not return a module.

Differential Revision: https://reviews.llvm.org/D156066

Added: 
    

Modified: 
    lldb/include/lldb/Target/Platform.h
    lldb/include/lldb/Target/Target.h
    lldb/source/Target/Platform.cpp
    lldb/source/Target/Target.cpp
    lldb/unittests/Target/LocateModuleCallbackTest.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Target/Platform.h b/lldb/include/lldb/Target/Platform.h
index a5a78fe0f86983..583e9a2e5a4c92 100644
--- a/lldb/include/lldb/Target/Platform.h
+++ b/lldb/include/lldb/Target/Platform.h
@@ -293,6 +293,11 @@ class Platform : public PluginInterface {
       lldb::ModuleSP &module_sp, const FileSpecList *module_search_paths_ptr,
       llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules, bool *did_create_ptr);
 
+  void CallLocateModuleCallbackIfSet(const ModuleSpec &module_spec,
+                                     lldb::ModuleSP &module_sp,
+                                     FileSpec &symbol_file_spec,
+                                     bool *did_create_ptr);
+
   virtual bool GetModuleSpec(const FileSpec &module_file_spec,
                              const ArchSpec &arch, ModuleSpec &module_spec);
 

diff  --git a/lldb/include/lldb/Target/Target.h b/lldb/include/lldb/Target/Target.h
index d5b4b8e18c2d2d..ed0ecbbddbf814 100644
--- a/lldb/include/lldb/Target/Target.h
+++ b/lldb/include/lldb/Target/Target.h
@@ -1625,12 +1625,6 @@ class Target : public std::enable_shared_from_this<Target>,
 
   Target(const Target &) = delete;
   const Target &operator=(const Target &) = delete;
-
-private:
-  void CallLocateModuleCallbackIfSet(const ModuleSpec &module_spec,
-                                     lldb::ModuleSP &module_sp,
-                                     FileSpec &symbol_file_spec,
-                                     bool &did_create_module);
 };
 
 } // namespace lldb_private

diff  --git a/lldb/source/Target/Platform.cpp b/lldb/source/Target/Platform.cpp
index 11a123fb6d583d..2802033cf63051 100644
--- a/lldb/source/Target/Platform.cpp
+++ b/lldb/source/Target/Platform.cpp
@@ -1564,14 +1564,167 @@ Status Platform::GetRemoteSharedModule(const ModuleSpec &module_spec,
     resolved_module_spec.GetUUID() = module_spec.GetUUID();
   }
 
+  // Call locate module callback if set. This allows users to implement their
+  // own module cache system. For example, to leverage build system artifacts,
+  // to bypass pulling files from remote platform, or to search symbol files
+  // from symbol servers.
+  FileSpec symbol_file_spec;
+  CallLocateModuleCallbackIfSet(resolved_module_spec, module_sp,
+                                symbol_file_spec, did_create_ptr);
+  if (module_sp) {
+    // The module is loaded.
+    if (symbol_file_spec) {
+      // 1. module_sp:loaded, symbol_file_spec:set
+      //      The callback found a module file and a symbol file for this
+      //      resolved_module_spec. Set the symbol file to the module.
+      module_sp->SetSymbolFileFileSpec(symbol_file_spec);
+    } else {
+      // 2. module_sp:loaded, symbol_file_spec:empty
+      //      The callback only found a module file for this
+      //      resolved_module_spec.
+    }
+    return Status();
+  }
+
+  // The module is not loaded by CallLocateModuleCallbackIfSet.
+  // 3. module_sp:empty, symbol_file_spec:set
+  //      The callback only found a symbol file for the module. We continue to
+  //      find a module file for this resolved_module_spec. and we will call
+  //      module_sp->SetSymbolFileFileSpec with the symbol_file_spec later.
+  // 4. module_sp:empty, symbol_file_spec:empty
+  //      The callback is not set. Or the callback did not find any module
+  //      files nor any symbol files. Or the callback failed, or something
+  //      went wrong. We continue to find a module file for this
+  //      resolved_module_spec.
+
   // Trying to find a module by UUID on local file system.
-  const auto error = module_resolver(resolved_module_spec);
+  const Status error = module_resolver(resolved_module_spec);
+  if (error.Success()) {
+    if (module_sp && symbol_file_spec) {
+      // Set the symbol file to the module if the locate modudle callback was
+      // called and returned only a symbol file.
+      module_sp->SetSymbolFileFileSpec(symbol_file_spec);
+    }
+    return error;
+  }
+
+  // Fallback to call GetCachedSharedModule on failure.
+  if (GetCachedSharedModule(resolved_module_spec, module_sp, did_create_ptr)) {
+    if (module_sp && symbol_file_spec) {
+      // Set the symbol file to the module if the locate modudle callback was
+      // called and returned only a symbol file.
+      module_sp->SetSymbolFileFileSpec(symbol_file_spec);
+    }
+    return Status();
+  }
+
+  return Status("Failed to call GetCachedSharedModule");
+}
+
+void Platform::CallLocateModuleCallbackIfSet(const ModuleSpec &module_spec,
+                                             lldb::ModuleSP &module_sp,
+                                             FileSpec &symbol_file_spec,
+                                             bool *did_create_ptr) {
+  if (!m_locate_module_callback) {
+    // Locate module callback is not set.
+    return;
+  }
+
+  FileSpec module_file_spec;
+  Status error =
+      m_locate_module_callback(module_spec, module_file_spec, symbol_file_spec);
+
+  // Locate module callback is set and called. Check the error.
+  Log *log = GetLog(LLDBLog::Platform);
   if (error.Fail()) {
-    if (GetCachedSharedModule(resolved_module_spec, module_sp, did_create_ptr))
-      return Status();
+    LLDB_LOGF(log, "%s: locate module callback failed: %s",
+              LLVM_PRETTY_FUNCTION, error.AsCString());
+    return;
   }
 
-  return error;
+  // The locate module callback was succeeded.
+  // Check the module_file_spec and symbol_file_spec values.
+  // 1. module:empty  symbol:empty  -> Failure
+  //    - The callback did not return any files.
+  // 2. module:exists symbol:exists -> Success
+  //    - The callback returned a module file and a symbol file.
+  // 3. module:exists symbol:empty  -> Success
+  //    - The callback returned only a module file.
+  // 4. module:empty  symbol:exists -> Success
+  //    - The callback returned only a symbol file.
+  //      For example, a breakpad symbol text file.
+  if (!module_file_spec && !symbol_file_spec) {
+    // This is '1. module:empty  symbol:empty  -> Failure'
+    // The callback did not return any files.
+    LLDB_LOGF(log,
+              "%s: locate module callback did not set both "
+              "module_file_spec and symbol_file_spec",
+              LLVM_PRETTY_FUNCTION);
+    return;
+  }
+
+  // If the callback returned a module file, it should exist.
+  if (module_file_spec && !FileSystem::Instance().Exists(module_file_spec)) {
+    LLDB_LOGF(log,
+              "%s: locate module callback set a non-existent file to "
+              "module_file_spec: %s",
+              LLVM_PRETTY_FUNCTION, module_file_spec.GetPath().c_str());
+    // Clear symbol_file_spec for the error.
+    symbol_file_spec.Clear();
+    return;
+  }
+
+  // If the callback returned a symbol file, it should exist.
+  if (symbol_file_spec && !FileSystem::Instance().Exists(symbol_file_spec)) {
+    LLDB_LOGF(log,
+              "%s: locate module callback set a non-existent file to "
+              "symbol_file_spec: %s",
+              LLVM_PRETTY_FUNCTION, symbol_file_spec.GetPath().c_str());
+    // Clear symbol_file_spec for the error.
+    symbol_file_spec.Clear();
+    return;
+  }
+
+  if (!module_file_spec && symbol_file_spec) {
+    // This is '4. module:empty  symbol:exists -> Success'
+    // The locate module callback returned only a symbol file. For example,
+    // a breakpad symbol text file. GetRemoteSharedModule will use this returned
+    // symbol_file_spec.
+    LLDB_LOGF(log, "%s: locate module callback succeeded: symbol=%s",
+              LLVM_PRETTY_FUNCTION, symbol_file_spec.GetPath().c_str());
+    return;
+  }
+
+  // This is one of the following.
+  // - 2. module:exists symbol:exists -> Success
+  //    - The callback returned a module file and a symbol file.
+  // - 3. module:exists symbol:empty  -> Success
+  //    - The callback returned Only a module file.
+  // Load the module file.
+  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_spec;
+  cached_module_spec.GetPlatformFileSpec() = module_spec.GetFileSpec();
+  cached_module_spec.SetObjectOffset(0);
+
+  error = ModuleList::GetSharedModule(cached_module_spec, module_sp, nullptr,
+                                      nullptr, did_create_ptr, false);
+  if (error.Success() && module_sp) {
+    // Succeeded to load the module file.
+    LLDB_LOGF(log, "%s: locate module callback succeeded: module=%s symbol=%s",
+              LLVM_PRETTY_FUNCTION, module_file_spec.GetPath().c_str(),
+              symbol_file_spec.GetPath().c_str());
+  } else {
+    LLDB_LOGF(log,
+              "%s: locate module callback succeeded but failed to load: "
+              "module=%s symbol=%s",
+              LLVM_PRETTY_FUNCTION, module_file_spec.GetPath().c_str(),
+              symbol_file_spec.GetPath().c_str());
+    // Clear module_sp and symbol_file_spec for the error.
+    module_sp.reset();
+    symbol_file_spec.Clear();
+  }
 }
 
 bool Platform::GetCachedSharedModule(const ModuleSpec &module_spec,

diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index f0c57ef83c4a14..ab68f447cfdb6f 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -2135,8 +2135,9 @@ ModuleSP Target::GetOrCreateModule(const ModuleSpec &module_spec, bool notify,
     // own module cache system. For example, to leverage build system artifacts,
     // to bypass pulling files from remote platform, or to search symbol files
     // from symbol servers.
-    CallLocateModuleCallbackIfSet(module_spec, module_sp, symbol_file_spec,
-                                  did_create_module);
+    if (m_platform_sp)
+      m_platform_sp->CallLocateModuleCallbackIfSet(
+          module_spec, module_sp, symbol_file_spec, &did_create_module);
 
     // The result of this CallLocateModuleCallbackIfSet is one of the following.
     // 1. module_sp:loaded, symbol_file_spec:set
@@ -2150,9 +2151,10 @@ ModuleSP Target::GetOrCreateModule(const ModuleSpec &module_spec, bool notify,
     //      to find a module file for this module_spec and we will call
     //      module_sp->SetSymbolFileFileSpec with the symbol_file_spec later.
     // 4. module_sp:empty, symbol_file_spec:empty
-    //      The callback is not set. Or the callback did not find any module
-    //      files nor any symbol files. Or the callback failed, or something
-    //      went wrong. We continue to find a module file for this module_spec.
+    //      Platform does not exist, the callback is not set, the callback did
+    //      not find any module files nor any symbol files, the callback failed,
+    //      or something went wrong. We continue to find a module file for this
+    //      module_spec.
 
     if (!module_sp) {
       // If there are image search path entries, try to use them to acquire a
@@ -2338,113 +2340,6 @@ ModuleSP Target::GetOrCreateModule(const ModuleSpec &module_spec, bool notify,
   return module_sp;
 }
 
-void Target::CallLocateModuleCallbackIfSet(const ModuleSpec &module_spec,
-                                           lldb::ModuleSP &module_sp,
-                                           FileSpec &symbol_file_spec,
-                                           bool &did_create_module) {
-  if (!m_platform_sp)
-    return;
-
-  Platform::LocateModuleCallback locate_module_callback =
-      m_platform_sp->GetLocateModuleCallback();
-  if (!locate_module_callback)
-    return;
-
-  FileSpec module_file_spec;
-  Status error =
-      locate_module_callback(module_spec, module_file_spec, symbol_file_spec);
-
-  // Locate module callback is set and called. Check the error.
-  Log *log = GetLog(LLDBLog::Target);
-  if (error.Fail()) {
-    LLDB_LOGF(log, "%s: locate module callback failed: %s",
-              LLVM_PRETTY_FUNCTION, error.AsCString());
-    return;
-  }
-
-  // The locate module callback was succeeded. It should returned
-  // 1. a combination of a module file and a symbol file.
-  // 2. or only a module file.
-  // 3. or only a symbol file. For example, a breakpad symbol text file.
-  //
-  // Check the module_file_spec and symbol_file_spec values.
-  // 1. module:empty  symbol:empty  -> Invalid
-  // 2. module:exists symbol:exists -> Success
-  // 3. module:exists symbol:empty  -> Success
-  // 4. module:empty  symbol:exists -> Success
-  if (!module_file_spec && !symbol_file_spec) {
-    // This is '1. module:empty symbol:empty -> Invalid'.
-    LLDB_LOGF(log,
-              "%s: locate module callback did not set both "
-              "module_file_spec and symbol_file_spec",
-              LLVM_PRETTY_FUNCTION);
-    return;
-  }
-
-  // The module file should exist.
-  if (module_file_spec && !FileSystem::Instance().Exists(module_file_spec)) {
-    LLDB_LOGF(log,
-              "%s: locate module callback set a non-existent file to "
-              "module_file_spec: %s",
-              LLVM_PRETTY_FUNCTION, module_file_spec.GetPath().c_str());
-    // Clear symbol_file_spec for the error.
-    symbol_file_spec.Clear();
-    return;
-  }
-
-  // The symbol file should exist.
-  if (symbol_file_spec && !FileSystem::Instance().Exists(symbol_file_spec)) {
-    LLDB_LOGF(log,
-              "%s: locate module callback set a non-existent file to "
-              "symbol_file_spec: %s",
-              LLVM_PRETTY_FUNCTION, symbol_file_spec.GetPath().c_str());
-    // Clear symbol_file_spec for the error.
-    symbol_file_spec.Clear();
-    return;
-  }
-
-  if (!module_file_spec && symbol_file_spec) {
-    // This is '4. module:empty symbol:exists -> Success'.
-    // The locate module callback returned only a symbol file. For example,
-    // a breakpad symbol text file. GetOrCreateModule will use this returned
-    // symbol_file_spec.
-    LLDB_LOGF(log, "%s: locate module callback succeeded: symbol=%s",
-              LLVM_PRETTY_FUNCTION, symbol_file_spec.GetPath().c_str());
-    return;
-  }
-
-  // The locate module callback returned
-  // - '2. module:exists symbol:exists -> Success'
-  //   - a combination of a module file and a symbol file.
-  // - Or '3. module:exists symbol:empty -> Success'
-  //   - only a module file.
-  // Load the module file.
-  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_spec;
-  cached_module_spec.GetPlatformFileSpec() = module_spec.GetFileSpec();
-  cached_module_spec.SetObjectOffset(0);
-
-  error = ModuleList::GetSharedModule(cached_module_spec, module_sp, nullptr,
-                                      nullptr, &did_create_module, false);
-  if (error.Success() && module_sp) {
-    // Succeeded to load the module file.
-    LLDB_LOGF(log, "%s: locate module callback succeeded: module=%s symbol=%s",
-              LLVM_PRETTY_FUNCTION, module_file_spec.GetPath().c_str(),
-              symbol_file_spec.GetPath().c_str());
-  } else {
-    LLDB_LOGF(log,
-              "%s: locate module callback succeeded but failed to load: "
-              "module=%s symbol=%s",
-              LLVM_PRETTY_FUNCTION, module_file_spec.GetPath().c_str(),
-              symbol_file_spec.GetPath().c_str());
-    // Clear module_sp and symbol_file_spec for the error.
-    module_sp.reset();
-    symbol_file_spec.Clear();
-  }
-}
-
 TargetSP Target::CalculateTarget() { return shared_from_this(); }
 
 ProcessSP Target::CalculateProcess() { return m_process_sp; }

diff  --git a/lldb/unittests/Target/LocateModuleCallbackTest.cpp b/lldb/unittests/Target/LocateModuleCallbackTest.cpp
index 3f8eb2f18ac7e5..0fa0f87efe53ff 100644
--- a/lldb/unittests/Target/LocateModuleCallbackTest.cpp
+++ b/lldb/unittests/Target/LocateModuleCallbackTest.cpp
@@ -231,6 +231,7 @@ class LocateModuleCallbackTest : public testing::Test {
     EXPECT_TRUE(m_process_sp);
 
     m_module_spec = GetTestModuleSpec();
+    m_module_spec_without_uuid = ModuleSpec(GetRemotePath(), ArchSpec(k_arch));
   }
 
   void TearDown() override {
@@ -244,15 +245,33 @@ class LocateModuleCallbackTest : public testing::Test {
   }
 
   void CheckCallbackArgs(const ModuleSpec &module_spec,
-                         FileSpec &module_file_spec,
-                         FileSpec &symbol_file_spec) {
-    EXPECT_TRUE(m_module_spec.Matches(module_spec,
-                                      /*exact_arch_match=*/true));
+                         FileSpec &module_file_spec, FileSpec &symbol_file_spec,
+                         const ModuleSpec &expected_module_spec,
+                         int expected_callback_call_count) {
+    EXPECT_TRUE(expected_module_spec.Matches(module_spec,
+                                             /*exact_arch_match=*/true));
     EXPECT_FALSE(module_file_spec);
     EXPECT_FALSE(symbol_file_spec);
 
-    EXPECT_EQ(m_callback_call_count, 0);
-    m_callback_call_count++;
+    EXPECT_EQ(++m_callback_call_count, expected_callback_call_count);
+  }
+
+  void CheckCallbackArgsWithUUID(const ModuleSpec &module_spec,
+                                 FileSpec &module_file_spec,
+                                 FileSpec &symbol_file_spec,
+                                 int expected_callback_call_count) {
+    CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec,
+                      m_module_spec, expected_callback_call_count);
+    EXPECT_TRUE(module_spec.GetUUID().IsValid());
+  }
+
+  void CheckCallbackArgsWithoutUUID(const ModuleSpec &module_spec,
+                                    FileSpec &module_file_spec,
+                                    FileSpec &symbol_file_spec,
+                                    int expected_callback_call_count) {
+    CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec,
+                      m_module_spec_without_uuid, expected_callback_call_count);
+    EXPECT_FALSE(module_spec.GetUUID().IsValid());
   }
 
 protected:
@@ -262,6 +281,7 @@ class LocateModuleCallbackTest : public testing::Test {
   TargetSP m_target_sp;
   ProcessSP m_process_sp;
   ModuleSpec m_module_spec;
+  ModuleSpec m_module_spec_without_uuid;
   ModuleSP m_module_sp;
   int m_callback_call_count = 0;
 };
@@ -331,14 +351,18 @@ TEST_F(LocateModuleCallbackTest, GetOrCreateModuleCallbackFailureNoCache) {
   // download the module and fails.
   BuildEmptyCacheDir(m_test_dir);
 
-  m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
-                                                FileSpec &module_file_spec,
-                                                FileSpec &symbol_file_spec) {
-    CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
-    return Status("The locate module callback failed");
-  });
+  int callback_call_count = 0;
+  m_platform_sp->SetLocateModuleCallback(
+      [this, &callback_call_count](const ModuleSpec &module_spec,
+                                   FileSpec &module_file_spec,
+                                   FileSpec &symbol_file_spec) {
+        CheckCallbackArgsWithUUID(module_spec, module_file_spec,
+                                  symbol_file_spec, ++callback_call_count);
+        return Status("The locate module callback failed");
+      });
 
   m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  ASSERT_EQ(callback_call_count, 2);
   ASSERT_FALSE(m_module_sp);
 }
 
@@ -348,14 +372,18 @@ TEST_F(LocateModuleCallbackTest, GetOrCreateModuleCallbackFailureCached) {
   // some reason.
   FileSpec uuid_view = BuildCacheDir(m_test_dir);
 
-  m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
-                                                FileSpec &module_file_spec,
-                                                FileSpec &symbol_file_spec) {
-    CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
-    return Status("The locate module callback failed");
-  });
+  int callback_call_count = 0;
+  m_platform_sp->SetLocateModuleCallback(
+      [this, &callback_call_count](const ModuleSpec &module_spec,
+                                   FileSpec &module_file_spec,
+                                   FileSpec &symbol_file_spec) {
+        CheckCallbackArgsWithUUID(module_spec, module_file_spec,
+                                  symbol_file_spec, ++callback_call_count);
+        return Status("The locate module callback failed");
+      });
 
   m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  ASSERT_EQ(callback_call_count, 2);
   CheckModule(m_module_sp);
   ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
   ASSERT_FALSE(m_module_sp->GetSymbolFileFileSpec());
@@ -368,16 +396,20 @@ TEST_F(LocateModuleCallbackTest, GetOrCreateModuleCallbackNoFiles) {
   // no files.
   FileSpec uuid_view = BuildCacheDir(m_test_dir);
 
-  m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
-                                                FileSpec &module_file_spec,
-                                                FileSpec &symbol_file_spec) {
-    CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
-    // The locate module callback succeeds but it does not set
-    // module_file_spec nor symbol_file_spec.
-    return Status();
-  });
+  int callback_call_count = 0;
+  m_platform_sp->SetLocateModuleCallback(
+      [this, &callback_call_count](const ModuleSpec &module_spec,
+                                   FileSpec &module_file_spec,
+                                   FileSpec &symbol_file_spec) {
+        CheckCallbackArgsWithUUID(module_spec, module_file_spec,
+                                  symbol_file_spec, ++callback_call_count);
+        // The locate module callback succeeds but it does not set
+        // module_file_spec nor symbol_file_spec.
+        return Status();
+      });
 
   m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  ASSERT_EQ(callback_call_count, 2);
   CheckModule(m_module_sp);
   ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
   ASSERT_FALSE(m_module_sp->GetSymbolFileFileSpec());
@@ -391,15 +423,19 @@ TEST_F(LocateModuleCallbackTest, GetOrCreateModuleCallbackNonExistentModule) {
   // non-existent module file.
   FileSpec uuid_view = BuildCacheDir(m_test_dir);
 
-  m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
-                                                FileSpec &module_file_spec,
-                                                FileSpec &symbol_file_spec) {
-    CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
-    module_file_spec.SetPath("/this path does not exist");
-    return Status();
-  });
+  int callback_call_count = 0;
+  m_platform_sp->SetLocateModuleCallback(
+      [this, &callback_call_count](const ModuleSpec &module_spec,
+                                   FileSpec &module_file_spec,
+                                   FileSpec &symbol_file_spec) {
+        CheckCallbackArgsWithUUID(module_spec, module_file_spec,
+                                  symbol_file_spec, ++callback_call_count);
+        module_file_spec.SetPath("/this path does not exist");
+        return Status();
+      });
 
   m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  ASSERT_EQ(callback_call_count, 2);
   CheckModule(m_module_sp);
   ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
   ASSERT_FALSE(m_module_sp->GetSymbolFileFileSpec());
@@ -413,18 +449,22 @@ TEST_F(LocateModuleCallbackTest, GetOrCreateModuleCallbackNonExistentSymbol) {
   // non-existent symbol file.
   FileSpec uuid_view = BuildCacheDir(m_test_dir);
 
-  m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
-                                                FileSpec &module_file_spec,
-                                                FileSpec &symbol_file_spec) {
-    CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
-    // The locate module callback returns a right module file.
-    module_file_spec.SetPath(GetInputFilePath(k_module_file));
-    // But it returns non-existent symbols file.
-    symbol_file_spec.SetPath("/this path does not exist");
-    return Status();
-  });
+  int callback_call_count = 0;
+  m_platform_sp->SetLocateModuleCallback(
+      [this, &callback_call_count](const ModuleSpec &module_spec,
+                                   FileSpec &module_file_spec,
+                                   FileSpec &symbol_file_spec) {
+        CheckCallbackArgsWithUUID(module_spec, module_file_spec,
+                                  symbol_file_spec, ++callback_call_count);
+        // The locate module callback returns a right module file.
+        module_file_spec.SetPath(GetInputFilePath(k_module_file));
+        // But it returns non-existent symbols file.
+        symbol_file_spec.SetPath("/this path does not exist");
+        return Status();
+      });
 
   m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  ASSERT_EQ(callback_call_count, 2);
   CheckModule(m_module_sp);
   ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
   ASSERT_TRUE(m_module_sp->GetSymbolFileFileSpec().GetPath().empty());
@@ -440,7 +480,8 @@ TEST_F(LocateModuleCallbackTest, GetOrCreateModuleCallbackSuccessWithModule) {
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
-    CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
+    CheckCallbackArgsWithUUID(module_spec, module_file_spec, symbol_file_spec,
+                              1);
     module_file_spec.SetPath(GetInputFilePath(k_module_file));
     return Status();
   });
@@ -465,7 +506,8 @@ TEST_F(LocateModuleCallbackTest,
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
-    CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
+    CheckCallbackArgsWithUUID(module_spec, module_file_spec, symbol_file_spec,
+                              1);
     module_file_spec.SetPath(GetInputFilePath(k_symbol_file));
     return Status();
   });
@@ -490,7 +532,8 @@ TEST_F(LocateModuleCallbackTest,
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
-    CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
+    CheckCallbackArgsWithUUID(module_spec, module_file_spec, symbol_file_spec,
+                              1);
     module_file_spec.SetPath(GetInputFilePath(k_symbol_file));
     symbol_file_spec.SetPath(GetInputFilePath(k_symbol_file));
     return Status();
@@ -516,7 +559,8 @@ TEST_F(LocateModuleCallbackTest,
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
-    CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
+    CheckCallbackArgsWithUUID(module_spec, module_file_spec, symbol_file_spec,
+                              1);
     module_file_spec.SetPath(GetInputFilePath(k_module_file));
     symbol_file_spec.SetPath(GetInputFilePath(k_symbol_file));
     return Status();
@@ -542,7 +586,8 @@ TEST_F(LocateModuleCallbackTest,
   m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
                                                 FileSpec &module_file_spec,
                                                 FileSpec &symbol_file_spec) {
-    CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
+    CheckCallbackArgsWithUUID(module_spec, module_file_spec, symbol_file_spec,
+                              1);
     module_file_spec.SetPath(GetInputFilePath(k_module_file));
     symbol_file_spec.SetPath(GetInputFilePath(k_breakpad_symbol_file));
     return Status();
@@ -565,15 +610,19 @@ TEST_F(LocateModuleCallbackTest,
   // along with the symbol file from the Inputs directory.
   FileSpec uuid_view = BuildCacheDir(m_test_dir);
 
-  m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
-                                                FileSpec &module_file_spec,
-                                                FileSpec &symbol_file_spec) {
-    CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
-    symbol_file_spec.SetPath(GetInputFilePath(k_symbol_file));
-    return Status();
-  });
+  int callback_call_count = 0;
+  m_platform_sp->SetLocateModuleCallback(
+      [this, &callback_call_count](const ModuleSpec &module_spec,
+                                   FileSpec &module_file_spec,
+                                   FileSpec &symbol_file_spec) {
+        CheckCallbackArgsWithUUID(module_spec, module_file_spec,
+                                  symbol_file_spec, ++callback_call_count);
+        symbol_file_spec.SetPath(GetInputFilePath(k_symbol_file));
+        return Status();
+      });
 
   m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  ASSERT_EQ(callback_call_count, 2);
   CheckModule(m_module_sp);
   ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
   ASSERT_EQ(m_module_sp->GetSymbolFileFileSpec(),
@@ -589,15 +638,51 @@ TEST_F(LocateModuleCallbackTest,
   // cache along with the symbol file from the Inputs directory.
   FileSpec uuid_view = BuildCacheDir(m_test_dir);
 
-  m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
-                                                FileSpec &module_file_spec,
-                                                FileSpec &symbol_file_spec) {
-    CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
-    symbol_file_spec.SetPath(GetInputFilePath(k_breakpad_symbol_file));
-    return Status();
-  });
+  int callback_call_count = 0;
+  m_platform_sp->SetLocateModuleCallback(
+      [this, &callback_call_count](const ModuleSpec &module_spec,
+                                   FileSpec &module_file_spec,
+                                   FileSpec &symbol_file_spec) {
+        CheckCallbackArgsWithUUID(module_spec, module_file_spec,
+                                  symbol_file_spec, ++callback_call_count);
+        symbol_file_spec.SetPath(GetInputFilePath(k_breakpad_symbol_file));
+        return Status();
+      });
+
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  ASSERT_EQ(callback_call_count, 2);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
+  ASSERT_EQ(m_module_sp->GetSymbolFileFileSpec(),
+            FileSpec(GetInputFilePath(k_breakpad_symbol_file)));
+  CheckUnstrippedSymbol(m_module_sp);
+  ModuleList::RemoveSharedModule(m_module_sp);
+}
+
+TEST_F(LocateModuleCallbackTest,
+       GetOrCreateModuleCallbackSuccessWithMultipleSymbols) {
+  // The get callback returns only a symbol file. The first call returns
+  // a breakpad symbol file and the second call returns a symbol file.
+  // Also the module is cached, so GetOrCreateModule should succeed to return
+  // the module from the cache along with the breakpad symbol file from the
+  // Inputs directory because GetOrCreateModule will use the first symbol file
+  // from the callback.
+  FileSpec uuid_view = BuildCacheDir(m_test_dir);
+
+  int callback_call_count = 0;
+  m_platform_sp->SetLocateModuleCallback(
+      [this, &callback_call_count](const ModuleSpec &module_spec,
+                                   FileSpec &module_file_spec,
+                                   FileSpec &symbol_file_spec) {
+        CheckCallbackArgsWithUUID(module_spec, module_file_spec,
+                                  symbol_file_spec, ++callback_call_count);
+        symbol_file_spec.SetPath(GetInputFilePath(
+            callback_call_count == 1 ? k_breakpad_symbol_file : k_symbol_file));
+        return Status();
+      });
 
   m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  ASSERT_EQ(callback_call_count, 2);
   CheckModule(m_module_sp);
   ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
   ASSERT_EQ(m_module_sp->GetSymbolFileFileSpec(),
@@ -612,15 +697,19 @@ TEST_F(LocateModuleCallbackTest,
   // cached, GetOrCreateModule should fail because of the missing module.
   BuildEmptyCacheDir(m_test_dir);
 
-  m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
-                                                FileSpec &module_file_spec,
-                                                FileSpec &symbol_file_spec) {
-    CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
-    symbol_file_spec.SetPath(GetInputFilePath(k_symbol_file));
-    return Status();
-  });
+  int callback_call_count = 0;
+  m_platform_sp->SetLocateModuleCallback(
+      [this, &callback_call_count](const ModuleSpec &module_spec,
+                                   FileSpec &module_file_spec,
+                                   FileSpec &symbol_file_spec) {
+        CheckCallbackArgsWithUUID(module_spec, module_file_spec,
+                                  symbol_file_spec, ++callback_call_count);
+        symbol_file_spec.SetPath(GetInputFilePath(k_symbol_file));
+        return Status();
+      });
 
   m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  ASSERT_EQ(callback_call_count, 2);
   ASSERT_FALSE(m_module_sp);
 }
 
@@ -630,14 +719,180 @@ TEST_F(LocateModuleCallbackTest,
   // cached, GetOrCreateModule should fail because of the missing module.
   BuildEmptyCacheDir(m_test_dir);
 
-  m_platform_sp->SetLocateModuleCallback([this](const ModuleSpec &module_spec,
-                                                FileSpec &module_file_spec,
-                                                FileSpec &symbol_file_spec) {
-    CheckCallbackArgs(module_spec, module_file_spec, symbol_file_spec);
-    symbol_file_spec.SetPath(GetInputFilePath(k_breakpad_symbol_file));
-    return Status();
-  });
+  int callback_call_count = 0;
+  m_platform_sp->SetLocateModuleCallback(
+      [this, &callback_call_count](const ModuleSpec &module_spec,
+                                   FileSpec &module_file_spec,
+                                   FileSpec &symbol_file_spec) {
+        CheckCallbackArgsWithUUID(module_spec, module_file_spec,
+                                  symbol_file_spec, ++callback_call_count);
+        symbol_file_spec.SetPath(GetInputFilePath(k_breakpad_symbol_file));
+        return Status();
+      });
 
   m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec, /*notify=*/false);
+  ASSERT_EQ(callback_call_count, 2);
   ASSERT_FALSE(m_module_sp);
 }
+
+TEST_F(LocateModuleCallbackTest,
+       GetOrCreateModuleCallbackSuccessWithModuleByPlatformUUID) {
+  // This is a simulation for Android remote platform debugging.
+  // The locate module callback first call fails because module_spec does not
+  // have UUID. Then, the callback second call returns a module file because the
+  // platform resolved the module_spec UUID from the target process.
+  // GetOrCreateModule should succeed to return the module from the Inputs
+  // directory.
+  BuildEmptyCacheDir(m_test_dir);
+
+  int callback_call_count = 0;
+  m_platform_sp->SetLocateModuleCallback(
+      [this, &callback_call_count](const ModuleSpec &module_spec,
+                                   FileSpec &module_file_spec,
+                                   FileSpec &symbol_file_spec) {
+        callback_call_count++;
+        if (callback_call_count == 1) {
+          // The module_spec does not have UUID on the first call.
+          CheckCallbackArgsWithoutUUID(module_spec, module_file_spec,
+                                       symbol_file_spec, callback_call_count);
+          return Status("Ignored empty UUID");
+        } else {
+          // The module_spec has UUID on the second call.
+          CheckCallbackArgsWithUUID(module_spec, module_file_spec,
+                                    symbol_file_spec, callback_call_count);
+          module_file_spec.SetPath(GetInputFilePath(k_module_file));
+          return Status();
+        }
+      });
+
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec_without_uuid,
+                                               /*notify=*/false);
+  ASSERT_EQ(callback_call_count, 2);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(),
+            FileSpec(GetInputFilePath(k_module_file)));
+  ASSERT_FALSE(m_module_sp->GetSymbolFileFileSpec());
+  CheckStrippedSymbol(m_module_sp);
+  ModuleList::RemoveSharedModule(m_module_sp);
+}
+
+TEST_F(LocateModuleCallbackTest,
+       GetOrCreateModuleCallbackSuccessWithSymbolByPlatformUUID) {
+  // Same as GetOrCreateModuleCallbackSuccessWithModuleByPlatformUUID,
+  // but with a symbol file. GetOrCreateModule should succeed to return the
+  // module file and the symbol file from the Inputs directory.
+  BuildEmptyCacheDir(m_test_dir);
+
+  int callback_call_count = 0;
+  m_platform_sp->SetLocateModuleCallback(
+      [this, &callback_call_count](const ModuleSpec &module_spec,
+                                   FileSpec &module_file_spec,
+                                   FileSpec &symbol_file_spec) {
+        callback_call_count++;
+        if (callback_call_count == 1) {
+          // The module_spec does not have UUID on the first call.
+          CheckCallbackArgsWithoutUUID(module_spec, module_file_spec,
+                                       symbol_file_spec, callback_call_count);
+          return Status("Ignored empty UUID");
+        } else {
+          // The module_spec has UUID on the second call.
+          CheckCallbackArgsWithUUID(module_spec, module_file_spec,
+                                    symbol_file_spec, callback_call_count);
+          module_file_spec.SetPath(GetInputFilePath(k_module_file));
+          symbol_file_spec.SetPath(GetInputFilePath(k_symbol_file));
+          return Status();
+        }
+      });
+
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec_without_uuid,
+                                               /*notify=*/false);
+  ASSERT_EQ(callback_call_count, 2);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(),
+            FileSpec(GetInputFilePath(k_module_file)));
+  ASSERT_EQ(m_module_sp->GetSymbolFileFileSpec(),
+            FileSpec(GetInputFilePath(k_symbol_file)));
+  CheckUnstrippedSymbol(m_module_sp);
+  ModuleList::RemoveSharedModule(m_module_sp);
+}
+
+TEST_F(LocateModuleCallbackTest,
+       GetOrCreateModuleCallbackSuccessWithBreakpadSymbolByPlatformUUID) {
+  // Same as GetOrCreateModuleCallbackSuccessWithModuleByPlatformUUID,
+  // but with a breakpad symbol file. GetOrCreateModule should succeed to return
+  // the module file and the symbol file from the Inputs directory.
+  BuildEmptyCacheDir(m_test_dir);
+
+  int callback_call_count = 0;
+  m_platform_sp->SetLocateModuleCallback(
+      [this, &callback_call_count](const ModuleSpec &module_spec,
+                                   FileSpec &module_file_spec,
+                                   FileSpec &symbol_file_spec) {
+        callback_call_count++;
+        if (callback_call_count == 1) {
+          // The module_spec does not have UUID on the first call.
+          CheckCallbackArgsWithoutUUID(module_spec, module_file_spec,
+                                       symbol_file_spec, callback_call_count);
+          return Status("Ignored empty UUID");
+        } else {
+          // The module_spec has UUID on the second call.
+          CheckCallbackArgsWithUUID(module_spec, module_file_spec,
+                                    symbol_file_spec, callback_call_count);
+          module_file_spec.SetPath(GetInputFilePath(k_module_file));
+          symbol_file_spec.SetPath(GetInputFilePath(k_breakpad_symbol_file));
+          return Status();
+        }
+      });
+
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec_without_uuid,
+                                               /*notify=*/false);
+  ASSERT_EQ(callback_call_count, 2);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(),
+            FileSpec(GetInputFilePath(k_module_file)));
+  ASSERT_EQ(m_module_sp->GetSymbolFileFileSpec(),
+            FileSpec(GetInputFilePath(k_breakpad_symbol_file)));
+  CheckUnstrippedSymbol(m_module_sp);
+  ModuleList::RemoveSharedModule(m_module_sp);
+}
+
+TEST_F(LocateModuleCallbackTest,
+       GetOrCreateModuleCallbackSuccessWithOnlyBreakpadSymbolByPlatformUUID) {
+  // This is a simulation for Android remote platform debugging.
+  // The locate module callback first call fails because module_spec does not
+  // have UUID. Then, the callback second call returns a breakpad symbol file
+  // for the UUID from the target process. GetOrCreateModule should succeed to
+  // return the module from the cache along with the symbol file from the Inputs
+  // directory.
+  FileSpec uuid_view = BuildCacheDir(m_test_dir);
+
+  int callback_call_count = 0;
+  m_platform_sp->SetLocateModuleCallback(
+      [this, &callback_call_count](const ModuleSpec &module_spec,
+                                   FileSpec &module_file_spec,
+                                   FileSpec &symbol_file_spec) {
+        callback_call_count++;
+        if (callback_call_count == 1) {
+          // The module_spec does not have UUID on the first call.
+          CheckCallbackArgsWithoutUUID(module_spec, module_file_spec,
+                                       symbol_file_spec, callback_call_count);
+          return Status("Ignored empty UUID");
+        } else {
+          // The module_spec has UUID on the second call.
+          CheckCallbackArgsWithUUID(module_spec, module_file_spec,
+                                    symbol_file_spec, callback_call_count);
+          symbol_file_spec.SetPath(GetInputFilePath(k_breakpad_symbol_file));
+          return Status();
+        }
+      });
+
+  m_module_sp = m_target_sp->GetOrCreateModule(m_module_spec_without_uuid,
+                                               /*notify=*/false);
+  ASSERT_EQ(callback_call_count, 2);
+  CheckModule(m_module_sp);
+  ASSERT_EQ(m_module_sp->GetFileSpec(), uuid_view);
+  ASSERT_EQ(m_module_sp->GetSymbolFileFileSpec(),
+            FileSpec(GetInputFilePath(k_breakpad_symbol_file)));
+  CheckUnstrippedSymbol(m_module_sp);
+  ModuleList::RemoveSharedModule(m_module_sp);
+}


        


More information about the lldb-commits mailing list