llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: David Spickett (DavidSpickett) <details> <summary>Changes</summary> This fixes a bug I found where copying a file from a remote Linux platform using the API would claim that it succeeded, but it had not. The local file was read only so it was not updated. The host Linux platform's copy did not have this bug. There are two tests, one which will run if the testing platform is not already a remote. This one spawns a "remote" platform, which is actually still on the host, but it's enough to take the remote code path in the POSIX platform implementation. The second uses the testing platform. If that's a remote, use that, if it's host then we use the host platform to check local copying behaviour. This test is not as specific as I'd like it to be, but I wanted something that could run on many platforms. Even if the copy fails for a reason other than permissions, at least we are checking that the failure is propagated. --- Full diff: https://github.com/llvm/llvm-project/pull/207365.diff 2 Files Affected: - (modified) lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp (+6-4) - (modified) lldb/test/API/python_api/sbplatform/TestSBPlatform.py (+48) ``````````diff diff --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp index b301890d2b698..1a62145083f8f 100644 --- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp +++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp @@ -255,10 +255,12 @@ lldb_private::Status PlatformPOSIX::GetFile( offset += n_read; } } - // Ignore the close error of src. - if (fd_src != UINT64_MAX) - CloseFile(fd_src, error); - // And close the dst file descriptot. + if (fd_src != UINT64_MAX) { + // Ignore the close error of src. + Status close_error; + CloseFile(fd_src, close_error); + } + // And close the dst file descriptor. if (fd_dst != UINT64_MAX && !FileCache::GetInstance().CloseFile(fd_dst, error)) { if (!error.Fail()) diff --git a/lldb/test/API/python_api/sbplatform/TestSBPlatform.py b/lldb/test/API/python_api/sbplatform/TestSBPlatform.py index 3c3e268af35eb..8a5f534be4605 100644 --- a/lldb/test/API/python_api/sbplatform/TestSBPlatform.py +++ b/lldb/test/API/python_api/sbplatform/TestSBPlatform.py @@ -1,7 +1,13 @@ """Test the SBPlatform APIs.""" +import os +import socket +from pathlib import Path +import lldbgdbserverutils + from lldbsuite.test.decorators import * from lldbsuite.test.lldbtest import * +from lldbsuite.test import lldbutil @skipIfWasm # no remote platform file/process APIs @@ -52,3 +58,45 @@ def test_SetCurrentPlatform_associated(self): dbg_platform = self.dbg.GetSelectedPlatform() self.assertEqual(dbg_platform.GetName(), "remote-netbsd") self.assertEqual(dbg_platform.GetWorkingDirectory(), self.getBuildDir()) + + def do_copy_test(self, platform): + source_name = "source-file" + source_path = self.getBuildArtifact(source_name) + Path(source_path).touch() + + remote_path = lldbutil.append_to_process_working_directory(self, source_name) + put_error = platform.Put( + lldb.SBFileSpec(source_path, True), lldb.SBFileSpec(remote_path, False) + ) + self.assertSuccess(put_error) + + destination = self.getBuildArtifact("destination-file") + # Make the file read only. + Path(destination).touch(mode=0o400) + self.addTearDownHook(lambda: os.remove(destination)) + + get_error = platform.Get( + lldb.SBFileSpec(remote_path, False), lldb.SBFileSpec(destination, True) + ) + # Ideally we would check that we failed due to permissions, but as this + # runs on many platforms, we do not know the exact form of the error message. + self.assertFailure(get_error) + + @skipIfDarwin # lldb-server not found correctly + @add_test_categories(["lldb-server"]) + # If we're already remote, no need to spawn a new remote platform. + @skipIfRemote + def test_get_reports_write_failure_remote(self): + if lldb.remote_platform: + platform = lldb.remote_platform + else: + platform = lldbutil.connect_to_new_remote_platform( + self, lldbgdbserverutils.get_lldb_server_exe() + ) + + self.do_copy_test(platform) + + @skipIfDarwin # lldb-server not found correctly + @add_test_categories(["lldb-server"]) + def test_get_reports_write_failure(self): + self.do_copy_test(self.dbg.GetSelectedPlatform()) `````````` </details> https://github.com/llvm/llvm-project/pull/207365 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
