From 7d04a947c07f18c304029a26104f188f9c7b26e5 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 12 Dec 2024 11:15:03 +0100 Subject: [PATCH] fix handling of quotes in rpath wrapper In some software the compiler must be invoked with an argument containing a literal single quote which requires escaping in the call. E.g.: `gcc -DFOO=\'value\'` However the current rpath wrapper would remove those single quotes causing errors during compilation or erronous behavior of the compiled binary. Fix by replacing the `eval` of the `rpath_args.py` output by `readarray -t`. An array asignment could be used (`CMD_ARGS=( $(rpath_args.py ...) )`) but that would break up arguments containing spaces. Quotes from the output are kept literally, so quoting to avoid this is not possible. `readarray -t` works and is widely available. Also some minor fixes related to quoting in the bash script template. --- easybuild/scripts/rpath_args.py | 12 ++-- .../scripts/rpath_wrapper_template.sh.in | 23 +++--- test/framework/toolchain.py | 70 ++++++++++++------- 3 files changed, 61 insertions(+), 44 deletions(-) diff --git a/easybuild/scripts/rpath_args.py b/easybuild/scripts/rpath_args.py index b477aa63d8..6bde5dddd7 100755 --- a/easybuild/scripts/rpath_args.py +++ b/easybuild/scripts/rpath_args.py @@ -161,8 +161,12 @@ def is_new_existing_path(new_path, paths): # add -rpath flags in front cmd_args = cmd_args_rpath + cmd_args -# wrap all arguments into single quotes to avoid further bash expansion -cmd_args = ["'%s'" % a.replace("'", "''") for a in cmd_args] -# output: statement to define $CMD_ARGS and $RPATH_ARGS -print("CMD_ARGS=(%s)" % ' '.join(cmd_args)) +def quote(arg): + """Quote the argument for use in a shell to avoid further expansion""" + # Wrap in single quotes and replace every single quote by '"'"' + # This terminates the single-quoted string, inserts a literal single quote and starts a new single-quotest-string + return "'" + arg.replace("'", "'\"'\"'") + "'" + + +print('\n'.join(cmd_args)) diff --git a/easybuild/scripts/rpath_wrapper_template.sh.in b/easybuild/scripts/rpath_wrapper_template.sh.in index 44a5aac43a..6ac6155d60 100644 --- a/easybuild/scripts/rpath_wrapper_template.sh.in +++ b/easybuild/scripts/rpath_wrapper_template.sh.in @@ -30,6 +30,7 @@ # before actually calling the original compiler/linker command. # # author: Kenneth Hoste (HPC-UGent) +# author: Alexander Grund (TU Dresden) set -e @@ -37,13 +38,13 @@ set -e function log { # escape percent signs, since this is a template script # that will templated using Python string templating - echo "($$) [$(date "+%%Y-%%m-%%d %%H:%%M:%%S")] $1" >> %(rpath_wrapper_log)s + echo "($$) [$(date "+%%Y-%%m-%%d %%H:%%M:%%S")] $1" >> '%(rpath_wrapper_log)s' } # command name -CMD=`basename $0` +CMD=$(basename $0) -log "found CMD: $CMD | original command: %(orig_cmd)s | orig args: '$(echo \"$@\")'" +log "found CMD: $CMD | original command: %(orig_cmd)s | orig args: $*)" # rpath_args.py script spits out statement that defines $CMD_ARGS # options for 'python' command (see https://docs.python.org/3/using/cmdline.html#miscellaneous-options) @@ -52,18 +53,12 @@ log "found CMD: $CMD | original command: %(orig_cmd)s | orig args: '$(echo \"$@\ # * -s: don’t add the user site-packages directory to sys.path; # * -S: disable the import of the module site and the site-dependent manipulations of sys.path that it entails; # (once we only support Python 3, we can (also) use -I (isolated mode) -log "%(python)s -E -O -s -S %(rpath_args_py)s $CMD '%(rpath_filter)s' '%(rpath_include)s' $(echo \"$@\")'" -rpath_args_out=$(%(python)s -E -O -s -S %(rpath_args_py)s $CMD '%(rpath_filter)s' '%(rpath_include)s' "$@") - -log "rpath_args_out: -$rpath_args_out" - -# define $CMD_ARGS by evaluating output of rpath_args.py script -eval $rpath_args_out +log "%(python)s -E -O -s -S %(rpath_args_py)s $CMD '%(rpath_filter)s' '%(rpath_include)s' $*" +readarray -t CMD_ARGS < <( %(python)s -E -O -s -S %(rpath_args_py)s $CMD '%(rpath_filter)s' '%(rpath_include)s' "$@" ) # exclude location of this wrapper from $PATH to avoid other potential wrappers calling this wrapper -export PATH=$(echo $PATH | tr ':' '\n' | grep -v "^%(wrapper_dir)s$" | tr '\n' ':') +export PATH=$(echo "$PATH" | tr ':' '\n' | grep -v "^%(wrapper_dir)s$" | tr '\n' ':') # call original command with modified list of command line arguments -log "running '%(orig_cmd)s $(echo ${CMD_ARGS[@]})'" -%(orig_cmd)s "${CMD_ARGS[@]}" +log "running: %(orig_cmd)s $(echo "${CMD_ARGS[@]}")" +'%(orig_cmd)s' "${CMD_ARGS[@]}" diff --git a/test/framework/toolchain.py b/test/framework/toolchain.py index beeb2e3512..6711cacbd0 100644 --- a/test/framework/toolchain.py +++ b/test/framework/toolchain.py @@ -2744,9 +2744,12 @@ def test_rpath_args_script(self): def test_toolchain_prepare_rpath(self): """Test toolchain.prepare under --rpath""" + # Code for a bash script that prints each passed argument on a new line + BASH_SCRIPT_PRINT_ARGS = '#!/bin/bash\nfor arg in "$@"; do echo "$arg"; done' + # put fake 'g++' command in place that just echos its arguments fake_gxx = os.path.join(self.test_prefix, 'fake', 'g++') - write_file(fake_gxx, '#!/bin/bash\necho "$@"') + write_file(fake_gxx, BASH_SCRIPT_PRINT_ARGS) adjust_permissions(fake_gxx, stat.S_IXUSR) os.environ['PATH'] = '%s:%s' % (os.path.join(self.test_prefix, 'fake'), os.getenv('PATH', '')) @@ -2789,7 +2792,7 @@ def test_toolchain_prepare_rpath(self): # Check that we can create a wrapper for a toolchain for which self.compilers() returns 'None' for the Fortran # compilers (i.e. Clang) fake_clang = os.path.join(self.test_prefix, 'fake', 'clang') - write_file(fake_clang, '#!/bin/bash\necho "$@"') + write_file(fake_clang, BASH_SCRIPT_PRINT_ARGS) adjust_permissions(fake_clang, stat.S_IXUSR) tc_clang = Clang(name='Clang', version='1') tc_clang.prepare_rpath_wrappers() @@ -2852,18 +2855,25 @@ def test_toolchain_prepare_rpath(self): '-L%s/foo' % self.test_prefix, '-L/bar', "'$FOO'", - '-DX="\\"\\""', + # C/C++ preprocessor value including a quotes + r'-DX1="\"\""', + r'-DX2="\"I\""', + r"-DY1=\'\'", + r"-DY2=\'J\'", ]) out, ec = run_cmd(cmd) self.assertEqual(ec, 0) - expected = ' '.join([ + expected = '\n'.join([ '-Wl,--disable-new-dtags', '-Wl,-rpath=%s/foo' % self.test_prefix, '%(user)s.c', '-L%s/foo' % self.test_prefix, '-L/bar', '$FOO', - '-DX=""', + '-DX1=""', + '-DX2="I"', + "-DY1=''", + "-DY2='J'", ]) self.assertEqual(out.strip(), expected % {'user': os.getenv('USER')}) @@ -2888,35 +2898,43 @@ def test_toolchain_prepare_rpath(self): for path in paths: mkdir(path, parents=True) args = ['-L%s' % x for x in paths] + path_with_spaces = os.path.join(self.test_prefix, 'prefix/with spaces') + mkdir(path_with_spaces) + args.append('-L"%s"' % path_with_spaces) cmd = "g++ ${USER}.c %s" % ' '.join(args) out, ec = run_cmd(cmd, simple=False) self.assertEqual(ec, 0) - expected = ' '.join([ + expected = '\n'.join([ '-Wl,--disable-new-dtags', - '-Wl,-rpath=%s/tmp/foo/' % self.test_prefix, - '-Wl,-rpath=%s/prefix/software/stubs/1.2.3/lib' % self.test_prefix, - '-Wl,-rpath=%s/prefix/software/foobar/4.5/notreallystubs' % self.test_prefix, - '-Wl,-rpath=%s/prefix/software/zlib/1.2.11/lib' % self.test_prefix, - '-Wl,-rpath=%s/prefix/software/foobar/4.5/stubsbutnotreally' % self.test_prefix, + '-Wl,-rpath=%(libdir)s/tmp/foo/', + '-Wl,-rpath=%(libdir)s/prefix/software/stubs/1.2.3/lib', + '-Wl,-rpath=%(libdir)s/prefix/software/foobar/4.5/notreallystubs', + '-Wl,-rpath=%(libdir)s/prefix/software/zlib/1.2.11/lib', + '-Wl,-rpath=%(libdir)s/prefix/software/foobar/4.5/stubsbutnotreally', + '-Wl,-rpath=%(path_with_spaces)s', '%(user)s.c', - '-L%s/prefix/software/CUDA/1.2.3/lib/stubs/' % self.test_prefix, - '-L%s/prefix/software/CUDA/1.2.3/stubs/lib/' % self.test_prefix, - '-L%s/tmp/foo/' % self.test_prefix, - '-L%s/prefix/software/stubs/1.2.3/lib' % self.test_prefix, - '-L%s/prefix/software/CUDA/1.2.3/lib/stubs' % self.test_prefix, - '-L%s/prefix/software/CUDA/1.2.3/stubs/lib' % self.test_prefix, - '-L%s/prefix/software/CUDA/1.2.3/lib64/stubs/' % self.test_prefix, - '-L%s/prefix/software/CUDA/1.2.3/stubs/lib64/' % self.test_prefix, - '-L%s/prefix/software/foobar/4.5/notreallystubs' % self.test_prefix, - '-L%s/prefix/software/CUDA/1.2.3/lib64/stubs' % self.test_prefix, - '-L%s/prefix/software/CUDA/1.2.3/stubs/lib64' % self.test_prefix, - '-L%s/prefix/software/zlib/1.2.11/lib' % self.test_prefix, - '-L%s/prefix/software/bleh/0/lib/stubs' % self.test_prefix, - '-L%s/prefix/software/foobar/4.5/stubsbutnotreally' % self.test_prefix, + '-L%(libdir)s/prefix/software/CUDA/1.2.3/lib/stubs/', + '-L%(libdir)s/prefix/software/CUDA/1.2.3/stubs/lib/', + '-L%(libdir)s/tmp/foo/', + '-L%(libdir)s/prefix/software/stubs/1.2.3/lib', + '-L%(libdir)s/prefix/software/CUDA/1.2.3/lib/stubs', + '-L%(libdir)s/prefix/software/CUDA/1.2.3/stubs/lib', + '-L%(libdir)s/prefix/software/CUDA/1.2.3/lib64/stubs/', + '-L%(libdir)s/prefix/software/CUDA/1.2.3/stubs/lib64/', + '-L%(libdir)s/prefix/software/foobar/4.5/notreallystubs', + '-L%(libdir)s/prefix/software/CUDA/1.2.3/lib64/stubs', + '-L%(libdir)s/prefix/software/CUDA/1.2.3/stubs/lib64', + '-L%(libdir)s/prefix/software/zlib/1.2.11/lib', + '-L%(libdir)s/prefix/software/bleh/0/lib/stubs', + '-L%(libdir)s/prefix/software/foobar/4.5/stubsbutnotreally', + '-L%(path_with_spaces)s', ]) - self.assertEqual(out.strip(), expected % {'user': os.getenv('USER')}) + self.assertEqual(out.strip(), expected % {'libdir': self.test_prefix, + 'user': os.getenv('USER'), + 'path_with_spaces': path_with_spaces, + }) # calling prepare() again should *not* result in wrapping the existing RPATH wrappers # this can happen when building extensions