[patch] bug-646856-01.patch

bug-646856-01.patch (text/plain), 3.68 KB, created by kfm on 2018-02-07 02:55:37 UTC

commit 0f7b946342e3d40048689d849369e789d052d66a
Author: Kerin Millar <[e-mail]>
Date:   Wed Feb 7 02:17:32 2018 +0000

    flag-o-matic: refactor strip-flags and fix non-functioning -O2 fallback
    
    The strip-flags() function tries to add -O2 to the stripped flag list, if
    it finds that it is reponsible for having stripped all -O flags. This
    code is defective because it relies on passing the literal name of the
    array containing the stripped flags to _is_flagq() for inspection, by
    way of indirection. This cannot possibly work because the array,
    entitled "new" is locally scoped to the calling function. As such, it is not
    visible to the _is_flagq() function.
    
    From inspecting the _is_flaqq() code, I could see that it was intended for
    one use case - dereferencing the scalar variables produced by
    all-flag-vars(). However, it appears to have had support for
    dereferencing array variables tacked on (in a very unsafe way). The sole
    case where array dereferencing is relied upon is in passing "new" from
    strip-flags() and - even then - it doesn't work, as explained above.
    
    Therefore, I elected to fix it by instead keeping a pseudo-boolean named
    has_o. For each flag that is added to the new list, if -O* is
    encountered, it sets has_o=1. Thus, _is_flagq() is no longer needed to
    assess whether -O* is missing in the new list.
    
    Also, I re-factored the code. It eliminates all potentially unsafe expansions,
    and no longer requires pathname expansion to be disabled. The read builtin
    is used to split scalar variables containing lists of flags, and is thus
    completely immune to the effects of globbing and pathname expansion. It
    uses an associative array to avoid repeatedly walking flag sequences.
    Micro-benchmarks indicate that the function is approximately 2.5 times faster.
    I have also attempted to make the code easier to read by declaring variables
    in accordance with their type, and by changing some of the variable names.

diff --git a/eclass/flag-o-matic.eclass b/eclass/flag-o-matic.eclass
index 14b84fbdbebe..e01eb80e2b67 100644
--- a/eclass/flag-o-matic.eclass
+++ b/eclass/flag-o-matic.eclass
@@ -382,40 +382,37 @@ filter-mfpmath() {
 # Strip *FLAGS of everything except known good/safe flags.  This runs over all
 # flags returned by all_flag_vars().
 strip-flags() {
-	local x y var
-
-	local ALLOWED_FLAGS
+	local var flag has_o
+	local -a ALLOWED_FLAGS old_flags new_flags
 	setup-allowed-flags
 
-	set -f	# disable pathname expansion
+	local -A safe
+	for flag in "${ALLOWED_FLAGS[@]}"; do
+		safe[$flag]=
+	done
 
-	for var in $(all-flag-vars) ; do
-		local new=()
-
-		for x in ${!var} ; do
-			local flag=${x%%=*}
-			for y in "${ALLOWED_FLAGS[@]}" ; do
-				if [[ -z ${flag%%${y}} ]] ; then
-					new+=( "${x}" )
-					break
+	for var in $(all-flag-vars); do
+		new_flags=()
+		read -ra old_flags <<<"${!var}"
+		for flag in "${old_flags[@]}"; do
+			if [[ -v "safe[${flag%%=*}]" ]]; then
+				new_flags+=( "$flag" )
+				if [[ $flag == -O* ]]; then
+					has_o=1
 				fi
-			done
+			fi
 		done
 
 		# In case we filtered out all optimization flags fallback to -O2
-		if _is_flagq ${var} "-O*" && ! _is_flagq new "-O*" ; then
-			new+=( -O2 )
+		if (( ! has_o )) && _is_flagq "$var" '-O*'; then
+			new_flags+=( -O2 )
 		fi
 
-		if [[ ${!var} != "${new[*]}" ]] ; then
-			einfo "strip-flags: ${var}: changed '${!var}' to '${new[*]}'"
+		if [[ ${old_flags[*]} != "${new_flags[*]}" ]]; then
+			einfo "strip-flags: ${var}: changed '${old_flags[*]}' to '${new_flags[*]}'"
+			export "$var=${new_flags[*]}"
 		fi
-		export ${var}="${new[*]}"
 	done
-
-	set +f	# re-enable pathname expansion
-
-	return 0
 }
 
 test-flag-PROG() {