This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Safe ICF for i386.


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
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]