[lldb] Fix write only file action to truncate the file (llvm#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

Author: kusmour

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 {

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')

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):

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);
+}

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
 ---------------------------------