https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/207365
>From 97f54757e2cfac2dbd5d065a6f13ade2867b0b51 Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Fri, 19 Jun 2026 15:20:59 +0000 Subject: [PATCH 1/2] [lldb][POSIX] Correctly report error copying from remote platform 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. This change fixes that bug and adds tests for platform get file. 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, fine, if it's host, then we check the local behaviour. (host Linux platform uses a different copy method which does not have this bug) 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. --- .../Plugins/Platform/POSIX/PlatformPOSIX.cpp | 10 ++-- .../python_api/sbplatform/TestSBPlatform.py | 48 +++++++++++++++++++ 2 files changed, 54 insertions(+), 4 deletions(-) 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()) >From a353e87615cc07e189056b6c11c2f540fcfd76ea Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Fri, 3 Jul 2026 12:46:09 +0000 Subject: [PATCH 2/2] change permissions before delete, potentially fix windows --- lldb/test/API/python_api/sbplatform/TestSBPlatform.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/python_api/sbplatform/TestSBPlatform.py b/lldb/test/API/python_api/sbplatform/TestSBPlatform.py index 8a5f534be4605..3d9bd315d6493 100644 --- a/lldb/test/API/python_api/sbplatform/TestSBPlatform.py +++ b/lldb/test/API/python_api/sbplatform/TestSBPlatform.py @@ -2,6 +2,7 @@ import os import socket +import stat from pathlib import Path import lldbgdbserverutils @@ -72,8 +73,13 @@ def do_copy_test(self, platform): destination = self.getBuildArtifact("destination-file") # Make the file read only. - Path(destination).touch(mode=0o400) - self.addTearDownHook(lambda: os.remove(destination)) + Path(destination).touch(mode=stat.S_IREAD, exist_ok=True) + + def remove_destination_file(): + os.chmod(destination, stat.S_IWRITE) + os.remove(destination) + + self.addTearDownHook(remove_destination_file) get_error = platform.Get( lldb.SBFileSpec(remote_path, False), lldb.SBFileSpec(destination, True) _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
