summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTodd Gamblin <tgamblin@llnl.gov>2024-09-21 17:25:16 -0700
committerTodd Gamblin <tgamblin@llnl.gov>2024-09-27 05:27:26 -0700
commit2613a14c43b11ab92181bdd7ba57cd5b99f128a8 (patch)
tree89b0c34094cdebf382d321376391240342c922d7
parenta76a48c42e8d6657f9dcb0500bff574651f317d9 (diff)
downloadspack-2613a14c43b11ab92181bdd7ba57cd5b99f128a8.tar.gz
spack-2613a14c43b11ab92181bdd7ba57cd5b99f128a8.tar.bz2
spack-2613a14c43b11ab92181bdd7ba57cd5b99f128a8.tar.xz
spack-2613a14c43b11ab92181bdd7ba57cd5b99f128a8.zip
`cc`: ensure that RPATHs passed to linker are unique
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>
-rwxr-xr-xlib/spack/env/cc105
-rw-r--r--lib/spack/spack/test/cc.py35
2 files changed, 114 insertions, 26 deletions
diff --git a/lib/spack/env/cc b/lib/spack/env/cc
index 44f8b9316a..45e5e26269 100755
--- a/lib/spack/env/cc
+++ b/lib/spack/env/cc
@@ -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
diff --git a/lib/spack/spack/test/cc.py b/lib/spack/spack/test/cc.py
index 4a394680f4..dc08fdfb04 100644
--- a/lib/spack/spack/test/cc.py
+++ b/lib/spack/spack/test/cc.py
@@ -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(