Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210130004401.2528717-1-ndesaulniers@google.com>
 <20210130004401.2528717-3-ndesaulniers@google.com> <CAK7LNAQW3XtBGAg6u+86wGc0tizDyezZ_f61JjkT15QH5BtGjA@mail.gmail.com>
In-Reply-To: <CAK7LNAQW3XtBGAg6u+86wGc0tizDyezZ_f61JjkT15QH5BtGjA@mail.gmail.com>
From:   Nick Desaulniers <ndesaulniers@google.com>
Date:   Wed, 3 Feb 2021 15:27:34 -0800
Message-ID: <CAKwvOdnFQ+Y+QzHLVs-XNFtbNL8s236x6zS3QAkQ-unPvhbfEA@mail.gmail.com>
Subject: Re: [PATCH v7 2/2] Kbuild: implement support for DWARF v5
To:     Masahiro Yamada <masahiroy@kernel.org>,
        Nick Clifton <nickc@redhat.com>
Cc:     Nathan Chancellor <natechancellor@gmail.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Sedat Dilek <sedat.dilek@gmail.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        clang-built-linux <clang-built-linux@googlegroups.com>,
        Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
        linux-arch <linux-arch@vger.kernel.org>,
        Jakub Jelinek <jakub@redhat.com>,
        Fangrui Song <maskray@google.com>,
        Caroline Tice <cmtice@google.com>, Yonghong Song <yhs@fb.com>,
        Jiri Olsa <jolsa@kernel.org>,
        Andrii Nakryiko <andrii@kernel.org>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Arvind Sankar <nivedita@alum.mit.edu>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Feb 3, 2021 at 3:07 PM Masahiro Yamada <masahiroy@kernel.org> 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.  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.
-- 
Thanks,
~Nick Desaulniers