[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

I think doing this in the module list is not the right place. Why? Some platforms might have multiple sysroot to check. iOS for instance has a directory for each device that Xcode has connected to which can be checked. I am fine with adding this ability to lldb_private::Platform, but I would just do it in there. Try GetRemoteSharedModule with the spec, if it fails, try again after modifying the spec to prepend the sysroot path. Possible even just check the sysroot path + path first if m_sdk_sysroot is filled in. I don't see the need to change ModuleList itself.



================
Comment at: include/lldb/Core/ModuleList.h:544-545
                                 bool *did_create_ptr,
-                                bool always_create = false);
+                                bool always_create = false,
+                                const char* sysroot = nullptr);
   static bool RemoveSharedModule(lldb::ModuleSP &module_sp);
----------------
Revert this? See my main comment


================
Comment at: source/Core/ModuleList.cpp:710-714
+                                   bool *did_create_ptr, bool always_create,
+                                   const char* sysroot) {
+  // Make sure no one else can try and get or create a module while this
+  // function is actively working on it by doing an extra lock on the
+  // global mutex list.
----------------
Revert


================
Comment at: source/Core/ModuleList.cpp:766-770
+  if (sysroot != nullptr)
+    resolved_module_spec.GetFileSpec().PrependPathComponent(sysroot);
+
+  module_sp.reset(new Module(resolved_module_spec));
   // Make sure there are a module and an object file since we can specify
----------------
Revert


================
Comment at: source/Target/Platform.cpp:228
         module_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
-        did_create_ptr, false);
+        did_create_ptr, false, m_sdk_sysroot.AsCString());
   return GetRemoteSharedModule(module_spec, process, module_sp,
----------------
Revert


================
Comment at: source/Target/Platform.cpp:230
   return GetRemoteSharedModule(module_spec, process, module_sp,
                                [&](const ModuleSpec &spec) {
                                  Status error = ModuleList::GetSharedModule(
----------------
Here just make a module spec that has m_sdk_sysroot prepended if m_sdk_sysroot is set, and check for that file first. If that succeeds, return that, else do the same code as before.


Repository:
  rL LLVM

https://reviews.llvm.org/D49685