Re: [PATCH] Fix wrong refactoring in cgraph_node::function_symbol

[Ilya Enkovich <enkovich dot gnu at gmail dot com>]

2014-08-13 16:48 GMT+04:00 Richard Biener <richard.guenther@gmail.com>:
> On Wed, Aug 13, 2014 at 2:25 PM, Ilya Enkovich <enkovich.gnu@gmail.com> wrote:
>> Hi,
>>
>> This patch is to fix wrong refactoring for cgraph_node::function_symbol introduced by this patch: https://gcc.gnu.org/ml/gcc-cvs/2014-07/msg00805.html.  Here is how function was refactored:
>>
>> -cgraph_function_node (struct cgraph_node *node, enum availability *availability)
>> +cgraph_node *
>> +cgraph_node::function_symbol (enum availability *availability)
>>  {
>> +  cgraph_node *node = NULL;
>> +
>>    do
>>      {
>> -      node = cgraph_function_or_thunk_node (node, availability);
>> +      node = ultimate_alias_target (availability);
>>        if (node->thunk.thunk_p)
>>         {
>>           node = node->callees->callee;
>>           if (availability)
>>             {
>>               enum availability a;
>> -             a = cgraph_function_body_availability (node);
>> +             a = node->get_availability ();
>>               if (a < *availability)
>>                 *availability = a;
>>             }
>> -         node = cgraph_function_or_thunk_node (node, availability);
>> +         node = node->ultimate_alias_target (availability);
>>         }
>>      } while (node && node->thunk.thunk_p);
>>    return node;
>>  }
>>
>> first ultimate_alias_target call always uses 'this' instead of 'node'.  This causes infinite loop.
>>
>> Patch was bootstrapped and regtested on linux-x86_64.  OK for trunk?
>
> Ok.  Do you have a testcase?

No, I found this problem testing pointer bounds checker, which
produces many instrumentation thunks.  It seems in a regular case we
do not have such thunks chains and one loop iteration is enough.

Ilya

>
> Thanks,
> Richard.
>
>> Thanks,
>> Ilya
>> --
>>
>> 2014-08-13  Ilya Enkovich  <ilya.enkovich@intel.com>
>>
>>         * cgraph.c (cgraph_node::function_symbol): Fix wrong
>>         cgraph_function_node to cgraph_node::function_symbol
>>         refactoring.
>>
>> diff --git a/gcc/cgraph.c b/gcc/cgraph.c
>> index 5a0b903..370a96a 100644
>> --- a/gcc/cgraph.c
>> +++ b/gcc/cgraph.c
>> @@ -3000,11 +3000,11 @@ cgraph_node::verify_cgraph_nodes (void)
>>  cgraph_node *
>>  cgraph_node::function_symbol (enum availability *availability)
>>  {
>> -  cgraph_node *node = NULL;
>> +  cgraph_node *node = this;
>>
>>    do
>>      {
>> -      node = ultimate_alias_target (availability);
>> +      node = node->ultimate_alias_target (availability);
>>        if (node->thunk.thunk_p)
>>         {
>>           node = node->callees->callee;