diff mbox series

[v3] purgatory: fix disabling debug info

Message ID 20230330182223.181775-1-hi@alyssa.is
State New
Headers
Series [v3] purgatory: fix disabling debug info |

Commit Message

Alyssa Ross March 30, 2023, 6:22 p.m. UTC 
  Since 32ef9e5054ec, -Wa,-gdwarf-2 is no longer used in KBUILD_AFLAGS.
Instead, it includes -g, the appropriate -gdwarf-* flag, and also the
-Wa versions of both of those if building with Clang and GNU as.  As a
result, debug info was being generated for the purgatory objects, even
though the intention was that it not be.

Fixes: 32ef9e5054ec ("Makefile.debug: re-enable debug info for .S files")
Signed-off-by: Alyssa Ross <hi@alyssa.is>
Cc: stable@vger.kernel.org
Acked-by: Nick Desaulniers <ndesaulniers@google.com>
---
v2: https://lore.kernel.org/r/20230326182120.194541-1-hi@alyssa.is

Difference from v2: replaced asflags-remove-y with every possible
debug flag with asflags-y += -g0, as suggested by Nick Desaulniers.

Additionally, I've CCed the x86 maintainers this time, since Masahiro
said he would like acks from subsystem maintainers, and
get_maintainer.pl didn't pick them the first time around.

 arch/riscv/purgatory/Makefile | 7 +------
 arch/x86/purgatory/Makefile   | 3 +--
 2 files changed, 2 insertions(+), 8 deletions(-)

Comments

Nathan Chancellor March 30, 2023, 10:29 p.m. UTC | #1 
On Thu, Mar 30, 2023 at 06:22:24PM +0000, Alyssa Ross wrote:
> Since 32ef9e5054ec, -Wa,-gdwarf-2 is no longer used in KBUILD_AFLAGS.
> Instead, it includes -g, the appropriate -gdwarf-* flag, and also the
> -Wa versions of both of those if building with Clang and GNU as.  As a
> result, debug info was being generated for the purgatory objects, even
> though the intention was that it not be.
> 
> Fixes: 32ef9e5054ec ("Makefile.debug: re-enable debug info for .S files")
> Signed-off-by: Alyssa Ross <hi@alyssa.is>
> Cc: stable@vger.kernel.org
> Acked-by: Nick Desaulniers <ndesaulniers@google.com>

This is definitely more future proof.

Reviewed-by: Nathan Chancellor <nathan@kernel.org>
Tested-by: Nathan Chancellor <nathan@kernel.org>

> ---
> v2: https://lore.kernel.org/r/20230326182120.194541-1-hi@alyssa.is
> 
> Difference from v2: replaced asflags-remove-y with every possible
> debug flag with asflags-y += -g0, as suggested by Nick Desaulniers.
> 
> Additionally, I've CCed the x86 maintainers this time, since Masahiro
> said he would like acks from subsystem maintainers, and
> get_maintainer.pl didn't pick them the first time around.
> 
>  arch/riscv/purgatory/Makefile | 7 +------
>  arch/x86/purgatory/Makefile   | 3 +--
>  2 files changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/riscv/purgatory/Makefile b/arch/riscv/purgatory/Makefile
> index d16bf715a586..9c1e71853ee7 100644
> --- a/arch/riscv/purgatory/Makefile
> +++ b/arch/riscv/purgatory/Makefile
> @@ -84,12 +84,7 @@ CFLAGS_string.o			+= $(PURGATORY_CFLAGS)
>  CFLAGS_REMOVE_ctype.o		+= $(PURGATORY_CFLAGS_REMOVE)
>  CFLAGS_ctype.o			+= $(PURGATORY_CFLAGS)
>  
> -AFLAGS_REMOVE_entry.o		+= -Wa,-gdwarf-2
> -AFLAGS_REMOVE_memcpy.o		+= -Wa,-gdwarf-2
> -AFLAGS_REMOVE_memset.o		+= -Wa,-gdwarf-2
> -AFLAGS_REMOVE_strcmp.o		+= -Wa,-gdwarf-2
> -AFLAGS_REMOVE_strlen.o		+= -Wa,-gdwarf-2
> -AFLAGS_REMOVE_strncmp.o		+= -Wa,-gdwarf-2
> +asflags-y			+= -g0
>  
>  $(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE
>  		$(call if_changed,ld)
> diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
> index 17f09dc26381..8e6c81b1c8f7 100644
> --- a/arch/x86/purgatory/Makefile
> +++ b/arch/x86/purgatory/Makefile
> @@ -69,8 +69,7 @@ CFLAGS_sha256.o			+= $(PURGATORY_CFLAGS)
>  CFLAGS_REMOVE_string.o		+= $(PURGATORY_CFLAGS_REMOVE)
>  CFLAGS_string.o			+= $(PURGATORY_CFLAGS)
>  
> -AFLAGS_REMOVE_setup-x86_$(BITS).o	+= -Wa,-gdwarf-2
> -AFLAGS_REMOVE_entry64.o			+= -Wa,-gdwarf-2
> +asflags-y			+= -g0
>  
>  $(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE
>  		$(call if_changed,ld)
> -- 
> 2.37.1
> 
>
Masahiro Yamada March 31, 2023, 3:42 p.m. UTC | #2 
On Fri, Mar 31, 2023 at 7:29 AM Nathan Chancellor <nathan@kernel.org> wrote:
>
> On Thu, Mar 30, 2023 at 06:22:24PM +0000, Alyssa Ross wrote:
> > Since 32ef9e5054ec, -Wa,-gdwarf-2 is no longer used in KBUILD_AFLAGS.
> > Instead, it includes -g, the appropriate -gdwarf-* flag, and also the
> > -Wa versions of both of those if building with Clang and GNU as.  As a
> > result, debug info was being generated for the purgatory objects, even
> > though the intention was that it not be.
> >
> > Fixes: 32ef9e5054ec ("Makefile.debug: re-enable debug info for .S files")
> > Signed-off-by: Alyssa Ross <hi@alyssa.is>
> > Cc: stable@vger.kernel.org
> > Acked-by: Nick Desaulniers <ndesaulniers@google.com>
>
> This is definitely more future proof.
>
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> Tested-by: Nathan Chancellor <nathan@kernel.org>



I prefer v3 since it is cleaner, but unfortunately
it does not work for Clang+GAS.


With v3 applied, I still see the debug info.



$ make LLVM=1 LLVM_IAS=0  arch/x86/purgatory/setup-x86_64.o
  UPD     include/config/kernel.release
  UPD     include/generated/utsrelease.h
  CALL    scripts/checksyscalls.sh
  DESCEND objtool
  INSTALL libsubcmd_headers
  AS      arch/x86/purgatory/setup-x86_64.o
$ readelf -S arch/x86/purgatory/setup-x86_64.o
There are 18 section headers, starting at offset 0x14d8:

Section Headers:
  [Nr] Name              Type             Address           Offset
       Size              EntSize          Flags  Link  Info  Align
  [ 0]                   NULL             0000000000000000  00000000
       0000000000000000  0000000000000000           0     0     0
  [ 1] .text             PROGBITS         0000000000000000  00000040
       0000000000000027  0000000000000000  AX       0     0     16
  [ 2] .rela.text        RELA             0000000000000000  000012f8
       0000000000000060  0000000000000018   I      15     1     8
  [ 3] .data             PROGBITS         0000000000000000  00000067
       0000000000000000  0000000000000000  WA       0     0     1
  [ 4] .bss              NOBITS           0000000000000000  00001000
       0000000000001000  0000000000000000  WA       0     0     4096
  [ 5] .rodata           PROGBITS         0000000000000000  00001000
       0000000000000020  0000000000000000   A       0     0     16
  [ 6] .rela.rodata      RELA             0000000000000000  00001358
       0000000000000018  0000000000000018   I      15     5     8
  [ 7] .debug_line       PROGBITS         0000000000000000  00001020
       000000000000005f  0000000000000000           0     0     1
  [ 8] .rela.debug_line  RELA             0000000000000000  00001370
       0000000000000018  0000000000000018   I      15     7     8
  [ 9] .debug_info       PROGBITS         0000000000000000  0000107f
       0000000000000027  0000000000000000           0     0     1
  [10] .rela.debug_info  RELA             0000000000000000  00001388
       0000000000000090  0000000000000018   I      15     9     8
  [11] .debug_abbrev     PROGBITS         0000000000000000  000010a6
       0000000000000014  0000000000000000           0     0     1
  [12] .debug_aranges    PROGBITS         0000000000000000  000010c0
       0000000000000030  0000000000000000           0     0     16
  [13] .rela.debug_[...] RELA             0000000000000000  00001418
       0000000000000030  0000000000000018   I      15    12     8
  [14] .debug_str        PROGBITS         0000000000000000  000010f0
       0000000000000054  0000000000000001  MS       0     0     1
  [15] .symtab           SYMTAB           0000000000000000  00001148
       0000000000000168  0000000000000018          16    12     8
  [16] .strtab           STRTAB           0000000000000000  000012b0
       0000000000000041  0000000000000000           0     0     1
  [17] .shstrtab         STRTAB           0000000000000000  00001448
       000000000000008d  0000000000000000           0     0     1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
  L (link order), O (extra OS processing required), G (group), T (TLS),
  C (compressed), x (unknown), o (OS specific), E (exclude),
  D (mbind), l (large), p (processor specific)






With -g0 given, GCC stops passing -g -gdwarf-4 down to GAS.


Clang does not do anything about -g0 for the external assembler.




I was thinking of dropping LLVM_IAS=0 support.
When we decide to give up -fno-integrated-as,
we can clean up the code in various places.


Anyway, v3 does not work in the current situation.


V2 works for all usecases.
Alyssa Ross March 31, 2023, 8:27 p.m. UTC | #3 
On Sat, Apr 01, 2023 at 12:42:13AM +0900, Masahiro Yamada wrote:
> On Fri, Mar 31, 2023 at 7:29 AM Nathan Chancellor <nathan@kernel.org> wrote:
> >
> > On Thu, Mar 30, 2023 at 06:22:24PM +0000, Alyssa Ross wrote:
> > > Since 32ef9e5054ec, -Wa,-gdwarf-2 is no longer used in KBUILD_AFLAGS.
> > > Instead, it includes -g, the appropriate -gdwarf-* flag, and also the
> > > -Wa versions of both of those if building with Clang and GNU as.  As a
> > > result, debug info was being generated for the purgatory objects, even
> > > though the intention was that it not be.
> > >
> > > Fixes: 32ef9e5054ec ("Makefile.debug: re-enable debug info for .S files")
> > > Signed-off-by: Alyssa Ross <hi@alyssa.is>
> > > Cc: stable@vger.kernel.org
> > > Acked-by: Nick Desaulniers <ndesaulniers@google.com>
> >
> > This is definitely more future proof.
> >
> > Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> > Tested-by: Nathan Chancellor <nathan@kernel.org>
>
>
>
> I prefer v3 since it is cleaner, but unfortunately
> it does not work for Clang+GAS.
>
>
> With v3 applied, I still see the debug info.
>
>
>
> $ make LLVM=1 LLVM_IAS=0  arch/x86/purgatory/setup-x86_64.o
>   UPD     include/config/kernel.release
>   UPD     include/generated/utsrelease.h
>   CALL    scripts/checksyscalls.sh
>   DESCEND objtool
>   INSTALL libsubcmd_headers
>   AS      arch/x86/purgatory/setup-x86_64.o
> $ readelf -S arch/x86/purgatory/setup-x86_64.o
> There are 18 section headers, starting at offset 0x14d8:
>
> Section Headers:
>   [Nr] Name              Type             Address           Offset
>        Size              EntSize          Flags  Link  Info  Align
>   [ 0]                   NULL             0000000000000000  00000000
>        0000000000000000  0000000000000000           0     0     0
>   [ 1] .text             PROGBITS         0000000000000000  00000040
>        0000000000000027  0000000000000000  AX       0     0     16
>   [ 2] .rela.text        RELA             0000000000000000  000012f8
>        0000000000000060  0000000000000018   I      15     1     8
>   [ 3] .data             PROGBITS         0000000000000000  00000067
>        0000000000000000  0000000000000000  WA       0     0     1
>   [ 4] .bss              NOBITS           0000000000000000  00001000
>        0000000000001000  0000000000000000  WA       0     0     4096
>   [ 5] .rodata           PROGBITS         0000000000000000  00001000
>        0000000000000020  0000000000000000   A       0     0     16
>   [ 6] .rela.rodata      RELA             0000000000000000  00001358
>        0000000000000018  0000000000000018   I      15     5     8
>   [ 7] .debug_line       PROGBITS         0000000000000000  00001020
>        000000000000005f  0000000000000000           0     0     1
>   [ 8] .rela.debug_line  RELA             0000000000000000  00001370
>        0000000000000018  0000000000000018   I      15     7     8
>   [ 9] .debug_info       PROGBITS         0000000000000000  0000107f
>        0000000000000027  0000000000000000           0     0     1
>   [10] .rela.debug_info  RELA             0000000000000000  00001388
>        0000000000000090  0000000000000018   I      15     9     8
>   [11] .debug_abbrev     PROGBITS         0000000000000000  000010a6
>        0000000000000014  0000000000000000           0     0     1
>   [12] .debug_aranges    PROGBITS         0000000000000000  000010c0
>        0000000000000030  0000000000000000           0     0     16
>   [13] .rela.debug_[...] RELA             0000000000000000  00001418
>        0000000000000030  0000000000000018   I      15    12     8
>   [14] .debug_str        PROGBITS         0000000000000000  000010f0
>        0000000000000054  0000000000000001  MS       0     0     1
>   [15] .symtab           SYMTAB           0000000000000000  00001148
>        0000000000000168  0000000000000018          16    12     8
>   [16] .strtab           STRTAB           0000000000000000  000012b0
>        0000000000000041  0000000000000000           0     0     1
>   [17] .shstrtab         STRTAB           0000000000000000  00001448
>        000000000000008d  0000000000000000           0     0     1
> Key to Flags:
>   W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
>   L (link order), O (extra OS processing required), G (group), T (TLS),
>   C (compressed), x (unknown), o (OS specific), E (exclude),
>   D (mbind), l (large), p (processor specific)
>
>
>
>
>
>
> With -g0 given, GCC stops passing -g -gdwarf-4 down to GAS.
>
>
> Clang does not do anything about -g0 for the external assembler.

You're right.  Thank you for your thoughtful testing — I forgot to check
LLVM for v3.  I thought maybe adding -Wa,-g0 would be enough, but it
turns out that GAS doesn't support that.  If -g has been specified,
there doesn't seem to be any way to disable debug info again later in
the command line.

So we probably can't do better than v2 while LLVM_IAS=0 is supported.
The only other option I see is (untested):

asflags-y			+= -g0
asflags-remove-y		+= -Wa,-g -Wa,-gdwarf-4 -Wa,-gdwarf-5

But I don't like that option, because it means there are two completely
different ways of doing it depending on the compiler setup, and it makes
it even less likely anybody would remember to update asflags-remove-y
when DWARF 6 comes around or whatever.

> I was thinking of dropping LLVM_IAS=0 support.
> When we decide to give up -fno-integrated-as,
> we can clean up the code in various places.
>
>
> Anyway, v3 does not work in the current situation.
>
>
> V2 works for all usecases.
>
>
>
>
> --
> Best Regards
> Masahiro Yamada
Masahiro Yamada April 1, 2023, 1:08 a.m. UTC | #4 
On Sat, Apr 1, 2023 at 5:27 AM Alyssa Ross <hi@alyssa.is> wrote:
>
> On Sat, Apr 01, 2023 at 12:42:13AM +0900, Masahiro Yamada wrote:
> > On Fri, Mar 31, 2023 at 7:29 AM Nathan Chancellor <nathan@kernel.org> wrote:
> > >
> > > On Thu, Mar 30, 2023 at 06:22:24PM +0000, Alyssa Ross wrote:
> > > > Since 32ef9e5054ec, -Wa,-gdwarf-2 is no longer used in KBUILD_AFLAGS.
> > > > Instead, it includes -g, the appropriate -gdwarf-* flag, and also the
> > > > -Wa versions of both of those if building with Clang and GNU as.  As a
> > > > result, debug info was being generated for the purgatory objects, even
> > > > though the intention was that it not be.
> > > >
> > > > Fixes: 32ef9e5054ec ("Makefile.debug: re-enable debug info for .S files")
> > > > Signed-off-by: Alyssa Ross <hi@alyssa.is>
> > > > Cc: stable@vger.kernel.org
> > > > Acked-by: Nick Desaulniers <ndesaulniers@google.com>
> > >
> > > This is definitely more future proof.
> > >
> > > Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> > > Tested-by: Nathan Chancellor <nathan@kernel.org>
> >
> >
> >
> > I prefer v3 since it is cleaner, but unfortunately
> > it does not work for Clang+GAS.
> >
> >
> > With v3 applied, I still see the debug info.
> >
> >
> >
> > $ make LLVM=1 LLVM_IAS=0  arch/x86/purgatory/setup-x86_64.o
> >   UPD     include/config/kernel.release
> >   UPD     include/generated/utsrelease.h
> >   CALL    scripts/checksyscalls.sh
> >   DESCEND objtool
> >   INSTALL libsubcmd_headers
> >   AS      arch/x86/purgatory/setup-x86_64.o
> > $ readelf -S arch/x86/purgatory/setup-x86_64.o
> > There are 18 section headers, starting at offset 0x14d8:
> >
> > Section Headers:
> >   [Nr] Name              Type             Address           Offset
> >        Size              EntSize          Flags  Link  Info  Align
> >   [ 0]                   NULL             0000000000000000  00000000
> >        0000000000000000  0000000000000000           0     0     0
> >   [ 1] .text             PROGBITS         0000000000000000  00000040
> >        0000000000000027  0000000000000000  AX       0     0     16
> >   [ 2] .rela.text        RELA             0000000000000000  000012f8
> >        0000000000000060  0000000000000018   I      15     1     8
> >   [ 3] .data             PROGBITS         0000000000000000  00000067
> >        0000000000000000  0000000000000000  WA       0     0     1
> >   [ 4] .bss              NOBITS           0000000000000000  00001000
> >        0000000000001000  0000000000000000  WA       0     0     4096
> >   [ 5] .rodata           PROGBITS         0000000000000000  00001000
> >        0000000000000020  0000000000000000   A       0     0     16
> >   [ 6] .rela.rodata      RELA             0000000000000000  00001358
> >        0000000000000018  0000000000000018   I      15     5     8
> >   [ 7] .debug_line       PROGBITS         0000000000000000  00001020
> >        000000000000005f  0000000000000000           0     0     1
> >   [ 8] .rela.debug_line  RELA             0000000000000000  00001370
> >        0000000000000018  0000000000000018   I      15     7     8
> >   [ 9] .debug_info       PROGBITS         0000000000000000  0000107f
> >        0000000000000027  0000000000000000           0     0     1
> >   [10] .rela.debug_info  RELA             0000000000000000  00001388
> >        0000000000000090  0000000000000018   I      15     9     8
> >   [11] .debug_abbrev     PROGBITS         0000000000000000  000010a6
> >        0000000000000014  0000000000000000           0     0     1
> >   [12] .debug_aranges    PROGBITS         0000000000000000  000010c0
> >        0000000000000030  0000000000000000           0     0     16
> >   [13] .rela.debug_[...] RELA             0000000000000000  00001418
> >        0000000000000030  0000000000000018   I      15    12     8
> >   [14] .debug_str        PROGBITS         0000000000000000  000010f0
> >        0000000000000054  0000000000000001  MS       0     0     1
> >   [15] .symtab           SYMTAB           0000000000000000  00001148
> >        0000000000000168  0000000000000018          16    12     8
> >   [16] .strtab           STRTAB           0000000000000000  000012b0
> >        0000000000000041  0000000000000000           0     0     1
> >   [17] .shstrtab         STRTAB           0000000000000000  00001448
> >        000000000000008d  0000000000000000           0     0     1
> > Key to Flags:
> >   W (write), A (alloc), X (execute), M (merge), S (strings), I (info),
> >   L (link order), O (extra OS processing required), G (group), T (TLS),
> >   C (compressed), x (unknown), o (OS specific), E (exclude),
> >   D (mbind), l (large), p (processor specific)
> >
> >
> >
> >
> >
> >
> > With -g0 given, GCC stops passing -g -gdwarf-4 down to GAS.
> >
> >
> > Clang does not do anything about -g0 for the external assembler.
>
> You're right.  Thank you for your thoughtful testing — I forgot to check
> LLVM for v3.  I thought maybe adding -Wa,-g0 would be enough, but it
> turns out that GAS doesn't support that.  If -g has been specified,
> there doesn't seem to be any way to disable debug info again later in
> the command line.
>
> So we probably can't do better than v2 while LLVM_IAS=0 is supported.
> The only other option I see is (untested):
>
> asflags-y                       += -g0
> asflags-remove-y                += -Wa,-g -Wa,-gdwarf-4 -Wa,-gdwarf-5
>
> But I don't like that option, because it means there are two completely
> different ways of doing it depending on the compiler setup, and it makes
> it even less likely anybody would remember to update asflags-remove-y
> when DWARF 6 comes around or whatever.


Agree.
This is uglier than v2.



If nobody comes up with a better idea,
I will pick up v2.
diff mbox series

Patch

diff --git a/arch/riscv/purgatory/Makefile b/arch/riscv/purgatory/Makefile
index d16bf715a586..9c1e71853ee7 100644
--- a/arch/riscv/purgatory/Makefile
+++ b/arch/riscv/purgatory/Makefile
@@ -84,12 +84,7 @@  CFLAGS_string.o			+= $(PURGATORY_CFLAGS)
 CFLAGS_REMOVE_ctype.o		+= $(PURGATORY_CFLAGS_REMOVE)
 CFLAGS_ctype.o			+= $(PURGATORY_CFLAGS)
 
-AFLAGS_REMOVE_entry.o		+= -Wa,-gdwarf-2
-AFLAGS_REMOVE_memcpy.o		+= -Wa,-gdwarf-2
-AFLAGS_REMOVE_memset.o		+= -Wa,-gdwarf-2
-AFLAGS_REMOVE_strcmp.o		+= -Wa,-gdwarf-2
-AFLAGS_REMOVE_strlen.o		+= -Wa,-gdwarf-2
-AFLAGS_REMOVE_strncmp.o		+= -Wa,-gdwarf-2
+asflags-y			+= -g0
 
 $(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE
 		$(call if_changed,ld)
diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
index 17f09dc26381..8e6c81b1c8f7 100644
--- a/arch/x86/purgatory/Makefile
+++ b/arch/x86/purgatory/Makefile
@@ -69,8 +69,7 @@  CFLAGS_sha256.o			+= $(PURGATORY_CFLAGS)
 CFLAGS_REMOVE_string.o		+= $(PURGATORY_CFLAGS_REMOVE)
 CFLAGS_string.o			+= $(PURGATORY_CFLAGS)
 
-AFLAGS_REMOVE_setup-x86_$(BITS).o	+= -Wa,-gdwarf-2
-AFLAGS_REMOVE_entry64.o			+= -Wa,-gdwarf-2
+asflags-y			+= -g0
 
 $(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE
 		$(call if_changed,ld)