From 664be1d1b7d632f34a4265bfc747801b63e4c5f2 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Tue, 16 Sep 2025 18:33:16 +0200 Subject: [PATCH 1/8] [Flang][runtime] Fix RENAME intrinsic, remove trailing blanks The RENAME intrinsic did not correctly remove trailing spaces from filenames. This PR introduces code to remove trailing blanks as documented by GFortran. --- flang-rt/lib/runtime/misc-intrinsic.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/flang-rt/lib/runtime/misc-intrinsic.cpp b/flang-rt/lib/runtime/misc-intrinsic.cpp index 4d1165f25687c..a3aa0953f776e 100644 --- a/flang-rt/lib/runtime/misc-intrinsic.cpp +++ b/flang-rt/lib/runtime/misc-intrinsic.cpp @@ -65,8 +65,20 @@ void RTDEF(Rename)(const Descriptor &path1, const Descriptor &path2, char *pathDst{EnsureNullTerminated( path2.OffsetElement(), path2.ElementBytes(), terminator)}; + // Trim trailing blanks + auto srcTrimPos{TrimTrailingSpaces(pathSrc, path1.ElementBytes())}; + auto dstTrimPos{TrimTrailingSpaces(pathDst, path2.ElementBytes())}; + char *srcPathTrim{ + static_cast(alloca((srcTrimPos + 1) * sizeof(char)))}; + char *dstPathTrim{ + static_cast(alloca((dstTrimPos + 1) * sizeof(char)))}; + std::strncpy(srcPathTrim, pathSrc, srcTrimPos); + std::strncpy(dstPathTrim, pathDst, dstTrimPos); + srcPathTrim[srcTrimPos] = '\0'; + dstPathTrim[dstTrimPos] = '\0'; + // We simply call rename(2) from POSIX - int result{rename(pathSrc, pathDst)}; + int result{rename(srcPathTrim, dstPathTrim)}; if (status) { // When an error has happened, int errorCode{0}; // Assume success From b20d0ab493ceb73f574b9c4f87493c875c499aab Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Tue, 16 Sep 2025 18:58:44 +0200 Subject: [PATCH 2/8] Update flang-rt/lib/runtime/misc-intrinsic.cpp Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- flang-rt/lib/runtime/misc-intrinsic.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flang-rt/lib/runtime/misc-intrinsic.cpp b/flang-rt/lib/runtime/misc-intrinsic.cpp index a3aa0953f776e..b8652b54aa61d 100644 --- a/flang-rt/lib/runtime/misc-intrinsic.cpp +++ b/flang-rt/lib/runtime/misc-intrinsic.cpp @@ -72,8 +72,8 @@ void RTDEF(Rename)(const Descriptor &path1, const Descriptor &path2, static_cast(alloca((srcTrimPos + 1) * sizeof(char)))}; char *dstPathTrim{ static_cast(alloca((dstTrimPos + 1) * sizeof(char)))}; - std::strncpy(srcPathTrim, pathSrc, srcTrimPos); - std::strncpy(dstPathTrim, pathDst, dstTrimPos); + std::memcpy(srcPathTrim, pathSrc, srcTrimPos); + std::memcpy(dstPathTrim, pathDst, dstTrimPos); srcPathTrim[srcTrimPos] = '\0'; dstPathTrim[dstTrimPos] = '\0'; From 6827079255dbd0183c413e9516d40d0e349024b3 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Wed, 17 Sep 2025 10:11:21 +0200 Subject: [PATCH 3/8] Introduce function IsNullTerminated --- flang-rt/include/flang-rt/runtime/tools.h | 3 +++ flang-rt/lib/runtime/tools.cpp | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/flang-rt/include/flang-rt/runtime/tools.h b/flang-rt/include/flang-rt/runtime/tools.h index 1939c4d907be4..648d32e78ac00 100644 --- a/flang-rt/include/flang-rt/runtime/tools.h +++ b/flang-rt/include/flang-rt/runtime/tools.h @@ -525,6 +525,9 @@ RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from, bool toIsContiguous, bool fromIsContiguous); RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from); +// Determines if a character string is null-terminated. +RT_API_ATTRS bool IsNullTerminated(char *str, std::size_t length); + // Ensures that a character string is null-terminated, allocating a /p length +1 // size memory for null-terminator if necessary. Returns the original or a newly // allocated null-terminated string (responsibility for deallocation is on the diff --git a/flang-rt/lib/runtime/tools.cpp b/flang-rt/lib/runtime/tools.cpp index 03ee982d913bb..b687a8088fdfa 100644 --- a/flang-rt/lib/runtime/tools.cpp +++ b/flang-rt/lib/runtime/tools.cpp @@ -273,9 +273,13 @@ RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from) { ShallowCopy(to, from, to.IsContiguous(), from.IsContiguous()); } +RT_API_ATTRS bool IsNullTerminated(char *str, std::size_t length) { + return !(runtime::memchr(str, '\0', length) == nullptr); +} + RT_API_ATTRS char *EnsureNullTerminated( char *str, std::size_t length, Terminator &terminator) { - if (runtime::memchr(str, '\0', length) == nullptr) { + if (!IsNullTerminated(str, length)) { char *newCmd{(char *)AllocateMemoryOrCrash(terminator, length + 1)}; runtime::memcpy(newCmd, str, length); newCmd[length] = '\0'; From 8b60514096adedf9fb937d9503b913714a5ad35d Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Wed, 17 Sep 2025 10:18:51 +0200 Subject: [PATCH 4/8] Add proper handling of null-terminated character strings --- flang-rt/lib/runtime/misc-intrinsic.cpp | 35 ++++++++++++++++--------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/flang-rt/lib/runtime/misc-intrinsic.cpp b/flang-rt/lib/runtime/misc-intrinsic.cpp index b8652b54aa61d..48fd54670854d 100644 --- a/flang-rt/lib/runtime/misc-intrinsic.cpp +++ b/flang-rt/lib/runtime/misc-intrinsic.cpp @@ -15,6 +15,8 @@ #include #include +#include "../../include/flang-rt/runtime/descriptor.h" + namespace Fortran::runtime { static RT_API_ATTRS void TransferImpl(Descriptor &result, @@ -60,25 +62,34 @@ void RTDEF(Rename)(const Descriptor &path1, const Descriptor &path2, const Descriptor *status, const char *sourceFile, int line) { Terminator terminator{sourceFile, line}; #if !defined(RT_DEVICE_COMPILATION) + // Get the raw strings (null-terminated) char *pathSrc{EnsureNullTerminated( path1.OffsetElement(), path1.ElementBytes(), terminator)}; char *pathDst{EnsureNullTerminated( path2.OffsetElement(), path2.ElementBytes(), terminator)}; + char *srcFilePath = pathSrc; + char *dstFilePath = pathDst; - // Trim trailing blanks - auto srcTrimPos{TrimTrailingSpaces(pathSrc, path1.ElementBytes())}; - auto dstTrimPos{TrimTrailingSpaces(pathDst, path2.ElementBytes())}; - char *srcPathTrim{ - static_cast(alloca((srcTrimPos + 1) * sizeof(char)))}; - char *dstPathTrim{ - static_cast(alloca((dstTrimPos + 1) * sizeof(char)))}; - std::memcpy(srcPathTrim, pathSrc, srcTrimPos); - std::memcpy(dstPathTrim, pathDst, dstTrimPos); - srcPathTrim[srcTrimPos] = '\0'; - dstPathTrim[dstTrimPos] = '\0'; + // Trim trailing blanks (if string have not been null-terminated) + if (!IsNullTerminated(path1.OffsetElement(), path1.ElementBytes())) { + auto srcTrimPos{TrimTrailingSpaces(pathSrc, path1.ElementBytes())}; + char *srcPathTrim{ + static_cast(alloca((srcTrimPos + 1) * sizeof(char)))}; + std::memcpy(srcPathTrim, pathSrc, srcTrimPos); + srcPathTrim[srcTrimPos] = '\0'; + srcFilePath = srcPathTrim; + } + if (!IsNullTerminated(path2.OffsetElement(), path2.ElementBytes())) { + auto dstTrimPos{TrimTrailingSpaces(pathDst, path2.ElementBytes())}; + char *dstPathTrim{ + static_cast(alloca((dstTrimPos + 1) * sizeof(char)))}; + std::memcpy(dstPathTrim, pathDst, dstTrimPos); + dstPathTrim[dstTrimPos] = '\0'; + dstFilePath = dstPathTrim; + } // We simply call rename(2) from POSIX - int result{rename(srcPathTrim, dstPathTrim)}; + int result{rename(srcFilePath, dstFilePath)}; if (status) { // When an error has happened, int errorCode{0}; // Assume success From 8e9305ed9e023e1b53c7efd800d503efafe908a0 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Wed, 17 Sep 2025 10:24:24 +0200 Subject: [PATCH 5/8] Remove sizeof --- flang-rt/lib/runtime/misc-intrinsic.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flang-rt/lib/runtime/misc-intrinsic.cpp b/flang-rt/lib/runtime/misc-intrinsic.cpp index 48fd54670854d..371c75fe2e116 100644 --- a/flang-rt/lib/runtime/misc-intrinsic.cpp +++ b/flang-rt/lib/runtime/misc-intrinsic.cpp @@ -74,7 +74,7 @@ void RTDEF(Rename)(const Descriptor &path1, const Descriptor &path2, if (!IsNullTerminated(path1.OffsetElement(), path1.ElementBytes())) { auto srcTrimPos{TrimTrailingSpaces(pathSrc, path1.ElementBytes())}; char *srcPathTrim{ - static_cast(alloca((srcTrimPos + 1) * sizeof(char)))}; + static_cast(alloca((srcTrimPos + 1)))}; std::memcpy(srcPathTrim, pathSrc, srcTrimPos); srcPathTrim[srcTrimPos] = '\0'; srcFilePath = srcPathTrim; @@ -82,7 +82,7 @@ void RTDEF(Rename)(const Descriptor &path1, const Descriptor &path2, if (!IsNullTerminated(path2.OffsetElement(), path2.ElementBytes())) { auto dstTrimPos{TrimTrailingSpaces(pathDst, path2.ElementBytes())}; char *dstPathTrim{ - static_cast(alloca((dstTrimPos + 1) * sizeof(char)))}; + static_cast(alloca((dstTrimPos + 1)))}; std::memcpy(dstPathTrim, pathDst, dstTrimPos); dstPathTrim[dstTrimPos] = '\0'; dstFilePath = dstPathTrim; From 58cbad69638445a0700b502b7c8bab903b15d981 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Wed, 17 Sep 2025 14:32:01 +0200 Subject: [PATCH 6/8] Revert "Introduce function IsNullTerminated" This reverts commit 6827079255dbd0183c413e9516d40d0e349024b3. --- flang-rt/include/flang-rt/runtime/tools.h | 3 --- flang-rt/lib/runtime/tools.cpp | 6 +----- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/flang-rt/include/flang-rt/runtime/tools.h b/flang-rt/include/flang-rt/runtime/tools.h index 648d32e78ac00..1939c4d907be4 100644 --- a/flang-rt/include/flang-rt/runtime/tools.h +++ b/flang-rt/include/flang-rt/runtime/tools.h @@ -525,9 +525,6 @@ RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from, bool toIsContiguous, bool fromIsContiguous); RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from); -// Determines if a character string is null-terminated. -RT_API_ATTRS bool IsNullTerminated(char *str, std::size_t length); - // Ensures that a character string is null-terminated, allocating a /p length +1 // size memory for null-terminator if necessary. Returns the original or a newly // allocated null-terminated string (responsibility for deallocation is on the diff --git a/flang-rt/lib/runtime/tools.cpp b/flang-rt/lib/runtime/tools.cpp index b687a8088fdfa..03ee982d913bb 100644 --- a/flang-rt/lib/runtime/tools.cpp +++ b/flang-rt/lib/runtime/tools.cpp @@ -273,13 +273,9 @@ RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from) { ShallowCopy(to, from, to.IsContiguous(), from.IsContiguous()); } -RT_API_ATTRS bool IsNullTerminated(char *str, std::size_t length) { - return !(runtime::memchr(str, '\0', length) == nullptr); -} - RT_API_ATTRS char *EnsureNullTerminated( char *str, std::size_t length, Terminator &terminator) { - if (!IsNullTerminated(str, length)) { + if (runtime::memchr(str, '\0', length) == nullptr) { char *newCmd{(char *)AllocateMemoryOrCrash(terminator, length + 1)}; runtime::memcpy(newCmd, str, length); newCmd[length] = '\0'; From b5101da74f363b28974a74514bcca32434d2773a Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Wed, 17 Sep 2025 14:36:14 +0200 Subject: [PATCH 7/8] Simplify code Simplification suggested by @jeanPerier --- flang-rt/lib/runtime/misc-intrinsic.cpp | 49 +++++++------------------ 1 file changed, 14 insertions(+), 35 deletions(-) diff --git a/flang-rt/lib/runtime/misc-intrinsic.cpp b/flang-rt/lib/runtime/misc-intrinsic.cpp index 371c75fe2e116..d87c914b2c9d7 100644 --- a/flang-rt/lib/runtime/misc-intrinsic.cpp +++ b/flang-rt/lib/runtime/misc-intrinsic.cpp @@ -61,35 +61,22 @@ RT_EXT_API_GROUP_BEGIN void RTDEF(Rename)(const Descriptor &path1, const Descriptor &path2, const Descriptor *status, const char *sourceFile, int line) { Terminator terminator{sourceFile, line}; -#if !defined(RT_DEVICE_COMPILATION) - // Get the raw strings (null-terminated) - char *pathSrc{EnsureNullTerminated( - path1.OffsetElement(), path1.ElementBytes(), terminator)}; - char *pathDst{EnsureNullTerminated( - path2.OffsetElement(), path2.ElementBytes(), terminator)}; - char *srcFilePath = pathSrc; - char *dstFilePath = pathDst; - // Trim trailing blanks (if string have not been null-terminated) - if (!IsNullTerminated(path1.OffsetElement(), path1.ElementBytes())) { - auto srcTrimPos{TrimTrailingSpaces(pathSrc, path1.ElementBytes())}; - char *srcPathTrim{ - static_cast(alloca((srcTrimPos + 1)))}; - std::memcpy(srcPathTrim, pathSrc, srcTrimPos); - srcPathTrim[srcTrimPos] = '\0'; - srcFilePath = srcPathTrim; - } - if (!IsNullTerminated(path2.OffsetElement(), path2.ElementBytes())) { - auto dstTrimPos{TrimTrailingSpaces(pathDst, path2.ElementBytes())}; - char *dstPathTrim{ - static_cast(alloca((dstTrimPos + 1)))}; - std::memcpy(dstPathTrim, pathDst, dstTrimPos); - dstPathTrim[dstTrimPos] = '\0'; - dstFilePath = dstPathTrim; - } + // Semantics for character strings: A null character (CHAR(0)) can be used to + // mark the end of the names in PATH1 and PATH2; otherwise, trailing blanks in + // the file names are ignored. + // (https://gcc.gnu.org/onlinedocs/gfortran/RENAME.html) +#if !defined(RT_DEVICE_COMPILATION) + // Trim tailing spaces, respect presences of null character when doing so. + auto pathSrc{SaveDefaultCharacter(path1.OffsetElement(), + TrimTrailingSpaces(path1.OffsetElement(), path1.ElementBytes()), + terminator)}; + auto pathDst{SaveDefaultCharacter(path2.OffsetElement(), + TrimTrailingSpaces(path2.OffsetElement(), path2.ElementBytes()), + terminator)}; - // We simply call rename(2) from POSIX - int result{rename(srcFilePath, dstFilePath)}; + // We can now simply call rename(2) from POSIX. + int result{rename(pathSrc.get(), pathDst.get())}; if (status) { // When an error has happened, int errorCode{0}; // Assume success @@ -99,14 +86,6 @@ void RTDEF(Rename)(const Descriptor &path1, const Descriptor &path2, } StoreIntToDescriptor(status, errorCode, terminator); } - - // Deallocate memory if EnsureNullTerminated dynamically allocated memory - if (pathSrc != path1.OffsetElement()) { - FreeMemory(pathSrc); - } - if (pathDst != path2.OffsetElement()) { - FreeMemory(pathDst); - } #else // !defined(RT_DEVICE_COMPILATION) terminator.Crash("RENAME intrinsic is only supported on host devices"); #endif // !defined(RT_DEVICE_COMPILATION) From 14cbc44b54336eb968d00ca1b6b923b0b5544272 Mon Sep 17 00:00:00 2001 From: Michael Klemm Date: Wed, 17 Sep 2025 14:42:24 +0200 Subject: [PATCH 8/8] Cleanup --- flang-rt/lib/runtime/misc-intrinsic.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/flang-rt/lib/runtime/misc-intrinsic.cpp b/flang-rt/lib/runtime/misc-intrinsic.cpp index d87c914b2c9d7..3812c990cd946 100644 --- a/flang-rt/lib/runtime/misc-intrinsic.cpp +++ b/flang-rt/lib/runtime/misc-intrinsic.cpp @@ -15,8 +15,6 @@ #include #include -#include "../../include/flang-rt/runtime/descriptor.h" - namespace Fortran::runtime { static RT_API_ATTRS void TransferImpl(Descriptor &result,