[bpf-next] bpf: fix build when CONFIG_DEBUG_INFO_BTF[_MODULES] is undefined

Commit Message

Alan Maguire June 23, 2024, 1:52 p.m. UTC

Kernel test robot reports that kernel build fails with
resilient split BTF changes.

Examining the associated config and code we see that
btf_relocate_id() is defined under CONFIG_DEBUG_INFO_BTF_MODULES.
Moving it outside the #ifdef solves the issue.

Reported-by: kernel test robot <[e-mail]>
Closes: https://lore.kernel.org/oe-kbuild-all/[e-mail]/
Signed-off-by: Alan Maguire <[e-mail]>
---
 kernel/bpf/btf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Alexei Starovoitov June 23, 2024, 2:35 p.m. UTC | #1

On Sun, Jun 23, 2024 at 6:52 AM Alan Maguire <[e-mail]> wrote:
>
> Kernel test robot reports that kernel build fails with
> resilient split BTF changes.
>
> Examining the associated config and code we see that
> btf_relocate_id() is defined under CONFIG_DEBUG_INFO_BTF_MODULES.
> Moving it outside the #ifdef solves the issue.
>
> Reported-by: kernel test robot <[e-mail]>
> Closes: https://lore.kernel.org/oe-kbuild-all/[e-mail]/
> Signed-off-by: Alan Maguire <[e-mail]>
> ---
>  kernel/bpf/btf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 8e12cb80ba73..4ff11779699e 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -6185,8 +6185,6 @@ struct btf *btf_parse_vmlinux(void)
>         return btf;
>  }
>
> -#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
> -
>  /* If .BTF_ids section was created with distilled base BTF, both base and
>   * split BTF ids will need to be mapped to actual base/split ids for
>   * BTF now that it has been relocated.
> @@ -6198,6 +6196,8 @@ static __u32 btf_relocate_id(const struct btf *btf, __u32 id)
>         return btf->base_id_map[id];
>  }
>
> +#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
> +

It doesn't fix it all. The 32 build is still failing:

../kernel/bpf/btf.c: In function ‘btf_populate_kfunc_set’:
-../kernel/bpf/btf.c:8251:36: error: implicit declaration of function
‘btf_relocate_id’; did you mean ‘btf_relocate’?
[-Wimplicit-function-declaration]
- 8251 |                 set->pairs[i].id = btf_relocate_id(btf,
set->pairs[i].id);
-      |                                    ^~~~~~~~~~~~~~~
-      |                                    btf_relocate
-

See build_32, build_clang, build_allmod failures in CI:
https://patchwork.kernel.org/project/netdevbpf/patch/[e-mail]/

Alan Maguire June 23, 2024, 7:04 p.m. UTC | #2

On 23/06/2024 15:35, Alexei Starovoitov wrote:
> On Sun, Jun 23, 2024 at 6:52 AM Alan Maguire <[e-mail]> wrote:
>>
>> Kernel test robot reports that kernel build fails with
>> resilient split BTF changes.
>>
>> Examining the associated config and code we see that
>> btf_relocate_id() is defined under CONFIG_DEBUG_INFO_BTF_MODULES.
>> Moving it outside the #ifdef solves the issue.
>>
>> Reported-by: kernel test robot <[e-mail]>
>> Closes: https://lore.kernel.org/oe-kbuild-all/[e-mail]/
>> Signed-off-by: Alan Maguire <[e-mail]>
>> ---
>>  kernel/bpf/btf.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
>> index 8e12cb80ba73..4ff11779699e 100644
>> --- a/kernel/bpf/btf.c
>> +++ b/kernel/bpf/btf.c
>> @@ -6185,8 +6185,6 @@ struct btf *btf_parse_vmlinux(void)
>>         return btf;
>>  }
>>
>> -#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
>> -
>>  /* If .BTF_ids section was created with distilled base BTF, both base and
>>   * split BTF ids will need to be mapped to actual base/split ids for
>>   * BTF now that it has been relocated.
>> @@ -6198,6 +6196,8 @@ static __u32 btf_relocate_id(const struct btf *btf, __u32 id)
>>         return btf->base_id_map[id];
>>  }
>>
>> +#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
>> +
> 
> It doesn't fix it all. The 32 build is still failing:
> 
> ../kernel/bpf/btf.c: In function ‘btf_populate_kfunc_set’:
> -../kernel/bpf/btf.c:8251:36: error: implicit declaration of function
> ‘btf_relocate_id’; did you mean ‘btf_relocate’?
> [-Wimplicit-function-declaration]
> - 8251 |                 set->pairs[i].id = btf_relocate_id(btf,
> set->pairs[i].id);
> -      |                                    ^~~~~~~~~~~~~~~
> -      |                                    btf_relocate
> -
> 
> See build_32, build_clang, build_allmod failures in CI:
> https://patchwork.kernel.org/project/netdevbpf/patch/[e-mail]/

I've been trying to reproduce this with no success I'm afraid. I may be
misreading but it appears that the diff from baseline to new build is
actually telling us the btf_relocate_id() issues went away

https://netdev.bots.linux.dev/static/nipa/864622/13708618/build_clang/stderr

shows (note the "-" in the diffs preceding the btf_relocate_id()
complaints):

New errors added
--- /tmp/tmp.tLVKGCnz0N	2024-06-23 07:09:50.097720906 -0700
+++ /tmp/tmp.5jUDaRbbAY	2024-06-23 07:10:36.751715396 -0700
@@ -9,24 +9,846 @@
-../kernel/bpf/btf.c:8251:22: error: call to undeclared function
'btf_relocate_id'; ISO C99 and later do not support implicit function
declarations [-Wimplicit-function-declaration]
- 8251 |                 set->pairs[i].id = btf_relocate_id(btf,
set->pairs[i].id);
-      |                                    ^
-../kernel/bpf/btf.c:8251:22: note: did you mean 'btf_relocate'?
-../include/linux/btf.h:556:5: note: 'btf_relocate' declared here
-  556 | int btf_relocate(struct btf *btf, const struct btf *base_btf,
__u32 **map_ids);
-      |     ^
-../kernel/bpf/btf.c:8376:37: error: call to undeclared function
'btf_relocate_id'; ISO C99 and later do not support implicit function
declarations [-Wimplicit-function-declaration]
- 8376 |                 ret = btf_check_kfunc_protos(btf,
btf_relocate_id(btf, kset->set->pairs[i].id),
-      |                                                   ^
-../kernel/bpf/btf.c:8440:17: error: call to undeclared function
'btf_relocate_id'; ISO C99 and later do not support implicit function
declarations [-Wimplicit-function-declaration]
- 8440 |                 dtor_btf_id = btf_relocate_id(btf,
dtors[i].kfunc_btf_id);
-      |                               ^
-../kernel/bpf/btf.c:8529:26: error: call to undeclared function
'btf_relocate_id'; ISO C99 and later do not support implicit function
declarations [-Wimplicit-function-declaration]
- 8529 |                 tab->dtors[i].btf_id = btf_relocate_id(btf,
tab->dtors[i].btf_id);
-      |                                        ^
-4 errors generated.
-make[5]: *** [../kernel/bpf/Makefile:60: kernel/bpf/btf.o] Error 1
-make[4]: *** [../scripts/Makefile.build:485: kernel/bpf] Error 2
-make[3]: *** [../scripts/Makefile.build:485: kernel] Error 2
-make[3]: *** Waiting for unfinished jobs....
-make[2]: *** [/home/nipa/bpf-next/wt-0/Makefile:1934: .] Error 2
-make[1]: *** [/home/nipa/bpf-next/wt-0/Makefile:240: __sub-make] Error 2
-make: *** [Makefile:240: __sub-make] Error 2
+WARNING: modpost: missing MODULE_DESCRIPTION() in vmlinux.o
+WARNING: modpost: missing MODULE_DESCRIPTION() in
arch/x86/kernel/cpu/mce/mce-inject.o
+WARNING: modpost: missing MODULE_DESCRIPTION() in
arch/x86/mm/testmmiotrace.o
+WARNING: modpost: missing MODULE_DESCRIPTION() in
arch/x86/crypto/crc32-pclmul.o
+WARNING: modpost: missing MODULE_DESCRIPTION() in
arch/x86/crypto/curve25519-x86_64.o

...

...and looking at

https://github.com/linux-netdev/nipa/blob/main/tests/patch/build_32bit/build_32bit.sh

...that appears to be a diff between old and new build logs. The new
issues all appear to be missing module license complaints in an
allmodconfig build.

I did find another issue in tools/lib/bpf/btf_relocate.c when compiling
with clang that I'll send a patch for, and there's an existing issue in
btf.c that generates a warning:

tools/testing/selftests/kvm/settings: warning: ignored by one of the
.gitignore files
../kernel/bpf/btf.c: In function ‘btf_seq_show’:
../kernel/bpf/btf.c:7544:29: warning: function ‘btf_seq_show’ might be a
candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format]
 7544 |         seq_vprintf((struct seq_file *)show->target, fmt, args);
      |                             ^~~~~~~~
../kernel/bpf/btf.c: In function ‘btf_snprintf_show’:
../kernel/bpf/btf.c:7581:9: warning: function ‘btf_snprintf_show’ might
be a candidate for ‘gnu_printf’ format attribute
[-Wsuggest-attribute=format]
 7581 |         len = vsnprintf(show->target, ssnprintf->len_left, fmt,
args);
      |         ^~~


...but I can't see how this fix is still causing failures in finding
btf_relocate_id(). There may be something I'm missing here of course.
Thanks!

Alan

Alexei Starovoitov June 23, 2024, 7:53 p.m. UTC | #3

On Sun, Jun 23, 2024 at 12:04 PM Alan Maguire <[e-mail]> wrote:
> I've been trying to reproduce this with no success I'm afraid. I may be
> misreading but it appears that the diff from baseline to new build is
> actually telling us the btf_relocate_id() issues went away
>
> https://netdev.bots.linux.dev/static/nipa/864622/13708618/build_clang/stderr
>
> shows (note the "-" in the diffs preceding the btf_relocate_id()
> complaints):

Ahh. Makes sense. Applied.

> I did find another issue in tools/lib/bpf/btf_relocate.c when compiling
> with clang that I'll send a patch for, and there's an existing issue in
> btf.c that generates a warning:

Thanks!

[e-mail] June 23, 2024, 8 p.m. UTC | #4

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <[e-mail]>:

On Sun, 23 Jun 2024 14:52:24 +0100 you wrote:
> Kernel test robot reports that kernel build fails with
> resilient split BTF changes.
> 
> Examining the associated config and code we see that
> btf_relocate_id() is defined under CONFIG_DEBUG_INFO_BTF_MODULES.
> Moving it outside the #ifdef solves the issue.
> 
> [...]

Here is the summary with links:
  - [bpf-next] bpf: fix build when CONFIG_DEBUG_INFO_BTF[_MODULES] is undefined
    https://git.kernel.org/bpf/bpf-next/c/5a532459aa91

You are awesome, thank you!

Patch

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 8e12cb80ba73..4ff11779699e 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6185,8 +6185,6 @@  struct btf *btf_parse_vmlinux(void)
 	return btf;
 }
 
-#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
-
 /* If .BTF_ids section was created with distilled base BTF, both base and
  * split BTF ids will need to be mapped to actual base/split ids for
  * BTF now that it has been relocated.
@@ -6198,6 +6196,8 @@  static __u32 btf_relocate_id(const struct btf *btf, __u32 id)
 	return btf->base_id_map[id];
 }
 
+#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
+
 static struct btf *btf_parse_module(const char *module_name, const void *data,
 				    unsigned int data_size, void *base_data,
 				    unsigned int base_data_size)