linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Nick Desaulniers <ndesaulniers@google.com>
To: Jakub Jelinek <jakub@redhat.com>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Arvind Sankar <nivedita@alum.mit.edu>
Cc: Nathan Chancellor <natechancellor@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Sedat Dilek <sedat.dilek@gmail.com>,
	LKML <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>,
	Fangrui Song <maskray@google.com>,
	Caroline Tice <cmtice@google.com>,
	Nick Clifton <nickc@redhat.com>, Yonghong Song <yhs@fb.com>,
	Jiri Olsa <jolsa@kernel.org>, Andrii Nakryiko <andrii@kernel.org>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Nathan Chancellor <nathan@kernel.org>
Subject: Re: [PATCH v6 1/2] Kbuild: make DWARF version a choice
Date: Fri, 29 Jan 2021 12:57:20 -0800	[thread overview]
Message-ID: <CAKwvOdk0zxewEOaFuqK0aSMz3vKNzDOgmez=-Dae4+bodsSg5w@mail.gmail.com> (raw)
In-Reply-To: <CAKwvOdkqcWOn6G7U6v37kc6gxZ=xbiZ1JtCd4XyCggMe=0v8iQ@mail.gmail.com>

On Fri, Jan 29, 2021 at 12:19 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Fri, Jan 29, 2021 at 12:17 PM Jakub Jelinek <jakub@redhat.com> wrote:
> >
> > On Fri, Jan 29, 2021 at 11:43:17AM -0800, Nick Desaulniers wrote:
> > > Modifies CONFIG_DEBUG_INFO_DWARF4 to be a member of a choice. Adds an
> > > explicit CONFIG_DEBUG_INFO_DWARF2, which is the default. Does so in a
> > > way that's forward compatible with existing configs, and makes adding
> > > future versions more straightforward.
> > >
> > > Suggested-by: Arvind Sankar <nivedita@alum.mit.edu>
> > > Suggested-by: Fangrui Song <maskray@google.com>
> > > Suggested-by: Nathan Chancellor <nathan@kernel.org>
> > > Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > > ---
> > >  Makefile          |  6 +++---
> > >  lib/Kconfig.debug | 21 ++++++++++++++++-----
> > >  2 files changed, 19 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 95ab9856f357..20141cd9319e 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -830,9 +830,9 @@ ifneq ($(LLVM_IAS),1)
> > >  KBUILD_AFLAGS        += -Wa,-gdwarf-2
> > >  endif
> > >
> > > -ifdef CONFIG_DEBUG_INFO_DWARF4
> > > -DEBUG_CFLAGS += -gdwarf-4
> > > -endif
> > > +dwarf-version-$(CONFIG_DEBUG_INFO_DWARF2) := 2
> > > +dwarf-version-$(CONFIG_DEBUG_INFO_DWARF4) := 4
> > > +DEBUG_CFLAGS += -gdwarf-$(dwarf-version-y)
> >
> > Why do you make DWARF2 the default?  That seems a big step back from what
> > the Makefile used to do before, where it defaulted to whatever DWARF version
> > the compiler defaulted to?
> > E.g. GCC 4.8 up to 10 defaults to -gdwarf-4 and GCC 11 will default to
> > -gdwarf-5.
> > DWARF2 is more than 27 years old standard, DWARF3 15 years old,
> > DWARF4 over 10 years old and DWARF5 almost 4 years old...
> > It is true that some tools aren't DWARF5 ready at this point, but with GCC
> > defaulting to that it will change quickly, but at least DWARF4 support has
> > been around for years.
>
> I agree with you; I also do not want to change the existing defaults
> in this series. That is a separate issue to address.

Thinking more about this over lunch...

I agree that DWARF v2 is quite old and I don't have a concrete reason
why the Linux kernel should continue to support it in 2021.

I agree that this patch takes away the compiler vendor's choice as to
what the implicit default choice is for dwarf version for the kernel.
(We, the Linux kernel, do so already for implicit default -std=gnuc*
as well).

I would not mind making this commit more explicit along the lines of:
"""
If you previously had not explicitly opted into
CONFIG_DEBUG_INFO_DWARF4, you will be opted in to
CONFIG_DEBUG_INFO_DWARF2 rather than the compiler's implicit default
(which changes over time).
"""
If you would rather see dwarf4 be the explicit default, that can be
done before or after this patch series, but to avoid further
"rope-a-dope" over getting DWARFv5 enabled, I suggest waiting until
after.

If Masahiro or Arvind (or whoever) feel differently about preserving
the previous "don't care" behavior related to DWARF version for
developers who had previously not opted in to
CONFIG_DEBUG_INFO_DWARF4, I can drop this patch, and resend v7 of
0002/0002 simply adding CONFIG_DEBUG_INFO_DWARF5 and making that and
CONFIG_DEBUG_INFO_DWARF4 depend on ! each other (I think).  But I'm
going to suggest we follow the Zen of Python: explicit is better than
implicit.  Supporting "I choose not to choose (my dwarf version)"
doesn't seem worthwhile to me, but could be convinced otherwise.
-- 
Thanks,
~Nick Desaulniers

  reply	other threads:[~2021-01-29 20:58 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-29 19:43 [PATCH v6 0/2] Kbuild: DWARF v5 support Nick Desaulniers
2021-01-29 19:43 ` [PATCH v6 1/2] Kbuild: make DWARF version a choice Nick Desaulniers
2021-01-29 20:17   ` Jakub Jelinek
2021-01-29 20:19     ` Nick Desaulniers
2021-01-29 20:57       ` Nick Desaulniers [this message]
2021-01-29 21:32         ` Arvind Sankar
2021-01-29 21:41           ` Jakub Jelinek
2021-01-29 22:40             ` Nick Desaulniers
2021-01-29 19:43 ` [PATCH v6 2/2] Kbuild: implement support for DWARF v5 Nick Desaulniers
2021-01-29 20:41   ` Sedat Dilek
2021-01-29 20:48     ` Nick Desaulniers
2021-01-29 20:54       ` Sedat Dilek
2021-01-29 21:09         ` Nick Desaulniers
2021-01-29 21:13           ` Sedat Dilek
2021-01-29 21:20             ` Sedat Dilek
2021-01-29 22:09               ` Nick Desaulniers
2021-01-29 22:11                 ` Sedat Dilek
2021-01-29 22:20                   ` Nick Desaulniers
2021-01-29 22:23                     ` Sedat Dilek
2021-01-29 22:31                       ` Nick Desaulniers
2021-01-29 22:42                         ` Sedat Dilek
2021-02-03  7:56                           ` Masahiro Yamada
2021-01-29 20:57       ` Jakub Jelinek
2021-01-29 21:05         ` Nick Desaulniers
2021-01-29 21:11           ` Jakub Jelinek
2021-01-29 22:05             ` Nick Desaulniers
2021-01-29 22:09               ` Jakub Jelinek
2021-01-29 23:25                 ` Nick Desaulniers
2021-01-29 23:30                   ` Nick Desaulniers
2021-01-29 22:27             ` Nick Desaulniers
2021-01-29 21:51   ` Fangrui Song
2021-01-29 22:07     ` Nick Desaulniers
2021-01-29 22:47   ` Fangrui Song
2021-01-29 20:03 ` [PATCH v6 0/2] Kbuild: DWARF v5 support Sedat Dilek
2021-01-30  0:08 ` Sedat Dilek
2021-01-30  0:46   ` Nick Desaulniers
2021-01-30  0:58     ` Fāng-ruì Sòng

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='CAKwvOdk0zxewEOaFuqK0aSMz3vKNzDOgmez=-Dae4+bodsSg5w@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=nathan@kernel.org \
    --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).