linux-next.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miroslav Benes <mbenes@suse.cz>
To: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Randy Dunlap <rdunlap@infradead.org>,
	Stephen Rothwell <sfr@canb.auug.org.au>,
	Linux Next Mailing List <linux-next@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	mjambor@suse.cz, mliska@suse.cz, pmladek@suse.cz,
	live-patching@vger.kernel.org
Subject: Re: linux-next: Tree for May 21 (objtool warnings)
Date: Tue, 26 May 2020 18:39:47 +0200 (CEST)	[thread overview]
Message-ID: <alpine.LSU.2.21.2005261809480.22611@pobox.suse.cz> (raw)
In-Reply-To: <20200526140113.ppjywpx7uir3vrlj@treble>

On Tue, 26 May 2020, Josh Poimboeuf wrote:

> On Mon, May 25, 2020 at 01:07:27PM +0200, Miroslav Benes wrote:
> > > I'll try to find out which optimization does this, because it is a 
> > > slightly different scenario than hiding __noreturn from the callees. 
> > > Probably -fno-ipa-pure-const again.
> > 
> > And it is indeed -fno-ipa-pure-const again.
> 
> It still seems odd to me that GCC's dead end detection seems to break
> with -fno-ipa-pure-const.  Do you know if these issues can be fixed on
> the GCC side?

It is odd. I asked Martin and Martin about that yesterday (CCed). It could 
be possible to enable just noreturn propagation for -flive-patching if I 
understood correctly. The attribute would need to be preserved in a 
patched function then, but that should be manageable.

Marking functions as __noreturn is one thing (I think it is useful on its 
own as mentioned in the older thread about -flive-patching), but 
__always_inline solution in this case is really arbitrary. I don't like 
this neverending "battle" with compilers much, so it would be nice to have 
some kind of generic solution (and I currently have no idea about that). 
Of course, declaring -flive-patching a failed experiment is an option if 
there is not a better way to deal with a dead end detection either in GCC 
or in objtool. I would not like it, but you're right that if there are 
more and more problems like this appearing, we'll have to deal with 
maintainers all over the place and ask them to maintain odd fixes just for 
the sake of -flive-patching. I don't know what the current numbers are 
though. We'd have to approach the problem of GCC optimizations from a 
different angle. Petr CCed (we talked about it yesterday as well).

But first, let's try to find a way with -flive-patching.

Reduced test case follows (courtesty of Martin Liska):

$ cat open.i
int global;

void
break_deleg_wait()
{
  asm(".byte 15, 0x0b");
  __builtin_unreachable();
}

void chmod_common_delegated_inode(int arg)
{
retry_deleg:
  if (arg)
    break_deleg_wait(global);
  else
    return;
  goto retry_deleg;
}

$ gcc open.i -c -Os -fno-omit-frame-pointer -fno-ipa-pure-const && ./tools/objtool/objtool check open.o
open.o: warning: objtool: chmod_common_delegated_inode()+0x18: unreachable instruction

$ gcc open.i -c -Os -fno-omit-frame-pointer && ./tools/objtool/objtool check open.o
[OK]
---

So it is a similar problem. There is no noreturn attribute anywhere 
(nothing to propagate from a caller to a callee). Here, the information 
about an unreachable code is not propagated to the caller 
(chmod_common_delegated_inode()).

Martins, would it be possible to extend -flive-patching to deal with this?

Miroslav

  reply	other threads:[~2020-05-26 16:39 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-21 14:12 linux-next: Tree for May 21 Stephen Rothwell
2020-05-21 15:09 ` Daniel Thompson
2020-05-22  2:34 ` linux-next: Tree for May 21 (objtool warnings) Randy Dunlap
2020-05-22 16:34   ` Josh Poimboeuf
2020-05-25 10:10   ` Miroslav Benes
2020-05-25 11:07     ` Miroslav Benes
2020-05-26 14:01       ` Josh Poimboeuf
2020-05-26 16:39         ` Miroslav Benes [this message]
2020-05-27  8:57           ` Martin Jambor
2020-05-27 15:09             ` Josh Poimboeuf
2020-05-25 18:04     ` Randy Dunlap

Reply instructions:

You may reply publicly 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=alpine.LSU.2.21.2005261809480.22611@pobox.suse.cz \
    --to=mbenes@suse.cz \
    --cc=jpoimboe@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-next@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mjambor@suse.cz \
    --cc=mliska@suse.cz \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.cz \
    --cc=rdunlap@infradead.org \
    --cc=sfr@canb.auug.org.au \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).