Fix broken escaping of commands in the build
authorLuke Drummond <luke.drummond@codeplay.com>
Fri, 16 Dec 2016 16:38:25 +0000 (16:38 +0000)
committerLuke Drummond <luke.drummond@codeplay.com>
Fri, 16 Dec 2016 16:38:25 +0000 (16:38 +0000)
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).

Subscribers: lldb-commits
Differential Revision: https://reviews.llvm.org/D26757

llvm-svn: 289956

lldb/CMakeLists.txt
lldb/scripts/CMakeLists.txt
lldb/scripts/Python/prepare_binding_Python.py

index 23198af..b860a93 100644 (file)
@@ -39,15 +39,19 @@ add_subdirectory(unittests)
 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")
@@ -78,7 +82,7 @@ if (NOT LLDB_DISABLE_PYTHON)
         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 ()
index d7c5e67..18389c6 100644 (file)
@@ -35,12 +35,13 @@ add_custom_command(
   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}"
+      ${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 107f897..cf64d49 100644 (file)
@@ -196,34 +196,37 @@ def do_swig_rebuild(options, dependency_file, config_build_dir, settings):
         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" + os.path.curdir,
+            "-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 @@ def run_python_script(script_and_args):
     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)