[PATCH 0/3] fix debug info for asm and DEBUG_INFO_SPLIT

[PATCH 0/3] fix debug info for asm and DEBUG_INFO_SPLIT

[Nick Desaulniers]

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")

Do so requires a fixup for as-option, which seems to be failing when
used in scripts/Makefile.debug.

Also includes a fix for DEBUG_INFO_SPLIT while I'm here.  Dmitrii
reports that this has been broken since gcc-11+ & clang-12+. I'm
guessing no one uses this .config option...since no one else has
reported it being broken yet...

Nick Desaulniers (3):
  Makefile.compiler: s/KBUILD_CFLAGS/KBUILD_AFLAGS/ for as-option
  Makefile.debug: re-enable debug info for .S files
  Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT

 arch/x86/boot/compressed/Makefile |  5 +++--
 scripts/Makefile.compiler         |  6 +++---
 scripts/Makefile.debug            | 26 +++++++++++++++++++-------
 3 files changed, 25 insertions(+), 12 deletions(-)

-- 
2.37.2.672.g94769d06f0-goog

[PATCH 1/3] Makefile.compiler: s/KBUILD_CFLAGS/KBUILD_AFLAGS/ for as-option

[Nick Desaulniers]

as-instr uses KBUILD_AFLAGS, but as-option uses KBUILD_CFLAGS.  This can
cause as-option to fail unexpectedly because clang will emit
-Werror,-Wunused-command-line-argument for various -m and -f flags for
assembler sources.

Callers of as-option (and as-instr) likely want to be adding flags to
KBUILD_AFLAGS/aflags-y, not KBUILD_CFLAGS/cflags-y.

Link: https://github.com/ClangBuiltLinux/linux/issues/1699
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 arch/x86/boot/compressed/Makefile | 5 +++--
 scripts/Makefile.compiler         | 6 +++---
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 35ce1a64068b..fb3db714a028 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -48,8 +48,6 @@ KBUILD_CFLAGS += -Wno-pointer-sign
 KBUILD_CFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=)
 KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
 KBUILD_CFLAGS += -D__DISABLE_EXPORTS
-# Disable relocation relaxation in case the link is not PIE.
-KBUILD_CFLAGS += $(call as-option,-Wa$(comma)-mrelax-relocations=no)
 KBUILD_CFLAGS += -include $(srctree)/include/linux/hidden.h
 
 # sev.c indirectly inludes inat-table.h which is generated during
@@ -58,6 +56,9 @@ KBUILD_CFLAGS += -include $(srctree)/include/linux/hidden.h
 CFLAGS_sev.o += -I$(objtree)/arch/x86/lib/
 
 KBUILD_AFLAGS  := $(KBUILD_CFLAGS) -D__ASSEMBLY__
+# Disable relocation relaxation in case the link is not PIE.
+KBUILD_AFLAGS += $(call as-option,-Wa$(comma)-mrelax-relocations=no)
+
 GCOV_PROFILE := n
 UBSAN_SANITIZE :=n
 
diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
index 94d0d40cddb3..d1739f0d3ce3 100644
--- a/scripts/Makefile.compiler
+++ b/scripts/Makefile.compiler
@@ -29,13 +29,13 @@ try-run = $(shell set -e;		\
 	fi)
 
 # as-option
-# Usage: cflags-y += $(call as-option,-Wa$(comma)-isa=foo,)
+# Usage: aflags-y += $(call as-option,-Wa$(comma)-isa=foo,)
 
 as-option = $(call try-run,\
-	$(CC) $(KBUILD_CFLAGS) $(1) -c -x assembler /dev/null -o "$$TMP",$(1),$(2))
+	$(CC) $(KBUILD_AFLAGS) $(1) -c -x assembler /dev/null -o "$$TMP",$(1),$(2))
 
 # as-instr
-# Usage: cflags-y += $(call as-instr,instr,option1,option2)
+# Usage: aflags-y += $(call as-instr,instr,option1,option2)
 
 as-instr = $(call try-run,\
 	printf "%b\n" "$(1)" | $(CC) $(KBUILD_AFLAGS) -c -x assembler -o "$$TMP" -,$(2),$(3))
-- 
2.37.2.672.g94769d06f0-goog

Re: [PATCH 1/3] Makefile.compiler: s/KBUILD_CFLAGS/KBUILD_AFLAGS/ for as-option

[Nathan Chancellor]

Hi Nick,

I think the title would be a little more readable if it was:

Makefile.compiler: Use KBUILD_AFLAGS for as-option

On Fri, Aug 26, 2022 at 11:10:33AM -0700, Nick Desaulniers wrote:
> as-instr uses KBUILD_AFLAGS, but as-option uses KBUILD_CFLAGS.  This can
> cause as-option to fail unexpectedly because clang will emit
> -Werror,-Wunused-command-line-argument for various -m and -f flags for
> assembler sources.
> 
> Callers of as-option (and as-instr) likely want to be adding flags to
> KBUILD_AFLAGS/aflags-y, not KBUILD_CFLAGS/cflags-y.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/1699
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  arch/x86/boot/compressed/Makefile | 5 +++--
>  scripts/Makefile.compiler         | 6 +++---
>  2 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index 35ce1a64068b..fb3db714a028 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -48,8 +48,6 @@ KBUILD_CFLAGS += -Wno-pointer-sign
>  KBUILD_CFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=)
>  KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
>  KBUILD_CFLAGS += -D__DISABLE_EXPORTS
> -# Disable relocation relaxation in case the link is not PIE.
> -KBUILD_CFLAGS += $(call as-option,-Wa$(comma)-mrelax-relocations=no)
>  KBUILD_CFLAGS += -include $(srctree)/include/linux/hidden.h
>  
>  # sev.c indirectly inludes inat-table.h which is generated during
> @@ -58,6 +56,9 @@ KBUILD_CFLAGS += -include $(srctree)/include/linux/hidden.h
>  CFLAGS_sev.o += -I$(objtree)/arch/x86/lib/
>  
>  KBUILD_AFLAGS  := $(KBUILD_CFLAGS) -D__ASSEMBLY__
> +# Disable relocation relaxation in case the link is not PIE.
> +KBUILD_AFLAGS += $(call as-option,-Wa$(comma)-mrelax-relocations=no)
> +

Commit 09e43968db40 ("x86/boot/compressed: Disable relocation
relaxation") added this to address
https://github.com/ClangBuiltLinux/linux/issues/1121, is it correct to
move it to only being used for the .S files in arch/x86/boot/compressed?

>  GCOV_PROFILE := n
>  UBSAN_SANITIZE :=n
>  
> diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
> index 94d0d40cddb3..d1739f0d3ce3 100644
> --- a/scripts/Makefile.compiler
> +++ b/scripts/Makefile.compiler
> @@ -29,13 +29,13 @@ try-run = $(shell set -e;		\
>  	fi)
>  
>  # as-option
> -# Usage: cflags-y += $(call as-option,-Wa$(comma)-isa=foo,)
> +# Usage: aflags-y += $(call as-option,-Wa$(comma)-isa=foo,)
>  
>  as-option = $(call try-run,\
> -	$(CC) $(KBUILD_CFLAGS) $(1) -c -x assembler /dev/null -o "$$TMP",$(1),$(2))
> +	$(CC) $(KBUILD_AFLAGS) $(1) -c -x assembler /dev/null -o "$$TMP",$(1),$(2))
>  
>  # as-instr
> -# Usage: cflags-y += $(call as-instr,instr,option1,option2)
> +# Usage: aflags-y += $(call as-instr,instr,option1,option2)
>  
>  as-instr = $(call try-run,\
>  	printf "%b\n" "$(1)" | $(CC) $(KBUILD_AFLAGS) -c -x assembler -o "$$TMP" -,$(2),$(3))
> -- 
> 2.37.2.672.g94769d06f0-goog
> 

Re: [PATCH 1/3] Makefile.compiler: s/KBUILD_CFLAGS/KBUILD_AFLAGS/ for as-option

[Nick Desaulniers]

On Fri, Aug 26, 2022 at 11:21 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> Hi Nick,
>
> I think the title would be a little more readable if it was:
>
> Makefile.compiler: Use KBUILD_AFLAGS for as-option

Thanks, yeah that looks better; I expect a v2 of this series will be necessary.

>
> On Fri, Aug 26, 2022 at 11:10:33AM -0700, Nick Desaulniers wrote:
> > as-instr uses KBUILD_AFLAGS, but as-option uses KBUILD_CFLAGS.  This can
> > cause as-option to fail unexpectedly because clang will emit
> > -Werror,-Wunused-command-line-argument for various -m and -f flags for
> > assembler sources.
> >
> > Callers of as-option (and as-instr) likely want to be adding flags to
> > KBUILD_AFLAGS/aflags-y, not KBUILD_CFLAGS/cflags-y.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/1699
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > ---
> >  arch/x86/boot/compressed/Makefile | 5 +++--
> >  scripts/Makefile.compiler         | 6 +++---
> >  2 files changed, 6 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> > index 35ce1a64068b..fb3db714a028 100644
> > --- a/arch/x86/boot/compressed/Makefile
> > +++ b/arch/x86/boot/compressed/Makefile
> > @@ -48,8 +48,6 @@ KBUILD_CFLAGS += -Wno-pointer-sign
> >  KBUILD_CFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=)
> >  KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
> >  KBUILD_CFLAGS += -D__DISABLE_EXPORTS
> > -# Disable relocation relaxation in case the link is not PIE.
> > -KBUILD_CFLAGS += $(call as-option,-Wa$(comma)-mrelax-relocations=no)
> >  KBUILD_CFLAGS += -include $(srctree)/include/linux/hidden.h
> >
> >  # sev.c indirectly inludes inat-table.h which is generated during
> > @@ -58,6 +56,9 @@ KBUILD_CFLAGS += -include $(srctree)/include/linux/hidden.h
> >  CFLAGS_sev.o += -I$(objtree)/arch/x86/lib/
> >
> >  KBUILD_AFLAGS  := $(KBUILD_CFLAGS) -D__ASSEMBLY__
> > +# Disable relocation relaxation in case the link is not PIE.
> > +KBUILD_AFLAGS += $(call as-option,-Wa$(comma)-mrelax-relocations=no)
> > +
>
> Commit 09e43968db40 ("x86/boot/compressed: Disable relocation
> relaxation") added this to address
> https://github.com/ClangBuiltLinux/linux/issues/1121, is it correct to
> move it to only being used for the .S files in arch/x86/boot/compressed?

+ Arvind

Hmm...that makes me think we might need two different as-option flags;
one that does use KBUILD_CFLAGS, and whose result is meant to be added
to cflags-y, then another that uses KBUILD_AFLAGS and is added to
aflagsy-y.

(My patch 2/3 in the series would use the latter)

Let's see what thoughts Masahiro has.

>
> >  GCOV_PROFILE := n
> >  UBSAN_SANITIZE :=n
> >
> > diff --git a/scripts/Makefile.compiler b/scripts/Makefile.compiler
> > index 94d0d40cddb3..d1739f0d3ce3 100644
> > --- a/scripts/Makefile.compiler
> > +++ b/scripts/Makefile.compiler
> > @@ -29,13 +29,13 @@ try-run = $(shell set -e;         \
> >       fi)
> >
> >  # as-option
> > -# Usage: cflags-y += $(call as-option,-Wa$(comma)-isa=foo,)
> > +# Usage: aflags-y += $(call as-option,-Wa$(comma)-isa=foo,)
> >
> >  as-option = $(call try-run,\
> > -     $(CC) $(KBUILD_CFLAGS) $(1) -c -x assembler /dev/null -o "$$TMP",$(1),$(2))
> > +     $(CC) $(KBUILD_AFLAGS) $(1) -c -x assembler /dev/null -o "$$TMP",$(1),$(2))
> >
> >  # as-instr
> > -# Usage: cflags-y += $(call as-instr,instr,option1,option2)
> > +# Usage: aflags-y += $(call as-instr,instr,option1,option2)
> >
> >  as-instr = $(call try-run,\
> >       printf "%b\n" "$(1)" | $(CC) $(KBUILD_AFLAGS) -c -x assembler -o "$$TMP" -,$(2),$(3))
> > --
> > 2.37.2.672.g94769d06f0-goog
> >



-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 1/3] Makefile.compiler: s/KBUILD_CFLAGS/KBUILD_AFLAGS/ for as-option

[Masahiro Yamada]

On Sat, Aug 27, 2022 at 3:35 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Fri, Aug 26, 2022 at 11:21 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > Hi Nick,
> >
> > I think the title would be a little more readable if it was:
> >
> > Makefile.compiler: Use KBUILD_AFLAGS for as-option
>
> Thanks, yeah that looks better; I expect a v2 of this series will be necessary.
>
> >
> > On Fri, Aug 26, 2022 at 11:10:33AM -0700, Nick Desaulniers wrote:
> > > as-instr uses KBUILD_AFLAGS, but as-option uses KBUILD_CFLAGS.  This can
> > > cause as-option to fail unexpectedly because clang will emit
> > > -Werror,-Wunused-command-line-argument for various -m and -f flags for
> > > assembler sources.
> > >
> > > Callers of as-option (and as-instr) likely want to be adding flags to
> > > KBUILD_AFLAGS/aflags-y, not KBUILD_CFLAGS/cflags-y.

Indeed, it has been weird since the day-1 of as-option.

commit cad8244840d1a148f638925758afd1cdf81fc839
Author: Paul Mundt <lethal@linux-sh.org>
Date:   Mon Jan 16 22:14:19 2006 -0800

    [PATCH] sh: Move CPU subtype configuration to its own Kconfig








> > >
> > > Link: https://github.com/ClangBuiltLinux/linux/issues/1699
> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > > ---
> > >  arch/x86/boot/compressed/Makefile | 5 +++--
> > >  scripts/Makefile.compiler         | 6 +++---
> > >  2 files changed, 6 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> > > index 35ce1a64068b..fb3db714a028 100644
> > > --- a/arch/x86/boot/compressed/Makefile
> > > +++ b/arch/x86/boot/compressed/Makefile
> > > @@ -48,8 +48,6 @@ KBUILD_CFLAGS += -Wno-pointer-sign
> > >  KBUILD_CFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=)
> > >  KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
> > >  KBUILD_CFLAGS += -D__DISABLE_EXPORTS
> > > -# Disable relocation relaxation in case the link is not PIE.
> > > -KBUILD_CFLAGS += $(call as-option,-Wa$(comma)-mrelax-relocations=no)
> > >  KBUILD_CFLAGS += -include $(srctree)/include/linux/hidden.h
> > >
> > >  # sev.c indirectly inludes inat-table.h which is generated during
> > > @@ -58,6 +56,9 @@ KBUILD_CFLAGS += -include $(srctree)/include/linux/hidden.h
> > >  CFLAGS_sev.o += -I$(objtree)/arch/x86/lib/
> > >
> > >  KBUILD_AFLAGS  := $(KBUILD_CFLAGS) -D__ASSEMBLY__
> > > +# Disable relocation relaxation in case the link is not PIE.
> > > +KBUILD_AFLAGS += $(call as-option,-Wa$(comma)-mrelax-relocations=no)
> > > +
> >
> > Commit 09e43968db40 ("x86/boot/compressed: Disable relocation
> > relaxation") added this to address
> > https://github.com/ClangBuiltLinux/linux/issues/1121, is it correct to
> > move it to only being used for the .S files in arch/x86/boot/compressed?

Good catch!


>
> + Arvind
>
> Hmm...that makes me think we might need two different as-option flags;
> one that does use KBUILD_CFLAGS, and whose result is meant to be added
> to cflags-y, then another that uses KBUILD_AFLAGS and is added to
> aflagsy-y.
>
> (My patch 2/3 in the series would use the latter)
>
> Let's see what thoughts Masahiro has.


How about this?

KBUILD_CFLAGS += $(call as-option,-Wa$(comma)-mrelax-relocations=no)

    to

KBUILD_CFLAGS += $(call cc-option,-Wa$(comma)-mrelax-relocations=no)


You can insert this one-liner before this patch.




In Kconfig, -Wa,<as-option> is tested by cc-option.
(See commit 4d0831e8a029c03f49f434f28b8faef9f0bd403f)





--
Best Regards
Masahiro Yamada

[PATCH 2/3] Makefile.debug: re-enable debug info for .S files

[Nick Desaulniers]

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@google.com>
Reported-by: Bill Wendling <morbo@google.com>
Reported-by: Greg Thelen <gthelen@google.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 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)
+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

Re: [PATCH 2/3] Makefile.debug: re-enable debug info for .S files

[Bill Wendling]

On Fri, Aug 26, 2022 at 11:10 AM Nick Desaulniers
<ndesaulniers@google.com> 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@google.com>
> Reported-by: Bill Wendling <morbo@google.com>
> Reported-by: Greg Thelen <gthelen@google.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  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.

-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
>

Re: [PATCH 2/3] Makefile.debug: re-enable debug info for .S files

[Nick Desaulniers]

On Fri, Aug 26, 2022 at 11:41 AM Bill Wendling <morbo@google.com> wrote:
>
> On Fri, Aug 26, 2022 at 11:10 AM Nick Desaulniers
> <ndesaulniers@google.com> 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@google.com>
> > Reported-by: Bill Wendling <morbo@google.com>
> > Reported-by: Greg Thelen <gthelen@google.com>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > ---
> >  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.

>
> -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

Re: [PATCH 2/3] Makefile.debug: re-enable debug info for .S files

[Nick Desaulniers]

On Fri, Aug 26, 2022 at 12:52 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Fri, Aug 26, 2022 at 11:41 AM Bill Wendling <morbo@google.com> wrote:
> >
> > On Fri, Aug 26, 2022 at 11:10 AM Nick Desaulniers
> > <ndesaulniers@google.com> 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@google.com>
> > > Reported-by: Bill Wendling <morbo@google.com>
> > > Reported-by: Greg Thelen <gthelen@google.com>
> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > > ---
> > >  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

Re: [PATCH 2/3] Makefile.debug: re-enable debug info for .S files

[Bill Wendling]

On Fri, Aug 26, 2022 at 1:16 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Fri, Aug 26, 2022 at 12:52 PM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > On Fri, Aug 26, 2022 at 11:41 AM Bill Wendling <morbo@google.com> wrote:
> > >
> > > On Fri, Aug 26, 2022 at 11:10 AM Nick Desaulniers
> > > <ndesaulniers@google.com> 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@google.com>
> > > > Reported-by: Bill Wendling <morbo@google.com>
> > > > Reported-by: Greg Thelen <gthelen@google.com>
> > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > > > ---
> > > >  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),)

I think for this you want "gcc-max-version".

>  IS_OLD_GCC = 1
>  endif
>  endif
> ```
>
> Thoughts?
>

I like this idea. Modifying "cc-ifversion" would be great, but you'd
have to somehow specify the checking versions for both gcc and clang
in the macro, and that's messy given the number of arguments that
already exist. It might be possible instead to implement
"cc-ifversion" in terms of the "*-{max,min}-version" macros.

-bw

> >
> > >
> > > -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

[PATCH 3/3] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT

[Nick Desaulniers]

Dmitrii, Fangrui, and Mashahiro note:

  Before GCC 11 and Clang 12 -gsplit-dwarf implicitly uses -g2.

Fix CONFIG_DEBUG_INFO_SPLIT for gcc-11+ & clang-12+ which now need -g
specified in order for -gsplit-dwarf to work at all.

Link: https://lore.kernel.org/lkml/20220815013317.26121-1-dmitrii.bundin.a@gmail.com/
Link: https://lore.kernel.org/lkml/CAK7LNARPAmsJD5XKAw7m_X2g7Fi-CAAsWDQiP7+ANBjkg7R7ng@mail.gmail.com/
Link: https://reviews.llvm.org/D80391
Reported-by: Dmitrii Bundin <dmitrii.bundin.a@gmail.com>
Reported-by: Fangrui Song <maskray@google.com>
Reported-by: Masahiro Yamada <masahiroy@kernel.org>
Suggested-by: Dmitrii Bundin <dmitrii.bundin.a@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 scripts/Makefile.debug | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug
index a7a6da7f6e7d..0f9912f7bd4c 100644
--- a/scripts/Makefile.debug
+++ b/scripts/Makefile.debug
@@ -1,10 +1,8 @@
-DEBUG_CFLAGS	:=
+DEBUG_CFLAGS	:= -g
+KBUILD_AFLAGS	+= -g
 
 ifdef CONFIG_DEBUG_INFO_SPLIT
 DEBUG_CFLAGS	+= -gsplit-dwarf
-else
-DEBUG_CFLAGS	+= -g
-KBUILD_AFLAGS	+= -g
 endif
 
 ifdef CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT
-- 
2.37.2.672.g94769d06f0-goog

Re: [PATCH 3/3] Makefile.debug: set -g unconditional on CONFIG_DEBUG_INFO_SPLIT

[Nick Desaulniers]

On Fri, Aug 26, 2022 at 11:10 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> Dmitrii, Fangrui, and Mashahiro note:
>
>   Before GCC 11 and Clang 12 -gsplit-dwarf implicitly uses -g2.
>
> Fix CONFIG_DEBUG_INFO_SPLIT for gcc-11+ & clang-12+ which now need -g
> specified in order for -gsplit-dwarf to work at all.

Looking at
commit 866ced950bcd ("kbuild: Support split debug info v4")
I'm curious whether -gsplit-dwarf needed to be mutually exclusive with
-g, possibly for older compilers? Andi, do you recall?
I have gcc-9 handy; that doesn't seem to be the case. I get the same
size binary with `-gsplit-dwarf` as I do with `-g -gsplit-dwarf`.  So
it does seem like mutual exclusion between the two is not necessary.

x86_64-gcc-5.5.0-nolibc-x86_64-linux.tar.gz from
https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/5.5.0/
seems to really need libisl.so.15...
$ cd /tmp
$ wget https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/5.5.0/x86_64-gcc-5.5.0-nolibc-x86_64-linux.tar.gz
$ echo "void foo(void) {}" > x.c
$ ./gcc-5.5.0-nolibc/x86_64-linux/bin/x86_64-linux-gcc-5.5.0
-gsplit-dwarf x.c -c
/tmp/gcc-5.5.0-nolibc/x86_64-linux/bin/../libexec/gcc/x86_64-linux/5.5.0/cc1:
error while loading shared libraries: libisl.so.15: cannot open shared
object file: No such file or directory
$ find /usr/lib -name libisl\*
/usr/lib/x86_64-linux-gnu/libisl.so.23.2.0
/usr/lib/x86_64-linux-gnu/libisl.so.23
/usr/lib/x86_64-linux-gnu/libisl.a
/usr/lib/x86_64-linux-gnu/libisl.so

>
> Link: https://lore.kernel.org/lkml/20220815013317.26121-1-dmitrii.bundin.a@gmail.com/
> Link: https://lore.kernel.org/lkml/CAK7LNARPAmsJD5XKAw7m_X2g7Fi-CAAsWDQiP7+ANBjkg7R7ng@mail.gmail.com/
> Link: https://reviews.llvm.org/D80391
> Reported-by: Dmitrii Bundin <dmitrii.bundin.a@gmail.com>
> Reported-by: Fangrui Song <maskray@google.com>
> Reported-by: Masahiro Yamada <masahiroy@kernel.org>
> Suggested-by: Dmitrii Bundin <dmitrii.bundin.a@gmail.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  scripts/Makefile.debug | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/Makefile.debug b/scripts/Makefile.debug
> index a7a6da7f6e7d..0f9912f7bd4c 100644
> --- a/scripts/Makefile.debug
> +++ b/scripts/Makefile.debug
> @@ -1,10 +1,8 @@
> -DEBUG_CFLAGS   :=
> +DEBUG_CFLAGS   := -g
> +KBUILD_AFLAGS  += -g
>
>  ifdef CONFIG_DEBUG_INFO_SPLIT
>  DEBUG_CFLAGS   += -gsplit-dwarf
> -else
> -DEBUG_CFLAGS   += -g
> -KBUILD_AFLAGS  += -g
>  endif
>
>  ifdef CONFIG_DEBUG_INFO_DWARF_TOOLCHAIN_DEFAULT
> --
> 2.37.2.672.g94769d06f0-goog
>


-- 
Thanks,
~Nick Desaulniers