Re: [PATCH 2/3] Makefile.debug: re-enable debug info for .S files
From: Nick Desaulniers
Date: Fri Aug 26 2022 - 16:20:07 EST
On Fri, Aug 26, 2022 at 12:52 PM Nick Desaulniers
<ndesaulniers@xxxxxxxxxx> wrote:
>
> On Fri, Aug 26, 2022 at 11:41 AM Bill Wendling <morbo@xxxxxxxxxx> wrote:
> >
> > On Fri, Aug 26, 2022 at 11:10 AM Nick Desaulniers
> > <ndesaulniers@xxxxxxxxxx> wrote:
> > >
> > > Alexey reported that the fraction of unknown filename instances in
> > > kallsyms grew from ~0.3% to ~10% recently; Bill and Greg tracked it down
> > > to assembler defined symbols, which regressed as a result of:
> > >
> > > commit b8a9092330da ("Kbuild: do not emit debug info for assembly with LLVM_IAS=1")
> > >
> > > In that commit, I allude to restoring debug info for assembler defined
> > > symbols in a follow up patch, but it seems I forgot to do so in
> > >
> > > commit a66049e2cf0e ("Kbuild: make DWARF version a choice")
> > >
> > > This patch does a few things:
> > > 1. Add -g to KBUILD_AFLAGS. This will instruct the compiler to instruct
> > > the assembler to emit debug info. But this can cause an issue for
> > > folks using a newer compiler but older assembler, because the
> > > implicit default DWARF version changed from v4 to v5 in gcc-11 and
> > > clang-14.
> > > 2. If the user is using CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT, use a
> > > version check to explicitly set -Wa,-gdwarf-<version> for the
> > > assembler. There's another problem with this; GAS only gained support
> > > for explicit DWARF versions 3-5 in the 2.36 GNU binutils release.
> > > 3. Wrap -Wa,-gdwarf-<version> in as-option call to test whether the
> > > assembler supports that explicit DWARF version.
> > >
> > > Link: https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=31bf18645d98b4d3d7357353be840e320649a67d
> > > Fixes: b8a9092330da ("Kbuild: do not emit debug info for assembly with LLVM_IAS=1")
> > > Reported-by: Alexey Alexandrov <aalexand@xxxxxxxxxx>
> > > Reported-by: Bill Wendling <morbo@xxxxxxxxxx>
> > > Reported-by: Greg Thelen <gthelen@xxxxxxxxxx>
> > > Signed-off-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> > > ---
> > > scripts/Makefile.debug | 22 ++++++++++++++++++----
> > > 1 file changed, 18 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug
> > > index 9f39b0130551..a7a6da7f6e7d 100644
> > > --- a/scripts/Makefile.debug
> > > +++ b/scripts/Makefile.debug
> > > @@ -4,18 +4,32 @@ ifdef CONFIG_DEBUG_INFO_SPLIT
> > > DEBUG_CFLAGS += -gsplit-dwarf
> > > else
> > > DEBUG_CFLAGS += -g
> > > +KBUILD_AFLAGS += -g
> > > endif
> > >
> > > -ifndef CONFIG_AS_IS_LLVM
> > > -KBUILD_AFLAGS += -Wa,-gdwarf-2
> > > +ifdef CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT
> > > +# gcc-11+, clang-14+
> > > +ifeq ($(shell [ $(CONFIG_GCC_VERSION) -ge 110000 -o $(CONFIG_CLANG_VERSION) -ge 140000 ] && echo y),y)
> >
> > Do you think this would be better as a macro? Maybe something like:
> >
> > if $(call cc-min-version,110000,140000)
> >
> > where the first argument is GCC's min version and second Clang's min
> > version. It would be more readable and reusable.
>
> Yeah!
>
> I was looking at cc-ifversion, which has a bug in that it specifically
> uses CONFIG_GCC_VERSION. I think I sent a series maybe a year or two
> ago trying to remove all users of that macro; I think most landed but
> not all and I never pursued it to completion. Also, I think there's
> one user remaining in the AMDGPU drivers; looks like they're been
> reducing their dependency on that SIMD hack I wrote for them years ago
> after propagating it to parts of their tree, but one user remains.
> Perhaps I can just open code it there, or replace it with something
> new like you suggest.
>
> Such a macro would need to consider whether CC_IS_GCC vs CC_IS_CLANG;
> I could imagine we might need a version check for both, or just one of
> the two compilers.
>
> Ugh, logical OR in GNU make is supported by use of the filter macro...yuck.
>
> ifneq ($(filter y, $(call gcc-min-version, 110000), $(call
> clang-min-version, 140000),)
> # add gcc-11+, clang-14+ flag
> endif
>
> or maybe as you suggest:
>
> ifneq ($(call cc-min-version,110000,140000),)
> # add gcc-11+, clang-14+ flag
> endif
>
> where the newly minted cc-min-version is implemented in terms of the
> newly minted gcc-min-version and clang-min-version. Then I can use
> cc-min-version in this series, and replace cc-ifversion in AMDGPU with
> gcc-min-version. That way, we create composable wrappers that are
> readable and reusable.
RFC:
```
diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
index d1739f0d3ce3..4809a4c9e5f2 100644
--- a/scripts/Makefile.compiler
+++ b/scripts/Makefile.compiler
@@ -65,6 +65,19 @@ cc-disable-warning = $(call try-run,\
# Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
cc-ifversion = $(shell [ $(CONFIG_GCC_VERSION)0 $(1) $(2)000 ] &&
echo $(3) || echo $(4))
+# gcc-min-version
+# Usage: cflags-$(call gcc-min-version, 70100) += -foo
+gcc-min-version = $(shell [ $(CONFIG_GCC_VERSION) -ge $(1) ] && echo y)
+
+# clang-min-version
+# Usage: cflags-$(call clang-min-version, 110000) += -foo
+clang-min-version = $(shell [ $(CONFIG_CLANG_VERSION) -ge $(1) ] && echo y)
+
+# cc-min-version
+# Usage: cflags-$(call cc-min-version, 701000, 110000)
+# ^ GCC ^ Clang
+cc-min-version = $(filter y, $(call gcc-min-version, $(1)), $(call
clang-min-version, $(2)))
+
# ld-option
# Usage: KBUILD_LDFLAGS += $(call ld-option, -X, -Y)
ld-option = $(call try-run, $(LD) $(KBUILD_LDFLAGS) $(1) -v,$(1),$(2),$(3))
diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug
index 0f9912f7bd4c..80c84f746219 100644
--- a/scripts/Makefile.debug
+++ b/scripts/Makefile.debug
@@ -7,7 +7,7 @@ endif
ifdef CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT
# gcc-11+, clang-14+
-ifeq ($(shell [ $(CONFIG_GCC_VERSION) -ge 110000 -o
$(CONFIG_CLANG_VERSION) -ge 140000 ] && echo y),y)
+ifeq ($(call cc-min-version, 110000, 140000), y)
dwarf-version-y := 5
else
dwarf-version-y := 4
```
I'd keep the replacement of cc-ifversion with gcc-min-version as a
distinct child patch, since I don't care about backports for that (and
there's probably more cc-ifversion users the further back you go) but
I think we might want cc-min-version in stable.
Technically, the above should also update
Documentation/kbuild/makefiles.rst; but I'm just testing this works;
it seems to.
Oh, it looks like cc-ifversion both permits checking max-version and
min version. But we can implement -ge and -lt with just one or the
other:
```
diff --git a/drivers/gpu/drm/amd/display/dc/dml/Makefile
b/drivers/gpu/drm/amd/display/dc/dml/Makefile
index 86a3b5bfd699..17e8fd42d0a5 100644
--- a/drivers/gpu/drm/amd/display/dc/dml/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dml/Makefile
@@ -34,7 +34,7 @@ dml_ccflags := -mhard-float -maltivec
endif
ifdef CONFIG_CC_IS_GCC
-ifeq ($(call cc-ifversion, -lt, 0701, y), y)
+ifeq ($(call gcc-min-version, 70100),)
IS_OLD_GCC = 1
endif
endif
```
Thoughts?
>
> >
> > -bw
> >
> > > +dwarf-version-y := 5
> > > +else
> > > +dwarf-version-y := 4
> > > endif
> > > -
> > > -ifndef CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT
> > > +else # !CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT
> > > dwarf-version-$(CONFIG_DEBUG_INFO_DWARF4) := 4
> > > dwarf-version-$(CONFIG_DEBUG_INFO_DWARF5) := 5
> > > DEBUG_CFLAGS += -gdwarf-$(dwarf-version-y)
> > > endif
> > >
> > > +# Binutils 2.35+ (or clang) required for -gdwarf-{4|5}.
> > > +# https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=31bf18645d98b4d3d7357353be840e320649a67d
> > > +ifneq ($(call as-option,-Wa$(comma)-gdwarf-$(dwarf-version-y)),)
> > > +KBUILD_AFLAGS += -Wa,-gdwarf-$(dwarf-version-y)
> > > +else
> > > +ifndef CONFIG_AS_IS_LLVM
> > > +KBUILD_AFLAGS += -Wa,-gdwarf-2
> > > +endif
> > > +endif
> > > +
> > > ifdef CONFIG_DEBUG_INFO_REDUCED
> > > DEBUG_CFLAGS += -fno-var-tracking
> > > ifdef CONFIG_CC_IS_GCC
> > > --
> > > 2.37.2.672.g94769d06f0-goog
> > >
>
>
>
> --
> Thanks,
> ~Nick Desaulniers
--
Thanks,
~Nick Desaulniers