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

[Dehao Chen <dehao at google dot com>]

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