bpf: Fixed GC mistakes in BPF builtins code.

  Hi everyone,

This patch fixes BPF CO-RE builtins support that missed information for
garbage collector (GC).

The BPF CO-RE implementation defines several data structures that keep
builtin information throught all of the compilation flow aside from
code.  This intentionally avoids having the builtin calls arguments
expressions/enum/types tree nodes within the compiling code in order to
avoid the compiler to optimize those away, based on information in
current compilation unit.
CO-RE builtins are target kernel specific and very little can be infered
from type inforamtion within the compilation unit.

Fault was triggered when attempting to compile some BPF kernel big
examples that revealed the lack of GC information.

Patch also removes some spurious includes of header files.

Best regards,
Cupertino
  

Hi Cuper.

OK.  Hopefully all the roots are marked now to avoid these nodes being
collected.

Thanks.

> Hi everyone,
>
> This patch fixes BPF CO-RE builtins support that missed information for
> garbage collector (GC).
>
> The BPF CO-RE implementation defines several data structures that keep
> builtin information throught all of the compilation flow aside from
> code.  This intentionally avoids having the builtin calls arguments
> expressions/enum/types tree nodes within the compiling code in order to
> avoid the compiler to optimize those away, based on information in
> current compilation unit.
> CO-RE builtins are target kernel specific and very little can be infered
> from type inforamtion within the compilation unit.
>
> Fault was triggered when attempting to compile some BPF kernel big
> examples that revealed the lack of GC information.
>
> Patch also removes some spurious includes of header files.
>
> Best regards,
> Cupertino
>
>
>
> commit c71b5c604189d04664c5b5ee155326fa4b79808b
> Author: Cupertino Miranda <cupertino.miranda@oracle.com>
> Date:   Tue Aug 8 11:12:00 2023 +0100
>
>     bpf: Fixed GC mistakes in BPF builtins code.
>     
>     This patches fixes problems with GC within the CO-RE builtins
>     implementation.
>     List of included headers was also reviseD.
>     
>     gcc/ChangeLog:
>     
>             * config/bpf/core-builtins.cc: Cleaned include headers.
>             (struct cr_builtins): Added GTY.
>             (cr_builtins_ref): Created.
>             (builtins_data) Changed to GC root.
>             (allocate_builtin_data): Changed.
>             Included gt-core-builtins.h.
>             * config/bpf/coreout.cc: (bpf_core_extra) Added GTY.
>             (bpf_core_extra_ref): Created.
>             (bpf_comment_info): Changed to GC root.
>             (bpf_core_reloc_add, output_btfext_header, btf_ext_init): Changed.
>
> diff --git a/gcc/config/bpf/core-builtins.cc b/gcc/config/bpf/core-builtins.cc
> index 575e63d8ea77..c3222b4c7804 100644
> --- a/gcc/config/bpf/core-builtins.cc
> +++ b/gcc/config/bpf/core-builtins.cc
> @@ -22,52 +22,23 @@ along with GCC; see the file COPYING3.  If not see
>  #include "config.h"
>  #include "system.h"
>  #include "coretypes.h"
> -#include "tm.h"
> +#include "target.h"
>  #include "rtl.h"
> -#include "regs.h"
> -#include "insn-config.h"
> -#include "insn-attr.h"
> -#include "recog.h"
>  #include "output.h"
> -#include "alias.h"
>  #include "tree.h"
>  #include "stringpool.h"
>  #include "attribs.h"
> -#include "varasm.h"
> -#include "stor-layout.h"
> -#include "calls.h"
>  #include "function.h"
> -#include "explow.h"
>  #include "memmodel.h"
>  #include "emit-rtl.h"
> -#include "reload.h"
> -#include "tm_p.h"
> -#include "target.h"
> -#include "basic-block.h"
>  #include "expr.h"
> -#include "optabs.h"
> -#include "bitmap.h"
> -#include "df.h"
> -#include "c-family/c-common.h"
>  #include "diagnostic.h"
> -#include "builtins.h"
> -#include "predict.h"
>  #include "langhooks.h"
> -#include "flags.h"
> -
> -#include "cfg.h"
> +#include "basic-block.h"
>  #include "gimple.h"
>  #include "gimple-iterator.h"
>  #include "gimple-walk.h"
>  #include "tree-pass.h"
> -#include "tree-iterator.h"
> -
> -#include "context.h"
> -#include "pass_manager.h"
> -
> -#include "gimplify.h"
> -#include "gimplify-me.h"
> -
>  #include "plugin.h"
>  
>  #include "ctfc.h"
> @@ -159,37 +130,41 @@ along with GCC; see the file COPYING3.  If not see
>      as a builtin.  */
>  
>  
> -struct cr_builtins
> +struct GTY(()) cr_builtins
>  {
>    tree type;
>    tree expr;
>    tree default_value;
>    rtx rtx_default_value;
> -  enum btf_core_reloc_kind kind; /* Recovered from proper argument.  */
> +  enum btf_core_reloc_kind kind;
>    enum bpf_builtins orig_builtin_code;
>    tree orig_arg_expr;
>  };
> +typedef struct cr_builtins *cr_builtins_ref;
>  
>  #define CORE_BUILTINS_DATA_EMPTY \
>    { NULL_TREE, NULL_TREE, NULL_TREE, NULL_RTX, BPF_RELO_INVALID, \
>      BPF_BUILTIN_UNUSED, NULL }
>  
>  /* Vector definition and its access function.  */
> -vec<struct cr_builtins> builtins_data;
> +static GTY(()) vec<cr_builtins_ref, va_gc> *builtins_data = NULL;
>  
>  static inline int
>  allocate_builtin_data ()
>  {
> -  struct cr_builtins data = CORE_BUILTINS_DATA_EMPTY;
> -  int ret = builtins_data.length ();
> -  builtins_data.safe_push (data);
> +  if (builtins_data == NULL)
> +    vec_alloc (builtins_data, 1);
> +
> +  cr_builtins_ref data = ggc_cleared_alloc<struct cr_builtins> ();
> +  int ret = builtins_data->length ();
> +  vec_safe_push (builtins_data, data);
>    return ret;
>  }
>  
>  static inline struct cr_builtins *
>  get_builtin_data (int index)
>  {
> -  return &builtins_data[index];
> +  return (*builtins_data)[index];
>  }
>  
>  typedef bool
> @@ -200,11 +175,12 @@ search_builtin_data (builtin_local_data_compare_fn callback,
>  		     struct cr_builtins *elem)
>  {
>    unsigned int i;
> -  for (i = 0; i < builtins_data.length (); i++)
> -    if ((callback != NULL && (callback) (elem, &builtins_data[i]))
> -       || (callback == NULL
> -	   && (builtins_data[i].orig_arg_expr == elem->orig_arg_expr)))
> -      return (int) i;
> +  if(builtins_data != NULL)
> +    for (i = 0; i < builtins_data->length (); i++)
> +      if ((callback != NULL && (callback) (elem, (*builtins_data)[i]))
> +	  || (callback == NULL
> +	      && ((*builtins_data)[i]->orig_arg_expr == elem->orig_arg_expr)))
> +	return (int) i;
>  
>    return -1;
>  }
> @@ -1392,3 +1368,5 @@ bpf_replace_core_move_operands (rtx *operands)
>  	  }
>        }
>  }
> +
> +#include "gt-core-builtins.h"
> diff --git a/gcc/config/bpf/coreout.cc b/gcc/config/bpf/coreout.cc
> index b84585fb104e..0c5d166298fb 100644
> --- a/gcc/config/bpf/coreout.cc
> +++ b/gcc/config/bpf/coreout.cc
> @@ -147,11 +147,12 @@ static char btf_ext_info_section_label[MAX_BTF_EXT_LABEL_BYTES];
>  
>  static GTY (()) vec<bpf_core_section_ref, va_gc> *bpf_core_sections;
>  
> -struct bpf_core_extra {
> +struct GTY(()) bpf_core_extra {
>    const char *accessor_str;
>    tree type;
>  };
> -static hash_map<bpf_core_reloc_ref, struct bpf_core_extra *> bpf_comment_info;
> +typedef struct bpf_core_extra *bpf_core_extra_ref;
> +static GTY(()) hash_map<bpf_core_reloc_ref, bpf_core_extra_ref> *bpf_comment_info;
>  
>  /* Create a new BPF CO-RE relocation record, and add it to the appropriate
>     CO-RE section.  */
> @@ -162,7 +163,7 @@ bpf_core_reloc_add (const tree type, const char * section_name,
>  		    enum btf_core_reloc_kind kind)
>  {
>    bpf_core_reloc_ref bpfcr = ggc_cleared_alloc<bpf_core_reloc_t> ();
> -  struct bpf_core_extra *info = ggc_cleared_alloc<struct bpf_core_extra> ();
> +  bpf_core_extra_ref info = ggc_cleared_alloc<struct bpf_core_extra> ();
>    ctf_container_ref ctfc = ctf_get_tu_ctfc ();
>  
>    /* Buffer the access string in the auxiliary strtab.  */
> @@ -173,7 +174,7 @@ bpf_core_reloc_add (const tree type, const char * section_name,
>  
>    info->accessor_str = accessor;
>    info->type = type;
> -  bpf_comment_info.put (bpfcr, info);
> +  bpf_comment_info->put (bpfcr, info);
>  
>    /* Add the CO-RE reloc to the appropriate section.  */
>    bpf_core_section_ref sec;
> @@ -288,7 +289,7 @@ output_btfext_header (void)
>  static void
>  output_asm_btfext_core_reloc (bpf_core_reloc_ref bpfcr)
>  {
> -  struct bpf_core_extra **info = bpf_comment_info.get (bpfcr);
> +  bpf_core_extra_ref *info = bpf_comment_info->get (bpfcr);
>    gcc_assert (info != NULL);
>  
>    bpfcr->bpfcr_astr_off += ctfc_get_strtab_len (ctf_get_tu_ctfc (),
> @@ -365,6 +366,7 @@ btf_ext_init (void)
>  			       btf_ext_label_num++);
>  
>    vec_alloc (bpf_core_sections, 1);
> +  bpf_comment_info = hash_map<bpf_core_reloc_ref, bpf_core_extra_ref>::create_ggc ();
>  }
>  
>  /* Output the entire .BTF.ext section.  */
  

Thanks! Pushed to master.

Jose E. Marchesi writes:

> Hi Cuper.
>
> OK.  Hopefully all the roots are marked now to avoid these nodes being
> collected.
>
> Thanks.
>
>> Hi everyone,
>>
>> This patch fixes BPF CO-RE builtins support that missed information for
>> garbage collector (GC).
>>
>> The BPF CO-RE implementation defines several data structures that keep
>> builtin information throught all of the compilation flow aside from
>> code.  This intentionally avoids having the builtin calls arguments
>> expressions/enum/types tree nodes within the compiling code in order to
>> avoid the compiler to optimize those away, based on information in
>> current compilation unit.
>> CO-RE builtins are target kernel specific and very little can be infered
>> from type inforamtion within the compilation unit.
>>
>> Fault was triggered when attempting to compile some BPF kernel big
>> examples that revealed the lack of GC information.
>>
>> Patch also removes some spurious includes of header files.
>>
>> Best regards,
>> Cupertino
>>
>>
>>
>> commit c71b5c604189d04664c5b5ee155326fa4b79808b
>> Author: Cupertino Miranda <cupertino.miranda@oracle.com>
>> Date:   Tue Aug 8 11:12:00 2023 +0100
>>
>>     bpf: Fixed GC mistakes in BPF builtins code.
>>
>>     This patches fixes problems with GC within the CO-RE builtins
>>     implementation.
>>     List of included headers was also reviseD.
>>
>>     gcc/ChangeLog:
>>
>>             * config/bpf/core-builtins.cc: Cleaned include headers.
>>             (struct cr_builtins): Added GTY.
>>             (cr_builtins_ref): Created.
>>             (builtins_data) Changed to GC root.
>>             (allocate_builtin_data): Changed.
>>             Included gt-core-builtins.h.
>>             * config/bpf/coreout.cc: (bpf_core_extra) Added GTY.
>>             (bpf_core_extra_ref): Created.
>>             (bpf_comment_info): Changed to GC root.
>>             (bpf_core_reloc_add, output_btfext_header, btf_ext_init): Changed.
>>
>> diff --git a/gcc/config/bpf/core-builtins.cc b/gcc/config/bpf/core-builtins.cc
>> index 575e63d8ea77..c3222b4c7804 100644
>> --- a/gcc/config/bpf/core-builtins.cc
>> +++ b/gcc/config/bpf/core-builtins.cc
>> @@ -22,52 +22,23 @@ along with GCC; see the file COPYING3.  If not see
>>  #include "config.h"
>>  #include "system.h"
>>  #include "coretypes.h"
>> -#include "tm.h"
>> +#include "target.h"
>>  #include "rtl.h"
>> -#include "regs.h"
>> -#include "insn-config.h"
>> -#include "insn-attr.h"
>> -#include "recog.h"
>>  #include "output.h"
>> -#include "alias.h"
>>  #include "tree.h"
>>  #include "stringpool.h"
>>  #include "attribs.h"
>> -#include "varasm.h"
>> -#include "stor-layout.h"
>> -#include "calls.h"
>>  #include "function.h"
>> -#include "explow.h"
>>  #include "memmodel.h"
>>  #include "emit-rtl.h"
>> -#include "reload.h"
>> -#include "tm_p.h"
>> -#include "target.h"
>> -#include "basic-block.h"
>>  #include "expr.h"
>> -#include "optabs.h"
>> -#include "bitmap.h"
>> -#include "df.h"
>> -#include "c-family/c-common.h"
>>  #include "diagnostic.h"
>> -#include "builtins.h"
>> -#include "predict.h"
>>  #include "langhooks.h"
>> -#include "flags.h"
>> -
>> -#include "cfg.h"
>> +#include "basic-block.h"
>>  #include "gimple.h"
>>  #include "gimple-iterator.h"
>>  #include "gimple-walk.h"
>>  #include "tree-pass.h"
>> -#include "tree-iterator.h"
>> -
>> -#include "context.h"
>> -#include "pass_manager.h"
>> -
>> -#include "gimplify.h"
>> -#include "gimplify-me.h"
>> -
>>  #include "plugin.h"
>>
>>  #include "ctfc.h"
>> @@ -159,37 +130,41 @@ along with GCC; see the file COPYING3.  If not see
>>      as a builtin.  */
>>
>>
>> -struct cr_builtins
>> +struct GTY(()) cr_builtins
>>  {
>>    tree type;
>>    tree expr;
>>    tree default_value;
>>    rtx rtx_default_value;
>> -  enum btf_core_reloc_kind kind; /* Recovered from proper argument.  */
>> +  enum btf_core_reloc_kind kind;
>>    enum bpf_builtins orig_builtin_code;
>>    tree orig_arg_expr;
>>  };
>> +typedef struct cr_builtins *cr_builtins_ref;
>>
>>  #define CORE_BUILTINS_DATA_EMPTY \
>>    { NULL_TREE, NULL_TREE, NULL_TREE, NULL_RTX, BPF_RELO_INVALID, \
>>      BPF_BUILTIN_UNUSED, NULL }
>>
>>  /* Vector definition and its access function.  */
>> -vec<struct cr_builtins> builtins_data;
>> +static GTY(()) vec<cr_builtins_ref, va_gc> *builtins_data = NULL;
>>
>>  static inline int
>>  allocate_builtin_data ()
>>  {
>> -  struct cr_builtins data = CORE_BUILTINS_DATA_EMPTY;
>> -  int ret = builtins_data.length ();
>> -  builtins_data.safe_push (data);
>> +  if (builtins_data == NULL)
>> +    vec_alloc (builtins_data, 1);
>> +
>> +  cr_builtins_ref data = ggc_cleared_alloc<struct cr_builtins> ();
>> +  int ret = builtins_data->length ();
>> +  vec_safe_push (builtins_data, data);
>>    return ret;
>>  }
>>
>>  static inline struct cr_builtins *
>>  get_builtin_data (int index)
>>  {
>> -  return &builtins_data[index];
>> +  return (*builtins_data)[index];
>>  }
>>
>>  typedef bool
>> @@ -200,11 +175,12 @@ search_builtin_data (builtin_local_data_compare_fn callback,
>>  		     struct cr_builtins *elem)
>>  {
>>    unsigned int i;
>> -  for (i = 0; i < builtins_data.length (); i++)
>> -    if ((callback != NULL && (callback) (elem, &builtins_data[i]))
>> -       || (callback == NULL
>> -	   && (builtins_data[i].orig_arg_expr == elem->orig_arg_expr)))
>> -      return (int) i;
>> +  if(builtins_data != NULL)
>> +    for (i = 0; i < builtins_data->length (); i++)
>> +      if ((callback != NULL && (callback) (elem, (*builtins_data)[i]))
>> +	  || (callback == NULL
>> +	      && ((*builtins_data)[i]->orig_arg_expr == elem->orig_arg_expr)))
>> +	return (int) i;
>>
>>    return -1;
>>  }
>> @@ -1392,3 +1368,5 @@ bpf_replace_core_move_operands (rtx *operands)
>>  	  }
>>        }
>>  }
>> +
>> +#include "gt-core-builtins.h"
>> diff --git a/gcc/config/bpf/coreout.cc b/gcc/config/bpf/coreout.cc
>> index b84585fb104e..0c5d166298fb 100644
>> --- a/gcc/config/bpf/coreout.cc
>> +++ b/gcc/config/bpf/coreout.cc
>> @@ -147,11 +147,12 @@ static char btf_ext_info_section_label[MAX_BTF_EXT_LABEL_BYTES];
>>
>>  static GTY (()) vec<bpf_core_section_ref, va_gc> *bpf_core_sections;
>>
>> -struct bpf_core_extra {
>> +struct GTY(()) bpf_core_extra {
>>    const char *accessor_str;
>>    tree type;
>>  };
>> -static hash_map<bpf_core_reloc_ref, struct bpf_core_extra *> bpf_comment_info;
>> +typedef struct bpf_core_extra *bpf_core_extra_ref;
>> +static GTY(()) hash_map<bpf_core_reloc_ref, bpf_core_extra_ref> *bpf_comment_info;
>>
>>  /* Create a new BPF CO-RE relocation record, and add it to the appropriate
>>     CO-RE section.  */
>> @@ -162,7 +163,7 @@ bpf_core_reloc_add (const tree type, const char * section_name,
>>  		    enum btf_core_reloc_kind kind)
>>  {
>>    bpf_core_reloc_ref bpfcr = ggc_cleared_alloc<bpf_core_reloc_t> ();
>> -  struct bpf_core_extra *info = ggc_cleared_alloc<struct bpf_core_extra> ();
>> +  bpf_core_extra_ref info = ggc_cleared_alloc<struct bpf_core_extra> ();
>>    ctf_container_ref ctfc = ctf_get_tu_ctfc ();
>>
>>    /* Buffer the access string in the auxiliary strtab.  */
>> @@ -173,7 +174,7 @@ bpf_core_reloc_add (const tree type, const char * section_name,
>>
>>    info->accessor_str = accessor;
>>    info->type = type;
>> -  bpf_comment_info.put (bpfcr, info);
>> +  bpf_comment_info->put (bpfcr, info);
>>
>>    /* Add the CO-RE reloc to the appropriate section.  */
>>    bpf_core_section_ref sec;
>> @@ -288,7 +289,7 @@ output_btfext_header (void)
>>  static void
>>  output_asm_btfext_core_reloc (bpf_core_reloc_ref bpfcr)
>>  {
>> -  struct bpf_core_extra **info = bpf_comment_info.get (bpfcr);
>> +  bpf_core_extra_ref *info = bpf_comment_info->get (bpfcr);
>>    gcc_assert (info != NULL);
>>
>>    bpfcr->bpfcr_astr_off += ctfc_get_strtab_len (ctf_get_tu_ctfc (),
>> @@ -365,6 +366,7 @@ btf_ext_init (void)
>>  			       btf_ext_label_num++);
>>
>>    vec_alloc (bpf_core_sections, 1);
>> +  bpf_comment_info = hash_map<bpf_core_reloc_ref, bpf_core_extra_ref>::create_ggc ();
>>  }
>>
>>  /* Output the entire .BTF.ext section.  */