Re: [linux-next:master 3357/8413] drivers/scsi/FlashPoint.c:1712:12: warning: stack frame size (1056) exceeds limit (1024) in 'FlashPoint_HandleInterrupt'

[Nick Desaulniers <ndesaulniers@google.com>]

On Tue, Jun 13, 2023 at 5:22 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Sat, Jun 10, 2023 at 12:58:23AM +0800, kernel test robot wrote:
> > First bad commit (maybe != root cause):
> >
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> > head:   53ab6975c12d1ad86c599a8927e8c698b144d669
> > commit: df8fc4e934c12b906d08050d7779f292b9c5c6b5 [3357/8413] kbuild: Enable -fstrict-flex-arrays=3
> > config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20230610/202306100035.VTusNhm4-lkp@intel.com/config)

^ I just checked this config. CONFIG_KASAN=y is not set, so this is
not a case of
https://github.com/ClangBuiltLinux/linux/issues/39

UBSAN is though (maybe a red herring) as well as GCOV and TSAN/KCSAN.

Disabling GCOV did not change the stack usage from allmodconfig.

Disable KCSAN dropped it down from 2272 to 2160.

Disabling UBSAN produced no warnings, and changed the inlining
behavior such that FlashPoint_HandleInterrupt only uses 656B rather
than 2272 via allmodconfig.

Seems specific to:
```
CONFIG_UBSAN=y
CONFIG_CC_HAS_UBSAN_ARRAY_BOUNDS=y
CONFIG_UBSAN_BOUNDS=y
CONFIG_UBSAN_ARRAY_BOUNDS=y
# CONFIG_UBSAN_SHIFT is not set
# CONFIG_UBSAN_UNREACHABLE is not set
# CONFIG_UBSAN_BOOL is not set
# CONFIG_UBSAN_ENUM is not set
CONFIG_UBSAN_SANITIZE_ALL=y
```
but adding these on top of powernv_defconfig I couldn't reproduce.  So
perhaps we can do a config bisection between allmodconfig and
powernv_defconfig to see what combination of configs in allmodconfig
is causing this to blow up.

> > compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
> > reproduce (this is a W=1 build):
> >         mkdir -p ~/bin
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # install powerpc cross compiling tool for clang build
> >         # apt-get install binutils-powerpc-linux-gnu
> >         # https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=df8fc4e934c12b906d08050d7779f292b9c5c6b5
> >         git remote add linux-next https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> >         git fetch --no-tags linux-next master
> >         git checkout df8fc4e934c12b906d08050d7779f292b9c5c6b5
> >         # save the config file
> >         mkdir build_dir && cp config build_dir/.config
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=powerpc olddefconfig
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash drivers/
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202306100035.VTusNhm4-lkp@intel.com/
> >
> > All warnings (new ones prefixed by >>):
> >
> >    In file included from drivers/scsi/BusLogic.c:51:
> > >> drivers/scsi/FlashPoint.c:1712:12: warning: stack frame size (1056) exceeds limit (1024) in 'FlashPoint_HandleInterrupt' [-Wframe-larger-than]
> >     1712 | static int FlashPoint_HandleInterrupt(void *pcard)
> >          |            ^
> >    1 warning generated.
>
> I wasn't able to generate the warning with my Clang, but it sure is
> close! Building with KCFLAGS=-Rpass-analysis=stack-frame-layout I see:
>
>
> In file included from ../drivers/scsi/BusLogic.c:51:
> ../drivers/scsi/FlashPoint.c:1713:1: remark:
> Function: FlashPoint_HandleInterrupt
> Offset: [SP-4], Type: Protector, Align: 4, Size: 4
> Offset: [SP-8], Type: Spill, Align: 8, Size: 4
> Offset: [SP-12], Type: Spill, Align: 4, Size: 4
> Offset: [SP-16], Type: Spill, Align: 16, Size: 4

Perhaps alignment 16B for a 4B variable leads to excessive padding?
Perhaps this config changes the alignment of a type?  No, looks like
most spills are the 4B w/ 4B alignment and the 16B alignment spills
are insignificant.

> Offset: [SP-20], Type: Spill, Align: 4, Size: 4
> Offset: [SP-24], Type: Spill, Align: 8, Size: 4
> Offset: [SP-28], Type: Spill, Align: 4, Size: 4
> Offset: [SP-32], Type: Spill, Align: 16, Size: 4
> Offset: [SP-36], Type: Spill, Align: 4, Size: 4
> Offset: [SP-40], Type: Spill, Align: 8, Size: 4
> Offset: [SP-44], Type: Spill, Align: 4, Size: 4
> Offset: [SP-48], Type: Spill, Align: 16, Size: 4
> Offset: [SP-52], Type: Spill, Align: 4, Size: 4
> Offset: [SP-56], Type: Spill, Align: 8, Size: 4
> Offset: [SP-60], Type: Spill, Align: 4, Size: 4
> Offset: [SP-64], Type: Spill, Align: 16, Size: 4
> Offset: [SP-68], Type: Spill, Align: 4, Size: 4
> Offset: [SP-72], Type: Spill, Align: 8, Size: 4
> Offset: [SP-76], Type: Variable, Align: 4, Size: 4
> Offset: [SP-80], Type: Spill, Align: 4, Size: 4
> Offset: [SP-84], Type: Spill, Align: 4, Size: 4
> ...[4 byte spills]...
> Offset: [SP-1012], Type: Variable, Align: 4, Size: 4
> Offset: [SP-1016], Type: Variable, Align: 4, Size: 4
> [-Rpass-analysis=stack-frame-layout]
>
> So something is very weird in FlashPoint_HandleInterrupt -- it has a
> single while loop with an internal if/else if/.../else chain. So I don't
> see why it would need such extensive spills...
>
> This seems like a missed optimization in Clang, maybe?
>
> --
> Kees Cook
>


-- 
Thanks,
~Nick Desaulniers