[Support] Prevent loss of file type flags when creating temporary

[tltao]

Non-binary output files from the compiler need the OF_Text flag set for encoding conversion to be performed correctly on z/OS.

I'm not sure if there's a portable way to test this, but if anyone has an idea I can add it.

[llvmbot]

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-backend-systemz

@llvm/pr-subscribers-llvm-support

Author: Tony Tao (tltao)

Changes

Non-binary output files from the compiler need the OF_Text flag set for encoding conversion to be performed correctly on z/OS.

I'm not sure if there's a portable way to test this, but if anyone has an idea I can add it.

---

Full diff: https://github.com/llvm/llvm-project/pull/167939.diff

1 Files Affected:

• (modified) llvm/lib/Support/VirtualOutputBackends.cpp (+8-1)

diff --git a/llvm/lib/Support/VirtualOutputBackends.cpp b/llvm/lib/Support/VirtualOutputBackends.cpp
index de59b8ab63a53..f44cfd419f8c7 100644
--- a/llvm/lib/Support/VirtualOutputBackends.cpp
+++ b/llvm/lib/Support/VirtualOutputBackends.cpp
@@ -269,8 +269,15 @@ Error OnDiskOutputFile::tryToCreateTemporary(std::optional<int> &FD) {
   return createDirectoriesOnDemand(OutputPath, Config, [&]() -> Error {
     int NewFD;
     SmallString<128> UniquePath;
+    sys::fs::OpenFlags OF = sys::fs::OF_None;
+    if (Config.getTextWithCRLF())
+      OF |= sys::fs::OF_TextWithCRLF;
+    else if (Config.getText())
+      OF |= sys::fs::OF_Text;
+    if (Config.getAppend())
+      OF |= sys::fs::OF_Append;
     if (std::error_code EC =
-            sys::fs::createUniqueFile(ModelPath, NewFD, UniquePath))
+            sys::fs::createUniqueFile(ModelPath, NewFD, UniquePath, OF))
       return make_error<TempFileOutputError>(ModelPath, OutputPath, EC);
 
     if (Config.getDiscardOnSignal())

[abhina-sree]

LGTM

[cachemeifyoucan]

I prefer writing a unit test and it doesn't need to be a portable test (it just needs to run on the platforms that hit this error).

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[cachemeifyoucan]

LGTM. Thanks.

[tltao]

@cachemeifyoucan There was a test failure on Windows in TEST(OnDiskBackendTest, Append) due to the assert assert((!(Disp == CD_CreateNew) || !(Flags & OF_Append)) in Path.inc. I fixed it by checking for getAtomicWrite() and not passing the OF_Append flag if true since to me, it doesn't make much sense to append to a temporary file. Does that sound reasonable or is there a better fix?

[cachemeifyoucan]

@cachemeifyoucan There was a test failure on Windows in TEST(OnDiskBackendTest, Append) due to the assert assert((!(Disp == CD_CreateNew) || !(Flags & OF_Append)) in Path.inc. I fixed it by checking for getAtomicWrite() and not passing the OF_Append flag if true since to me, it doesn't make much sense to append to a temporary file. Does that sound reasonable or is there a better fix?

Sounds reasonable. Atomic append is not OF_Append since writing to temporary. Add a comment on this will be a plus.