Prefer executable files from sysroot over files from local filesystem

[yuri]

Summary

In D49685 sysroot behaviour was partially fixed. But files from local filesystem with same path still has priority over files from sysroot.

This patch fixes it by removing fallback to local filesystem from RemoteAwarePlatform::GetModuleSpec(). It is not actually required because higher level code do such fallback itself. See, for example, resolver in Platform::GetSharedModule().

[yuri]

Build failure is a bug of tidy script and is not caused by the patch itself.

/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/lldb/source/Target/RemoteAwarePlatform.cpp:9:10: error: 'lldb/Target/RemoteAwarePlatform.h' file not found [clang-diagnostic-error]
#include "lldb/Target/RemoteAwarePlatform.h"
         ^

[labath]

Thanks for doing this. There are so many levels at which the search for modules is retried and I am glad that we can solve this problem by getting rid of one of them. Unfortunately, all of that complexity also means it's hard to guarantee that this won't break anything, but this patch is so simple, I think it's worth giving it a shot...

I just have a question about the test case inline...

lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py

221

Would it be possible to make this test not depend on the existence of /bin/sh? Maybe if you place the "wrong" file in $BUILD/a.out, and the "right" file at $BUILD/sysroot/$BUILD/a.out and then check that the "right" file was selected?

I guess that will require creating a new core file with a sufficiently long path so that it can be replaced by the right runtime value.

[yuri]

lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py

221

Yes, it's possible.
Can you suggest how to upload binary file into review? Looks like git diff with --binary flag is not supported.

[yuri]

Do not depend on existance of /bin/sh anymore

[yuri]

@labath, please take a look

[labath]

Sorry, this got burried in my inbox. I think this looks good. I don't believe this test will pass on windows, because of the different path styles. I don't think it's possible to meaningfully test this on windows, as it requires the path in the core file to match a host path. So, I think you can just slap @skipIfWindows on the test.

Do you have commit access?

[yuri]

Added @skipIfWindows

[yuri]

I don't believe this test will pass on windows, because of the different path styles. I don't think it's possible to meaningfully test this on windows, as it requires the path in the core file to match a host path. So, I think you can just slap @skipIfWindows on the test.

Added

Do you have commit access?

No, I don't. Please commit this change

lldb/source/Target/RemoteAwarePlatform.cpp

 
 bool RemoteAwarePlatform::GetModuleSpec(const FileSpec &module_file_spec,
                                         const ArchSpec &arch,
                                         ModuleSpec &module_spec) {
   if (m_remote_platform_sp)
     return m_remote_platform_sp->GetModuleSpec(module_file_spec, arch,
                                                module_spec);
 
-  return Platform::GetModuleSpec(module_file_spec, arch, module_spec);
+  return false;
 }
 
 Status RemoteAwarePlatform::RunShellCommand(
     const char *command, const FileSpec &working_dir, int *status_ptr,
     int *signo_ptr, std::string *command_output,
     const Timeout<std::micro> &timeout) {
   if (IsHost())
     return Host::RunShellCommand(command, working_dir, status_ptr, signo_ptr,

lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py

def test_i386_sysroot(self):
         process = target.LoadCore("linux-i386.core")
 
         # Check that we found a.out from the sysroot
         self.check_all(process, self._i386_pid, self._i386_regions, "a.out")
 
         self.dbg.DeleteTarget(target)
 
     @skipIf(triple='^mips')
+    @skipIfLLVMTargetMissing("X86")
+    @skipIfWindows
+    def test_x86_64_sysroot(self):
+        """Test that sysroot has more priority then local filesystem."""
+
+        # Copy wrong executable to the location outside of sysroot
+        exe_outside = os.path.join(self.getBuildDir(), "bin", "a.out")
+        lldbutil.mkdir_p(os.path.dirname(exe_outside))
+        shutil.copyfile("altmain.out", exe_outside)
+
+        # Copy correct executable to the location inside sysroot
+        tmp_sysroot = os.path.join(self.getBuildDir(), "mock_sysroot")
+        exe_inside = os.path.join(tmp_sysroot, os.path.relpath(exe_outside, "/"))
+        lldbutil.mkdir_p(os.path.dirname(exe_inside))
+        shutil.copyfile("linux-x86_64.out", exe_inside)
+
+        # Prepare patched core file
+        core_file = os.path.join(self.getBuildDir(), "patched.core")
+        with open("linux-x86_64.core", "rb") as f:
+            core = f.read()
+        core = replace_path(core, "/test" * 817 + "/a.out", exe_outside)
+        with open(core_file, "wb") as f:
+            f.write(core)
+
+        # Set sysroot and load core
+        self.runCmd("platform select remote-linux --sysroot '%s'" % tmp_sysroot)
+        target = self.dbg.CreateTarget(None)
+        self.assertTrue(target, VALID_TARGET)
+        process = target.LoadCore(core_file)
+
+        # Check that we found executable from the sysroot
+        mod_path = str(target.GetModuleAtIndex(0).GetFileSpec())
+        self.assertEqual(mod_path, exe_inside)
+        self.check_all(process, self._x86_64_pid, self._x86_64_regions, "a.out")
+
+        self.dbg.DeleteTarget(target)
+
+    @skipIf(triple='^mips')
     @skipIfLLVMTargetMissing("ARM")
     def test_arm_core(self):
         # check 32 bit ARM core file
         target = self.dbg.CreateTarget(None)
         self.assertTrue(target, VALID_TARGET)
         process = target.LoadCore("linux-arm.core")
 
         values = {}
class LinuxCoreTestCase(TestBase):
 
     def do_test(self, filename, pid, region_count, thread_name):
         target = self.dbg.CreateTarget(filename + ".out")
         process = target.LoadCore(filename + ".core")
 
         self.check_all(process, pid, region_count, thread_name)
 
         self.dbg.DeleteTarget(target)
+
+def replace_path(binary, replace_from, replace_to):
+    src = replace_from.encode()
+    dst = replace_to.encode()
+    dst += b"\0" * (len(src) - len(dst))
+    return binary.replace(src, dst)