[PATCH 0/3] Correct recent GCC incompatible changes.

[PATCH 0/3] Correct recent GCC incompatible changes.

[Cupertino Miranda]

Hi everyone,

Apologies for the previous patches which did not include a cover letter.
My wish was to send 3 indepepdent patches but after the initial mistake lets keep
this as a series although they are all independent from themselves.

The changes in this patch series is related to recovering GCC support to
build the selftests.
A few tests and a makefile change have broken the support for GCC in the
last few months.

Looking forward to your comments.

Best regards,
Cupertino

Cupertino Miranda (3):
  selftests/bpf: Disable strict aliasing for verifier_nocsr.c
  selftest/bpf: _GNU_SOURCE redefined in g++
  selftest/bpf: Adapt inline asm operand constraint for GCC support

 tools/testing/selftests/bpf/Makefile                          | 3 ++-
 .../testing/selftests/bpf/progs/tailcall_bpf2bpf_hierarchy2.c | 4 ++--
 .../testing/selftests/bpf/progs/tailcall_bpf2bpf_hierarchy3.c | 4 ++--
 3 files changed, 6 insertions(+), 5 deletions(-)

-- 
2.30.2

[PATCH 1/3] selftests/bpf: Disable strict aliasing for verifier_nocsr.c

[Cupertino Miranda]

verfifier_nocsr.c fails to compile in GCC. The reason behind it was
initially explained in commit 27a90b14b93d3b2e1efd10764e456af7e2a42991.

"A few BPF selftests perform type punning and they may break strict
aliasing rules, which are exploited by both GCC and clang by default
while optimizing.  This can lead to broken compiled programs."

Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
Cc: Eduard Zingerman <eddyz87@gmail.com>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Yonghong Song <yonghong.song@linux.dev>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Jose Marchesi <jose.marchesi@oracle.com>
Cc: David Faust <david.faust@oracle.com>
---
 tools/testing/selftests/bpf/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 00bde031a469..ded6e22b3076 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -58,6 +58,7 @@ progs/test_pkt_md_access.c-CFLAGS := -fno-strict-aliasing
 progs/test_sk_lookup.c-CFLAGS := -fno-strict-aliasing
 progs/timer_crash.c-CFLAGS := -fno-strict-aliasing
 progs/test_global_func9.c-CFLAGS := -fno-strict-aliasing
+progs/verifier_nocsr.c-CFLAGS := -fno-strict-aliasing
 
 ifneq ($(LLVM),)
 # Silence some warnings when compiled with clang
-- 
2.30.2

[PATCH 2/3] selftest/bpf: _GNU_SOURCE redefined in g++

[Cupertino Miranda]

The following commit:

commit cc937dad85aea4ab9e4f9827d7ea55932c86906b
Author: Edward Liaw <edliaw@google.com>
Date:   Tue Jun 25 22:34:45 2024 +0000

    selftests: centralize -D_GNU_SOURCE= to CFLAGS in lib.mk

introduces "-D_GNU_SOURCE=" to generic CFLAGS used within bpf selfttests
makefiles which include lib.mk.
g++ by default sets the _GNU_SOURCE flag internally which
reports the following warning and subsequent error:

<command-line>: error: "_GNU_SOURCE" redefined [-Werror]
<command-line>: note: this is the location of the previous definition

This patch removes that _GNU_SOURCE definition from CFLAGS when
compiling CPP files.

Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Eduard Zingerman <eddyz87@gmail.com>
Cc: Yonghong Song <yonghong.song@linux.dev>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Jose Marchesi <jose.marchesi@oracle.com>
Cc: David Faust <david.faust@oracle.com>
---
 tools/testing/selftests/bpf/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index ded6e22b3076..f06c51bfd522 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -741,7 +741,7 @@ $(OUTPUT)/xdp_features: xdp_features.c $(OUTPUT)/network_helpers.o $(OUTPUT)/xdp
 # Make sure we are able to include and link libbpf against c++.
 $(OUTPUT)/test_cpp: test_cpp.cpp $(OUTPUT)/test_core_extern.skel.h $(BPFOBJ)
 	$(call msg,CXX,,$@)
-	$(Q)$(CXX) $(CFLAGS) $(filter %.a %.o %.cpp,$^) $(LDLIBS) -o $@
+	$(Q)$(CXX) $(subst -D_GNU_SOURCE=,,$(CFLAGS)) $(filter %.a %.o %.cpp,$^) $(LDLIBS) -o $@
 
 # Benchmark runner
 $(OUTPUT)/bench_%.o: benchs/bench_%.c bench.h $(BPFOBJ)
-- 
2.30.2

[PATCH 3/3] selftest/bpf: Adapt inline asm operand constraint for GCC support

[Cupertino Miranda]

GCC errors when compiling tailcall_bpf2bpf_hierarchy2.c and
tailcall_bpf2bpf_hierarchy3.c with the following error:

progs/tailcall_bpf2bpf_hierarchy2.c: In function 'tailcall_bpf2bpf_hierarchy_2':
progs/tailcall_bpf2bpf_hierarchy2.c:66:9: error: input operand constraint contains '+'
   66 |         asm volatile (""::"r+"(ret));
      |         ^~~

Changed implementation to make use of __sink macro that abstracts the
desired behaviour.

The proposed change seems valid for both GCC and CLANG.

Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
Cc: Leon Hwang <hffilwlqm@gmail.com>
Cc: Eduard Zingerman <eddyz87@gmail.com>
Cc: Andrii Nakryiko <andrii@kernel.org>
Cc: Yonghong Song <yonghong.song@linux.dev>
Cc: Alexei Starovoitov <ast@kernel.org>
Cc: Jose Marchesi <jose.marchesi@oracle.com>
Cc: David Faust <david.faust@oracle.com>
---
 .../testing/selftests/bpf/progs/tailcall_bpf2bpf_hierarchy2.c | 4 ++--
 .../testing/selftests/bpf/progs/tailcall_bpf2bpf_hierarchy3.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf_hierarchy2.c b/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf_hierarchy2.c
index 37604b0b97af..72fd0d577506 100644
--- a/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf_hierarchy2.c
+++ b/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf_hierarchy2.c
@@ -58,12 +58,12 @@ __retval(33)
 SEC("tc")
 int tailcall_bpf2bpf_hierarchy_2(struct __sk_buff *skb)
 {
-	volatile int ret = 0;
+	int ret = 0;
 
 	subprog_tail0(skb);
 	subprog_tail1(skb);
 
-	asm volatile (""::"r+"(ret));
+	__sink(ret);
 	return (count1 << 16) | count0;
 }
 
diff --git a/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf_hierarchy3.c b/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf_hierarchy3.c
index 0cdbb781fcbc..a7fb91cb05b7 100644
--- a/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf_hierarchy3.c
+++ b/tools/testing/selftests/bpf/progs/tailcall_bpf2bpf_hierarchy3.c
@@ -51,11 +51,11 @@ __retval(33)
 SEC("tc")
 int tailcall_bpf2bpf_hierarchy_3(struct __sk_buff *skb)
 {
-	volatile int ret = 0;
+	int ret = 0;
 
 	bpf_tail_call_static(skb, &jmp_table0, 0);
 
-	asm volatile (""::"r+"(ret));
+	__sink(ret);
 	return ret;
 }
 
-- 
2.30.2

Re: [PATCH 2/3] selftest/bpf: _GNU_SOURCE redefined in g++

[Andrii Nakryiko]

On Mon, Aug 19, 2024 at 8:11 AM Cupertino Miranda
<cupertino.miranda@oracle.com> wrote:
>
> The following commit:
>
> commit cc937dad85aea4ab9e4f9827d7ea55932c86906b
> Author: Edward Liaw <edliaw@google.com>
> Date:   Tue Jun 25 22:34:45 2024 +0000
>
>     selftests: centralize -D_GNU_SOURCE= to CFLAGS in lib.mk
>
> introduces "-D_GNU_SOURCE=" to generic CFLAGS used within bpf selfttests
> makefiles which include lib.mk.
> g++ by default sets the _GNU_SOURCE flag internally which
> reports the following warning and subsequent error:
>
> <command-line>: error: "_GNU_SOURCE" redefined [-Werror]
> <command-line>: note: this is the location of the previous definition
>
> This patch removes that _GNU_SOURCE definition from CFLAGS when
> compiling CPP files.
>
> Signed-off-by: Cupertino Miranda <cupertino.miranda@oracle.com>
> Cc: Andrii Nakryiko <andrii@kernel.org>
> Cc: Eduard Zingerman <eddyz87@gmail.com>
> Cc: Yonghong Song <yonghong.song@linux.dev>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Jose Marchesi <jose.marchesi@oracle.com>
> Cc: David Faust <david.faust@oracle.com>
> ---
>  tools/testing/selftests/bpf/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index ded6e22b3076..f06c51bfd522 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -741,7 +741,7 @@ $(OUTPUT)/xdp_features: xdp_features.c $(OUTPUT)/network_helpers.o $(OUTPUT)/xdp
>  # Make sure we are able to include and link libbpf against c++.
>  $(OUTPUT)/test_cpp: test_cpp.cpp $(OUTPUT)/test_core_extern.skel.h $(BPFOBJ)
>         $(call msg,CXX,,$@)
> -       $(Q)$(CXX) $(CFLAGS) $(filter %.a %.o %.cpp,$^) $(LDLIBS) -o $@
> +       $(Q)$(CXX) $(subst -D_GNU_SOURCE=,,$(CFLAGS)) $(filter %.a %.o %.cpp,$^) $(LDLIBS) -o $@
>

The fix for this issue has already landed in bpf/master, so we are
just waiting for bpf/master to be merged into bpf-next/master.

>  # Benchmark runner
>  $(OUTPUT)/bench_%.o: benchs/bench_%.c bench.h $(BPFOBJ)
> --
> 2.30.2
>

Re: [PATCH 0/3] Correct recent GCC incompatible changes.

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Mon, 19 Aug 2024 16:11:26 +0100 you wrote:
> Hi everyone,
> 
> Apologies for the previous patches which did not include a cover letter.
> My wish was to send 3 indepepdent patches but after the initial mistake lets keep
> this as a series although they are all independent from themselves.
> 
> The changes in this patch series is related to recovering GCC support to
> build the selftests.
> A few tests and a makefile change have broken the support for GCC in the
> last few months.
> 
> [...]

Here is the summary with links:
  - [1/3] selftests/bpf: Disable strict aliasing for verifier_nocsr.c
    https://git.kernel.org/bpf/bpf-next/c/2aa93695081d
  - [2/3] selftest/bpf: _GNU_SOURCE redefined in g++
    (no matching commit)
  - [3/3] selftest/bpf: Adapt inline asm operand constraint for GCC support
    https://git.kernel.org/bpf/bpf-next/c/d9075ac631ce

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH 0/3] Correct recent GCC incompatible changes.

[Yonghong Song]

On 8/19/24 8:11 AM, Cupertino Miranda wrote:
> Hi everyone,
>
> Apologies for the previous patches which did not include a cover letter.
> My wish was to send 3 indepepdent patches but after the initial mistake lets keep
> this as a series although they are all independent from themselves.
>
> The changes in this patch series is related to recovering GCC support to
> build the selftests.
> A few tests and a makefile change have broken the support for GCC in the
> last few months.

Cupertino, it would be great if we can speed up to add CI test with gcc-bpf, even
just for compilation only.

Re: [PATCH 0/3] Correct recent GCC incompatible changes.

[Cupertino Miranda]

Hi Yonghong,

Working on it. I am almost done!

Cupertino

On 20-08-2024 20:42, Yonghong Song wrote:
> 
> On 8/19/24 8:11 AM, Cupertino Miranda wrote:
>> Hi everyone,
>>
>> Apologies for the previous patches which did not include a cover letter.
>> My wish was to send 3 indepepdent patches but after the initial 
>> mistake lets keep
>> this as a series although they are all independent from themselves.
>>
>> The changes in this patch series is related to recovering GCC support to
>> build the selftests.
>> A few tests and a makefile change have broken the support for GCC in the
>> last few months.
> 
> Cupertino, it would be great if we can speed up to add CI test with 
> gcc-bpf, even
> just for compilation only.
> 

Re: [PATCH 0/3] Correct recent GCC incompatible changes.

[Yonghong Song]

On 8/22/24 10:02 AM, Cupertino Miranda wrote:
> Hi Yonghong,
>
> Working on it. I am almost done!

Great. Thanks!

>
> Cupertino
>
> On 20-08-2024 20:42, Yonghong Song wrote:
>>
>> On 8/19/24 8:11 AM, Cupertino Miranda wrote:
>>> Hi everyone,
>>>
>>> Apologies for the previous patches which did not include a cover 
>>> letter.
>>> My wish was to send 3 indepepdent patches but after the initial 
>>> mistake lets keep
>>> this as a series although they are all independent from themselves.
>>>
>>> The changes in this patch series is related to recovering GCC 
>>> support to
>>> build the selftests.
>>> A few tests and a makefile change have broken the support for GCC in 
>>> the
>>> last few months.
>>
>> Cupertino, it would be great if we can speed up to add CI test with 
>> gcc-bpf, even
>> just for compilation only.
>>