ldrumm created this revision.
ldrumm added reviewers: zturner, granata.enrico.
ldrumm added a subscriber: LLDB.
Herald added a subscriber: mgorny.

A combination of broken escaping in CMake and in the python swig generation 
scripts meant that the swig generation step would fail whenever there were 
spaces or special characters in parameters passed to swig.

The fix for this in `CMakeLists` is to use the `VERBATIM` option on all 
`COMMAND`-based custom builders relying on CMake to properly escape each 
argument in the generated file.

Within the python swig scripts, the fix is to call `subprocess.Popen` with a 
list of raw argument strings rather than ones that are incorrectly manually 
escaped, then passed to a shell subprocess via `subprocess.Popen(' 
'.join(params))`. This also prevents nasty things happening such as accidental 
command-injection.

This allows us to have the swig / python executables in paths containing 
special chars and spaces, (or on shared storage on Win32, e.g \\some\path or 
C:\Program Files\swig\swig.exe).


https://reviews.llvm.org/D26757

Files:
  CMakeLists.txt
  scripts/CMakeLists.txt
  scripts/Python/prepare_binding_Python.py

Index: scripts/Python/prepare_binding_Python.py
===================================================================
--- scripts/Python/prepare_binding_Python.py
+++ scripts/Python/prepare_binding_Python.py
@@ -196,34 +196,37 @@
         temp_dep_file_path = dependency_file + ".tmp"
 
     # Build the SWIG args list
-    command = [
-        options.swig_executable,
-        "-c++",
-        "-shadow",
-        "-python",
-        "-threads",
-        "-I\"%s\"" % os.path.normcase(
-            os.path.join(options.src_root, "include")),
-        "-I\"%s\"" % os.path.normcase("./."),
-        "-D__STDC_LIMIT_MACROS",
-        "-D__STDC_CONSTANT_MACROS"]
-    if options.target_platform == "Darwin":
-        command.append("-D__APPLE__")
-    if options.generate_dependency_file:
-        command.append("-MMD -MF \"%s\"" % temp_dep_file_path)
-    command.extend([
-        "-outdir", "\"%s\"" % config_build_dir,
-        "-o", "\"%s\"" % settings.output_file,
-        "\"%s\"" % settings.input_file
-    ])
-    logging.info("running swig with: %s", command)
+    is_darwin = options.target_platform == "Darwin"
+    gen_deps = options.generate_dependency_file
+    darwin_extras = ["-D__APPLE__"] if is_darwin else []
+    deps_args = ["-MMD", "-MF", temp_dep_file_path] if gen_deps else []
+    command = ([
+            options.swig_executable,
+            "-c++",
+            "-shadow",
+            "-python",
+            "-threads",
+            "-I" + os.path.normpath(os.path.join(options.src_root, "include")),
+            "-I.",
+            "-D__STDC_LIMIT_MACROS",
+            "-D__STDC_CONSTANT_MACROS"
+        ]
+        + darwin_extras
+        + deps_args
+        + [
+            "-outdir", config_build_dir,
+            "-o", settings.output_file,
+            settings.input_file
+        ]
+    )
+    logging.info("running swig with: %r", command)
 
     # Execute swig
     process = subprocess.Popen(
-        ' '.join(command),
+        command,
         stdout=subprocess.PIPE,
         stderr=subprocess.PIPE,
-        shell=True)
+    )
     # Wait for SWIG process to terminate
     swig_stdout, swig_stderr = process.communicate()
     return_code = process.returncode
@@ -265,15 +268,14 @@
     the command line arguments to pass to it.
     """
     command = [sys.executable] + script_and_args
-    command_line = " ".join(command)
-    process = subprocess.Popen(command, shell=False)
+    process = subprocess.Popen(command)
     script_stdout, script_stderr = process.communicate()
     return_code = process.returncode
     if return_code != 0:
-        logging.error("failed to run '%s': %s", command_line, script_stderr)
+        logging.error("failed to run %r: %r", command, script_stderr)
         sys.exit(return_code)
     else:
-        logging.info("ran script '%s'", command_line)
+        logging.info("ran script %r'", command)
         if script_stdout is not None:
             logging.info("output: %s", script_stdout)
 
Index: scripts/CMakeLists.txt
===================================================================
--- scripts/CMakeLists.txt
+++ scripts/CMakeLists.txt
@@ -34,13 +34,15 @@
   DEPENDS ${SWIG_HEADERS}
   DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/Python/prepare_binding_Python.py
   DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/Python/modify-python-lldb.py
-  COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/prepare_bindings.py
-          ${framework_arg}
-          "--srcRoot=${LLDB_SOURCE_DIR}"
-          "--targetDir=${LLDB_PYTHON_TARGET_DIR}"
-          "--cfgBldDir=${CMAKE_CURRENT_BINARY_DIR}"
-          "--prefix=${CMAKE_BINARY_DIR}"
-          "--swigExecutable=${SWIG_EXECUTABLE}"
+  COMMAND
+    ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/prepare_bindings.py
+      ${framework_arg}
+      --srcRoot=${LLDB_SOURCE_DIR}
+      --targetDir=${LLDB_PYTHON_TARGET_DIR}
+      --cfgBldDir=${CMAKE_CURRENT_BINARY_DIR}
+      --prefix=${CMAKE_BINARY_DIR}
+      --swigExecutable=${SWIG_EXECUTABLE}
+  VERBATIM
   COMMENT "Python script building LLDB Python wrapper")
 set_source_files_properties(${LLDB_WRAP_PYTHON} PROPERTIES GENERATED 1)
 set_source_files_properties(${CMAKE_CURRENT_BINARY_DIR}/lldb.py PROPERTIES GENERATED 1)
Index: CMakeLists.txt
===================================================================
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -39,15 +39,19 @@
 add_subdirectory(lit)
 
 if (NOT LLDB_DISABLE_PYTHON)
-    # Add a Post-Build Event to copy over Python files and create the symlink to liblldb.so for the Python API(hardlink on Windows)
-    add_custom_target( finish_swig ALL
-        COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py
-        "--srcRoot=${LLDB_SOURCE_DIR}"
-        "--targetDir=${LLDB_PYTHON_TARGET_DIR}"
-        "--cfgBldDir=${CMAKE_CURRENT_BINARY_DIR}/scripts"
-        "--prefix=${CMAKE_BINARY_DIR}"
-        "--cmakeBuildConfiguration=${CMAKE_CFG_INTDIR}"
-        "--lldbLibDir=lib${LLVM_LIBDIR_SUFFIX}" ${FINISH_EXTRA_ARGS}
+    # Add a Post-Build Event to copy over Python files and create the symlink
+    # to liblldb.so for the Python API(hardlink on Windows)
+    add_custom_target(finish_swig ALL
+        COMMAND
+           ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py
+               --srcRoot=${LLDB_SOURCE_DIR}
+               --targetDir=${LLDB_PYTHON_TARGET_DIR}
+               --cfgBldDir=${CMAKE_CURRENT_BINARY_DIR}/scripts
+               --prefix=${CMAKE_BINARY_DIR}
+               --cmakeBuildConfiguration=${CMAKE_CFG_INTDIR}
+               --lldbLibDir=lib${LLVM_LIBDIR_SUFFIX}
+           ${FINISH_EXTRA_ARGS}
+        VERBATIM
         DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py
         DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/scripts/lldb.py
         COMMENT "Python script sym-linking LLDB Python API")
@@ -72,7 +76,7 @@
         add_custom_command(
             TARGET finish_swig
             POST_BUILD
-            COMMAND "${CMAKE_COMMAND}" -E copy ${PYTHON_DLL_NATIVE_PATH} ${LLDB_BIN_DIR}
+            COMMAND ${CMAKE_COMMAND} -E copy ${PYTHON_DLL_NATIVE_PATH} ${LLDB_BIN_DIR} VERBATIM
             COMMENT "Copying Python DLL to LLDB binaries directory.")
     endif ()
 endif ()
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to