https://github.com/kusmour updated https://github.com/llvm/llvm-project/pull/112657
>From 6ee1560ed4c0c2e77943b90fdf97523309f1e754 Mon Sep 17 00:00:00 2001 From: Wanyi Ye <wa...@meta.com> Date: Mon, 14 Oct 2024 22:37:50 -0700 Subject: [PATCH] [lldb] Fix write only file action to truncate the file --- lldb/source/Host/common/FileAction.cpp | 2 +- .../API/commands/settings/TestSettings.py | 53 +++++++++++++++++++ .../python_api/process/io/TestProcessIO.py | 30 +++++++++++ lldb/unittests/Host/FileActionTest.cpp | 25 +++++++++ llvm/docs/ReleaseNotes.md | 2 + 5 files changed, 111 insertions(+), 1 deletion(-) diff --git a/lldb/source/Host/common/FileAction.cpp b/lldb/source/Host/common/FileAction.cpp index f980d3224640e0..e1c3e14a165ea9 100644 --- a/lldb/source/Host/common/FileAction.cpp +++ b/lldb/source/Host/common/FileAction.cpp @@ -41,7 +41,7 @@ bool FileAction::Open(int fd, const FileSpec &file_spec, bool read, else if (read) m_arg = O_NOCTTY | O_RDONLY; else - m_arg = O_NOCTTY | O_CREAT | O_WRONLY; + m_arg = O_NOCTTY | O_CREAT | O_WRONLY | O_TRUNC; m_file_spec = file_spec; return true; } else { diff --git a/lldb/test/API/commands/settings/TestSettings.py b/lldb/test/API/commands/settings/TestSettings.py index 385acceb7a8b5c..2dd813f6b155b3 100644 --- a/lldb/test/API/commands/settings/TestSettings.py +++ b/lldb/test/API/commands/settings/TestSettings.py @@ -528,6 +528,59 @@ def test_set_error_output_path(self): output, exe=False, startstr="This message should go to standard out." ) + @skipIfDarwinEmbedded # <rdar://problem/34446098> debugserver on ios etc can't write files + def test_same_error_output_path(self): + """Test that setting target.error and output-path to the same file path for the launched process works.""" + self.build() + + exe = self.getBuildArtifact("a.out") + self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET) + + # Set the error-path and output-path and verify both are set. + self.runCmd( + "settings set target.error-path '{0}'".format( + lldbutil.append_to_process_working_directory(self, "output.txt") + ) + ) + self.runCmd( + "settings set target.output-path '{0}".format( + lldbutil.append_to_process_working_directory(self, "output.txt") + ) + ) + # And add hooks to restore the original settings during tearDown(). + self.addTearDownHook(lambda: self.runCmd("settings clear target.output-path")) + self.addTearDownHook(lambda: self.runCmd("settings clear target.error-path")) + + self.expect( + "settings show target.error-path", + SETTING_MSG("target.error-path"), + substrs=["target.error-path (file)", 'output.txt"'], + ) + + self.expect( + "settings show target.output-path", + SETTING_MSG("target.output-path"), + substrs=["target.output-path (file)", 'output.txt"'], + ) + + self.runCmd( + "process launch --working-dir '{0}'".format( + self.get_process_working_directory() + ), + RUN_SUCCEEDED, + ) + + output = lldbutil.read_file_from_process_wd(self, "output.txt") + err_message = "This message should go to standard error." + out_message = "This message should go to standard out." + # Error msg should get flushed by the output msg + self.expect(output, exe=False, substrs=[out_message]) + self.assertNotIn( + err_message, + output, + "Race condition when both stderr/stdout redirects to the same file", + ) + def test_print_dictionary_setting(self): self.runCmd("settings clear target.env-vars") self.runCmd('settings set target.env-vars ["MY_VAR"]=some-value') diff --git a/lldb/test/API/python_api/process/io/TestProcessIO.py b/lldb/test/API/python_api/process/io/TestProcessIO.py index 5bb91d2758312d..3b5c7c48c51f4d 100644 --- a/lldb/test/API/python_api/process/io/TestProcessIO.py +++ b/lldb/test/API/python_api/process/io/TestProcessIO.py @@ -95,6 +95,36 @@ def test_stdout_stderr_redirection(self): error = self.read_error_file_and_delete() self.check_process_output(output, error) + @skipIfWindows # stdio manipulation unsupported on Windows + @expectedFlakeyLinux(bugnumber="llvm.org/pr26437") + @skipIfDarwinEmbedded # debugserver can't create/write files on the device + def test_stdout_stderr_redirection_to_existing_files(self): + """Exercise SBLaunchInfo::AddOpenFileAction() for STDOUT and STDERR without redirecting STDIN to output files already exist.""" + self.setup_test() + self.build() + self.create_target() + self.write_file_with_placeholder(self.output_file) + self.write_file_with_placeholder(self.error_file) + self.redirect_stdout() + self.redirect_stderr() + self.run_process(True) + output = self.read_output_file_and_delete() + error = self.read_error_file_and_delete() + self.check_process_output(output, error) + + def write_file_with_placeholder(self, target_file): + placeholder = "This content should be overwritten." + if lldb.remote_platform: + self.runCmd( + 'platform file write "{target}" -d "{data}"'.format( + target=target_file, data=placeholder + ) + ) + else: + f = open(target_file, "w") + f.write(placeholder) + f.close() + # target_file - path on local file system or remote file system if running remote # local_file - path on local system def read_file_and_delete(self, target_file, local_file): diff --git a/lldb/unittests/Host/FileActionTest.cpp b/lldb/unittests/Host/FileActionTest.cpp index b208169aac20e6..3d2c722552c9c2 100644 --- a/lldb/unittests/Host/FileActionTest.cpp +++ b/lldb/unittests/Host/FileActionTest.cpp @@ -6,6 +6,8 @@ // //===----------------------------------------------------------------------===// +#include <fcntl.h> + #include "lldb/Host/FileAction.h" #include "gtest/gtest.h" @@ -17,3 +19,26 @@ TEST(FileActionTest, Open) { EXPECT_EQ(Action.GetAction(), FileAction::eFileActionOpen); EXPECT_EQ(Action.GetFileSpec(), FileSpec("/tmp")); } + +TEST(FileActionTest, OpenReadWrite) { + FileAction Action; + Action.Open(48, FileSpec("/tmp_0"), /*read*/ true, /*write*/ true); + EXPECT_TRUE(Action.GetActionArgument() & (O_NOCTTY | O_CREAT | O_RDWR)); + EXPECT_FALSE(Action.GetActionArgument() & O_RDONLY); + EXPECT_FALSE(Action.GetActionArgument() & O_WRONLY); +} + +TEST(FileActionTest, OpenReadOnly) { + FileAction Action; + Action.Open(49, FileSpec("/tmp_1"), /*read*/ true, /*write*/ false); + EXPECT_TRUE(Action.GetActionArgument() & (O_NOCTTY | O_RDONLY)); + EXPECT_FALSE(Action.GetActionArgument() & O_WRONLY); +} + +TEST(FileActionTest, OpenWriteOnly) { + FileAction Action; + Action.Open(50, FileSpec("/tmp_2"), /*read*/ false, /*write*/ true); + EXPECT_TRUE(Action.GetActionArgument() & + (O_NOCTTY | O_CREAT | O_WRONLY | O_TRUNC)); + EXPECT_FALSE(Action.GetActionArgument() & O_RDONLY); +} diff --git a/llvm/docs/ReleaseNotes.md b/llvm/docs/ReleaseNotes.md index 706546980cf671..e47f4494a3637a 100644 --- a/llvm/docs/ReleaseNotes.md +++ b/llvm/docs/ReleaseNotes.md @@ -273,6 +273,8 @@ Changes to LLDB * LLDB can now read the `fpmr` register from AArch64 Linux processes and core files. +* Program stdout/stderr redirection will now open the file with O_TRUNC flag, make sure to truncate the file if path already exists. + * eg. `settings set target.output-path/target.error-path <path/to/file>` Changes to BOLT --------------------------------- _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits