Skip to content

Commit

Permalink
fix handling of quotes in rpath wrapper
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@Flamefire
Flamefire committed Dec 12, 2024
1 parent 5661bf5 commit 7d04a94
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 44 deletions.
12 changes: 8 additions & 4 deletions easybuild/scripts/rpath_args.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
23 changes: 9 additions & 14 deletions easybuild/scripts/rpath_wrapper_template.sh.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,21 @@
# before actually calling the original compiler/linker command.
#
# author: Kenneth Hoste (HPC-UGent)
# author: Alexander Grund (TU Dresden)

set -e

# logging function
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)
Expand All @@ -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[@]}"
70 changes: 44 additions & 26 deletions test/framework/toolchain.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', ''))

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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')})

Expand All @@ -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
Expand Down

0 comments on commit 7d04a94

Please sign in to comment.