This revision was automatically updated to reflect the committed changes.
Closed by commit rG8c61c9b02b49: [lldb][LocateModuleCallback] Call locate 
module callback in Platform too (authored by splhack).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156066/new/

https://reviews.llvm.org/D156066

Files:
  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

Index: lldb/unittests/Target/LocateModuleCallbackTest.cpp
===================================================================
--- lldb/unittests/Target/LocateModuleCallbackTest.cpp
+++ lldb/unittests/Target/LocateModuleCallbackTest.cpp
@@ -231,6 +231,7 @@
     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 @@
   }
 
   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 @@
   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 @@
   // 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 @@
   // 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 @@
   // 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 @@
   // 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 @@
   // 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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   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 @@
   // 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 @@
   // 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 @@
   // 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 @@
   // 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);
+}
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -2135,8 +2135,9 @@
     // 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 @@
     //      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 @@
   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; }
Index: lldb/source/Target/Platform.cpp
===================================================================
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -1564,14 +1564,167 @@
     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,
Index: lldb/include/lldb/Target/Target.h
===================================================================
--- lldb/include/lldb/Target/Target.h
+++ lldb/include/lldb/Target/Target.h
@@ -1625,12 +1625,6 @@
 
   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
Index: lldb/include/lldb/Target/Platform.h
===================================================================
--- lldb/include/lldb/Target/Platform.h
+++ lldb/include/lldb/Target/Platform.h
@@ -293,6 +293,11 @@
       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);
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to