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.mira...@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. */
