2020-03-31 21:58:00

by Slava Bacherikov

[permalink] [raw]
Subject: [PATCH v2 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF

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 <[email protected]>
Reported-by: Jann Horn <[email protected]>
Reported-by: Liu Yiding <[email protected]>
Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
---
lib/Kconfig.debug | 1 +
1 file changed, 1 insertion(+)

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


2020-03-31 22:42:55

by KP Singh

[permalink] [raw]
Subject: Re: [PATCH v2 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF

On 01-Apr 00:55, 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 <[email protected]>
> Reported-by: Jann Horn <[email protected]>
> Reported-by: Liu Yiding <[email protected]>
> Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")

I can say that I also got invalid BTF when I tried using
DEBUG_INFO_REDUCED.

So here's a first one from of these from me :)

Acked-by: KP Singh <[email protected]>

> ---
> lib/Kconfig.debug | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index f61d834e02fe..9ae288e2a6c0 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -223,6 +223,7 @@ config DEBUG_INFO_DWARF4
> config DEBUG_INFO_BTF
> bool "Generate BTF typeinfo"
> depends on DEBUG_INFO
> + depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED && !GCC_PLUGIN_RANDSTRUCT
> help
> Generate deduplicated BTF type information from DWARF debug info.
> Turning this on expects presence of pahole tool, which will convert
> --
> 2.24.1
>

2020-04-01 00:06:07

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH v2 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF

On Tue, Mar 31, 2020 at 2:57 PM Slava Bacherikov <[email protected]> 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 <[email protected]>
> Reported-by: Jann Horn <[email protected]>
> Reported-by: Liu Yiding <[email protected]>
> Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
> ---

LGTM, but let's wait on Kees about COMPILE_TEST dependency...

Acked-by: Andrii Nakryiko <[email protected]>

> lib/Kconfig.debug | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index f61d834e02fe..9ae288e2a6c0 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -223,6 +223,7 @@ config DEBUG_INFO_DWARF4
> config DEBUG_INFO_BTF
> bool "Generate BTF typeinfo"
> depends on DEBUG_INFO
> + depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED && !GCC_PLUGIN_RANDSTRUCT
> help
> Generate deduplicated BTF type information from DWARF debug info.
> Turning this on expects presence of pahole tool, which will convert
> --
> 2.24.1
>

2020-04-01 07:36:30

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v2 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF

On Wed, Apr 01, 2020 at 12:55:37AM +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 <[email protected]>
> Reported-by: Jann Horn <[email protected]>
> Reported-by: Liu Yiding <[email protected]>
> Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
> ---
> lib/Kconfig.debug | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index f61d834e02fe..9ae288e2a6c0 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -223,6 +223,7 @@ config DEBUG_INFO_DWARF4
> config DEBUG_INFO_BTF
> bool "Generate BTF typeinfo"
> depends on DEBUG_INFO
> + depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED && !GCC_PLUGIN_RANDSTRUCT
> help
> Generate deduplicated BTF type information from DWARF debug info.
> Turning this on expects presence of pahole tool, which will convert

Please make this:

depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED
depends on COMPILE_TEST || !GCC_PLUGIN_RANDSTRUCT

-Kees

--
Kees Cook

2020-04-01 14:24:17

by Slava Bacherikov

[permalink] [raw]
Subject: [PATCH v3 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF

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 <[email protected]>
Reported-by: Jann Horn <[email protected]>
Reported-by: Liu Yiding <[email protected]>
Acked-by: KP Singh <[email protected]>
Acked-by: Andrii Nakryiko <[email protected]>
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

2020-04-01 14:40:10

by Slava Bacherikov

[permalink] [raw]
Subject: Re: [PATCH v3 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF



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 <[email protected]>
> Reported-by: Jann Horn <[email protected]>
> Reported-by: Liu Yiding <[email protected]>
> Acked-by: KP Singh <[email protected]>
> Acked-by: Andrii Nakryiko <[email protected]>
> 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
>

2020-04-01 15:51:29

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v3 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF

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 <[email protected]>

Very cool; thanks! :)

Reviewed-by: Kees Cook <[email protected]>

-Kees

> Reported-by: Jann Horn <[email protected]>
> Reported-by: Liu Yiding <[email protected]>
> Acked-by: KP Singh <[email protected]>
> Acked-by: Andrii Nakryiko <[email protected]>
> 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

--
Kees Cook

2020-04-01 18:25:07

by Slava Bacherikov

[permalink] [raw]
Subject: Re: [PATCH v3 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF



01.04.2020 20:46, Andrii Nakryiko пишет:
> On Wed, Apr 1, 2020 at 7:38 AM Slava Bacherikov <[email protected]> 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 <[email protected]>
>>> Reported-by: Jann Horn <[email protected]>
>>> Reported-by: Liu Yiding <[email protected]>
>>> Acked-by: KP Singh <[email protected]>
>>> Acked-by: Andrii Nakryiko <[email protected]>
>>> 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
>>>

2020-04-01 18:40:15

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH v3 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF

On Wed, Apr 1, 2020 at 7:38 AM Slava Bacherikov <[email protected]> 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 <[email protected]>
> > Reported-by: Jann Horn <[email protected]>
> > Reported-by: Liu Yiding <[email protected]>
> > Acked-by: KP Singh <[email protected]>
> > Acked-by: Andrii Nakryiko <[email protected]>
> > 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
> >

2020-04-02 15:35:52

by Slava Bacherikov

[permalink] [raw]
Subject: [PATCH v4 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF

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 <[email protected]>
Reported-by: Jann Horn <[email protected]>
Reported-by: Liu Yiding <[email protected]>
Acked-by: KP Singh <[email protected]>
Acked-by: Andrii Nakryiko <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
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

2020-04-02 15:41:09

by Slava Bacherikov

[permalink] [raw]
Subject: Re: [PATCH v4 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF



02.04.2020 18:33, Slava Bacherikov wrote:
> + depends on DEBUG_INFO || COMPILE_TEST

Andrii are you fine by this ?

2020-04-02 19:40:37

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH v4 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF

On Thu, Apr 2, 2020 at 8:40 AM Slava Bacherikov <[email protected]> wrote:
>
>
>
> 02.04.2020 18:33, Slava Bacherikov wrote:
> > + depends on DEBUG_INFO || COMPILE_TEST
>
> Andrii are you fine by this ?

I think it needs a good comment explaining this weirdness, at least.
As I said, if there is no DEBUG_INFO, there is not point in doing
DWARF-to-BTF conversion, even more -- it actually might fail, I
haven't checked what pahole does in that case. So I'd rather drop
GCC_PLUGIN_RANDSTRUCT is that's the issue here. DEBUG_INFO_SPLIT and
DEBUG_INFO_REDUCED look good.

2020-04-02 20:38:20

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v4 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF

On Thu, Apr 02, 2020 at 12:31:36PM -0700, Andrii Nakryiko wrote:
> On Thu, Apr 2, 2020 at 8:40 AM Slava Bacherikov <[email protected]> wrote:
> >
> >
> >
> > 02.04.2020 18:33, Slava Bacherikov wrote:
> > > + depends on DEBUG_INFO || COMPILE_TEST
> >
> > Andrii are you fine by this ?
>
> I think it needs a good comment explaining this weirdness, at least.
> As I said, if there is no DEBUG_INFO, there is not point in doing
> DWARF-to-BTF conversion, even more -- it actually might fail, I
> haven't checked what pahole does in that case. So I'd rather drop
> GCC_PLUGIN_RANDSTRUCT is that's the issue here. DEBUG_INFO_SPLIT and
> DEBUG_INFO_REDUCED look good.

The DEBUG_INFO is separate, AIUI -- it sounds like BTF may entirely
break on a compile with weird DWARF configs.

The GCC_PLUGIN_RANDSTRUCT issue is separate: it doesn't make sense to
run a kernel built with BTF and GCC_PLUGIN_RANDSTRUCT. But they should
have nothing to do with each other with regard to compilation. So, to
keep GCC_PLUGIN_RANDSTRUCT disable for "real" builds but leave it on for
all*config, randconfig, etc, I'd like to keep the || COMPILE_TEST,
otherwise GCC_PLUGIN_RANDSTRUCT won't be part of the many CIs doing
compilation testing.

And FWIW, I'm fine to let GCC_PLUGIN_RANDSTRUCT and BTF build together.
But if they want to be depends-conflicted, I wanted to keep the test
compile trap door.

--
Kees Cook

2020-04-02 20:39:51

by Slava Bacherikov

[permalink] [raw]
Subject: Re: [PATCH v4 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF



02.04.2020 23:34, Kees Cook wrote:
> On Thu, Apr 02, 2020 at 12:31:36PM -0700, Andrii Nakryiko wrote:
>> On Thu, Apr 2, 2020 at 8:40 AM Slava Bacherikov <[email protected]> wrote:
>>>
>>>
>>>
>>> 02.04.2020 18:33, Slava Bacherikov wrote:
>>>> + depends on DEBUG_INFO || COMPILE_TEST
>>>
>>> Andrii are you fine by this ?
>>
>> I think it needs a good comment explaining this weirdness, at least.
>> As I said, if there is no DEBUG_INFO, there is not point in doing
>> DWARF-to-BTF conversion, even more -- it actually might fail, I
>> haven't checked what pahole does in that case. So I'd rather drop
>> GCC_PLUGIN_RANDSTRUCT is that's the issue here. DEBUG_INFO_SPLIT and
>> DEBUG_INFO_REDUCED look good.

Yesterday before sending it I tested it against latest bpf git with
allyesconfig and it compiled fine, even worked in vm ;)
>
> The DEBUG_INFO is separate, AIUI -- it sounds like BTF may entirely
> break on a compile with weird DWARF configs.
>
> The GCC_PLUGIN_RANDSTRUCT issue is separate: it doesn't make sense to
> run a kernel built with BTF and GCC_PLUGIN_RANDSTRUCT. But they should
> have nothing to do with each other with regard to compilation. So, to
> keep GCC_PLUGIN_RANDSTRUCT disable for "real" builds but leave it on for
> all*config, randconfig, etc, I'd like to keep the || COMPILE_TEST,
> otherwise GCC_PLUGIN_RANDSTRUCT won't be part of the many CIs doing
> compilation testing.
>
> And FWIW, I'm fine to let GCC_PLUGIN_RANDSTRUCT and BTF build together.
> But if they want to be depends-conflicted, I wanted to keep the test
> compile trap door.
>

Oh, seems I misunderstood you, if everyone agree I'll drop it.

2020-04-02 20:45:08

by Slava Bacherikov

[permalink] [raw]
Subject: [PATCH v5 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF

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 <[email protected]>
Reported-by: Jann Horn <[email protected]>
Reported-by: Liu Yiding <[email protected]>
Acked-by: KP Singh <[email protected]>
Acked-by: Andrii Nakryiko <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
---
lib/Kconfig.debug | 2 ++
1 file changed, 2 insertions(+)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index f61d834e02fe..6118d99117da 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -223,6 +223,8 @@ config DEBUG_INFO_DWARF4
config DEBUG_INFO_BTF
bool "Generate BTF typeinfo"
depends on DEBUG_INFO
+ 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

2020-04-02 21:08:07

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH v5 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF

On Thu, Apr 2, 2020 at 1:44 PM Slava Bacherikov <[email protected]> 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 <[email protected]>
> Reported-by: Jann Horn <[email protected]>
> Reported-by: Liu Yiding <[email protected]>
> Acked-by: KP Singh <[email protected]>
> Acked-by: Andrii Nakryiko <[email protected]>
> Reviewed-by: Kees Cook <[email protected]>
> Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")
> ---
> lib/Kconfig.debug | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index f61d834e02fe..6118d99117da 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -223,6 +223,8 @@ config DEBUG_INFO_DWARF4
> config DEBUG_INFO_BTF
> bool "Generate BTF typeinfo"
> depends on DEBUG_INFO
> + depends on !DEBUG_INFO_SPLIT && !DEBUG_INFO_REDUCED
> + depends on !GCC_PLUGIN_RANDSTRUCT || COMPILE_TEST

Given what Kees explained, I think this looks good. Thanks!

> help
> Generate deduplicated BTF type information from DWARF debug info.
> Turning this on expects presence of pahole tool, which will convert

2020-04-02 22:50:46

by Daniel Borkmann

[permalink] [raw]
Subject: Re: [PATCH v5 bpf] kbuild: fix dependencies for DEBUG_INFO_BTF

On 4/2/20 10:41 PM, 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 <[email protected]>
> Reported-by: Jann Horn <[email protected]>
> Reported-by: Liu Yiding <[email protected]>
> Acked-by: KP Singh <[email protected]>
> Acked-by: Andrii Nakryiko <[email protected]>
> Reviewed-by: Kees Cook <[email protected]>
> Fixes: e83b9f55448a ("kbuild: add ability to generate BTF type info for vmlinux")

Applied, thanks!