[llvm] efc6d33 - [lldb] Fix write only file action to truncate the file (#112657)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 29 11:22:56 PDT 2024
Author: Wanyi
Date: 2024-10-29T14:22:51-04:00
New Revision: efc6d33be9f4b4d0f0e8d3d5f198f2616b75792b
URL: https://github.com/llvm/llvm-project/commit/efc6d33be9f4b4d0f0e8d3d5f198f2616b75792b
DIFF: https://github.com/llvm/llvm-project/commit/efc6d33be9f4b4d0f0e8d3d5f198f2616b75792b.diff
LOG: [lldb] Fix write only file action to truncate the file (#112657)
When `FileAction` opens file with write access, it doesn't clear the
file nor append to the end of the file if it already exists. Instead, it
writes from cursor index 0.
For example, by using the settings `target.output-path` and
`target.error-path`, lldb will redirect process stdout/stderr to files.
It then calls this function to write to the files which the above
symptoms appear.
## Test
- Added unit test checking the file flags
- Added 2 api tests checking
- File content overwritten if the file path already exists
- Stdout and stderr redirection to the same file doesn't change its
behavior
Added:
Modified:
lldb/source/Host/common/FileAction.cpp
lldb/test/API/commands/settings/TestSettings.py
lldb/test/API/python_api/process/io/TestProcessIO.py
lldb/unittests/Host/FileActionTest.cpp
llvm/docs/ReleaseNotes.md
Removed:
################################################################################
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 92a45d845f1db8..d5c650e74eeb28 100644
--- a/llvm/docs/ReleaseNotes.md
+++ b/llvm/docs/ReleaseNotes.md
@@ -301,6 +301,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 llvm-commits
mailing list