Re: [PATH] Patch to fix -finstrument-functions-exclude-function-list handling of namespaces and escaped commas

[Jeff Law <law@redhat.com>]

On 6/12/19 12:25 PM, Oliver Browne wrote:
> Patch fixes following PRs:
> c++/90816 - -finstrument-functions-exclude-function-list improperly
> handles namespace/class definitions
> c++/90809 - -finstrument-functions-exclude-function-list mishandles
> comma escaping
> 
> Fixes as follows:
> At flag_instrument_functions_exclude_p [gimplify.c]
> Using lang_hooks.decl_printable_name (fndecl, 1) to get namespace /
> class information as part of printable name to allow for
> inclusion of namespace / class specification when passing symbols to
> -finstrument-functions-exclude-function-list. Was
> previously lang_hooks.decl_printable_name (fndecl, 0).
> 
> At add_comma_separated_to_vector [opts.c]
> Added writing of a null character to w after primary loop finishes, to
> account for offset between r and w when r reaches end of
> passed string.
> 
> from Oliver Browne <oliverbrowne627@gmail.com>
> PR c++/90816
> PR c++/90809
>  * gimplify.c (flag_instrument_functions_exclude_p): include namespace
>    information as part of decl name
>  * opts.c (add_comma_separated_to_vector): add null character to correct
>    position in last token added to token vector
> Index: gimplify.c
> ===================================================================
> --- gimplify.c 2019-06-12 19:07:26.872077000 +0100
> +++ gimplify.c 2019-06-12 18:55:10.609255000 +0100
> @@ -13987,11 +13987,17 @@ flag_instrument_functions_exclude_p (tre
>      {
>        const char *name;
> -      int i;
> +      unsigned int i;
>        char *s;
> 
> -      name = lang_hooks.decl_printable_name (fndecl, 0);
> -      FOR_EACH_VEC_ELT (*v, i, s)
> +      name = lang_hooks.decl_printable_name (fndecl, 1);
> +   for(i = 0; i < v->length(); i++){
> + s = (*v)[i];
> + if(strstr(name, s) != NULL){
> +   return(true);
> + }
> +   }
> +/*      FOR_EACH_VEC_ELT (*v, i, s)
>   if (strstr (name, s) != NULL)
> -   return true;
> + return true;*/
>      }
So why did you drop the FOR_EACH_VEC_ELT and open-code the loop?  I
don't see that as being a necessary change.  Leaving the
FOR_EACH_VEC_ELT in place would also avoid the mis-formatting you've
introduced as well as removing clutter of a commented-out hunk of code.

> 
> @@ -14278,3 +14284,3 @@ gimplify_hasher::equal (const elt_t *p1,
> 
>    return true;
> -}
> \ No newline at end of file
> +}
> Index: opts.c
> ===================================================================
> --- opts.c 2019-06-12 19:10:04.354612000 +0100
> +++ opts.c 2019-06-12 18:53:43.675852000 +0100
> @@ -263,7 +263,8 @@ add_comma_separated_to_vector (void **pv
>   *w++ = *r++;
>      }
> -  if (*token_start != '\0')
> +  *w = '\0';
> +  if (*token_start != '\0'){
>      v->safe_push (token_start);
> -
> +  }
>    *pvec = v;
So why introduce the unnecessary { } scope?  And why do it in a way that
is different than 99% of the other code in GCC (where the { and } will
be on individual lines with 2 spaces of indention?

Jeff