> On 6/12/24 09:55, Jose E. Marchesi wrote:
>>
>> Hi Faust.
>> Thanks for the patch.
>> Please see a question below.
>>
>>> This patch enables -gprune-btf by default in the BPF backend when
>>> generating BTF information, and fixes BPF CO-RE generation when using
>>> -gprune-btf.
>>>
>>> When generating BPF CO-RE information, we must ensure that types used
>>> in CO-RE relocations always have sufficient BTF information emited so
>>> that the CO-RE relocations can be processed by a BPF loader. The BTF
>>> pruning algorithm on its own does not have sufficient information to
>>> determine which types are used in a BPF CO-RE relocation, so this
>>> information must be supplied by the BPF backend, using a new
>>> btf_mark_type_used function.
>>>
>>> Co-authored-by: Cupertino Miranda <cupertino.mira...@oracle.com>
>>>
>>> gcc/
>>> * btfout.cc (btf_mark_type_used): New.
>>> * ctfc.h (btf_mark_type_used): Declare it here.
>>> * config/bpf/bpf.cc (bpf_option_override): Enable -gprune-btf
>>> by default if -gbtf is enabled.
>>> * config/bpf/core-builtins.cc (extra_fn): New typedef.
>>> (compute_field_expr): Add callback parameter, and call it if supplied.
>>> Fix computation for MEM_REF.
>>> (mark_component_type_as_used): New.
>>> (bpf_mark_types_as_used): Likewise.
>>> (bpf_expand_core_builtin): Call here.
>>> * doc/invoke.texi (Debugging Options): Note that -gprune-btf is
>>> enabled by default for BPF target when generating BTF.
>>>
>>> gcc/testsuite/
>>> * gcc.dg/debug/btf/btf-variables-5.c: Adjust one test for bpf-*-*
>>> target.
>>> ---
>>> gcc/btfout.cc | 22 ++++++
>>> gcc/config/bpf/bpf.cc | 5 ++
>>> gcc/config/bpf/core-builtins.cc | 71 +++++++++++++++++--
>>> gcc/ctfc.h | 1 +
>>> gcc/doc/invoke.texi | 3 +
>>> .../gcc.dg/debug/btf/btf-variables-5.c | 6 +-
>>> 6 files changed, 100 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
>>> index 34d8cec0a2e3..083ca48d6279 100644
>>> --- a/gcc/btfout.cc
>>> +++ b/gcc/btfout.cc
>>> @@ -1503,6 +1503,28 @@ btf_assign_datasec_ids (ctf_container_ref ctfc)
>>> }
>>> }
>>>
>>> +
>>> +/* Manually mark that type T is used to ensure it will not be pruned.
>>> + Used by the BPF backend when generating BPF CO-RE to mark types used
>>> + in CO-RE relocations. */
>>> +
>>> +void
>>> +btf_mark_type_used (tree t)
>>> +{
>>> + /* If we are not going to prune anyway, this is a no-op. */
>>> + if (!debug_prune_btf)
>>> + return;
>>> +
>>> + gcc_assert (TYPE_P (t));
>>> + ctf_container_ref ctfc = ctf_get_tu_ctfc ();
>>> + ctf_dtdef_ref dtd = ctf_lookup_tree_type (ctfc, t);
>>> +
>>> + if (!dtd)
>>> + return;
>>> +
>>> + btf_add_used_type (ctfc, dtd, false, false, true);
>>> +}
>>> +
>>> /* Callback used for assembling the only-used-types list. Note that this
>>> is
>>> the same as btf_type_list_cb above, but the hash_set traverse requires a
>>> different function signature. */
>>> diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
>>> index dd1bfe38d29b..c62af7a6efa7 100644
>>> --- a/gcc/config/bpf/bpf.cc
>>> +++ b/gcc/config/bpf/bpf.cc
>>> @@ -221,6 +221,11 @@ bpf_option_override (void)
>>> && !(target_flags_explicit & MASK_BPF_CORE))
>>> target_flags |= MASK_BPF_CORE;
>>>
>>> + /* -gbtf implies -gprune-btf for BPF target. */
>>> + if (btf_debuginfo_p ())
>>> + SET_OPTION_IF_UNSET (&global_options, &global_options_set,
>>> + debug_prune_btf, true);
>>> +
>>> /* Determine available features from ISA setting (-mcpu=). */
>>> if (bpf_has_jmpext == -1)
>>> bpf_has_jmpext = (bpf_isa >= ISA_V2);
>>> diff --git a/gcc/config/bpf/core-builtins.cc
>>> b/gcc/config/bpf/core-builtins.cc
>>> index 232bebcadbd5..86e2e9d6e39f 100644
>>> --- a/gcc/config/bpf/core-builtins.cc
>>> +++ b/gcc/config/bpf/core-builtins.cc
>>> @@ -624,13 +624,20 @@ bpf_core_get_index (const tree node, bool *valid)
>>>
>>> ALLOW_ENTRY_CAST is an input arguments and specifies if the function
>>> should
>>> consider as valid expressions in which NODE entry is a cast expression
>>> (or
>>> - tree code nop_expr). */
>>> + tree code nop_expr).
>>> +
>>> + EXTRA_FN is a callback function to allow extra functionality with this
>>> + function traversal. Currently used for marking used type during expand
>>> + pass. */
>>> +
>>> +typedef void (*extra_fn) (tree);
>>>
>>> static unsigned char
>>> compute_field_expr (tree node, unsigned int *accessors,
>>> bool *valid,
>>> tree *access_node,
>>> - bool allow_entry_cast = true)
>>> + bool allow_entry_cast = true,
>>> + extra_fn callback = NULL)
>>> {
>>> unsigned char n = 0;
>>> unsigned int fake_accessors[MAX_NR_ACCESSORS];
>>> @@ -647,6 +654,9 @@ compute_field_expr (tree node, unsigned int *accessors,
>>>
>>> *access_node = node;
>>>
>>> + if (callback != NULL)
>>> + callback (node);
>>> +
>>> switch (TREE_CODE (node))
>>> {
>>> case INDIRECT_REF:
>>> @@ -664,17 +674,19 @@ compute_field_expr (tree node, unsigned int
>>> *accessors,
>>> case COMPONENT_REF:
>>> n = compute_field_expr (TREE_OPERAND (node, 0), accessors,
>>> valid,
>>> - access_node, false);
>>> + access_node, false, callback);
>>> accessors[n] = bpf_core_get_index (TREE_OPERAND (node, 1), valid);
>>> return n + 1;
>>> case ARRAY_REF:
>>> case ARRAY_RANGE_REF:
>>> - case MEM_REF:
>>> n = compute_field_expr (TREE_OPERAND (node, 0), accessors,
>>> valid,
>>> - access_node, false);
>>> + access_node, false, callback);
>>> accessors[n++] = bpf_core_get_index (node, valid);
>>> return n;
>>> + case MEM_REF:
>>> + accessors[0] = bpf_core_get_index (node, valid);
>>> + return 1;
>>> case NOP_EXPR:
>>> if (allow_entry_cast == true)
>>> {
>>> @@ -683,7 +695,7 @@ compute_field_expr (tree node, unsigned int *accessors,
>>> }
>>> n = compute_field_expr (TREE_OPERAND (node, 0), accessors,
>>> valid,
>>> - access_node, false);
>>> + access_node, false, callback);
>>> return n;
>>>
>>> case ADDR_EXPR:
>>> @@ -1549,6 +1561,51 @@ bpf_resolve_overloaded_core_builtin (location_t loc,
>>> tree fndecl,
>>> return construct_builtin_core_reloc (loc, fndecl, args, argsvec->length
>>> ());
>>> }
>>>
>>> +/* Callback function for bpf_mark_field_expr_types_as_used. */
>>> +
>>> +static void
>>> +mark_component_type_as_used (tree node)
>>> +{
>>> + if (TREE_CODE (node) == COMPONENT_REF)
>>> + btf_mark_type_used (TREE_TYPE (TREE_OPERAND (node, 0)));
>>> +}
>>
>> This means that the callback is only marking as used these types
>> reachable from the CO-RE builtin arguments that are referenced by
>> indexation or field name or pointer?
> The callback is only marking as used the types which are present in the
> CO-RE builtin argument by the chain of field accesses. It is not
> marking other types which may be part of those structures but are not
> used by the CO-RE builtin. The intent is to ensure type information
> needed to perform the CO-RE relocation is present, while allowing other
> extraneous types to be pruned.
>
> For example if we have:
>
> struct A {
> struct B *b;
> struct C *c;
> };
>
> struct B {
> int x;
> int y;
> };
>
> struct C {
> int w;
> int z;
> char stuff[400];
> struct some_other_huge_struct *ptr_to_huge;
> ...
> int member_number_800_at_offset_10620;
> };
>
> struct A my_a = ...;
> __builtin_preserve_access_index (my_a->b.x);
>
> In this case, we will mark struct B as used, but not struct C.
>
> Supposing struct C is otherwise unused, then in the pruned BTF
> we may get like:
>
> [1] BTF_KIND_INT "int" bits=32 ..
> [2] BTF_KIND_STRUCT "A"
> member "b" type=3
> member "c" type=5
> [3] BTF_KIND_PTR type=4
> [4] BTF_KIND_STRUCT "B"
> member "x" type=1
> member "y" type=1
> [5] BTF_KIND_PTR type=6
> [6] BTF_KIND_FWD "C" fwd_kind=struct
>
> Where the full type information for C is pruned because it is not
> necessary for this program, yet everything necessary to perform the
> CO-RE relocation correctly is present.
>
> Does that make sense to answer your question?
Yes, thats what I understood.
OK for this particular patch for master.
Thanks!
I believe you need some global maintainer to chime in for patches 4/6
and 6/6.
>
>>
>>> +
>>> +/* Mark types needed for BPF CO-RE relocations as used. Doing so ensures
>>> that
>>> + these types do not get pruned from the BTF information. */
>>> +
>>> +static void
>>> +bpf_mark_types_as_used (struct cr_builtins *data)
>>> +{
>>> + tree expr = data->expr;
>>> + switch (data->kind)
>>> + {
>>> + case BPF_RELO_FIELD_BYTE_OFFSET:
>>> + case BPF_RELO_FIELD_BYTE_SIZE:
>>> + case BPF_RELO_FIELD_EXISTS:
>>> + case BPF_RELO_FIELD_SIGNED:
>>> + case BPF_RELO_FIELD_LSHIFT_U64:
>>> + case BPF_RELO_FIELD_RSHIFT_U64:
>>> + if (TREE_CODE (expr) == ADDR_EXPR)
>>> + expr = TREE_OPERAND (expr, 0);
>>> +
>>> + expr = root_for_core_field_info (expr);
>>> + compute_field_expr (data->expr, NULL, NULL, NULL, false,
>>> + mark_component_type_as_used);
>>> + break;
>>> + case BPF_RELO_TYPE_ID_LOCAL:
>>> + case BPF_RELO_TYPE_ID_TARGET:
>>> + case BPF_RELO_TYPE_EXISTS:
>>> + case BPF_RELO_TYPE_SIZE:
>>> + case BPF_RELO_ENUMVAL_EXISTS:
>>> + case BPF_RELO_ENUMVAL_VALUE:
>>> + case BPF_RELO_TYPE_MATCHES:
>>> + btf_mark_type_used (data->type);
>>> + break;
>>> + default:
>>> + gcc_unreachable ();
>>> + }
>>> +}
>>> +
>>> /* Used in bpf_expand_builtin. This function is called in RTL expand
>>> stage to
>>> convert the internal __builtin_core_reloc in unspec:UNSPEC_CORE_RELOC
>>> RTL,
>>> which will contain a third argument that is the index in the vec
>>> collected
>>> @@ -1567,6 +1624,8 @@ bpf_expand_core_builtin (tree exp, enum bpf_builtins
>>> code)
>>> tree index = CALL_EXPR_ARG (exp, 0);
>>> struct cr_builtins *data = get_builtin_data (TREE_INT_CST_LOW (index));
>>>
>>> + bpf_mark_types_as_used (data);
>>> +
>>> rtx v = expand_normal (data->default_value);
>>> rtx i = expand_normal (index);
>>> return gen_rtx_UNSPEC (DImode,
>>> diff --git a/gcc/ctfc.h b/gcc/ctfc.h
>>> index 29267dc036d1..41e1169f271d 100644
>>> --- a/gcc/ctfc.h
>>> +++ b/gcc/ctfc.h
>>> @@ -457,6 +457,7 @@ extern ctf_dtdef_ref ctf_lookup_tree_type
>>> (ctf_container_ref, const tree);
>>>
>>> typedef bool (*funcs_traverse_callback) (ctf_dtdef_ref, void *);
>>> bool traverse_btf_func_types (funcs_traverse_callback, void *);
>>> +extern void btf_mark_type_used (tree);
>>>
>>> /* CTF section does not emit location information; at this time, location
>>> information is needed for BTF CO-RE use-cases. */
>>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>>> index 8479fd5cf2b8..0afd686733d0 100644
>>> --- a/gcc/doc/invoke.texi
>>> +++ b/gcc/doc/invoke.texi
>>> @@ -12023,6 +12023,9 @@ It is primarily useful when compiling for the BPF
>>> target, to minimize
>>> the size of the resulting object, and to eliminate BTF information
>>> which is not immediately relevant to the BPF program loading process.
>>>
>>> +This option is enabled by default for the BPF target when generating
>>> +BTF information.
>>> +
>>> @opindex gctf
>>> @item -gctf
>>> @itemx -gctf@var{level}
>>> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-variables-5.c
>>> b/gcc/testsuite/gcc.dg/debug/btf/btf-variables-5.c
>>> index 8aae76cacabd..a08130cfc072 100644
>>> --- a/gcc/testsuite/gcc.dg/debug/btf/btf-variables-5.c
>>> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-variables-5.c
>>> @@ -11,9 +11,11 @@
>>> /* { dg-final { scan-assembler-times "\[\t \]0xe000000\[\t
>>> \]+\[^\n\]*btv_info" 1 } } */
>>> /* { dg-final { scan-assembler-times "\[\t \]0x1\[\t
>>> \]+\[^\n\]*btv_linkage" 1 } } */
>>>
>>> -/* Expect 2 array types, one of which is unsized. */
>>> +/* Expect 2 array types, one of which is unsized. For BPF target,
>>> -gprune-btf
>>> + is the default and will remove the unsized array type. */
>>> /* { dg-final { scan-assembler-times "\[\t \]0x4\[\t
>>> \]+\[^\n\]*bta_nelems" 1 } } */
>>> -/* { dg-final { scan-assembler-times "\[\t \]0\[\t \]+\[^\n\]*bta_nelems"
>>> 1 } } */
>>> +/* { dg-final { scan-assembler-times "\[\t \]0\[\t \]+\[^\n\]*bta_nelems"
>>> 1 { target { !bpf-*-* } } } } */
>>> +/* { dg-final { scan-assembler-times "\[\t \]0\[\t \]+\[^\n\]*bta_nelems"
>>> 0 { target { bpf-*-* } } } } */
>>>
>>> extern const char FOO[];
>>> const char FOO[] = "foo";
