From 18145c17f0eaafde25d230b80a5e7c92596a3326 Mon Sep 17 00:00:00 2001
From: kangalio <me@kangalio.net>
Date: Sat, 3 May 2025 19:31:54 +0200
Subject: [PATCH] Fix Vulkan glslc invocation command lines

They were shoehorned through a Windows-style string representation, where the entire command line is just a space-separated string of arguments. If any arguments contain spaces, you have to escape them. This was implemented in https://github.com/ggml-org/llama.cpp/pull/8573, however, the issue remains on Unix.

Usually, this would NOT be an issue on Unix, because it doesn't have the braindead Windows-style API that only takes a flat string as a command line. In Unix, you can pass the arguments as a legit array. However, Llama.cpp didn't make use of that; instead running `sh -c <the flattened argument string>`. Thereby inflicting the same problems onto itself as Windows has.

Instead of bandaid-fixing this by also slapping quotes around the path arguments (which would also break apart as soon as the path contains quote characters), I decided to fix it properly.

So, the code now doesn't wrap the command line in `sh -c`, but passes the arguments directly, circumventing the need to do any brittle escaping.

This also necessitated replacing the `coopmat ? "" : "-O"` parameter with a proper conditional parameter. Because the empty string parameter (rightfully) confused glslc.
---
 .../vulkan-shaders/vulkan-shaders-gen.cpp     | 42 ++++++++++++++++---
 1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/ggml/src/ggml-vulkan/vulkan-shaders/vulkan-shaders-gen.cpp b/ggml/src/ggml-vulkan/vulkan-shaders/vulkan-shaders-gen.cpp
index cf74625cc56d5..cd425ae89581a 100644
--- a/ggml/src/ggml-vulkan/vulkan-shaders/vulkan-shaders-gen.cpp
+++ b/ggml/src/ggml-vulkan/vulkan-shaders/vulkan-shaders-gen.cpp
@@ -67,7 +67,15 @@ const std::vector<std::string> type_names = {
 };
 
 namespace {
-void execute_command(const std::string& command, std::string& stdout_str, std::string& stderr_str) {
+void execute_command(
+    #ifdef _WIN32
+    const std::string& command,
+    #else
+    const std::vector<std::string>& command,
+    #endif
+    std::string& stdout_str,
+    std::string& stderr_str
+) {
 #ifdef _WIN32
     HANDLE stdout_read, stdout_write;
     HANDLE stderr_read, stderr_write;
@@ -136,7 +144,15 @@ int stdout_pipe[2];
         dup2(stderr_pipe[1], STDERR_FILENO);
         close(stdout_pipe[1]);
         close(stderr_pipe[1]);
-        execl("/bin/sh", "sh", "-c", command.c_str(), (char*) nullptr);
+
+        std::vector<char*> argv;
+        argv.reserve(command.size());
+        for (const auto& part : command) {
+            argv.push_back(const_cast<char*>(part.c_str()));
+        }
+        argv.push_back(nullptr);
+        execv(argv[0], argv.data());
+
         _exit(EXIT_FAILURE);
     } else {
         close(stdout_pipe[1]);
@@ -220,13 +236,19 @@ void string_to_spv_func(const std::string& _name, const std::string& in_fname, c
 
     std::string target_env = (name.find("_cm2") != std::string::npos) ? "--target-env=vulkan1.3" : "--target-env=vulkan1.2";
 
+    std::vector<std::string> cmd = {GLSLC, "-fshader-stage=compute", target_env};
+    
     // disable spirv-opt for coopmat shaders for https://github.com/ggerganov/llama.cpp/issues/10734
-    std::string opt_level = coopmat ? "" : "-O";
+    if (!coopmat) cmd.push_back("-O");
 
     #ifdef _WIN32
-        std::vector<std::string> cmd = {GLSLC, "-fshader-stage=compute", target_env, opt_level, "\"" + in_path + "\"", "-o", "\"" + out_fname + "\""};
+        cmd.push_back("\"" + in_path + "\"");
+        cmd.push_back("-o");
+        cmd.push_back("\"" + out_fname + "\"");
     #else
-        std::vector<std::string> cmd = {GLSLC, "-fshader-stage=compute", target_env, opt_level, in_path, "-o",  out_fname};
+        cmd.push_back(in_path);
+        cmd.push_back("-o");
+        cmd.push_back(out_fname);
     #endif
 
     #ifdef GGML_VULKAN_SHADER_DEBUG_INFO
@@ -250,7 +272,15 @@ void string_to_spv_func(const std::string& _name, const std::string& in_fname, c
         // }
         // std::cout << std::endl;
 
-        execute_command(command, stdout_str, stderr_str);
+        execute_command(
+            #ifdef _WIN32
+            command,
+            #else
+            cmd,
+            #endif
+            stdout_str,
+            stderr_str
+        );
         if (!stderr_str.empty()) {
             std::cerr << "cannot compile " << name << "\n\n" << command << "\n\n" << stderr_str << std::endl;
             return;