Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20191021101057.23861-1-daniel.thompson@linaro.org>
In-Reply-To: <20191021101057.23861-1-daniel.thompson@linaro.org>
From:   Doug Anderson <dianders@chromium.org>
Date:   Mon, 21 Oct 2019 10:01:34 -0700
Message-ID: <CAD=FV=X9ESYnfWJLR1gsLwswBsTkbEh1-C7ki3=3AgFzUKkn3g@mail.gmail.com>
Subject: Re: [PATCH v2] kdb: Avoid array subscript warnings on non-SMP builds
To:     Daniel Thompson <daniel.thompson@linaro.org>
Cc:     Jason Wessel <jason.wessel@windriver.com>,
        kgdb-bugreport@lists.sourceforge.net,
        LKML <linux-kernel@vger.kernel.org>,
        Patch Tracking <patches@linaro.org>,
        kbuild test robot <lkp@intel.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi,

On Mon, Oct 21, 2019 at 3:11 AM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> Recent versions of gcc (reported on gcc-7.4) issue array subscript
> warnings for builds where SMP is not enabled.
>
> kernel/debug/debug_core.c: In function 'kdb_dump_stack_on_cpu':
> kernel/debug/debug_core.c:452:17: warning: array subscript is outside array
> +bounds [-Warray-bounds]
>      if (!(kgdb_info[cpu].exception_state & DCPU_IS_SLAVE)) {
>            ~~~~~~~~~^~~~~
>    kernel/debug/debug_core.c:469:33: warning: array subscript is outside array
> +bounds [-Warray-bounds]
>      kgdb_info[cpu].exception_state |= DCPU_WANT_BT;
>    kernel/debug/debug_core.c:470:18: warning: array subscript is outside array
> +bounds [-Warray-bounds]
>      while (kgdb_info[cpu].exception_state & DCPU_WANT_BT)
>
> There is no bug here but there is scope to improve the code
> generation for non-SMP systems (whilst also silencing the warning).
>
> Reported-by: kbuild test robot <lkp@intel.com>
> Fixes: 2277b492582d ("kdb: Fix stack crawling on 'running' CPUs that aren't the master")
> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
>
> Notes:
>     Changes in v2:
>
>      - Moved the IS_ENABLED(CONFIG_SMP) test to the first (slightly easier
>        to read the code, improves code generation a little)
>      - Sent out as a proper patch e-mail ;-)
>
>  kernel/debug/debug_core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
> index 70e86b4b4932..2b7c9b67931d 100644
> --- a/kernel/debug/debug_core.c
> +++ b/kernel/debug/debug_core.c
> @@ -444,7 +444,7 @@ int dbg_remove_all_break(void)
>  #ifdef CONFIG_KGDB_KDB
>  void kdb_dump_stack_on_cpu(int cpu)
>  {
> -       if (cpu == raw_smp_processor_id()) {
> +       if (cpu == raw_smp_processor_id() || !IS_ENABLED(CONFIG_SMP)) {

At first I thought maybe your code would be less efficient than:

  if (!IS_ENABLED(CONFIG_SMP) || cpu == raw_smp_processor_id())

...since the compiler would be still be required to "call"
raw_smp_processor_id() in the non-SMP case.  ...but then I realized
that seems to be a macro in the non-SMP case and just resolves to 0.
...so while the compiler would still be "required" to execute the
first part of the if test, it should be able to realize that it's a
no-op.

Reviewed-by: Douglas Anderson <dianders@chromium.org>

-Doug


>                 dump_stack();
>                 return;
>         }
> --
> 2.21.0
>