All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next] selftests/bpf: fix a few clang compilation errors
@ 2022-05-11 18:47 Yonghong Song
  2022-05-11 19:08 ` David Vernet
  2022-05-11 20:00 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 4+ messages in thread
From: Yonghong Song @ 2022-05-11 18:47 UTC (permalink / raw)
  To: bpf; +Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team

With latest clang, I got the following compilation errors:
  .../prog_tests/test_tunnel.c:291:6: error: variable 'local_ip_map_fd' is used uninitialized
     whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
       if (attach_tc_prog(&tc_hook, -1, set_dst_prog_fd))
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  .../bpf/prog_tests/test_tunnel.c:312:6: note: uninitialized use occurs here
        if (local_ip_map_fd >= 0)
            ^~~~~~~~~~~~~~~
  ...
  .../prog_tests/kprobe_multi_test.c:346:6: error: variable 'err' is used uninitialized
      whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
        if (IS_ERR(map))
            ^~~~~~~~~~~
  .../prog_tests/kprobe_multi_test.c:388:6: note: uninitialized use occurs here
        if (err) {
            ^~~

This patch fixed the above compilation errors.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c | 4 +++-
 tools/testing/selftests/bpf/prog_tests/test_tunnel.c       | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
index 816eacededd1..586dc52d6fb9 100644
--- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
@@ -343,8 +343,10 @@ static int get_syms(char ***symsp, size_t *cntp)
 		return -EINVAL;
 
 	map = hashmap__new(symbol_hash, symbol_equal, NULL);
-	if (IS_ERR(map))
+	if (IS_ERR(map)) {
+		err = libbpf_get_error(map);
 		goto error;
+	}
 
 	while (fgets(buf, sizeof(buf), f)) {
 		/* skip modules */
diff --git a/tools/testing/selftests/bpf/prog_tests/test_tunnel.c b/tools/testing/selftests/bpf/prog_tests/test_tunnel.c
index 071c9c91b50f..3bba4a2a0530 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_tunnel.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_tunnel.c
@@ -246,7 +246,7 @@ static void test_vxlan_tunnel(void)
 {
 	struct test_tunnel_kern *skel = NULL;
 	struct nstoken *nstoken;
-	int local_ip_map_fd;
+	int local_ip_map_fd = -1;
 	int set_src_prog_fd, get_src_prog_fd;
 	int set_dst_prog_fd;
 	int key = 0, ifindex = -1;
@@ -319,7 +319,7 @@ static void test_ip6vxlan_tunnel(void)
 {
 	struct test_tunnel_kern *skel = NULL;
 	struct nstoken *nstoken;
-	int local_ip_map_fd;
+	int local_ip_map_fd = -1;
 	int set_src_prog_fd, get_src_prog_fd;
 	int set_dst_prog_fd;
 	int key = 0, ifindex = -1;
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: fix a few clang compilation errors
  2022-05-11 18:47 [PATCH bpf-next] selftests/bpf: fix a few clang compilation errors Yonghong Song
@ 2022-05-11 19:08 ` David Vernet
  2022-05-11 22:10   ` Yonghong Song
  2022-05-11 20:00 ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 4+ messages in thread
From: David Vernet @ 2022-05-11 19:08 UTC (permalink / raw)
  To: Yonghong Song
  Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team

On Wed, May 11, 2022 at 11:47:35AM -0700, Yonghong Song wrote:
> With latest clang, I got the following compilation errors:
>   .../prog_tests/test_tunnel.c:291:6: error: variable 'local_ip_map_fd' is used uninitialized
>      whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>        if (attach_tc_prog(&tc_hook, -1, set_dst_prog_fd))
>             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   .../bpf/prog_tests/test_tunnel.c:312:6: note: uninitialized use occurs here
>         if (local_ip_map_fd >= 0)
>             ^~~~~~~~~~~~~~~
>   ...
>   .../prog_tests/kprobe_multi_test.c:346:6: error: variable 'err' is used uninitialized
>       whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>         if (IS_ERR(map))
>             ^~~~~~~~~~~
>   .../prog_tests/kprobe_multi_test.c:388:6: note: uninitialized use occurs here
>         if (err) {
>             ^~~
> 
> This patch fixed the above compilation errors.

I'd argue that these are real bugs that the compiler happens to have
caught, and that the patch should perhaps be framed as fixing them rather
than as avoiding compilation failures, but that might be unnecessarily
nit-picky and I don't feel strongly about it.

> 
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
>  tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c | 4 +++-
>  tools/testing/selftests/bpf/prog_tests/test_tunnel.c       | 4 ++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> index 816eacededd1..586dc52d6fb9 100644
> --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
> @@ -343,8 +343,10 @@ static int get_syms(char ***symsp, size_t *cntp)
>  		return -EINVAL;
>  
>  	map = hashmap__new(symbol_hash, symbol_equal, NULL);
> -	if (IS_ERR(map))
> +	if (IS_ERR(map)) {
> +		err = libbpf_get_error(map);
>  		goto error;
> +	}
>  
>  	while (fgets(buf, sizeof(buf), f)) {
>  		/* skip modules */
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_tunnel.c b/tools/testing/selftests/bpf/prog_tests/test_tunnel.c
> index 071c9c91b50f..3bba4a2a0530 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_tunnel.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_tunnel.c
> @@ -246,7 +246,7 @@ static void test_vxlan_tunnel(void)
>  {
>  	struct test_tunnel_kern *skel = NULL;
>  	struct nstoken *nstoken;
> -	int local_ip_map_fd;
> +	int local_ip_map_fd = -1;
>  	int set_src_prog_fd, get_src_prog_fd;
>  	int set_dst_prog_fd;
>  	int key = 0, ifindex = -1;
> @@ -319,7 +319,7 @@ static void test_ip6vxlan_tunnel(void)
>  {
>  	struct test_tunnel_kern *skel = NULL;
>  	struct nstoken *nstoken;
> -	int local_ip_map_fd;
> +	int local_ip_map_fd = -1;
>  	int set_src_prog_fd, get_src_prog_fd;
>  	int set_dst_prog_fd;
>  	int key = 0, ifindex = -1;
> -- 
> 2.30.2
> 

I'm a bit surprised this ever successfully compiled. What version of clang
did you have to upgrade to in order to see this error? IIRC I've used
-Wsometimes-uninitialized on much older versions of clang.

Anyways, looks good to me.

Acked-by: David Vernet <void@manifault.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: fix a few clang compilation errors
  2022-05-11 18:47 [PATCH bpf-next] selftests/bpf: fix a few clang compilation errors Yonghong Song
  2022-05-11 19:08 ` David Vernet
@ 2022-05-11 20:00 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-05-11 20:00 UTC (permalink / raw)
  To: Yonghong Song; +Cc: bpf, ast, andrii, daniel, kernel-team

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Wed, 11 May 2022 11:47:35 -0700 you wrote:
> With latest clang, I got the following compilation errors:
>   .../prog_tests/test_tunnel.c:291:6: error: variable 'local_ip_map_fd' is used uninitialized
>      whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>        if (attach_tc_prog(&tc_hook, -1, set_dst_prog_fd))
>             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   .../bpf/prog_tests/test_tunnel.c:312:6: note: uninitialized use occurs here
>         if (local_ip_map_fd >= 0)
>             ^~~~~~~~~~~~~~~
>   ...
>   .../prog_tests/kprobe_multi_test.c:346:6: error: variable 'err' is used uninitialized
>       whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>         if (IS_ERR(map))
>             ^~~~~~~~~~~
>   .../prog_tests/kprobe_multi_test.c:388:6: note: uninitialized use occurs here
>         if (err) {
>             ^~~
> 
> [...]

Here is the summary with links:
  - [bpf-next] selftests/bpf: fix a few clang compilation errors
    https://git.kernel.org/bpf/bpf-next/c/fd0ad6f1d10c

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



^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH bpf-next] selftests/bpf: fix a few clang compilation errors
  2022-05-11 19:08 ` David Vernet
@ 2022-05-11 22:10   ` Yonghong Song
  0 siblings, 0 replies; 4+ messages in thread
From: Yonghong Song @ 2022-05-11 22:10 UTC (permalink / raw)
  To: David Vernet
  Cc: bpf, Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team



On 5/11/22 12:08 PM, David Vernet wrote:
> On Wed, May 11, 2022 at 11:47:35AM -0700, Yonghong Song wrote:
>> With latest clang, I got the following compilation errors:
>>    .../prog_tests/test_tunnel.c:291:6: error: variable 'local_ip_map_fd' is used uninitialized
>>       whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>>         if (attach_tc_prog(&tc_hook, -1, set_dst_prog_fd))
>>              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>    .../bpf/prog_tests/test_tunnel.c:312:6: note: uninitialized use occurs here
>>          if (local_ip_map_fd >= 0)
>>              ^~~~~~~~~~~~~~~
>>    ...
>>    .../prog_tests/kprobe_multi_test.c:346:6: error: variable 'err' is used uninitialized
>>        whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>>          if (IS_ERR(map))
>>              ^~~~~~~~~~~
>>    .../prog_tests/kprobe_multi_test.c:388:6: note: uninitialized use occurs here
>>          if (err) {
>>              ^~~
>>
>> This patch fixed the above compilation errors.
> 
> I'd argue that these are real bugs that the compiler happens to have
> caught, and that the patch should perhaps be framed as fixing them rather
> than as avoiding compilation failures, but that might be unnecessarily
> nit-picky and I don't feel strongly about it.
> 
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>>   tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c | 4 +++-
>>   tools/testing/selftests/bpf/prog_tests/test_tunnel.c       | 4 ++--
>>   2 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
>> index 816eacededd1..586dc52d6fb9 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/kprobe_multi_test.c
>> @@ -343,8 +343,10 @@ static int get_syms(char ***symsp, size_t *cntp)
>>   		return -EINVAL;
>>   
>>   	map = hashmap__new(symbol_hash, symbol_equal, NULL);
>> -	if (IS_ERR(map))
>> +	if (IS_ERR(map)) {
>> +		err = libbpf_get_error(map);
>>   		goto error;
>> +	}
>>   
>>   	while (fgets(buf, sizeof(buf), f)) {
>>   		/* skip modules */
>> diff --git a/tools/testing/selftests/bpf/prog_tests/test_tunnel.c b/tools/testing/selftests/bpf/prog_tests/test_tunnel.c
>> index 071c9c91b50f..3bba4a2a0530 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/test_tunnel.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/test_tunnel.c
>> @@ -246,7 +246,7 @@ static void test_vxlan_tunnel(void)
>>   {
>>   	struct test_tunnel_kern *skel = NULL;
>>   	struct nstoken *nstoken;
>> -	int local_ip_map_fd;
>> +	int local_ip_map_fd = -1;
>>   	int set_src_prog_fd, get_src_prog_fd;
>>   	int set_dst_prog_fd;
>>   	int key = 0, ifindex = -1;
>> @@ -319,7 +319,7 @@ static void test_ip6vxlan_tunnel(void)
>>   {
>>   	struct test_tunnel_kern *skel = NULL;
>>   	struct nstoken *nstoken;
>> -	int local_ip_map_fd;
>> +	int local_ip_map_fd = -1;
>>   	int set_src_prog_fd, get_src_prog_fd;
>>   	int set_dst_prog_fd;
>>   	int key = 0, ifindex = -1;
>> -- 
>> 2.30.2
>>
> 
> I'm a bit surprised this ever successfully compiled. What version of clang
> did you have to upgrade to in order to see this error? IIRC I've used
> -Wsometimes-uninitialized on much older versions of clang.

I compiled with latest llvm-project source (llvm15 main branch).
Since latest pahole (built from source) by default will do parallel
dwarf parsing and btf generation when build vmlinux, you might need this 
patch as well for pahole:
   https://lore.kernel.org/bpf/20220511220249.525908-1-yhs@fb.com/

> 
> Anyways, looks good to me.
> 
> Acked-by: David Vernet <void@manifault.com>

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-05-11 22:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-11 18:47 [PATCH bpf-next] selftests/bpf: fix a few clang compilation errors Yonghong Song
2022-05-11 19:08 ` David Vernet
2022-05-11 22:10   ` Yonghong Song
2022-05-11 20:00 ` patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.