[3/3] btf: correct generation for extern funcs [PR106773]
Checks
Commit Message
The eBPF loader expects to find entries for functions declared as extern
in the corresponding BTF_KIND_DATASEC record, but we were not generating
these entries.
This patch adds support for the 'extern' linkage of function types in
BTF, and creates entries for for them BTF_KIND_DATASEC records as needed.
PR target/106773
gcc/
* btfout.cc (get_section_name): New function.
(btf_collect_datasec): Use it here. Process functions, marking them
'extern' and generating DATASEC entries for them as appropriate. Move
creation of BTF_KIND_FUNC records to here...
(btf_dtd_emit_preprocess_cb): ... from here.
gcc/testsuite/
* gcc.dg/debug/btf/btf-datasec-2.c: New test.
* gcc.dg/debug/btf/btf-function-6.c: New test.
include/
* btf.h (struct btf_var_secinfo): Update comments with notes about
extern functions.
---
gcc/btfout.cc | 129 ++++++++++++------
.../gcc.dg/debug/btf/btf-datasec-2.c | 28 ++++
.../gcc.dg/debug/btf/btf-function-6.c | 19 +++
include/btf.h | 9 +-
4 files changed, 139 insertions(+), 46 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c
create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-function-6.c
Comments
On 12/7/22 12:57, David Faust wrote:
> The eBPF loader expects to find entries for functions declared as extern
> in the corresponding BTF_KIND_DATASEC record, but we were not generating
> these entries.
>
> This patch adds support for the 'extern' linkage of function types in
> BTF, and creates entries for for them BTF_KIND_DATASEC records as needed.
>
> PR target/106773
>
> gcc/
>
> * btfout.cc (get_section_name): New function.
> (btf_collect_datasec): Use it here. Process functions, marking them
> 'extern' and generating DATASEC entries for them as appropriate. Move
> creation of BTF_KIND_FUNC records to here...
> (btf_dtd_emit_preprocess_cb): ... from here.
>
> gcc/testsuite/
>
> * gcc.dg/debug/btf/btf-datasec-2.c: New test.
> * gcc.dg/debug/btf/btf-function-6.c: New test.
>
> include/
>
> * btf.h (struct btf_var_secinfo): Update comments with notes about
> extern functions.
> ---
> gcc/btfout.cc | 129 ++++++++++++------
> .../gcc.dg/debug/btf/btf-datasec-2.c | 28 ++++
> .../gcc.dg/debug/btf/btf-function-6.c | 19 +++
> include/btf.h | 9 +-
> 4 files changed, 139 insertions(+), 46 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c
> create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-function-6.c
>
> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
> index 05f3a3f9b6e..d7ead377ec5 100644
> --- a/gcc/btfout.cc
> +++ b/gcc/btfout.cc
> @@ -294,7 +294,35 @@ btf_datasec_push_entry (ctf_container_ref ctfc, const char *secname,
> ds.entries.safe_push (info);
>
> datasecs.safe_push (ds);
> - num_types_created++;
> +}
> +
> +
> +/* Return the section name, as of interest to btf_collect_datasec, for the
> + given symtab node. Note that this deliberately returns NULL for objects
> + which do not go in a section btf_collect_datasec cares about. */
"Dot, space, space, new sentence."
> +static const char *
> +get_section_name (symtab_node *node)
> +{
> + const char *section_name = node->get_section ();
> +
> + if (section_name == NULL)
> + {
> + switch (categorize_decl_for_section (node->decl, 0))
> + {
> + case SECCAT_BSS:
> + section_name = ".bss";
> + break;
> + case SECCAT_DATA:
> + section_name = ".data";
> + break;
> + case SECCAT_RODATA:
> + section_name = ".rodata";
> + break;
> + default:;
> + }
> + }
> +
> + return section_name;
> }
>
> /* Construct all BTF_KIND_DATASEC records for CTFC. One such record is created
> @@ -305,7 +333,60 @@ btf_datasec_push_entry (ctf_container_ref ctfc, const char *secname,
> static void
> btf_collect_datasec (ctf_container_ref ctfc)
> {
> - /* See cgraph.h struct symtab_node, which varpool_node extends. */
> + cgraph_node *func;
> + FOR_EACH_FUNCTION (func)
> + {
> + dw_die_ref die = lookup_decl_die (func->decl);
> + if (die == NULL)
> + continue;
> +
> + ctf_dtdef_ref dtd = ctf_dtd_lookup (ctfc, die);
> + if (dtd == NULL)
> + continue;
> +
> + /* Functions actually get two types: a BTF_KIND_FUNC_PROTO, and
> + also a BTF_KIND_FUNC. But the CTF container only allocates one
> + type per function, which matches closely with BTF_KIND_FUNC_PROTO.
> + For each such function, also allocate a BTF_KIND_FUNC entry.
> + These will be output later. */
"Dot, space, space, new sentence."
> + ctf_dtdef_ref func_dtd = ggc_cleared_alloc<ctf_dtdef_t> ();
> + func_dtd->dtd_data = dtd->dtd_data;
> + func_dtd->dtd_data.ctti_type = dtd->dtd_type;
> + func_dtd->linkage = dtd->linkage;
> + func_dtd->dtd_type = num_types_added + num_types_created;
> +
> + /* Only the BTF_KIND_FUNC type actually references the name. The
> + BTF_KIND_FUNC_PROTO is always anonymous. */
> + dtd->dtd_data.ctti_name = 0;
> +
> + vec_safe_push (funcs, func_dtd);
> + num_types_created++;
> +
> + /* Mark any 'extern' funcs and add DATASEC entries for them. */
> + if (DECL_EXTERNAL (func->decl))
> + {
> + func_dtd->linkage = BTF_LINKAGE_EXTERN;
> +
What is the expected BTF when both decl and definition are present:
extern int extfunc(int x);
int extfunc (int x) {
int y = foo ();
return y;
}
> + const char *section_name = get_section_name (func);
> + /* Note: get_section_name () returns NULL for functions in text
> + section. This is intentional, since we do not want to generate
> + DATASEC entries for them. */
"Dot, space, space, new sentence."
> + if (section_name == NULL)
> + continue;
> +
> + struct btf_var_secinfo info;
> +
> + /* +1 for the sentinel type not in the types map. */
> + info.type = func_dtd->dtd_type + 1;
> +
> + /* Both zero at compile time. */
> + info.size = 0;
> + info.offset = 0;
> +
> + btf_datasec_push_entry (ctfc, section_name, info);
> + }
> + }
> +
> varpool_node *node;
> FOR_EACH_VARIABLE (node)
> {
> @@ -317,28 +398,13 @@ btf_collect_datasec (ctf_container_ref ctfc)
> if (dvd == NULL)
> continue;
>
> - const char *section_name = node->get_section ();
> /* Mark extern variables. */
> if (DECL_EXTERNAL (node->decl))
> dvd->dvd_visibility = BTF_LINKAGE_EXTERN;
>
> + const char *section_name = get_section_name (node);
> if (section_name == NULL)
> - {
> - switch (categorize_decl_for_section (node->decl, 0))
> - {
> - case SECCAT_BSS:
> - section_name = ".bss";
> - break;
> - case SECCAT_DATA:
> - section_name = ".data";
> - break;
> - case SECCAT_RODATA:
> - section_name = ".rodata";
> - break;
> - default:
> - continue;
> - }
> - }
> + continue;
>
> struct btf_var_secinfo info;
>
> @@ -363,6 +429,8 @@ btf_collect_datasec (ctf_container_ref ctfc)
>
> btf_datasec_push_entry (ctfc, section_name, info);
> }
> +
> + num_types_created += datasecs.length ();
> }
>
> /* Return true if the type ID is that of a type which will not be emitted (for
> @@ -461,29 +529,6 @@ btf_dtd_emit_preprocess_cb (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
> if (!btf_emit_id_p (dtd->dtd_type))
> return;
>
> - uint32_t btf_kind
> - = get_btf_kind (CTF_V2_INFO_KIND (dtd->dtd_data.ctti_info));
> -
> - if (btf_kind == BTF_KIND_FUNC_PROTO)
> - {
> - /* Functions actually get two types: a BTF_KIND_FUNC_PROTO, and
> - also a BTF_KIND_FUNC. But the CTF container only allocates one
> - type per function, which matches closely with BTF_KIND_FUNC_PROTO.
> - For each such function, also allocate a BTF_KIND_FUNC entry.
> - These will be output later. */
> - ctf_dtdef_ref func_dtd = ggc_cleared_alloc<ctf_dtdef_t> ();
> - func_dtd->dtd_data = dtd->dtd_data;
> - func_dtd->dtd_data.ctti_type = dtd->dtd_type;
> - func_dtd->linkage = dtd->linkage;
> -
> - vec_safe_push (funcs, func_dtd);
> - num_types_created++;
> -
> - /* Only the BTF_KIND_FUNC type actually references the name. The
> - BTF_KIND_FUNC_PROTO is always anonymous. */
> - dtd->dtd_data.ctti_name = 0;
> - }
> -
> ctfc->ctfc_num_vlen_bytes += btf_calc_num_vbytes (dtd);
> }
>
> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c
> new file mode 100644
> index 00000000000..f4b298cf019
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c
> @@ -0,0 +1,28 @@
> +/* Test BTF generation of DATASEC records for extern functions.
> +
> + Only functions declared extern should have entries in DATASEC records. */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O0 -gbtf -dA" } */
> +
> +/* Expect one DATASEC with vlen=1 (.foo_sec) and one with vlen=2 (.bar_sec) */
> +/* { dg-final { scan-assembler-times "0xf000002\[\t \]+\[^\n\]*btt_info" 1 } } */
> +/* { dg-final { scan-assembler-times "0xf000001\[\t \]+\[^\n\]*btt_info" 1 } } */
> +
> +/* Function entries should have offset and size of 0 at compile time. */
> +/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 3 } } */
> +/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_size" 3 } } */
> +
> +extern int foo (int a) __attribute__((section(".foo_sec")));
> +
> +
> +extern int bar (int b) __attribute__((section(".bar_sec")));
> +extern void chacha (void) __attribute__((section(".bar_sec")));
> +
> +__attribute__((section(".foo_sec")))
> +void baz (int *x)
> +{
> + chacha ();
> +
> + *x = foo (bar (*x));
> +}
> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-function-6.c b/gcc/testsuite/gcc.dg/debug/btf/btf-function-6.c
> new file mode 100644
> index 00000000000..48a946ab14b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-function-6.c
> @@ -0,0 +1,19 @@
> +/* Test BTF extern linkage for functions.
> +
> + We expect to see one BTF_KIND_FUNC type with global linkage (foo), and
> + one BTF_KIND_FUNC type with extern linkage (extfunc). */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O0 -gbtf -dA" } */
> +
> +/* { dg-final { scan-assembler-times "btt_info: kind=12, kflag=0, linkage=2" 1 } } */
> +/* { dg-final { scan-assembler-times "btt_info: kind=12, kflag=0, linkage=1" 1 } } */
> +
> +extern int extfunc(int a, int b);
> +
> +int foo (int x) {
> +
> + int y = extfunc (x, x+1);
> +
> + return y;
> +}
> diff --git a/include/btf.h b/include/btf.h
> index 9a757ce5bc9..f41ea94b75f 100644
> --- a/include/btf.h
> +++ b/include/btf.h
> @@ -186,12 +186,13 @@ struct btf_var
> };
>
> /* BTF_KIND_DATASEC is followed by VLEN struct btf_var_secinfo entries,
> - which describe all BTF_KIND_VAR types contained in the section. */
> + which describe all BTF_KIND_VAR or extern BTF_KIND_FUNC types contained
> + in the section. */
> struct btf_var_secinfo
> {
> - uint32_t type; /* Type of variable. */
> - uint32_t offset; /* In-section offset of variable (in bytes). */
> - uint32_t size; /* Size (in bytes) of variable. */
> + uint32_t type; /* Type of BTF_KIND_VAR or BTF_KIND_FUNC item. */
> + uint32_t offset; /* In-section offset (in bytes) of item. */
> + uint32_t size; /* Size (in bytes) of item. */
> };
>
> /* BTF_KIND_ENUM64 is followed by VLEN struct btf_enum64 entries,
On 12/8/22 23:36, Indu Bhagat wrote:
> On 12/7/22 12:57, David Faust wrote:
>> The eBPF loader expects to find entries for functions declared as extern
>> in the corresponding BTF_KIND_DATASEC record, but we were not generating
>> these entries.
>>
>> This patch adds support for the 'extern' linkage of function types in
>> BTF, and creates entries for for them BTF_KIND_DATASEC records as needed.
>>
>> PR target/106773
>>
>> gcc/
>>
>> * btfout.cc (get_section_name): New function.
>> (btf_collect_datasec): Use it here. Process functions, marking them
>> 'extern' and generating DATASEC entries for them as appropriate. Move
>> creation of BTF_KIND_FUNC records to here...
>> (btf_dtd_emit_preprocess_cb): ... from here.
>>
>> gcc/testsuite/
>>
>> * gcc.dg/debug/btf/btf-datasec-2.c: New test.
>> * gcc.dg/debug/btf/btf-function-6.c: New test.
>>
>> include/
>>
>> * btf.h (struct btf_var_secinfo): Update comments with notes about
>> extern functions.
>> ---
>> gcc/btfout.cc | 129 ++++++++++++------
>> .../gcc.dg/debug/btf/btf-datasec-2.c | 28 ++++
>> .../gcc.dg/debug/btf/btf-function-6.c | 19 +++
>> include/btf.h | 9 +-
>> 4 files changed, 139 insertions(+), 46 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c
>> create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-function-6.c
>>
>> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
>> index 05f3a3f9b6e..d7ead377ec5 100644
>> --- a/gcc/btfout.cc
>> +++ b/gcc/btfout.cc
>> @@ -294,7 +294,35 @@ btf_datasec_push_entry (ctf_container_ref ctfc, const char *secname,
>> ds.entries.safe_push (info);
>>
>> datasecs.safe_push (ds);
>> - num_types_created++;
>> +}
>> +
>> +
>> +/* Return the section name, as of interest to btf_collect_datasec, for the
>> + given symtab node. Note that this deliberately returns NULL for objects
>> + which do not go in a section btf_collect_datasec cares about. */
>
> "Dot, space, space, new sentence."
>
>> +static const char *
>> +get_section_name (symtab_node *node)
>> +{
>> + const char *section_name = node->get_section ();
>> +
>> + if (section_name == NULL)
>> + {
>> + switch (categorize_decl_for_section (node->decl, 0))
>> + {
>> + case SECCAT_BSS:
>> + section_name = ".bss";
>> + break;
>> + case SECCAT_DATA:
>> + section_name = ".data";
>> + break;
>> + case SECCAT_RODATA:
>> + section_name = ".rodata";
>> + break;
>> + default:;
>> + }
>> + }
>> +
>> + return section_name;
>> }
>>
>> /* Construct all BTF_KIND_DATASEC records for CTFC. One such record is created
>> @@ -305,7 +333,60 @@ btf_datasec_push_entry (ctf_container_ref ctfc, const char *secname,
>> static void
>> btf_collect_datasec (ctf_container_ref ctfc)
>> {
>> - /* See cgraph.h struct symtab_node, which varpool_node extends. */
>> + cgraph_node *func;
>> + FOR_EACH_FUNCTION (func)
>> + {
>> + dw_die_ref die = lookup_decl_die (func->decl);
>> + if (die == NULL)
>> + continue;
>> +
>> + ctf_dtdef_ref dtd = ctf_dtd_lookup (ctfc, die);
>> + if (dtd == NULL)
>> + continue;
>> +
>> + /* Functions actually get two types: a BTF_KIND_FUNC_PROTO, and
>> + also a BTF_KIND_FUNC. But the CTF container only allocates one
>> + type per function, which matches closely with BTF_KIND_FUNC_PROTO.
>> + For each such function, also allocate a BTF_KIND_FUNC entry.
>> + These will be output later. */
>
> "Dot, space, space, new sentence."
>
>> + ctf_dtdef_ref func_dtd = ggc_cleared_alloc<ctf_dtdef_t> ();
>> + func_dtd->dtd_data = dtd->dtd_data;
>> + func_dtd->dtd_data.ctti_type = dtd->dtd_type;
>> + func_dtd->linkage = dtd->linkage;
>> + func_dtd->dtd_type = num_types_added + num_types_created;
>> +
>> + /* Only the BTF_KIND_FUNC type actually references the name. The
>> + BTF_KIND_FUNC_PROTO is always anonymous. */
>> + dtd->dtd_data.ctti_name = 0;
>> +
>> + vec_safe_push (funcs, func_dtd);
>> + num_types_created++;
>> +
>> + /* Mark any 'extern' funcs and add DATASEC entries for them. */
>> + if (DECL_EXTERNAL (func->decl))
>> + {
>> + func_dtd->linkage = BTF_LINKAGE_EXTERN;
>> +
>
> What is the expected BTF when both decl and definition are present:
>
> extern int extfunc(int x);
> int extfunc (int x) {
> int y = foo ();
> return y;
> }
Using clang implementation as the reference, a single FUNC record
for "extfunc" with "global" linkage:
$ cat extfuncdef.c
extern int extfunc (int x);
int extfunc (int x) {
int y = foo ();
return y;
}
$ clang -target bpf -c -g extfuncdef.c -o extfuncdef.o
$ /usr/sbin/bpftool btf dump file extfuncdef.o
[1] FUNC_PROTO '(anon)' ret_type_id=2 vlen=1
'(anon)' type_id=2
[2] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED
[3] FUNC 'extfunc' type_id=1 linkage=global
With this patch we do the same in GCC.
>
>> + const char *section_name = get_section_name (func);
>> + /* Note: get_section_name () returns NULL for functions in text
>> + section. This is intentional, since we do not want to generate
>> + DATASEC entries for them. */
>
> "Dot, space, space, new sentence."
>
>> + if (section_name == NULL)
>> + continue;
>> +
>> + struct btf_var_secinfo info;
>> +
>> + /* +1 for the sentinel type not in the types map. */
>> + info.type = func_dtd->dtd_type + 1;
>> +
>> + /* Both zero at compile time. */
>> + info.size = 0;
>> + info.offset = 0;
>> +
>> + btf_datasec_push_entry (ctfc, section_name, info);
>> + }
>> + }
>> +
>> varpool_node *node;
>> FOR_EACH_VARIABLE (node)
>> {
>> @@ -317,28 +398,13 @@ btf_collect_datasec (ctf_container_ref ctfc)
>> if (dvd == NULL)
>> continue;
>>
>> - const char *section_name = node->get_section ();
>> /* Mark extern variables. */
>> if (DECL_EXTERNAL (node->decl))
>> dvd->dvd_visibility = BTF_LINKAGE_EXTERN;
>>
>> + const char *section_name = get_section_name (node);
>> if (section_name == NULL)
>> - {
>> - switch (categorize_decl_for_section (node->decl, 0))
>> - {
>> - case SECCAT_BSS:
>> - section_name = ".bss";
>> - break;
>> - case SECCAT_DATA:
>> - section_name = ".data";
>> - break;
>> - case SECCAT_RODATA:
>> - section_name = ".rodata";
>> - break;
>> - default:
>> - continue;
>> - }
>> - }
>> + continue;
>>
>> struct btf_var_secinfo info;
>>
>> @@ -363,6 +429,8 @@ btf_collect_datasec (ctf_container_ref ctfc)
>>
>> btf_datasec_push_entry (ctfc, section_name, info);
>> }
>> +
>> + num_types_created += datasecs.length ();
>> }
>>
>> /* Return true if the type ID is that of a type which will not be emitted (for
>> @@ -461,29 +529,6 @@ btf_dtd_emit_preprocess_cb (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>> if (!btf_emit_id_p (dtd->dtd_type))
>> return;
>>
>> - uint32_t btf_kind
>> - = get_btf_kind (CTF_V2_INFO_KIND (dtd->dtd_data.ctti_info));
>> -
>> - if (btf_kind == BTF_KIND_FUNC_PROTO)
>> - {
>> - /* Functions actually get two types: a BTF_KIND_FUNC_PROTO, and
>> - also a BTF_KIND_FUNC. But the CTF container only allocates one
>> - type per function, which matches closely with BTF_KIND_FUNC_PROTO.
>> - For each such function, also allocate a BTF_KIND_FUNC entry.
>> - These will be output later. */
>> - ctf_dtdef_ref func_dtd = ggc_cleared_alloc<ctf_dtdef_t> ();
>> - func_dtd->dtd_data = dtd->dtd_data;
>> - func_dtd->dtd_data.ctti_type = dtd->dtd_type;
>> - func_dtd->linkage = dtd->linkage;
>> -
>> - vec_safe_push (funcs, func_dtd);
>> - num_types_created++;
>> -
>> - /* Only the BTF_KIND_FUNC type actually references the name. The
>> - BTF_KIND_FUNC_PROTO is always anonymous. */
>> - dtd->dtd_data.ctti_name = 0;
>> - }
>> -
>> ctfc->ctfc_num_vlen_bytes += btf_calc_num_vbytes (dtd);
>> }
>>
>> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c
>> new file mode 100644
>> index 00000000000..f4b298cf019
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c
>> @@ -0,0 +1,28 @@
>> +/* Test BTF generation of DATASEC records for extern functions.
>> +
>> + Only functions declared extern should have entries in DATASEC records. */
>> +
>> +/* { dg-do compile } */
>> +/* { dg-options "-O0 -gbtf -dA" } */
>> +
>> +/* Expect one DATASEC with vlen=1 (.foo_sec) and one with vlen=2 (.bar_sec) */
>> +/* { dg-final { scan-assembler-times "0xf000002\[\t \]+\[^\n\]*btt_info" 1 } } */
>> +/* { dg-final { scan-assembler-times "0xf000001\[\t \]+\[^\n\]*btt_info" 1 } } */
>> +
>> +/* Function entries should have offset and size of 0 at compile time. */
>> +/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 3 } } */
>> +/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_size" 3 } } */
>> +
>> +extern int foo (int a) __attribute__((section(".foo_sec")));
>> +
>> +
>> +extern int bar (int b) __attribute__((section(".bar_sec")));
>> +extern void chacha (void) __attribute__((section(".bar_sec")));
>> +
>> +__attribute__((section(".foo_sec")))
>> +void baz (int *x)
>> +{
>> + chacha ();
>> +
>> + *x = foo (bar (*x));
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-function-6.c b/gcc/testsuite/gcc.dg/debug/btf/btf-function-6.c
>> new file mode 100644
>> index 00000000000..48a946ab14b
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-function-6.c
>> @@ -0,0 +1,19 @@
>> +/* Test BTF extern linkage for functions.
>> +
>> + We expect to see one BTF_KIND_FUNC type with global linkage (foo), and
>> + one BTF_KIND_FUNC type with extern linkage (extfunc). */
>> +
>> +/* { dg-do compile } */
>> +/* { dg-options "-O0 -gbtf -dA" } */
>> +
>> +/* { dg-final { scan-assembler-times "btt_info: kind=12, kflag=0, linkage=2" 1 } } */
>> +/* { dg-final { scan-assembler-times "btt_info: kind=12, kflag=0, linkage=1" 1 } } */
>> +
>> +extern int extfunc(int a, int b);
>> +
>> +int foo (int x) {
>> +
>> + int y = extfunc (x, x+1);
>> +
>> + return y;
>> +}
>> diff --git a/include/btf.h b/include/btf.h
>> index 9a757ce5bc9..f41ea94b75f 100644
>> --- a/include/btf.h
>> +++ b/include/btf.h
>> @@ -186,12 +186,13 @@ struct btf_var
>> };
>>
>> /* BTF_KIND_DATASEC is followed by VLEN struct btf_var_secinfo entries,
>> - which describe all BTF_KIND_VAR types contained in the section. */
>> + which describe all BTF_KIND_VAR or extern BTF_KIND_FUNC types contained
>> + in the section. */
>> struct btf_var_secinfo
>> {
>> - uint32_t type; /* Type of variable. */
>> - uint32_t offset; /* In-section offset of variable (in bytes). */
>> - uint32_t size; /* Size (in bytes) of variable. */
>> + uint32_t type; /* Type of BTF_KIND_VAR or BTF_KIND_FUNC item. */
>> + uint32_t offset; /* In-section offset (in bytes) of item. */
>> + uint32_t size; /* Size (in bytes) of item. */
>> };
>>
>> /* BTF_KIND_ENUM64 is followed by VLEN struct btf_enum64 entries,
>
On 12/12/22 12:31, David Faust wrote:
>
>
> On 12/8/22 23:36, Indu Bhagat wrote:
>> On 12/7/22 12:57, David Faust wrote:
>>> The eBPF loader expects to find entries for functions declared as extern
>>> in the corresponding BTF_KIND_DATASEC record, but we were not generating
>>> these entries.
>>>
>>> This patch adds support for the 'extern' linkage of function types in
>>> BTF, and creates entries for for them BTF_KIND_DATASEC records as needed.
>>>
>>> PR target/106773
>>>
>>> gcc/
>>>
>>> * btfout.cc (get_section_name): New function.
>>> (btf_collect_datasec): Use it here. Process functions, marking them
>>> 'extern' and generating DATASEC entries for them as appropriate. Move
>>> creation of BTF_KIND_FUNC records to here...
>>> (btf_dtd_emit_preprocess_cb): ... from here.
>>>
>>> gcc/testsuite/
>>>
>>> * gcc.dg/debug/btf/btf-datasec-2.c: New test.
>>> * gcc.dg/debug/btf/btf-function-6.c: New test.
>>>
>>> include/
>>>
>>> * btf.h (struct btf_var_secinfo): Update comments with notes about
>>> extern functions.
>>> ---
>>> gcc/btfout.cc | 129 ++++++++++++------
>>> .../gcc.dg/debug/btf/btf-datasec-2.c | 28 ++++
>>> .../gcc.dg/debug/btf/btf-function-6.c | 19 +++
>>> include/btf.h | 9 +-
>>> 4 files changed, 139 insertions(+), 46 deletions(-)
>>> create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c
>>> create mode 100644 gcc/testsuite/gcc.dg/debug/btf/btf-function-6.c
>>>
>>> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
>>> index 05f3a3f9b6e..d7ead377ec5 100644
>>> --- a/gcc/btfout.cc
>>> +++ b/gcc/btfout.cc
>>> @@ -294,7 +294,35 @@ btf_datasec_push_entry (ctf_container_ref ctfc, const char *secname,
>>> ds.entries.safe_push (info);
>>>
>>> datasecs.safe_push (ds);
>>> - num_types_created++;
>>> +}
>>> +
>>> +
>>> +/* Return the section name, as of interest to btf_collect_datasec, for the
>>> + given symtab node. Note that this deliberately returns NULL for objects
>>> + which do not go in a section btf_collect_datasec cares about. */
>>
>> "Dot, space, space, new sentence."
>>
>>> +static const char *
>>> +get_section_name (symtab_node *node)
>>> +{
>>> + const char *section_name = node->get_section ();
>>> +
>>> + if (section_name == NULL)
>>> + {
>>> + switch (categorize_decl_for_section (node->decl, 0))
>>> + {
>>> + case SECCAT_BSS:
>>> + section_name = ".bss";
>>> + break;
>>> + case SECCAT_DATA:
>>> + section_name = ".data";
>>> + break;
>>> + case SECCAT_RODATA:
>>> + section_name = ".rodata";
>>> + break;
>>> + default:;
>>> + }
>>> + }
>>> +
>>> + return section_name;
>>> }
>>>
>>> /* Construct all BTF_KIND_DATASEC records for CTFC. One such record is created
>>> @@ -305,7 +333,60 @@ btf_datasec_push_entry (ctf_container_ref ctfc, const char *secname,
>>> static void
>>> btf_collect_datasec (ctf_container_ref ctfc)
>>> {
>>> - /* See cgraph.h struct symtab_node, which varpool_node extends. */
>>> + cgraph_node *func;
>>> + FOR_EACH_FUNCTION (func)
>>> + {
>>> + dw_die_ref die = lookup_decl_die (func->decl);
>>> + if (die == NULL)
>>> + continue;
>>> +
>>> + ctf_dtdef_ref dtd = ctf_dtd_lookup (ctfc, die);
>>> + if (dtd == NULL)
>>> + continue;
>>> +
>>> + /* Functions actually get two types: a BTF_KIND_FUNC_PROTO, and
>>> + also a BTF_KIND_FUNC. But the CTF container only allocates one
>>> + type per function, which matches closely with BTF_KIND_FUNC_PROTO.
>>> + For each such function, also allocate a BTF_KIND_FUNC entry.
>>> + These will be output later. */
>>
>> "Dot, space, space, new sentence."
>>
>>> + ctf_dtdef_ref func_dtd = ggc_cleared_alloc<ctf_dtdef_t> ();
>>> + func_dtd->dtd_data = dtd->dtd_data;
>>> + func_dtd->dtd_data.ctti_type = dtd->dtd_type;
>>> + func_dtd->linkage = dtd->linkage;
>>> + func_dtd->dtd_type = num_types_added + num_types_created;
>>> +
>>> + /* Only the BTF_KIND_FUNC type actually references the name. The
>>> + BTF_KIND_FUNC_PROTO is always anonymous. */
>>> + dtd->dtd_data.ctti_name = 0;
>>> +
>>> + vec_safe_push (funcs, func_dtd);
>>> + num_types_created++;
>>> +
>>> + /* Mark any 'extern' funcs and add DATASEC entries for them. */
>>> + if (DECL_EXTERNAL (func->decl))
>>> + {
>>> + func_dtd->linkage = BTF_LINKAGE_EXTERN;
>>> +
>>
>> What is the expected BTF when both decl and definition are present:
>>
>> extern int extfunc(int x);
>> int extfunc (int x) {
>> int y = foo ();
>> return y;
>> }
>
> Using clang implementation as the reference, a single FUNC record
> for "extfunc" with "global" linkage:
>
> $ cat extfuncdef.c
> extern int extfunc (int x);
> int extfunc (int x) {
> int y = foo ();
> return y;
> }
>
> $ clang -target bpf -c -g extfuncdef.c -o extfuncdef.o
>
> $ /usr/sbin/bpftool btf dump file extfuncdef.o
> [1] FUNC_PROTO '(anon)' ret_type_id=2 vlen=1
> '(anon)' type_id=2
> [2] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED
> [3] FUNC 'extfunc' type_id=1 linkage=global
>
> With this patch we do the same in GCC.
>
OK. Thanks for confirming.
>>
>>> + const char *section_name = get_section_name (func);
>>> + /* Note: get_section_name () returns NULL for functions in text
>>> + section. This is intentional, since we do not want to generate
>>> + DATASEC entries for them. */
>>
>> "Dot, space, space, new sentence."
>>
>>> + if (section_name == NULL)
>>> + continue;
>>> +
>>> + struct btf_var_secinfo info;
>>> +
>>> + /* +1 for the sentinel type not in the types map. */
>>> + info.type = func_dtd->dtd_type + 1;
>>> +
>>> + /* Both zero at compile time. */
>>> + info.size = 0;
>>> + info.offset = 0;
>>> +
>>> + btf_datasec_push_entry (ctfc, section_name, info);
>>> + }
>>> + }
>>> +
>>> varpool_node *node;
>>> FOR_EACH_VARIABLE (node)
>>> {
>>> @@ -317,28 +398,13 @@ btf_collect_datasec (ctf_container_ref ctfc)
>>> if (dvd == NULL)
>>> continue;
>>>
>>> - const char *section_name = node->get_section ();
>>> /* Mark extern variables. */
>>> if (DECL_EXTERNAL (node->decl))
>>> dvd->dvd_visibility = BTF_LINKAGE_EXTERN;
>>>
>>> + const char *section_name = get_section_name (node);
>>> if (section_name == NULL)
>>> - {
>>> - switch (categorize_decl_for_section (node->decl, 0))
>>> - {
>>> - case SECCAT_BSS:
>>> - section_name = ".bss";
>>> - break;
>>> - case SECCAT_DATA:
>>> - section_name = ".data";
>>> - break;
>>> - case SECCAT_RODATA:
>>> - section_name = ".rodata";
>>> - break;
>>> - default:
>>> - continue;
>>> - }
>>> - }
>>> + continue;
>>>
>>> struct btf_var_secinfo info;
>>>
>>> @@ -363,6 +429,8 @@ btf_collect_datasec (ctf_container_ref ctfc)
>>>
>>> btf_datasec_push_entry (ctfc, section_name, info);
>>> }
>>> +
>>> + num_types_created += datasecs.length ();
>>> }
>>>
>>> /* Return true if the type ID is that of a type which will not be emitted (for
>>> @@ -461,29 +529,6 @@ btf_dtd_emit_preprocess_cb (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
>>> if (!btf_emit_id_p (dtd->dtd_type))
>>> return;
>>>
>>> - uint32_t btf_kind
>>> - = get_btf_kind (CTF_V2_INFO_KIND (dtd->dtd_data.ctti_info));
>>> -
>>> - if (btf_kind == BTF_KIND_FUNC_PROTO)
>>> - {
>>> - /* Functions actually get two types: a BTF_KIND_FUNC_PROTO, and
>>> - also a BTF_KIND_FUNC. But the CTF container only allocates one
>>> - type per function, which matches closely with BTF_KIND_FUNC_PROTO.
>>> - For each such function, also allocate a BTF_KIND_FUNC entry.
>>> - These will be output later. */
>>> - ctf_dtdef_ref func_dtd = ggc_cleared_alloc<ctf_dtdef_t> ();
>>> - func_dtd->dtd_data = dtd->dtd_data;
>>> - func_dtd->dtd_data.ctti_type = dtd->dtd_type;
>>> - func_dtd->linkage = dtd->linkage;
>>> -
>>> - vec_safe_push (funcs, func_dtd);
>>> - num_types_created++;
>>> -
>>> - /* Only the BTF_KIND_FUNC type actually references the name. The
>>> - BTF_KIND_FUNC_PROTO is always anonymous. */
>>> - dtd->dtd_data.ctti_name = 0;
>>> - }
>>> -
>>> ctfc->ctfc_num_vlen_bytes += btf_calc_num_vbytes (dtd);
>>> }
>>>
>>> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c
>>> new file mode 100644
>>> index 00000000000..f4b298cf019
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-datasec-2.c
>>> @@ -0,0 +1,28 @@
>>> +/* Test BTF generation of DATASEC records for extern functions.
>>> +
>>> + Only functions declared extern should have entries in DATASEC records. */
>>> +
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-O0 -gbtf -dA" } */
>>> +
>>> +/* Expect one DATASEC with vlen=1 (.foo_sec) and one with vlen=2 (.bar_sec) */
>>> +/* { dg-final { scan-assembler-times "0xf000002\[\t \]+\[^\n\]*btt_info" 1 } } */
>>> +/* { dg-final { scan-assembler-times "0xf000001\[\t \]+\[^\n\]*btt_info" 1 } } */
>>> +
>>> +/* Function entries should have offset and size of 0 at compile time. */
>>> +/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 3 } } */
>>> +/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_size" 3 } } */
>>> +
>>> +extern int foo (int a) __attribute__((section(".foo_sec")));
>>> +
>>> +
>>> +extern int bar (int b) __attribute__((section(".bar_sec")));
>>> +extern void chacha (void) __attribute__((section(".bar_sec")));
>>> +
>>> +__attribute__((section(".foo_sec")))
>>> +void baz (int *x)
>>> +{
>>> + chacha ();
>>> +
>>> + *x = foo (bar (*x));
>>> +}
>>> diff --git a/gcc/testsuite/gcc.dg/debug/btf/btf-function-6.c b/gcc/testsuite/gcc.dg/debug/btf/btf-function-6.c
>>> new file mode 100644
>>> index 00000000000..48a946ab14b
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.dg/debug/btf/btf-function-6.c
>>> @@ -0,0 +1,19 @@
>>> +/* Test BTF extern linkage for functions.
>>> +
>>> + We expect to see one BTF_KIND_FUNC type with global linkage (foo), and
>>> + one BTF_KIND_FUNC type with extern linkage (extfunc). */
>>> +
>>> +/* { dg-do compile } */
>>> +/* { dg-options "-O0 -gbtf -dA" } */
>>> +
>>> +/* { dg-final { scan-assembler-times "btt_info: kind=12, kflag=0, linkage=2" 1 } } */
>>> +/* { dg-final { scan-assembler-times "btt_info: kind=12, kflag=0, linkage=1" 1 } } */
>>> +
>>> +extern int extfunc(int a, int b);
>>> +
>>> +int foo (int x) {
>>> +
>>> + int y = extfunc (x, x+1);
>>> +
>>> + return y;
>>> +}
>>> diff --git a/include/btf.h b/include/btf.h
>>> index 9a757ce5bc9..f41ea94b75f 100644
>>> --- a/include/btf.h
>>> +++ b/include/btf.h
>>> @@ -186,12 +186,13 @@ struct btf_var
>>> };
>>>
>>> /* BTF_KIND_DATASEC is followed by VLEN struct btf_var_secinfo entries,
>>> - which describe all BTF_KIND_VAR types contained in the section. */
>>> + which describe all BTF_KIND_VAR or extern BTF_KIND_FUNC types contained
>>> + in the section. */
>>> struct btf_var_secinfo
>>> {
>>> - uint32_t type; /* Type of variable. */
>>> - uint32_t offset; /* In-section offset of variable (in bytes). */
>>> - uint32_t size; /* Size (in bytes) of variable. */
>>> + uint32_t type; /* Type of BTF_KIND_VAR or BTF_KIND_FUNC item. */
>>> + uint32_t offset; /* In-section offset (in bytes) of item. */
>>> + uint32_t size; /* Size (in bytes) of item. */
>>> };
>>>
>>> /* BTF_KIND_ENUM64 is followed by VLEN struct btf_enum64 entries,
>>
@@ -294,7 +294,35 @@ btf_datasec_push_entry (ctf_container_ref ctfc, const char *secname,
ds.entries.safe_push (info);
datasecs.safe_push (ds);
- num_types_created++;
+}
+
+
+/* Return the section name, as of interest to btf_collect_datasec, for the
+ given symtab node. Note that this deliberately returns NULL for objects
+ which do not go in a section btf_collect_datasec cares about. */
+static const char *
+get_section_name (symtab_node *node)
+{
+ const char *section_name = node->get_section ();
+
+ if (section_name == NULL)
+ {
+ switch (categorize_decl_for_section (node->decl, 0))
+ {
+ case SECCAT_BSS:
+ section_name = ".bss";
+ break;
+ case SECCAT_DATA:
+ section_name = ".data";
+ break;
+ case SECCAT_RODATA:
+ section_name = ".rodata";
+ break;
+ default:;
+ }
+ }
+
+ return section_name;
}
/* Construct all BTF_KIND_DATASEC records for CTFC. One such record is created
@@ -305,7 +333,60 @@ btf_datasec_push_entry (ctf_container_ref ctfc, const char *secname,
static void
btf_collect_datasec (ctf_container_ref ctfc)
{
- /* See cgraph.h struct symtab_node, which varpool_node extends. */
+ cgraph_node *func;
+ FOR_EACH_FUNCTION (func)
+ {
+ dw_die_ref die = lookup_decl_die (func->decl);
+ if (die == NULL)
+ continue;
+
+ ctf_dtdef_ref dtd = ctf_dtd_lookup (ctfc, die);
+ if (dtd == NULL)
+ continue;
+
+ /* Functions actually get two types: a BTF_KIND_FUNC_PROTO, and
+ also a BTF_KIND_FUNC. But the CTF container only allocates one
+ type per function, which matches closely with BTF_KIND_FUNC_PROTO.
+ For each such function, also allocate a BTF_KIND_FUNC entry.
+ These will be output later. */
+ ctf_dtdef_ref func_dtd = ggc_cleared_alloc<ctf_dtdef_t> ();
+ func_dtd->dtd_data = dtd->dtd_data;
+ func_dtd->dtd_data.ctti_type = dtd->dtd_type;
+ func_dtd->linkage = dtd->linkage;
+ func_dtd->dtd_type = num_types_added + num_types_created;
+
+ /* Only the BTF_KIND_FUNC type actually references the name. The
+ BTF_KIND_FUNC_PROTO is always anonymous. */
+ dtd->dtd_data.ctti_name = 0;
+
+ vec_safe_push (funcs, func_dtd);
+ num_types_created++;
+
+ /* Mark any 'extern' funcs and add DATASEC entries for them. */
+ if (DECL_EXTERNAL (func->decl))
+ {
+ func_dtd->linkage = BTF_LINKAGE_EXTERN;
+
+ const char *section_name = get_section_name (func);
+ /* Note: get_section_name () returns NULL for functions in text
+ section. This is intentional, since we do not want to generate
+ DATASEC entries for them. */
+ if (section_name == NULL)
+ continue;
+
+ struct btf_var_secinfo info;
+
+ /* +1 for the sentinel type not in the types map. */
+ info.type = func_dtd->dtd_type + 1;
+
+ /* Both zero at compile time. */
+ info.size = 0;
+ info.offset = 0;
+
+ btf_datasec_push_entry (ctfc, section_name, info);
+ }
+ }
+
varpool_node *node;
FOR_EACH_VARIABLE (node)
{
@@ -317,28 +398,13 @@ btf_collect_datasec (ctf_container_ref ctfc)
if (dvd == NULL)
continue;
- const char *section_name = node->get_section ();
/* Mark extern variables. */
if (DECL_EXTERNAL (node->decl))
dvd->dvd_visibility = BTF_LINKAGE_EXTERN;
+ const char *section_name = get_section_name (node);
if (section_name == NULL)
- {
- switch (categorize_decl_for_section (node->decl, 0))
- {
- case SECCAT_BSS:
- section_name = ".bss";
- break;
- case SECCAT_DATA:
- section_name = ".data";
- break;
- case SECCAT_RODATA:
- section_name = ".rodata";
- break;
- default:
- continue;
- }
- }
+ continue;
struct btf_var_secinfo info;
@@ -363,6 +429,8 @@ btf_collect_datasec (ctf_container_ref ctfc)
btf_datasec_push_entry (ctfc, section_name, info);
}
+
+ num_types_created += datasecs.length ();
}
/* Return true if the type ID is that of a type which will not be emitted (for
@@ -461,29 +529,6 @@ btf_dtd_emit_preprocess_cb (ctf_container_ref ctfc, ctf_dtdef_ref dtd)
if (!btf_emit_id_p (dtd->dtd_type))
return;
- uint32_t btf_kind
- = get_btf_kind (CTF_V2_INFO_KIND (dtd->dtd_data.ctti_info));
-
- if (btf_kind == BTF_KIND_FUNC_PROTO)
- {
- /* Functions actually get two types: a BTF_KIND_FUNC_PROTO, and
- also a BTF_KIND_FUNC. But the CTF container only allocates one
- type per function, which matches closely with BTF_KIND_FUNC_PROTO.
- For each such function, also allocate a BTF_KIND_FUNC entry.
- These will be output later. */
- ctf_dtdef_ref func_dtd = ggc_cleared_alloc<ctf_dtdef_t> ();
- func_dtd->dtd_data = dtd->dtd_data;
- func_dtd->dtd_data.ctti_type = dtd->dtd_type;
- func_dtd->linkage = dtd->linkage;
-
- vec_safe_push (funcs, func_dtd);
- num_types_created++;
-
- /* Only the BTF_KIND_FUNC type actually references the name. The
- BTF_KIND_FUNC_PROTO is always anonymous. */
- dtd->dtd_data.ctti_name = 0;
- }
-
ctfc->ctfc_num_vlen_bytes += btf_calc_num_vbytes (dtd);
}
new file mode 100644
@@ -0,0 +1,28 @@
+/* Test BTF generation of DATASEC records for extern functions.
+
+ Only functions declared extern should have entries in DATASEC records. */
+
+/* { dg-do compile } */
+/* { dg-options "-O0 -gbtf -dA" } */
+
+/* Expect one DATASEC with vlen=1 (.foo_sec) and one with vlen=2 (.bar_sec) */
+/* { dg-final { scan-assembler-times "0xf000002\[\t \]+\[^\n\]*btt_info" 1 } } */
+/* { dg-final { scan-assembler-times "0xf000001\[\t \]+\[^\n\]*btt_info" 1 } } */
+
+/* Function entries should have offset and size of 0 at compile time. */
+/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_offset" 3 } } */
+/* { dg-final { scan-assembler-times "0\[\t \]+\[^\n\]*bts_size" 3 } } */
+
+extern int foo (int a) __attribute__((section(".foo_sec")));
+
+
+extern int bar (int b) __attribute__((section(".bar_sec")));
+extern void chacha (void) __attribute__((section(".bar_sec")));
+
+__attribute__((section(".foo_sec")))
+void baz (int *x)
+{
+ chacha ();
+
+ *x = foo (bar (*x));
+}
new file mode 100644
@@ -0,0 +1,19 @@
+/* Test BTF extern linkage for functions.
+
+ We expect to see one BTF_KIND_FUNC type with global linkage (foo), and
+ one BTF_KIND_FUNC type with extern linkage (extfunc). */
+
+/* { dg-do compile } */
+/* { dg-options "-O0 -gbtf -dA" } */
+
+/* { dg-final { scan-assembler-times "btt_info: kind=12, kflag=0, linkage=2" 1 } } */
+/* { dg-final { scan-assembler-times "btt_info: kind=12, kflag=0, linkage=1" 1 } } */
+
+extern int extfunc(int a, int b);
+
+int foo (int x) {
+
+ int y = extfunc (x, x+1);
+
+ return y;
+}
@@ -186,12 +186,13 @@ struct btf_var
};
/* BTF_KIND_DATASEC is followed by VLEN struct btf_var_secinfo entries,
- which describe all BTF_KIND_VAR types contained in the section. */
+ which describe all BTF_KIND_VAR or extern BTF_KIND_FUNC types contained
+ in the section. */
struct btf_var_secinfo
{
- uint32_t type; /* Type of variable. */
- uint32_t offset; /* In-section offset of variable (in bytes). */
- uint32_t size; /* Size (in bytes) of variable. */
+ uint32_t type; /* Type of BTF_KIND_VAR or BTF_KIND_FUNC item. */
+ uint32_t offset; /* In-section offset (in bytes) of item. */
+ uint32_t size; /* Size (in bytes) of item. */
};
/* BTF_KIND_ENUM64 is followed by VLEN struct btf_enum64 entries,