Re: [PATCH v6 1/2] Kbuild: make DWARF version a choice

[Arvind Sankar]

On Fri, Jan 29, 2021 at 12:57:20PM -0800, Nick Desaulniers wrote:
> On Fri, Jan 29, 2021 at 12:19 PM Nick Desaulniers
> <ndesaulniers@xxxxxxxxxx> wrote:
> >
> > On Fri, Jan 29, 2021 at 12:17 PM Jakub Jelinek <jakub@xxxxxxxxxx> 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@xxxxxxxxxxxx>
> > > > Suggested-by: Fangrui Song <maskray@xxxxxxxxxx>
> > > > Suggested-by: Nathan Chancellor <nathan@xxxxxxxxxx>
> > > > Suggested-by: Masahiro Yamada <masahiroy@xxxxxxxxxx>
> > > > Signed-off-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> > > > ---
> > > >  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

Given what Jakub is saying, i.e. it was previously impossible to get
dwarf2 with gcc, and you get dwarf4 whether or not DEBUG_INFO_DWARF4 was
actually selected, we should make the default choice DEBUG_INFO_DWARF4
with the new menu to avoid surprising users. We should probably just
drop DWARF2 and make the menu in this patch have only DWARF4, and then
add DWARF5 as the second choice. The menu is still a good thing for
future-proofing even if it only has two options currently.

Thanks.