zhangyiss/spack
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

cc: ensure that RPATHs passed to linker are unique

Browse Source 
macOS Sequoia's linker will complain if RPATHs on the CLI are specified more than once.
To avoid errors due to this, make `cc` only append unique RPATHs to the final args list.

This required a few improvements to the logic in `cc`:

1. List functions in `cc` didn't have any way to append unique elements to a list. Add a
   `contains()` shell function that works like our other list functions. Use it to implement
   an optional `"unique"` argument to `append()` and an `extend_unique()`. Use that to add
   RPATHs to the `args_list`.

2. In the pure `ld` case, we weren't actually parsing `RPATH` arguments separately as we
   do for `ccld`. Fix this by adding *another* nested case statement for raw `RPATH`
   parsing. There are now 3 places where we deal with `-rpath` and friends, but I don't
   see a great way to unify them, as `-Wl,`, `-Xlinker`, and raw `-rpath` arguments are
   all ever so slightly different.

3. Fix ordering of assertions to make `pytest` diffs more intelligible. The meaning of
   `+` and `-` in diffs changed in `pytest` 6.0 and the "preferred" order for assertions
   became `assert actual == expected` instead of the other way around.

Signed-off-by: Todd Gamblin <tgamblin@llnl.gov>
This commit is contained in:
Todd Gamblin 2024-09-21 17:25:16 -07:00
parent a76a48c42e
commit 2613a14c43
2 changed files with 114 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

105
lib/spack/env/cc vendored
View File

@ -101,10 +101,9 @@ setsep() {
esac
}
# prepend LISTNAME ELEMENT [SEP]
# prepend LISTNAME ELEMENT
#
# Prepend ELEMENT to the list stored in the variable LISTNAME,
# assuming the list is separated by SEP.
# Prepend ELEMENT to the list stored in the variable LISTNAME.
# Handles empty lists and single-element lists.
prepend() {
varname="$1"
@ -119,18 +118,39 @@ prepend() {
fi
}
# append LISTNAME ELEMENT [SEP]
# contains LISTNAME ELEMENT
#
# Append ELEMENT to the list stored in the variable LISTNAME,
# assuming the list is separated by SEP.
# Test whether LISTNAME contains ELEMENT.
# Set $? to 1 if LISTNAME does not contain ELEMENT.
# Set $? to 0 if LISTNAME does not contain ELEMENT.
contains() {
varname="$1"
elt="$2"
setsep "$varname"
# the list may: 1) only contain the element, 2) start with the element,
# 3) contain the element in the middle, or 4) end wtih the element.
eval "[ \"\${$varname}\" = \"$elt\" ]" \
|| eval "[ \"\${$varname#${elt}${sep}}\" != \"\${$varname}\" ]" \
|| eval "[ \"\${$varname#*${sep}${elt}${sep}}\" != \"\${$varname}\" ]" \
|| eval "[ \"\${$varname%${sep}${elt}}\" != \"\${$varname}\" ]"
}
# append LISTNAME ELEMENT [unique]
#
# Append ELEMENT to the list stored in the variable LISTNAME.
# Handles empty lists and single-element lists.
#
# If the third argument is provided and if it is the string 'unique',
# this will not append if ELEMENT is already in the list LISTNAME.
append() {
varname="$1"
elt="$2"
if empty "$varname"; then
eval "$varname=\"\${elt}\""
else
elif [ "$3" != "unique" ] || ! contains "$varname" "$elt" ; then
# Get the appropriate separator for the list we're appending to.
setsep "$varname"
eval "$varname=\"\${$varname}${sep}\${elt}\""
@ -148,10 +168,21 @@ extend() {
if [ "$sep" != " " ]; then
IFS="$sep"
fi
eval "for elt in \${$2}; do append $1 \"$3\${elt}\"; done"
eval "for elt in \${$2}; do append $1 \"$3\${elt}\" ${_append_args}; done"
unset IFS
}
# extend_unique LISTNAME1 LISTNAME2 [PREFIX]
#
# Append the elements stored in the variable LISTNAME2 to the list
# stored in LISTNAME1, if they are not already present.
# If PREFIX is provided, prepend it to each element.
extend_unique() {
_append_args="unique"
extend "$@"
unset _append_args
}
# preextend LISTNAME1 LISTNAME2 [PREFIX]
#
# Prepend the elements stored in the list at LISTNAME2
@ -682,7 +713,32 @@ categorize_arguments() {
"$dtags_to_strip")
;;
*)
append return_other_args_list "$1"
# if mode is not ld, we can just add to other args
if [ "$mode" != "ld" ]; then
append return_other_args_list "$1"
shift
continue
fi
# if we're in linker mode, we need to parse raw RPATH args
case "$1" in
-rpath=*)
arg="${1#-rpath=}"
append_path_lists return_rpath_dirs_list "$arg"
;;
--rpath=*)
arg="${1#--rpath=}"
append_path_lists return_rpath_dirs_list "$arg"
;;
-rpath|--rpath)
shift
[ $# -eq 0 ] && break # ignore -rpath without value
append_path_lists return_rpath_dirs_list "$1"
;;
*)
append return_other_args_list "$1"
;;
esac
;;
esac
shift
@ -890,35 +946,34 @@ extend args_list system_spack_flags_lib_dirs_list "-L"
extend args_list system_lib_dirs_list "-L"
# RPATHs arguments
rpath_prefix=""
case "$mode" in
ccld)
if [ -n "$dtags_to_add" ] ; then
append args_list "$linker_arg$dtags_to_add"
fi
extend args_list spack_store_spack_flags_rpath_dirs_list "$rpath"
extend args_list spack_store_rpath_dirs_list "$rpath"
extend args_list spack_flags_rpath_dirs_list "$rpath"
extend args_list rpath_dirs_list "$rpath"
extend args_list system_spack_flags_rpath_dirs_list "$rpath"
extend args_list system_rpath_dirs_list "$rpath"
rpath_prefix="$rpath"
;;
ld)
if [ -n "$dtags_to_add" ] ; then
append args_list "$dtags_to_add"
fi
extend args_list spack_store_spack_flags_rpath_dirs_list "-rpath${lsep}"
extend args_list spack_store_rpath_dirs_list "-rpath${lsep}"
extend args_list spack_flags_rpath_dirs_list "-rpath${lsep}"
extend args_list rpath_dirs_list "-rpath${lsep}"
extend args_list system_spack_flags_rpath_dirs_list "-rpath${lsep}"
extend args_list system_rpath_dirs_list "-rpath${lsep}"
rpath_prefix="-rpath${lsep}"
;;
esac
# if mode is ccld or ld, extend RPATH lists with the prefix determined above
if [ -n "$rpath_prefix" ]; then
extend_unique args_list spack_store_spack_flags_rpath_dirs_list "$rpath_prefix"
extend_unique args_list spack_store_rpath_dirs_list "$rpath_prefix"
extend_unique args_list spack_flags_rpath_dirs_list "$rpath_prefix"
extend_unique args_list rpath_dirs_list "$rpath_prefix"
extend_unique args_list system_spack_flags_rpath_dirs_list "$rpath_prefix"
extend_unique args_list system_rpath_dirs_list "$rpath_prefix"
fi
# Other arguments from the input command
extend args_list other_args_list
extend args_list spack_flags_other_args_list

35
lib/spack/spack/test/cc.py
View File

@ -199,7 +199,7 @@ def check_args(cc, args, expected):
"""
with set_env(SPACK_TEST_COMMAND="dump-args"):
cc_modified_args = cc(*args, output=str).strip().split("\n")
assert expected == cc_modified_args
assert cc_modified_args == expected
def check_args_contents(cc, args, must_contain, must_not_contain):
@ -354,6 +354,39 @@ def test_fc_flags(wrapper_environment, wrapper_flags):
)
def test_ld_flags_with_redundant_rpaths(wrapper_environment, wrapper_flags):
check_args(
ld,
test_args + test_rpaths, # ensure thesee are made unique
["ld"]
+ test_include_paths
+ test_library_paths
+ ["--disable-new-dtags"]
+ test_rpaths
+ test_args_without_paths
+ spack_ldlibs,
)
def test_cc_flags_with_redundant_rpaths(wrapper_environment, wrapper_flags):
check_args(
cc,
test_args + test_wl_rpaths + test_wl_rpaths, # ensure thesee are made unique
[real_cc]
+ target_args
+ test_include_paths
+ ["-Lfoo"]
+ test_library_paths
+ ["-Wl,--disable-new-dtags"]
+ test_wl_rpaths
+ test_args_without_paths
+ spack_cppflags
+ spack_cflags
+ ["-Wl,--gc-sections"]
+ spack_ldlibs,
)
def test_always_cflags(wrapper_environment, wrapper_flags):
with set_env(SPACK_ALWAYS_CFLAGS="-always1 -always2"):
check_args(