Safe ICF for i386.

[Sriraman Tallam]

Hi Ralf,

My shell scripting can suck, let me make those changes with another
patch. I committed the original patch already.

Thanks,
-Sriraman.

On Wed, Mar 3, 2010 at 6:24 PM, Ralf Wildenhues <Ralf.Wildenhues@gmx.de> wrote:
> 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
>