[flang-commits] [flang] [flang] Fix execute_command_line cmdstat is not set when error occurs (PR #93023)

[Yi Wu via flang-commits]

================
@@ -64,22 +64,40 @@ void CheckAndStoreIntToDescriptor(
 // the CMDSTAT variable is not present, error termination is initiated.
 int TerminationCheck(int status, const Descriptor *cmdstat,
     const Descriptor *cmdmsg, Terminator &terminator) {
-  if (status == -1) {
-    if (!cmdstat) {
-      terminator.Crash("Execution error with system status code: %d", status);
-    } else {
-      StoreIntToDescriptor(cmdstat, EXECL_ERR, terminator);
-      CheckAndCopyCharsToDescriptor(cmdmsg, "Execution error");
-    }
-  }
 #ifdef _WIN32
   // On WIN32 API std::system returns exit status directly
   int exitStatusVal{status};
-  if (exitStatusVal == 1) {
 #else
   int exitStatusVal{WEXITSTATUS(status)};
-  if (exitStatusVal == 127 || exitStatusVal == 126) {
+  if (exitStatusVal == 1) {
+    if (!cmdstat) {
+      terminator.Crash("General Error with exit status code: 1");
+    } else {
+      StoreIntToDescriptor(cmdstat, INVALID_CL_ERR, terminator);
+      CheckAndCopyCharsToDescriptor(cmdmsg,
+          "General Error: a catch-all exit code for a variety of general "
+          "errors.");
+    }
+  } else if (exitStatusVal == 126) {
+    if (!cmdstat) {
+      terminator.Crash("Command cannot execute with exit status code: 126");
+    } else {
+      StoreIntToDescriptor(cmdstat, INVALID_CL_ERR, terminator);
+      CheckAndCopyCharsToDescriptor(cmdmsg,
+          "Command cannot execute: command was found, but it could not be "
+          "executed.");
+    }
+  } else if (exitStatusVal == 127) {
+    if (!cmdstat) {
+      terminator.Crash("Command not found with exit status code: 127");
+    } else {
+      StoreIntToDescriptor(cmdstat, INVALID_CL_ERR, terminator);
+      CheckAndCopyCharsToDescriptor(cmdmsg,
+          "Command not found: command was not found in the system's PATH");
+    }
+  } else
 #endif
+  if (exitStatusVal != 0) {
----------------
yi-wu-arm wrote:

Thanks a lot for your inputs, I agree that give specific `CMD_STAT` for common exit code is a good idea.

---

`exitStatusVal != 0` was meant to be a general error message, covers all other error code that wasn't specified by the if-else. Otherwise `CMD_MSG` and `CMD_STAT` would only be set if error code is 1, 126, 127.

---

>BTW, `NO_SUPPORT_ERR` seems is not used anywhere otherwise.  

On Windows
>https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/system-wsystem?view=msvc-170
>A return value of -1 indicates an error, and errno is set to one of the following values  

| Value | Description |
| --- | --- |
| E2BIG | The argument list (which is system-dependent) is too large. |
| ENOENT | The command interpreter can't be found. |
| ENOEXEC | The command-interpreter file can't be executed because the format isn't valid. |
| ENOMEM | Not enough memory is available to execute command; or available memory has been corrupted; or a non-valid block exists, which indicates that the calling process has been allocated incorrectly. |

On Linux: (`std::system` is achieved by `fork()`)
>https://man7.org/linux/man-pages/man3/system.3.html
>If a child process could not be created, or its status could not be retrieved, the return value is -1 and [errno](https://man7.org/linux/man-pages/man3/errno.3.html) is set to indicate the error.

In other words, a return code of -1 could mean `EXECL_ERR` **or** `NO_SUPPORT_ERR` based on the value of errno, thats why I used `EXECL_ERR`, for safety. On a second thought, I think it would be best to report the value of `errno` as well.


https://github.com/llvm/llvm-project/pull/93023