Re: [PATCH v2] pinctrl: ingenic: Improve unreachable code generation

[Linus Walleij]

On Thu, Feb 20, 2020 at 4:35 PM Josh Poimboeuf <jpoimboe@xxxxxxxxxx> 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@xxxxxxxxxxxxx>
> Signed-off-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>

Patch applied.

Yours,
Linus Walleij