[PATCH] bpf: Correct BTF for kernel_helper attributed decls.

[PATCH] bpf: Correct BTF for kernel_helper attributed decls.

[Cupertino Miranda]

Hi everyone,

This patch address the problem reported in:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113225

Looking forward to your review.

Cheers,
Cupertino


This patch fix a problem with kernel_helper attribute BTF information,
which incorrectly generates BTF_KIND_FUNC entry.
This BTF entry although accurate with traditional extern function
declarations, once the function is attributed with kernel_helper, it is
semantically incompatible of the kernel helpers in BPF infrastructure.

gcc/ChangeLog:
	PR target/113225
	* btfout.cc (btf_collect_datasec): Skip creating BTF info for
	extern and kernel_helper attributed function decls.
gcc/testsuite/ChangeLog:
	* gcc.target/bpf/attr-kernel-helper.c: New test.
---
 gcc/btfout.cc                                     |  7 +++++++
 gcc/testsuite/gcc.target/bpf/attr-kernel-helper.c | 15 +++++++++++++++
 2 files changed, 22 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/bpf/attr-kernel-helper.c

diff --git a/gcc/btfout.cc b/gcc/btfout.cc
index 04218adc9e66..39e7bec43bfb 100644
--- a/gcc/btfout.cc
+++ b/gcc/btfout.cc
@@ -35,6 +35,8 @@ along with GCC; see the file COPYING3.  If not see
 #include "diagnostic-core.h"
 #include "cgraph.h"
 #include "varasm.h"
+#include "stringpool.h"
+#include "attribs.h"
 #include "dwarf2out.h" /* For lookup_decl_die.  */
 
 static int btf_label_num;
@@ -429,6 +431,11 @@ btf_collect_datasec (ctf_container_ref ctfc)
       if (dtd == NULL)
 	continue;
 
+      if (DECL_EXTERNAL (func->decl)
+	  && (lookup_attribute ("kernel_helper",
+				DECL_ATTRIBUTES (func->decl))) != NULL_TREE)
+	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.
diff --git a/gcc/testsuite/gcc.target/bpf/attr-kernel-helper.c b/gcc/testsuite/gcc.target/bpf/attr-kernel-helper.c
new file mode 100644
index 000000000000..7c5a0007c979
--- /dev/null
+++ b/gcc/testsuite/gcc.target/bpf/attr-kernel-helper.c
@@ -0,0 +1,15 @@
+/* Basic test for kernel_helper attribute BTF information.  */
+
+/* { dg-do compile } */
+/* { dg-options "-O0 -dA -gbtf" } */
+
+extern int foo_helper(int) __attribute((kernel_helper(42)));
+extern int foo_nohelper(int);
+
+int bar (int arg)
+{
+  return foo_helper (arg) + foo_nohelper (arg);
+}
+
+/* { dg-final { scan-assembler-times "BTF_KIND_FUNC 'foo_nohelper'" 1 } } */
+/* { dg-final { scan-assembler-times "BTF_KIND_FUNC 'foo_helper'" 0 } } */
-- 
2.30.2

Re: [PATCH] bpf: Correct BTF for kernel_helper attributed decls.

[David Faust]

Hi Cupetino,

On 1/8/24 03:05, Cupertino Miranda wrote:
> Hi everyone,
> 
> This patch address the problem reported in:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113225
> 
> Looking forward to your review.

LGTM, thanks. Please apply.

> 
> Cheers,
> Cupertino
> 
> 
> This patch fix a problem with kernel_helper attribute BTF information,
> which incorrectly generates BTF_KIND_FUNC entry.
> This BTF entry although accurate with traditional extern function
> declarations, once the function is attributed with kernel_helper, it is
> semantically incompatible of the kernel helpers in BPF infrastructure.
> 
> gcc/ChangeLog:
> 	PR target/113225
> 	* btfout.cc (btf_collect_datasec): Skip creating BTF info for
> 	extern and kernel_helper attributed function decls.
> gcc/testsuite/ChangeLog:
> 	* gcc.target/bpf/attr-kernel-helper.c: New test.
> ---
>  gcc/btfout.cc                                     |  7 +++++++
>  gcc/testsuite/gcc.target/bpf/attr-kernel-helper.c | 15 +++++++++++++++
>  2 files changed, 22 insertions(+)
>  create mode 100644 gcc/testsuite/gcc.target/bpf/attr-kernel-helper.c
> 
> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
> index 04218adc9e66..39e7bec43bfb 100644
> --- a/gcc/btfout.cc
> +++ b/gcc/btfout.cc
> @@ -35,6 +35,8 @@ along with GCC; see the file COPYING3.  If not see
>  #include "diagnostic-core.h"
>  #include "cgraph.h"
>  #include "varasm.h"
> +#include "stringpool.h"
> +#include "attribs.h"
>  #include "dwarf2out.h" /* For lookup_decl_die.  */
>  
>  static int btf_label_num;
> @@ -429,6 +431,11 @@ btf_collect_datasec (ctf_container_ref ctfc)
>        if (dtd == NULL)
>  	continue;
>  
> +      if (DECL_EXTERNAL (func->decl)
> +	  && (lookup_attribute ("kernel_helper",
> +				DECL_ATTRIBUTES (func->decl))) != NULL_TREE)
> +	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.
> diff --git a/gcc/testsuite/gcc.target/bpf/attr-kernel-helper.c b/gcc/testsuite/gcc.target/bpf/attr-kernel-helper.c
> new file mode 100644
> index 000000000000..7c5a0007c979
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/bpf/attr-kernel-helper.c
> @@ -0,0 +1,15 @@
> +/* Basic test for kernel_helper attribute BTF information.  */
> +
> +/* { dg-do compile } */
> +/* { dg-options "-O0 -dA -gbtf" } */
> +
> +extern int foo_helper(int) __attribute((kernel_helper(42)));
> +extern int foo_nohelper(int);
> +
> +int bar (int arg)
> +{
> +  return foo_helper (arg) + foo_nohelper (arg);
> +}
> +
> +/* { dg-final { scan-assembler-times "BTF_KIND_FUNC 'foo_nohelper'" 1 } } */
> +/* { dg-final { scan-assembler-times "BTF_KIND_FUNC 'foo_helper'" 0 } } */

Re: [PATCH] bpf: Correct BTF for kernel_helper attributed decls.

[Cupertino Miranda]

Thanks! Committed.

David Faust writes:

> Hi Cupetino,
>
> On 1/8/24 03:05, Cupertino Miranda wrote:
>> Hi everyone,
>>
>> This patch address the problem reported in:
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113225
>>
>> Looking forward to your review.
>
> LGTM, thanks. Please apply.
>
>>
>> Cheers,
>> Cupertino
>>
>>
>> This patch fix a problem with kernel_helper attribute BTF information,
>> which incorrectly generates BTF_KIND_FUNC entry.
>> This BTF entry although accurate with traditional extern function
>> declarations, once the function is attributed with kernel_helper, it is
>> semantically incompatible of the kernel helpers in BPF infrastructure.
>>
>> gcc/ChangeLog:
>> 	PR target/113225
>> 	* btfout.cc (btf_collect_datasec): Skip creating BTF info for
>> 	extern and kernel_helper attributed function decls.
>> gcc/testsuite/ChangeLog:
>> 	* gcc.target/bpf/attr-kernel-helper.c: New test.
>> ---
>>  gcc/btfout.cc                                     |  7 +++++++
>>  gcc/testsuite/gcc.target/bpf/attr-kernel-helper.c | 15 +++++++++++++++
>>  2 files changed, 22 insertions(+)
>>  create mode 100644 gcc/testsuite/gcc.target/bpf/attr-kernel-helper.c
>>
>> diff --git a/gcc/btfout.cc b/gcc/btfout.cc
>> index 04218adc9e66..39e7bec43bfb 100644
>> --- a/gcc/btfout.cc
>> +++ b/gcc/btfout.cc
>> @@ -35,6 +35,8 @@ along with GCC; see the file COPYING3.  If not see
>>  #include "diagnostic-core.h"
>>  #include "cgraph.h"
>>  #include "varasm.h"
>> +#include "stringpool.h"
>> +#include "attribs.h"
>>  #include "dwarf2out.h" /* For lookup_decl_die.  */
>>
>>  static int btf_label_num;
>> @@ -429,6 +431,11 @@ btf_collect_datasec (ctf_container_ref ctfc)
>>        if (dtd == NULL)
>>  	continue;
>>
>> +      if (DECL_EXTERNAL (func->decl)
>> +	  && (lookup_attribute ("kernel_helper",
>> +				DECL_ATTRIBUTES (func->decl))) != NULL_TREE)
>> +	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.
>> diff --git a/gcc/testsuite/gcc.target/bpf/attr-kernel-helper.c b/gcc/testsuite/gcc.target/bpf/attr-kernel-helper.c
>> new file mode 100644
>> index 000000000000..7c5a0007c979
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.target/bpf/attr-kernel-helper.c
>> @@ -0,0 +1,15 @@
>> +/* Basic test for kernel_helper attribute BTF information.  */
>> +
>> +/* { dg-do compile } */
>> +/* { dg-options "-O0 -dA -gbtf" } */
>> +
>> +extern int foo_helper(int) __attribute((kernel_helper(42)));
>> +extern int foo_nohelper(int);
>> +
>> +int bar (int arg)
>> +{
>> +  return foo_helper (arg) + foo_nohelper (arg);
>> +}
>> +
>> +/* { dg-final { scan-assembler-times "BTF_KIND_FUNC 'foo_nohelper'" 1 } } */
>> +/* { dg-final { scan-assembler-times "BTF_KIND_FUNC 'foo_helper'" 0 } } */