lkml.org 
[lkml]   [2020]   [Feb]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] pinctrl: ingenic: Make unreachable path more robust


Le ven., févr. 14, 2020 at 14:37, Josh Poimboeuf <jpoimboe@redhat.com>
a écrit :
> On Fri, Feb 14, 2020 at 04:02:18PM -0300, Paul Cercueil wrote:
>> Hi Josh,
>>
>>
>> Le ven., févr. 14, 2020 at 10:37, Josh Poimboeuf
>> <jpoimboe@redhat.com> a
>> écrit :
>> > 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 return in
>> the
>> > previous loop.
>> >
>> > However, if a bug were to get introduced later, for example if an
>> > additional case were added to the first loop without adjusting the
>> > second loop, it would result in nasty undefined behavior: most
>> likely
>> > the function's generated code would fall through to the next
>> function.
>> >
>> > 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.
>> >
>> > So change the unreachable() to BUG() so that the unreachable
>> code, if
>> > ever executed, would panic instead of introducing undefined
>> behavior.
>> > This also makes objtool happy.
>>
>> I don't like the idea that you change this driver's code just to
>> work around
>> a bug in objtool, and I don't like the idea of working around a
>> future bug
>> that shouldn't be introduced in the first place.
>
> It's not an objtool bug. It's a byproduct of the fact that GCC's
> undefined behavior is inscrutable, and there's no way to determine
> that
> it actually *wants* to jump to a random function.
>
> And anyway, regardless of objtool, the patch is meant to make the code
> more robust.
>
> Do you not agree that BUG (defined behavior) is more robust than
> unreachable (undefined behavior)?

It's a dead code path. That would be an undefined behaviour, if it was
taken, but it's not.

-Paul


\
 
 \ /
  Last update: 2020-02-15 03:39    [W:0.017 / U:0.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site