[GOOGLE] Workaround a bug in AutoFDO which may leads to infinite loop for inlined assembly

Thanks for the comment, new patch attached:


On Sun, Apr 21, 2013 at 5:00 PM, Teresa Johnson <tejohnson@google.com> wrote:
> On Sun, Apr 21, 2013 at 4:51 PM, Dehao Chen <dehao@google.com> wrote:
>> This patch fixed a bug in getting inline stacks: if there is no
>> location info attached to a block, we should *not* try to get its
>> function name because it could result in infinite loop.
>
> Is the infinite loop within get_function_decl_from_block itself? If
> so, for extra safety perhaps that routine should check if the location
> is unknown and return early if it is. One other minor comment below,
> otherwise it looks ok to me.
>
> Teresa
>
>>
>> Bootstrapped and passed all regression tests.
>>
>> Ok for gcc-4_7 branch?
>>
>> Thanks,
>> Dehao
>>
>> Index: gcc/auto-profile.c
>> ===================================================================
>> --- gcc/auto-profile.c (revision 198117)
>> +++ gcc/auto-profile.c (working copy)
>> @@ -662,9 +662,10 @@ get_inline_stack_by_stmt (gimple stmt, tree decl,
>>         block && (TREE_CODE (block) == BLOCK);
>>         block = BLOCK_SUPERCONTEXT (block))
>>      {
>> -      tree decl = get_function_decl_from_block (block);
>> +      tree decl;
>>        if (LOCATION_LOCUS (BLOCK_SOURCE_LOCATION (block)) == UNKNOWN_LOCATION)
>>   continue;
>> +      decl = get_function_decl_from_block (block);
>>        loc = BLOCK_SOURCE_LOCATION (block);
>
> Do you want to move up the assignment to loc and use loc in the new if
> statement?
>
>>        pos_stack[idx].file = expand_location (loc).file;
>>        pos_stack[idx].line = expand_location (loc).line;
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413

Looks ok to me.
Teresa

On Sun, Apr 21, 2013 at 7:15 PM, Dehao Chen <dehao@google.com> wrote:
> Thanks for the comment, new patch attached:
>
> Index: gcc/auto-profile.c
> ===================================================================
> --- gcc/auto-profile.c (revision 198117)
> +++ gcc/auto-profile.c (working copy)
> @@ -623,6 +623,10 @@ static tree
>  get_function_decl_from_block (tree block)
>  {
>    tree decl;
> +
> +  if (LOCATION_LOCUS (BLOCK_SOURCE_LOCATION (block) == UNKNOWN_LOCATION))
> +    return NULL_TREE;
> +
>    for (decl = BLOCK_ABSTRACT_ORIGIN (block);
>         decl && (TREE_CODE (decl) == BLOCK);
>         decl = BLOCK_ABSTRACT_ORIGIN (decl))
> @@ -662,10 +666,12 @@ get_inline_stack_by_stmt (gimple stmt, tree decl,
>         block && (TREE_CODE (block) == BLOCK);
>         block = BLOCK_SUPERCONTEXT (block))
>      {
> -      tree decl = get_function_decl_from_block (block);
> -      if (LOCATION_LOCUS (BLOCK_SOURCE_LOCATION (block)) == UNKNOWN_LOCATION)
> +      tree decl;
> +      loc = BLOCK_SOURCE_LOCATION (block);
> +
> +      if (LOCATION_LOCUS (loc) == UNKNOWN_LOCATION)
>   continue;
> -      loc = BLOCK_SOURCE_LOCATION (block);
> +      decl = get_function_decl_from_block (block);
>        pos_stack[idx].file = expand_location (loc).file;
>        pos_stack[idx].line = expand_location (loc).line;
>        pos_stack[idx - 1].func =
>
> On Sun, Apr 21, 2013 at 5:00 PM, Teresa Johnson <tejohnson@google.com> wrote:
>> On Sun, Apr 21, 2013 at 4:51 PM, Dehao Chen <dehao@google.com> wrote:
>>> This patch fixed a bug in getting inline stacks: if there is no
>>> location info attached to a block, we should *not* try to get its
>>> function name because it could result in infinite loop.
>>
>> Is the infinite loop within get_function_decl_from_block itself? If
>> so, for extra safety perhaps that routine should check if the location
>> is unknown and return early if it is. One other minor comment below,
>> otherwise it looks ok to me.
>>
>> Teresa
>>
>>>
>>> Bootstrapped and passed all regression tests.
>>>
>>> Ok for gcc-4_7 branch?
>>>
>>> Thanks,
>>> Dehao
>>>
>>> Index: gcc/auto-profile.c
>>> ===================================================================
>>> --- gcc/auto-profile.c (revision 198117)
>>> +++ gcc/auto-profile.c (working copy)
>>> @@ -662,9 +662,10 @@ get_inline_stack_by_stmt (gimple stmt, tree decl,
>>>         block && (TREE_CODE (block) == BLOCK);
>>>         block = BLOCK_SUPERCONTEXT (block))
>>>      {
>>> -      tree decl = get_function_decl_from_block (block);
>>> +      tree decl;
>>>        if (LOCATION_LOCUS (BLOCK_SOURCE_LOCATION (block)) == UNKNOWN_LOCATION)
>>>   continue;
>>> +      decl = get_function_decl_from_block (block);
>>>        loc = BLOCK_SOURCE_LOCATION (block);
>>
>> Do you want to move up the assignment to loc and use loc in the new if
>> statement?
>>
>>>        pos_stack[idx].file = expand_location (loc).file;
>>>        pos_stack[idx].line = expand_location (loc).line;
>>
>>
>>
>> --
>> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413