[Lldb-commits] [lldb] [llvm] [lldb] Fix write only file action to truncate the file (PR #112657)
via lldb-commits
lldb-commits at lists.llvm.org
Thu Oct 24 20:28:36 PDT 2024
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 <wanyi at 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
---------------------------------
More information about the lldb-commits
mailing list