Re: Safe ICF for i386.

[Ralf Wildenhues <Ralf dot Wildenhues at gmx dot de>]

Hello,

* Sriraman Tallam wrote on Thu, Mar 04, 2010 at 02:37:59AM CET:
> --- testsuite/icf_safe_so_test.sh	13 Feb 2010 02:04:21 -0000	1.1
> +++ testsuite/icf_safe_so_test.sh	4 Mar 2010 01:32:23 -0000
> @@ -25,8 +25,26 @@
>  # to verify if identical code folding in safe mode correctly folds
>  # functions in a shared object.
>  
> +error_if_symbol_absent()
> +{
> +    is_symbol_present $1 $2
> +    if [ $? != 0 ];
> +    then

These three lines are cleaner written as
    if is_symbol_present $1 $2; then

> +      echo "Symbol" $2 "not present, possibly folded."
> +      exit 1
> +    fi
> +}
> +
> +is_symbol_present()
> +{
> +    result=`grep $2 $1`
> +    return $?

As Ian already noted, this is problematic.  Not all shells propagate the
exit status from a command substitution inside an assignment to the $?
variable.  These two lines are better replaced by
   grep $2 $1 >/dev/null 2>&1

> +}
> +
>  check_nofold()
>  {
> +    error_if_symbol_absent $1 $2
> +    error_if_symbol_absent $1 $3
>      func_addr_1=`grep $2 $1 | awk '{print $1}'`
>      func_addr_2=`grep $3 $1 | awk '{print $1}'`
>      if [ $func_addr_1 = $func_addr_2 ];
> @@ -38,6 +56,18 @@ check_nofold()
>  
>  check_fold()
>  {
> +    is_symbol_present $1 $2
> +    if [ $? != 0 ];
> +    then

See above.

> +      return 0
> +    fi
> +
> +    is_symbol_present $1 $3
> +    if [ $? != 0 ];
> +    then

See above.

> +      return 0
> +    fi
> +
>      func_addr_1=`grep $2 $1 | awk '{print $1}'`
>      func_addr_2=`grep $3 $1 | awk '{print $1}'`
>      if [ $func_addr_1 != $func_addr_2 ];
> @@ -49,21 +79,31 @@ check_fold()
>  
>  arch_specific_safe_fold()
>  {
> -    grep_x86_64=`grep -q "Advanced Micro Devices X86-64" $2`
> -    if [ $? == 0 ];
> +    if [ $1 == 0 ];
>      then
> -      check_fold $1 $3 $4
> +      check_fold $2 $3 $4
>      else
> -      check_nofold $1 $3 $4
> +      check_nofold $2 $3 $4
>      fi
>  }
>  
> -check_nofold icf_safe_so_test_1.stdout "foo_prot" "foo_hidden"
> -check_nofold icf_safe_so_test_1.stdout "foo_prot" "foo_internal"
> -check_nofold icf_safe_so_test_1.stdout "foo_prot" "foo_static"
> -check_nofold icf_safe_so_test_1.stdout "foo_hidden" "foo_internal"
> -check_nofold icf_safe_so_test_1.stdout "foo_hidden" "foo_static"
> -check_nofold icf_safe_so_test_1.stdout "foo_internal" "foo_static"
> -arch_specific_safe_fold icf_safe_so_test_1.stdout icf_safe_so_test_2.stdout \
> -  "foo_glob" "bar_glob"
> +X86_32_specific_safe_fold()
> +{
> +    grep -q -e "Intel 80386" $1 >& /dev/null

grep -q is not portable, just omit the -q.  I don't think >& is
portable, so prefer >/dev/null 2>&1 instead.

> +    arch_specific_safe_fold $? $2 $3 $4
> +}
>  
> +X86_64_specific_safe_fold()
> +{
> +    grep -q -e "Advanced Micro Devices X86-64" $1 >& /dev/null

See above.

> +    arch_specific_safe_fold $? $2 $3 $4
> +}

> diff -u -u -p -r1.2 icf_safe_test.sh
> --- testsuite/icf_safe_test.sh	13 Feb 2010 02:04:21 -0000	1.2
> +++ testsuite/icf_safe_test.sh	4 Mar 2010 01:32:23 -0000

>  arch_specific_safe_fold()
>  {
> -    grep_x86_64=`grep -q "Advanced Micro Devices X86-64" $2`
> +    grep_x86=`grep -q -e "Advanced Micro Devices X86-64" -e "Intel 80386" $2`
>      if [ $? == 0 ];

See above.

>      then

Cheers,
Ralf