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

[Masahiro Yamada]

On Thu, Feb 4, 2021 at 8:27 AM Nick Desaulniers <ndesaulniers@xxxxxxxxxx> wrote:
>
> On Wed, Feb 3, 2021 at 3:07 PM Masahiro Yamada <masahiroy@xxxxxxxxxx> 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