by Masahiro Yamada

[permalink] [raw]

Subject: Re: [PATCH v7 2/2] Kbuild: implement support for DWARF v5

On Thu, Feb 4, 2021 at 8:36 AM Jakub Jelinek <[email protected]> wrote:
>
> On Thu, Feb 04, 2021 at 08:06:12AM +0900, Masahiro Yamada wrote:
> > GCC never outputs '.file 0', which is why
> > this test is only needed for Clang, correct?
>
> No, GCC outputs .file 0 if it during configure time detected assembler that
> supports it and doesn't have any of the known bugs related to it.
> But that means kernel doesn't need to care because GCC already took care of
> that.

OK, then I am fine with the current code.

Thanks for the clarification.

--
Best Regards
Masahiro Yamada

2021-02-04 00:38:53

by Masahiro Yamada

[permalink] [raw]

Subject: Re: [PATCH v7 2/2] Kbuild: implement support for DWARF v5

On Thu, Feb 4, 2021 at 8:27 AM Nick Desaulniers <[email protected]> wrote:
>
> On Wed, Feb 3, 2021 at 3:07 PM Masahiro Yamada <[email protected]> wrote:
> >
> > Nick, the patch set is getting simpler and simpler,
> > and almost good enough to be merged.
>
> I agree. I think Sedat pointed out a binutils 2.35.2 release; thanks
> to Nick Clifton for that.
>
> >
> >
> > Please let me ask two questions below.
> >
> > There has been a lot of discussion, and
> > I might have missed the context.
> >
> > > --- a/lib/Kconfig.debug
> > > +++ b/lib/Kconfig.debug
> > > @@ -268,6 +268,24 @@ config DEBUG_INFO_DWARF4
> > > It makes the debug information larger, but it significantly
> > > improves the success of resolving variables in gdb on optimized code.
> > >
> > > +config DEBUG_INFO_DWARF5
> > > + bool "Generate DWARF Version 5 debuginfo"
> > > + depends on GCC_VERSION >= 50000 || CC_IS_CLANG
> > > + depends on CC_IS_GCC || $(success,$(srctree)/scripts/test_dwarf5_support.sh $(CC) $(CLANG_FLAGS))
> >
> > Q1.
> >
> > This "CC_IS_GCC ||" was introduced by v4.
> >
> > GCC never outputs '.file 0', which is why
> > this test is only needed for Clang, correct?
>
> This test script is only needed when compiling with clang but without
> its integrated assembler. It checks that when clang is used as the
> driver, but GAS is used as the assembler, that GAS will be able to
> decode the DWARF v5 assembler additions Clang will produce without
> needing an explicit -Wa,-gdwarf-5 flag passed.
>
> Technically, it is unnecessary for `LLVM=1 LLVM_IAS=1` or `CC=clang
> LLVM_IAS=1` (ie. clang+clang's integrated assembler). But there is no
> way to express AS_IS_IAS today in KConfig (similar to
> CC_IS_{GCC|CLANG} or LD_IS_LLD). I don't think that's necessary;
> whether or not clang's integrated assembler is used, when using clang,
> run the simple check.
>
> > > --- /dev/null
> > > +++ b/scripts/test_dwarf5_support.sh
> > > @@ -0,0 +1,8 @@
> > > +#!/bin/sh
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +
> > > +# Test that the assembler doesn't need -Wa,-gdwarf-5 when presented with DWARF
> > > +# v5 input, such as `.file 0` and `md5 0x00`. Should be fixed in GNU binutils
> > > +# 2.35.2. https://sourceware.org/bugzilla/show_bug.cgi?id=25611
> >
> >
> > I saw the following links in v6.
> >
> > https://sourceware.org/bugzilla/show_bug.cgi?id=25612
> > https://sourceware.org/bugzilla/show_bug.cgi?id=25614
> >
> > They were dropped in v7. Why?
> >
> > I just thought they were good to know...
>
> While having fixes for those bugs is required, technically
> https://sourceware.org/bugzilla/show_bug.cgi?id=25611 is the latest
> bug which was fixed. Testing for a fix of
> https://sourceware.org/bugzilla/show_bug.cgi?id=25611 implies that
> fixes for 25612 and 25614 exist due to the order they were fixed in
> GAS.

It is difficult to know the patch order in the binutils project.

Personally, I prefer having all the three references here.
Otherwise, it is difficult to understand why
this script is doing such complex checks.

> Technically, you could argue that this script is quite GAS
> centric; given an arbitrary "assembler" the test should check a few
> things. Realistically, I think that's overkill based on what
> assemblers are in use today; we can always grow the script should we
> identify other tests additional assemblers may need to pass, but until
> then, I suspect YAGNI. Maybe there's a more precise name for the
> script to reflect that, but that gets close to "what color shall we
> paint the bikeshed?" Given the number of folks on the thread, plz no.

No argument with this regard. I agree with you.

> --
> Thanks,
> ~Nick Desaulniers

--
Best Regards
Masahiro Yamada