Date:   Mon, 20 Mar 2023 13:42:43 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>
Cc:     Randy Dunlap <rdunlap@...radead.org>, linux-kernel@...r.kernel.org,
        Yoshinori Sato <ysato@...rs.sourceforge.jp>,
        Rich Felker <dalias@...c.org>, stable@...r.kernel.org
Subject: Re: [PATCH 7/7 v4] sh: mcount.S: fix build error when PRINTK is not enabled

Hi Adrian,

On Mon, Mar 20, 2023 at 10:13 AM John Paul Adrian Glaubitz
<glaubitz@...sik.fu-berlin.de> wrote:
> On Mon, 2023-03-20 at 09:16 +0100, Geert Uytterhoeven wrote:
> > On Sun, Mar 19, 2023 at 9:49 PM John Paul Adrian Glaubitz
> > <glaubitz@...sik.fu-berlin.de> wrote:
> > > On Sun, 2023-03-05 at 20:00 -0800, Randy Dunlap wrote:
> > > > Fix a build error in mcount.S when CONFIG_PRINTK is not enabled.
> > > > Fixes this build error:
> > > >
> > > > sh2-linux-ld: arch/sh/lib/mcount.o: in function `stack_panic':
> > > > (.text+0xec): undefined reference to `dump_stack'
> > > >
> > > > Fixes: e460ab27b6c3 ("sh: Fix up stack overflow check with ftrace disabled.")
> > > > Signed-off-by: Randy Dunlap <rdunlap@...radead.org>
> > > > Cc: John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>
> > > > Cc: Yoshinori Sato <ysato@...rs.sourceforge.jp>
> > > > Cc: Rich Felker <dalias@...c.org>
> > > > Suggested-by: Geert Uytterhoeven <geert@...ux-m68k.org>
> > > > Cc: stable@...r.kernel.org
> > > > ---
> > > > v2: add PRINTK to STACK_DEBUG dependency (thanks, Geert)
> > > > v3: skipped
> > > > v4: refresh & resend
> > > >
> > > >  arch/sh/Kconfig.debug |    2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff -- a/arch/sh/Kconfig.debug b/arch/sh/Kconfig.debug
> > > > --- a/arch/sh/Kconfig.debug
> > > > +++ b/arch/sh/Kconfig.debug
> > > > @@ -15,7 +15,7 @@ config SH_STANDARD_BIOS
> > > >
> > > >  config STACK_DEBUG
> > > >       bool "Check for stack overflows"
> > > > -     depends on DEBUG_KERNEL
> > > > +     depends on DEBUG_KERNEL && PRINTK
> > > >       help
> > > >         This option will cause messages to be printed if free stack space
> > > >         drops below a certain limit. Saying Y here will add overhead to
> > >
> > > I can't really test this change as the moment I am enabling CONFIG_STACK_DEBUG,
> > > the build fails with:
> > >
> > >   CC      scripts/mod/devicetable-offsets.s
> > > sh4-linux-gcc: error: -pg and -fomit-frame-pointer are incompatible
> > > make[1]: *** [scripts/Makefile.build:252: scripts/mod/empty.o] Error 1
> > > make[1]: *** Waiting for unfinished jobs....
> > > sh4-linux-gcc: error: -pg and -fomit-frame-pointer are incompatible
> > > make[1]: *** [scripts/Makefile.build:114: scripts/mod/devicetable-offsets.s] Error 1
> > > make: *** [Makefile:1286: prepare0] Error 2
> > >
> > > So, I assume we need to strip -fomit-frame-pointer from KBUILD_CFLAGS, correct?
> > >
> > > I tried this change, but that doesn't fix it for me:
> > >
> > > diff --git a/arch/sh/Makefile b/arch/sh/Makefile
> > > index 5c8776482530..83f535b73835 100644
> > > --- a/arch/sh/Makefile
> > > +++ b/arch/sh/Makefile
> > > @@ -173,6 +173,7 @@ KBUILD_AFLAGS               += $(cflags-y)
> > >
> > >  ifeq ($(CONFIG_MCOUNT),y)
> > >    KBUILD_CFLAGS += -pg
> > > +  KBUILD_CFLAGS := $(subst -fomit-frame-pointer,,$(KBUILD_CFLAGS))
> > >  endif
> > >
> > >  ifeq ($(CONFIG_DWARF_UNWINDER),y)
> > >
> > > Any ideas?
> >
> > Please try with "+=" instead of ":=".
>
> That doesn't work either. I tried the following, but that didn't strip -fomit-frame-pointer:

Oops, obviously all of that happened before my morning coffee ;-)

Makefile has:

    ifdef CONFIG_FRAME_POINTER
    KBUILD_CFLAGS   += -fno-omit-frame-pointer -fno-optimize-sibling-calls
    KBUILD_RUSTFLAGS += -Cforce-frame-pointers=y
    else
    # Some targets (ARM with Thumb2, for example), can't be built with frame
    # pointers.  For those, we don't have FUNCTION_TRACER automatically
    # select FRAME_POINTER.  However, FUNCTION_TRACER adds -pg, and this is
    # incompatible with -fomit-frame-pointer with current GCC, so we don't use
    # -fomit-frame-pointer with FUNCTION_TRACER.
    # In the Rust target specification, "frame-pointer" is set explicitly
    # to "may-omit".
    ifndef CONFIG_FUNCTION_TRACER
    KBUILD_CFLAGS   += -fomit-frame-pointer
    endif
    endif

Your config probably has CONFIG_FRAME_POINTER set?

    arch/sh/Kconfig.debug=config DWARF_UNWINDER
    arch/sh/Kconfig.debug-  bool "Enable the DWARF unwinder for stacktraces"
    arch/sh/Kconfig.debug-  depends on DEBUG_KERNEL
    arch/sh/Kconfig.debug:  select FRAME_POINTER

You should make sure that cannot happen when CONFIG_FUNCTION_TRACER
is enabled. I.e. make DWARF_UNWINDER depend on !FUNCTION_TRACER?

Other architectures do something similar:

    arch/sparc/Kconfig.debug:config FRAME_POINTER
    arch/sparc/Kconfig.debug-       bool
    arch/sparc/Kconfig.debug-       depends on MCOUNT

    arch/x86/Kconfig.debug:config FRAME_POINTER
    arch/x86/Kconfig.debug- depends on !UNWINDER_ORC && !UNWINDER_GUESS
    arch/x86/Kconfig.debug- bool

Probably you need to adjust the following, too:

    lib/Kconfig.debug:config FRAME_POINTER
    lib/Kconfig.debug-      bool "Compile the kernel with frame pointers"
    lib/Kconfig.debug-      depends on DEBUG_KERNEL && (M68K || UML ||
SUPERH) || ARCH_WANT_FRAME_POINTERS
    lib/Kconfig.debug-      default y if (DEBUG_INFO && UML) ||
ARCH_WANT_FRAME_POINTERS

i.e. drop SUPERH from the list above, and select ARCH_WANT_FRAME_POINTERS
if !FUNCTION_TRACER.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Powered by blists - more mailing lists