Linux-GPIO Archive on lore.kernel.org
 help / color / Atom feed
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Paul Cercueil <paul@crapouillou.net>
Cc: Linus Walleij <linus.walleij@linaro.org>,
	linux-gpio@vger.kernel.org, linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Randy Dunlap <rdunlap@infradead.org>
Subject: Re: [PATCH] pinctrl: ingenic: Make unreachable path more robust
Date: Fri, 14 Feb 2020 14:37:38 -0600
Message-ID: <20200214203738.af3y4gskukctvvum@treble> (raw)
In-Reply-To: <1581706938.3.5@crapouillou.net>

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)?

-- 
Josh


  reply index

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-14 16:37 Josh Poimboeuf
2020-02-14 19:02 ` Paul Cercueil
2020-02-14 20:37   ` Josh Poimboeuf [this message]
2020-02-15  2:37     ` Paul Cercueil
2020-02-17 15:18       ` Josh Poimboeuf
2020-02-20  1:36         ` Paul Cercueil
2020-02-14 21:52 ` Randy Dunlap

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200214203738.af3y4gskukctvvum@treble \
    --to=jpoimboe@redhat.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paul@crapouillou.net \
    --cc=peterz@infradead.org \
    --cc=rdunlap@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-GPIO Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-gpio/0 linux-gpio/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-gpio linux-gpio/ https://lore.kernel.org/linux-gpio \
		linux-gpio@vger.kernel.org
	public-inbox-index linux-gpio

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-gpio


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git