| Message ID | bc20fdbcb826512cf76b7dfd0972740875931b19.1582212881.git.jpoimboe@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] pinctrl: ingenic: Improve unreachable code generation | expand |
On Thu, Feb 20, 2020 at 4:35 PM Josh Poimboeuf <jpoimboe@redhat.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@infradead.org> > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Patch applied. Yours, Linus Walleij
diff --git a/drivers/pinctrl/pinctrl-ingenic.c b/drivers/pinctrl/pinctrl-ingenic.c index 96f04d121ebd..13c7d3351ed5 100644 --- a/drivers/pinctrl/pinctrl-ingenic.c +++ b/drivers/pinctrl/pinctrl-ingenic.c @@ -2158,7 +2158,8 @@ static int ingenic_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin, break; default: - unreachable(); + /* unreachable */ + break; } }
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@infradead.org> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- drivers/pinctrl/pinctrl-ingenic.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)