[flang-commits] [flang] [flang] Fix const cast issue in FreeMemory function call in execute_command_line (PR #77906)

[Yi Wu via flang-commits]

https://github.com/yi-wu-arm updated https://github.com/llvm/llvm-project/pull/77906

>From 2a36ab7f05771662c361a301f02039757694151a Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Fri, 12 Jan 2024 11:01:06 +0000
Subject: [PATCH 1/4] Fix const cast issue in FreeMemory function call

The FreeMemory function only accepts a void pointer,
but it was being called with a const char pointer,
resulting in a type-casting issue.
To address this, the commit replaces the direct cast with a
const_cast<void *>(static_cast<const void *>(newCmd).
---
 flang/runtime/execute.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp
index 48773ae8114b0b..fa2b7eca0dde69 100644
--- a/flang/runtime/execute.cpp
+++ b/flang/runtime/execute.cpp
@@ -199,7 +199,7 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait,
   }
   // Deallocate memory if EnsureNullTerminated dynamically allocated memory
   if (newCmd != command.OffsetElement()) {
-    FreeMemory((void *)newCmd);
+    FreeMemory(const_cast<void *>(static_cast<const void *>(newCmd)));
   }
 }
 

>From 0525df1ba81764c10c6f0f4ab1d7a6d616430c6a Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Fri, 12 Jan 2024 13:40:43 +0000
Subject: [PATCH 2/4] remove const

---
 flang/runtime/execute.cpp | 4 ++--
 flang/runtime/tools.cpp   | 4 ++--
 flang/runtime/tools.h     | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp
index fa2b7eca0dde69..3a818670c96452 100644
--- a/flang/runtime/execute.cpp
+++ b/flang/runtime/execute.cpp
@@ -116,7 +116,7 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait,
     const Descriptor *exitstat, const Descriptor *cmdstat,
     const Descriptor *cmdmsg, const char *sourceFile, int line) {
   Terminator terminator{sourceFile, line};
-  const char *newCmd{EnsureNullTerminated(
+  char *newCmd{EnsureNullTerminated(
       command.OffsetElement(), command.ElementBytes(), terminator)};
 
   if (exitstat) {
@@ -199,7 +199,7 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait,
   }
   // Deallocate memory if EnsureNullTerminated dynamically allocated memory
   if (newCmd != command.OffsetElement()) {
-    FreeMemory(const_cast<void *>(static_cast<const void *>(newCmd)));
+    FreeMemory((void *)newCmd);
   }
 }
 
diff --git a/flang/runtime/tools.cpp b/flang/runtime/tools.cpp
index 6d2d86586c5fe6..e653323ed1de03 100644
--- a/flang/runtime/tools.cpp
+++ b/flang/runtime/tools.cpp
@@ -173,8 +173,8 @@ RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from) {
   ShallowCopy(to, from, to.IsContiguous(), from.IsContiguous());
 }
 
-RT_API_ATTRS const char *EnsureNullTerminated(
-    const char *str, std::size_t length, Terminator &terminator) {
+RT_API_ATTRS char *EnsureNullTerminated(
+    char *str, std::size_t length, Terminator &terminator) {
   if (std::memchr(str, '\0', length) == nullptr) {
     char *newCmd{(char *)AllocateMemoryOrCrash(terminator, length + 1)};
     std::memcpy(newCmd, str, length);
diff --git a/flang/runtime/tools.h b/flang/runtime/tools.h
index 47398a910ce73d..89e5069995748b 100644
--- a/flang/runtime/tools.h
+++ b/flang/runtime/tools.h
@@ -441,8 +441,8 @@ RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from);
 // size memory for null-terminator if necessary. Returns the original or a newly
 // allocated null-terminated string (responsibility for deallocation is on the
 // caller).
-RT_API_ATTRS const char *EnsureNullTerminated(
-    const char *str, std::size_t length, Terminator &terminator);
+RT_API_ATTRS char *EnsureNullTerminated(
+    char *str, std::size_t length, Terminator &terminator);
 
 RT_API_ATTRS bool IsValidCharDescriptor(const Descriptor *value);
 

>From 25d7a795387741e3dbf4c0906d56d1edcf8333b4 Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Fri, 12 Jan 2024 14:29:33 +0000
Subject: [PATCH 3/4] doc fixes, add more detail to sync and async execution

---
 flang/docs/Intrinsics.md | 58 +++++++++++++++++++++++++++-------------
 1 file changed, 40 insertions(+), 18 deletions(-)

diff --git a/flang/docs/Intrinsics.md b/flang/docs/Intrinsics.md
index 190d98569c8656..20fd1dfe9beb61 100644
--- a/flang/docs/Intrinsics.md
+++ b/flang/docs/Intrinsics.md
@@ -847,7 +847,7 @@ used in constant expressions have currently no folding support at all.
 
 #### Usage and Info
 
-- **Standard:** Fortran 2008 and later, specified in 16.9.73
+- **Standard:** Fortran 2008 and later, specified in Section 16.9.73
 - **Class:** Subroutine
 - **Syntax:** `CALL EXECUTE_COMMAND_LINE(COMMAND [, WAIT, EXITSTAT, CMDSTAT, CMDMSG ])`
 - **Arguments:**
@@ -862,28 +862,50 @@ used in constant expressions have currently no folding support at all.
 
 #### Implementation Specifics
 
-- **`COMMAND`:**
-  - Must be preset.
+##### `COMMAND`:
 
-- **`WAIT`:**
-  - If set to `false`, the command is executed asynchronously. If not preset or set to `false`, it is executed synchronously.
-  - Sync: achieved by passing command into `std::system` on all systems.
-  - Async: achieved by calling a `fork()` on POSIX-compatible systems, or `CreateProcess()` on Windows.
+- Must be preset.
 
-- **`CMDSTAT`:**
+##### `WAIT`:
+
+- If set to `false`, the command is executed asynchronously.
+- If not preset or set to `true`, it is executed synchronously.
+- Synchronous execution is achieved by passing the command into `std::system` on all systems.
+- Asynchronous execution is achieved by calling `fork()` on POSIX-compatible systems or `CreateProcess()` on Windows.
+
+##### `EXITSTAT`:
+
+- Synchronous execution:
+  - Inferred by the return value of `std::system(cmd)`.
+    - On POSIX-compatible systems: return value is first passed into `WEXITSTATUS(status)`, then assigned to `EXITSTAT`.
+    - On Windows, the value is directly assigned as the return value of `std::system()`.
+- Asynchronous execution:
+  - Value is not modified.
+
+##### `CMDSTAT`:
+
+- Synchronous execution:
   - -2: No error condition occurs, but `WAIT` is present with the value `false`, and the processor does not support asynchronous execution.
   - -1: The processor does not support command line execution.
   - \+ (positive value): An error condition occurs.
-    - 1: Fork Error, where `pid_t < 0`, would only occur on POSIX-compatible systems.
-    - 2: Execution Error, a command exits with status -1.
-    - 3: Invalid Command Error, determined by the exit code depending on the system.
-      - On Windows, if the exit code is 1.
-      - On POSIX-compatible systems, if the exit code is 127 or 126.
-    - 4: Signal error, either it is stopped or killed by signal, would only occur on POSIX-compatible systems.
+    - 1: Fork Error (occurs only on POSIX-compatible systems).
+    - 2: Execution Error (command exits with status -1).
+    - 3: Invalid Command Error (determined by the exit code depending on the system).
+      - On Windows: exit code is 1.
+      - On POSIX-compatible systems: exit code is 127 or 126.
+    - 4: Signal error (either stopped or killed by signal, occurs only on POSIX-compatible systems).
   - 0: Otherwise.
+- Asynchronous execution:
+  - 0 will always be assigned.
+
+##### `CMDMSG`:
 
-- **`CMDMSG`:**
-  - If an error condition occurs, it is assigned an explanatory message. Otherwise, it remains unchanged.
+- Synchronous execution:
+  - If an error condition occurs, it is assigned an explanatory message; otherwise, it remains unchanged.
+  - If a condition occurs that would assign a nonzero value to `CMDSTAT` but the `CMDSTAT` variable is not present, error termination is initiated (applies to both POSIX-compatible systems and Windows).
+- Asynchronous execution:
+  - The value is unchanged.
   - If a condition occurs that would assign a nonzero value to `CMDSTAT` but the `CMDSTAT` variable is not present, error termination is initiated.
-    - On POSIX-compatible systems, this applies to both synchronous and asynchronous error termination. When the execution mode is set to async with error termination, the child process (async process) will be terminated with no effect on the parent process (continues).
-    - On Windows, this only applies to synchronous error termination.
+    - On POSIX-compatible systems, the child process (async process) will be terminated with no effect on the parent process (continues).
+    - On Windows, error termination is not initiated.
+

>From 207b1a23c306f2bbbcf02946bd56a862bd7ad015 Mon Sep 17 00:00:00 2001
From: Yi Wu <yi.wu2 at arm.com>
Date: Fri, 12 Jan 2024 17:11:56 +0000
Subject: [PATCH 4/4] word fix, change from "section" to "subclause"

---
 flang/docs/Intrinsics.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/flang/docs/Intrinsics.md b/flang/docs/Intrinsics.md
index 20fd1dfe9beb61..5ade2574032977 100644
--- a/flang/docs/Intrinsics.md
+++ b/flang/docs/Intrinsics.md
@@ -847,7 +847,7 @@ used in constant expressions have currently no folding support at all.
 
 #### Usage and Info
 
-- **Standard:** Fortran 2008 and later, specified in Section 16.9.73
+- **Standard:** Fortran 2008 and later, specified in subclause 16.9.73
 - **Class:** Subroutine
 - **Syntax:** `CALL EXECUTE_COMMAND_LINE(COMMAND [, WAIT, EXITSTAT, CMDSTAT, CMDMSG ])`
 - **Arguments:**