Re: [PATCH 2/3] media: vicodec: avoic clang frame size warning

From: Arnd Bergmann <arnd@arndb.de>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: Hans Verkuil <hans.verkuil@cisco.com>,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Mark Brown <broonie@kernel.org>,
	Nathan Chancellor <natechancellor@gmail.com>,
	Dafna Hirschfeld <dafna3@gmail.com>,
	Tom aan de Wiel <tom.aandewiel@gmail.com>,
	Linux Media Mailing List <linux-media@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/3] media: vicodec: avoic clang frame size warning
Date: Tue, 19 Feb 2019 20:14:08 +0100	[thread overview]
Message-ID: <CAK8P3a0C6V0e8Z0uFSwrQ9V0pHqHWKwsHaQ5pVXDEpun5SVh3w@mail.gmail.com> (raw)
In-Reply-To: <CAKwvOdm2fr6Xh9Tezexq4RGinkJy6P0_L6jQOMoZfArbgmaKJQ@mail.gmail.com>

On Tue, Feb 19, 2019 at 8:02 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
> On Tue, Feb 19, 2019 at 9:02 AM Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > Clang-9 makes some different inlining decisions compared to gcc, which
> > leads to a warning about a possible stack overflow problem when building
> > with CONFIG_KASAN, including when setting asan-stack=0, which avoids
> > most other frame overflow warnings:
> >
> > drivers/media/platform/vicodec/codec-fwht.c:673:12: error: stack frame size of 2224 bytes in function 'encode_plane'
> >
> > Manually adding noinline_for_stack annotations in those functions
>
> Thanks for the fix! In general, for -Wstack-frame-larger-than=
> warnings, is it possible that these sets of stack frames are already
> too large if entered?  Sure, inlining was a little aggressive, causing
> more stack space use than maybe otherwise necessary at runtime, but
> isn't it also possible that "no inlining" a stack frame can still be a
> problem should the stack frame be entered?  Doesn't the kernel have a
> way of estimating the stack depth for any given frame?  I guess I was
> always curious if the best fix for these kind of warnings was to
> non-stack allocate (kmalloc) certain locally allocated structs, or
> no-inline the function.  Surely there's cases where no-inlining is
> safe, but I was curious if it's still maybe dangerous to enter the
> problematic child most stack frame?

What I think is happening here is that llvm fails to combine the
stack allocations for the inlined functions in certain conditions,
while gcc can reuse it here. We had similar issues in gcc
a few years ago, and they got fixed there, but I have not looked
at this one in more detail. My guess is that it's related to
the bug I mentioned in patch 3.

      Arnd