[llvm] [Flang][runtime] Fix RENAME intrinsic, remove trailing blanks (PR #159123)
Michael Klemm via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 17 05:44:40 PDT 2025
https://github.com/mjklemm updated https://github.com/llvm/llvm-project/pull/159123
>From 664be1d1b7d632f34a4265bfc747801b63e4c5f2 Mon Sep 17 00:00:00 2001
From: Michael Klemm <michael.klemm at amd.com>
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<char *>(alloca((srcTrimPos + 1) * sizeof(char)))};
+ char *dstPathTrim{
+ static_cast<char *>(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 <michael.klemm at amd.com>
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 at 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<char *>(alloca((srcTrimPos + 1) * sizeof(char)))};
char *dstPathTrim{
static_cast<char *>(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 <michael.klemm at amd.com>
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 <michael.klemm at amd.com>
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 <cstdio>
#include <cstring>
+#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<char *>(alloca((srcTrimPos + 1) * sizeof(char)))};
- char *dstPathTrim{
- static_cast<char *>(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<char *>(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<char *>(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 <michael.klemm at amd.com>
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<char *>(alloca((srcTrimPos + 1) * sizeof(char)))};
+ static_cast<char *>(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<char *>(alloca((dstTrimPos + 1) * sizeof(char)))};
+ static_cast<char *>(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 <michael.klemm at amd.com>
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 <michael.klemm at amd.com>
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<char *>(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<char *>(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 <michael.klemm at amd.com>
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 <cstdio>
#include <cstring>
-#include "../../include/flang-rt/runtime/descriptor.h"
-
namespace Fortran::runtime {
static RT_API_ATTRS void TransferImpl(Descriptor &result,
More information about the llvm-commits
mailing list