diff mbox series

[v3,bpf] kbuild: fix dependencies for DEBUG_INFO_BTF

Message ID 20200401142057.453892-1-slava@bacher09.org
State Changes Requested
Delegated to: BPF Maintainers
Headers show
Series [v3,bpf] kbuild: fix dependencies for DEBUG_INFO_BTF | expand

Commit Message

Slava Bacherikov April 1, 2020, 2:20 p.m. UTC 
Currently turning on DEBUG_INFO_SPLIT when DEBUG_INFO_BTF is also
enabled will produce invalid btf file, since gen_btf function in
link-vmlinux.sh script doesn't handle *.dwo files.

Enabling DEBUG_INFO_REDUCED will also produce invalid btf file, and
using GCC_PLUGIN_RANDSTRUCT with BTF makes no sense.

Signed-off-by: Slava Bacherikov <slava@bacher09.org>
Reported-by: Jann Horn <jannh@google.com>
Reported-by: Liu Yiding <liuyd.fnst@cn.fujitsu.com>
Acked-by: KP Singh <kpsingh@google.com>
Acked-by: Andrii Nakryiko <andriin@fb.com>
Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
---
 lib/Kconfig.debug | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Slava Bacherikov April 1, 2020, 2:37 p.m. UTC | #1 
01.04.2020 17:20, Slava Bacherikov wrotes:
> Currently turning on DEBUG_INFO_SPLIT when DEBUG_INFO_BTF is also
> enabled will produce invalid btf file, since gen_btf function in
> link-vmlinux.sh script doesn't handle *.dwo files.
> 
> Enabling DEBUG_INFO_REDUCED will also produce invalid btf file, and
> using GCC_PLUGIN_RANDSTRUCT with BTF makes no sense.
> 
> Signed-off-by: Slava Bacherikov <slava@bacher09.org>
> Reported-by: Jann Horn <jannh@google.com>
> Reported-by: Liu Yiding <liuyd.fnst@cn.fujitsu.com>
> Acked-by: KP Singh <kpsingh@google.com>
> Acked-by: Andrii Nakryiko <andriin@fb.com>
> Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
> ---
>  lib/Kconfig.debug | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index f61d834e02fe..b94227be2d62 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -222,7 +222,9 @@ config DEBUG_INFO_DWARF4
>  
>  config DEBUG_INFO_BTF
>  	bool "Generate BTF typeinfo"
> -	depends on DEBUG_INFO
> +	depends on DEBUG_INFO || COMPILE_TEST
I had to add this, since DEBUG_INFO which depends on:

	DEBUG_KERNEL && !COMPILE_TEST

would block DEBUG_INFO_BTF when COMPILE_TEST is turned on.

In that case allyesconfig will emit both:

CONFIG_DEBUG_INFO_BTF=y
CONFIG_GCC_PLUGIN_RANDSTRUCT=y



> +	depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED
> +	depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST
>  	help
>  	  Generate deduplicated BTF type information from DWARF debug info.
>  	  Turning this on expects presence of pahole tool, which will convert
>
Kees Cook April 1, 2020, 3:49 p.m. UTC | #2 
On Wed, Apr 01, 2020 at 05:20:58PM +0300, Slava Bacherikov wrote:
> Currently turning on DEBUG_INFO_SPLIT when DEBUG_INFO_BTF is also
> enabled will produce invalid btf file, since gen_btf function in
> link-vmlinux.sh script doesn't handle *.dwo files.
> 
> Enabling DEBUG_INFO_REDUCED will also produce invalid btf file, and
> using GCC_PLUGIN_RANDSTRUCT with BTF makes no sense.
> 
> Signed-off-by: Slava Bacherikov <slava@bacher09.org>

Very cool; thanks! :)

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

> Reported-by: Jann Horn <jannh@google.com>
> Reported-by: Liu Yiding <liuyd.fnst@cn.fujitsu.com>
> Acked-by: KP Singh <kpsingh@google.com>
> Acked-by: Andrii Nakryiko <andriin@fb.com>
> Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
> ---
>  lib/Kconfig.debug | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index f61d834e02fe..b94227be2d62 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -222,7 +222,9 @@ config DEBUG_INFO_DWARF4
>  
>  config DEBUG_INFO_BTF
>  	bool "Generate BTF typeinfo"
> -	depends on DEBUG_INFO
> +	depends on DEBUG_INFO || COMPILE_TEST
> +	depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED
> +	depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST
>  	help
>  	  Generate deduplicated BTF type information from DWARF debug info.
>  	  Turning this on expects presence of pahole tool, which will convert
Andrii Nakryiko April 1, 2020, 5:46 p.m. UTC | #3 
On Wed, Apr 1, 2020 at 7:38 AM Slava Bacherikov <slava@bacher09.org> wrote:
>
>
>
> 01.04.2020 17:20, Slava Bacherikov wrotes:
> > Currently turning on DEBUG_INFO_SPLIT when DEBUG_INFO_BTF is also
> > enabled will produce invalid btf file, since gen_btf function in
> > link-vmlinux.sh script doesn't handle *.dwo files.
> >
> > Enabling DEBUG_INFO_REDUCED will also produce invalid btf file, and
> > using GCC_PLUGIN_RANDSTRUCT with BTF makes no sense.
> >
> > Signed-off-by: Slava Bacherikov <slava@bacher09.org>
> > Reported-by: Jann Horn <jannh@google.com>
> > Reported-by: Liu Yiding <liuyd.fnst@cn.fujitsu.com>
> > Acked-by: KP Singh <kpsingh@google.com>
> > Acked-by: Andrii Nakryiko <andriin@fb.com>
> > Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
> > ---
> >  lib/Kconfig.debug | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index f61d834e02fe..b94227be2d62 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -222,7 +222,9 @@ config DEBUG_INFO_DWARF4
> >
> >  config DEBUG_INFO_BTF
> >       bool "Generate BTF typeinfo"
> > -     depends on DEBUG_INFO
> > +     depends on DEBUG_INFO || COMPILE_TEST
> I had to add this, since DEBUG_INFO which depends on:
>
>         DEBUG_KERNEL && !COMPILE_TEST
>
> would block DEBUG_INFO_BTF when COMPILE_TEST is turned on.
>

Sorry if I'm being dense here. But what's the point in enabling
DEBUG_INFO_BTF if there is no *valid* DWARF info available for
DWARF-to-BTF conversion?


> In that case allyesconfig will emit both:
>
> CONFIG_DEBUG_INFO_BTF=y
> CONFIG_GCC_PLUGIN_RANDSTRUCT=y

Which I thought is exactly what we wanted to avoid. Not sure what's
the point of compiling kernel (even if it's the one that is not
supposed to ever run) that apriori has broken BTF? If it was
acceptable to not have DEBUG_INFO for COMPILE_TEST, why it's not
acceptable to not have DEBUG_INFO_BTF in that situation as well?

>
>
>
> > +     depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED
> > +     depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST
> >       help
> >         Generate deduplicated BTF type information from DWARF debug info.
> >         Turning this on expects presence of pahole tool, which will convert
> >
Slava Bacherikov April 1, 2020, 6:24 p.m. UTC | #4 
01.04.2020 20:46, Andrii Nakryiko пишет:
> On Wed, Apr 1, 2020 at 7:38 AM Slava Bacherikov <slava@bacher09.org> wrote:
>>
>>
>>
>> 01.04.2020 17:20, Slava Bacherikov wrotes:
>>> Currently turning on DEBUG_INFO_SPLIT when DEBUG_INFO_BTF is also
>>> enabled will produce invalid btf file, since gen_btf function in
>>> link-vmlinux.sh script doesn't handle *.dwo files.
>>>
>>> Enabling DEBUG_INFO_REDUCED will also produce invalid btf file, and
>>> using GCC_PLUGIN_RANDSTRUCT with BTF makes no sense.
>>>
>>> Signed-off-by: Slava Bacherikov <slava@bacher09.org>
>>> Reported-by: Jann Horn <jannh@google.com>
>>> Reported-by: Liu Yiding <liuyd.fnst@cn.fujitsu.com>
>>> Acked-by: KP Singh <kpsingh@google.com>
>>> Acked-by: Andrii Nakryiko <andriin@fb.com>
>>> Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
>>> ---
>>>  lib/Kconfig.debug | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
>>> index f61d834e02fe..b94227be2d62 100644
>>> --- a/lib/Kconfig.debug
>>> +++ b/lib/Kconfig.debug
>>> @@ -222,7 +222,9 @@ config DEBUG_INFO_DWARF4
>>>
>>>  config DEBUG_INFO_BTF
>>>       bool "Generate BTF typeinfo"
>>> -     depends on DEBUG_INFO
>>> +     depends on DEBUG_INFO || COMPILE_TEST
>> I had to add this, since DEBUG_INFO which depends on:
>>
>>         DEBUG_KERNEL && !COMPILE_TEST
>>
>> would block DEBUG_INFO_BTF when COMPILE_TEST is turned on.
>>
> 
> Sorry if I'm being dense here. But what's the point in enabling
> DEBUG_INFO_BTF if there is no *valid* DWARF info available for
> DWARF-to-BTF conversion?

As I mention in [0] there is no point in having `!GCC_PLUGIN_RANDSTRUCT
|| COMPILE_TEST` without `DEBUG_INFO || COMPILE_TEST`, since without it
COMPILE_TEST would block DEBUG_INFO_BTF and because of that
GCC_PLUGIN_RANDSTRUCT would be never blocked by BTF.

As far as I understood from [1] main point for all these these things
with COMPILE_TEST is to be able to check if kernel could be compiled
with all these options (e.g. check syntax, build scripts, etc).

I can rollback `DEBUG_INFO || COMPILE_TEST`, but in that case there is
no point in `GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST` since COMPILE_TEST
in that case will not affect anything here, regardless from it's value.

[0]:
https://lore.kernel.org/bpf/202004010029.167BA4AA1F@keescook/T/#m2f493902d6aed09d30e5c4144a0164459386339d
[1]:
https://lore.kernel.org/bpf/202004010029.167BA4AA1F@keescook/T/#m8f25fab3476c9619249fee9ae692acb98c02cdc7
> 
> 
>> In that case allyesconfig will emit both:
>>
>> CONFIG_DEBUG_INFO_BTF=y
>> CONFIG_GCC_PLUGIN_RANDSTRUCT=y
> 
> Which I thought is exactly what we wanted to avoid. Not sure what's
> the point of compiling kernel (even if it's the one that is not
> supposed to ever run) that apriori has broken BTF? If it was
> acceptable to not have DEBUG_INFO for COMPILE_TEST, why it's not
> acceptable to not have DEBUG_INFO_BTF in that situation as well?
> 
>>
>>
>>
>>> +     depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED
>>> +     depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST
>>>       help
>>>         Generate deduplicated BTF type information from DWARF debug info.
>>>         Turning this on expects presence of pahole tool, which will convert
>>>
diff mbox series

Patch

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index f61d834e02fe..b94227be2d62 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -222,7 +222,9 @@  config DEBUG_INFO_DWARF4
 
 config DEBUG_INFO_BTF
 	bool "Generate BTF typeinfo"
-	depends on DEBUG_INFO
+	depends on DEBUG_INFO || COMPILE_TEST
+	depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED
+	depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST
 	help
 	  Generate deduplicated BTF type information from DWARF debug info.
 	  Turning this on expects presence of pahole tool, which will convert