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?
>
>> +
>> +/* 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";