[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CACRpkdbXLKpzOwc7pkdPEN8DAQk64P=5qTo3HfW5tvZnYL70UA@mail.gmail.com>
Date: Fri, 21 Feb 2020 16:31:05 +0100
From: Linus Walleij <linus.walleij@...aro.org>
To: Josh Poimboeuf <jpoimboe@...hat.com>
Cc: Paul Cercueil <paul@...pouillou.net>,
"open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Peter Zijlstra <peterz@...radead.org>,
Randy Dunlap <rdunlap@...radead.org>
Subject: Re: [PATCH v2] pinctrl: ingenic: Improve unreachable code generation
On Thu, Feb 20, 2020 at 4:35 PM Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> In the second loop of ingenic_pinconf_set(), it annotates the switch
> default case as unreachable(). The annotation is technically correct,
> because that same case would have resulted in an early function return
> in the previous loop.
>
> However, the compiled code is suboptimal. GCC seems to work extra hard
> to ensure that the unreachable code path triggers undefined behavior.
> The function would fall through to start executing whatever function
> happens to be next in the compilation unit.
>
> This is problematic because:
>
> a) it adds unnecessary 'ensure undefined behavior' logic, and
> corresponding i-cache footprint; and
>
> b) it's less robust -- if a bug were to be introduced, falling through
> to the next function would be catastrophic.
>
> Yet another issue is that, while objtool normally understands
> unreachable() annotations, there's one special case where it doesn't:
> when the annotation occurs immediately after a 'ret' instruction. That
> happens to be the case here because unreachable() is immediately before
> the return.
>
> Remove the unreachable() annotation and replace it with a comment. This
> simplifies the code generation and changes the unreachable error path to
> just silently return instead of corrupting execution.
>
> This fixes the following objtool warning:
>
> drivers/pinctrl/pinctrl-ingenic.o: warning: objtool: ingenic_pinconf_set() falls through to next function ingenic_pinconf_group_set()
>
> Reported-by: Randy Dunlap <rdunlap@...radead.org>
> Signed-off-by: Josh Poimboeuf <jpoimboe@...hat.com>
Patch applied.
Yours,
Linus Walleij
Powered by blists - more mailing lists