linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nick Desaulniers <ndesaulniers@google.com>
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>
Subject: Re: [PATCH v7 2/2] Kbuild: implement support for DWARF v5
Date: Wed, 3 Feb 2021 15:27:34 -0800	[thread overview]
Message-ID: <CAKwvOdnFQ+Y+QzHLVs-XNFtbNL8s236x6zS3QAkQ-unPvhbfEA@mail.gmail.com> (raw)
In-Reply-To: <CAK7LNAQW3XtBGAg6u+86wGc0tizDyezZ_f61JjkT15QH5BtGjA@mail.gmail.com>

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

  reply	other threads:[~2021-02-03 23:28 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-30  0:43 [PATCH v7 0/2] Kbuild: DWARF v5 support Nick Desaulniers
2021-01-30  0:44 ` [PATCH v7 1/2] Kbuild: make DWARF version a choice Nick Desaulniers
2021-01-30  1:52   ` Nathan Chancellor
2021-02-03 22:23     ` Masahiro Yamada
2021-02-03 22:25       ` Masahiro Yamada
2021-02-03 23:16       ` Nick Desaulniers
2021-02-04  0:29         ` Masahiro Yamada
2021-02-04  6:15           ` Nick Desaulniers
2021-02-04  3:32       ` Fangrui Song
2021-02-04  6:08         ` Nick Desaulniers
2021-02-04  6:13     ` Nick Desaulniers
2021-02-04 10:39   ` Mark Wielaard
2021-02-04 19:18     ` Nick Desaulniers
2021-02-04 19:56       ` Mark Wielaard
2021-02-04 20:04         ` Nick Desaulniers
2021-02-04 20:28           ` Mark Wielaard
2021-02-04 22:06             ` Nick Desaulniers
2021-02-04 22:36               ` Fangrui Song
2021-02-05 12:49               ` Mark Wielaard
2021-02-05 21:18                 ` Nick Desaulniers
2021-02-06 15:11                   ` Mark Wielaard
2021-01-30  0:44 ` [PATCH v7 2/2] Kbuild: implement support for DWARF v5 Nick Desaulniers
2021-01-30  1:53   ` Nathan Chancellor
2021-01-30 23:10   ` Sedat Dilek
2021-01-30 23:39     ` Sedat Dilek
2021-01-31  0:37     ` Fāng-ruì Sòng
2021-01-31  0:39       ` Sedat Dilek
2021-02-03 23:06   ` Masahiro Yamada
2021-02-03 23:27     ` Nick Desaulniers [this message]
2021-02-04  0:33       ` Masahiro Yamada
2021-02-03 23:36     ` Jakub Jelinek
2021-02-04  0:33       ` Masahiro Yamada
2021-01-30 23:59 ` [PATCH v7 0/2] Kbuild: DWARF v5 support Sedat Dilek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKwvOdnFQ+Y+QzHLVs-XNFtbNL8s236x6zS3QAkQ-unPvhbfEA@mail.gmail.com \
    --to=ndesaulniers@google.com \
    --cc=acme@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=andrii@kernel.org \
    --cc=clang-built-linux@googlegroups.com \
    --cc=cmtice@google.com \
    --cc=jakub@redhat.com \
    --cc=jolsa@kernel.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masahiroy@kernel.org \
    --cc=maskray@google.com \
    --cc=natechancellor@gmail.com \
    --cc=nickc@redhat.com \
    --cc=nivedita@alum.mit.edu \
    --cc=sedat.dilek@gmail.com \
    --cc=yhs@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).