Skip to content

Commit

Permalink
[lldb] Fix write only file action to truncate the file (#112657)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kusmour authored Oct 29, 2024
1 parent 8e14c6c commit efc6d33
Show file tree
Hide file tree
Showing 5 changed files with 111 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lldb/source/Host/common/FileAction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
53 changes: 53 additions & 0 deletions lldb/test/API/commands/settings/TestSettings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down
30 changes: 30 additions & 0 deletions lldb/test/API/python_api/process/io/TestProcessIO.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
25 changes: 25 additions & 0 deletions lldb/unittests/Host/FileActionTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
//
//===----------------------------------------------------------------------===//

#include <fcntl.h>

#include "lldb/Host/FileAction.h"
#include "gtest/gtest.h"

Expand All @@ -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);
}
2 changes: 2 additions & 0 deletions llvm/docs/ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
---------------------------------
Expand Down

0 comments on commit efc6d33

Please sign in to comment.