mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Petr Tesarik <ptesarik@suse.cz>
To: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: LKML <linux-kernel@vger.kernel.org>
Subject: Re: Definition of BUG on x86
Date: Thu, 19 Feb 2009 12:40:48 +0100	[thread overview]
Message-ID: <1235043648.15053.35.camel@nathan.suse.cz> (raw)
In-Reply-To: <499C4786.5010504@goop.org>

Jeremy Fitzhardinge píše v St 18. 02. 2009 v 09:38 -0800:
> Petr Tesarik wrote:
> > Hi Jeremy,
> >
> > while looking at the disassembly of a recent kernel, I noticed that
> > BUG() without CONFIG_DEBUG_BUGVERBOSE now produces a sequence like this:
> >
> > 0xc02c2f11 <schedule+581>:      ud2a
> > 0xc02c2f13 <schedule+583>:      jmp    0xc02c2f13 <schedule+583>
> >
> > I dug back into the git log and found out that this was last changed by
> > you in commit 91768d6c2bad0d2766a166f13f2f57e197de3458.
> >
> > I might be missing something, but why do we need that extra endless loop
> > there? Isn't ud2a simply a no-return? Especially given that there is no
> > such for(;;) for the CONFIG_DEBUG_BUGVERBOSE case.
> >
> > Now, it's no big deal, but it seems to me that the second instruction is
> > totally useless and merely occupies 2 bytes in the i-cache...
> >
> > Why not get rid of it like this?
> >   
> 
> The trouble is that the compiler doesn't know that the asm never 
> returns, and so it starts to complain about things that might happen 
> after the BUG returns.  In other words, the loop is just there to shut 
> the compiler up.  I agree that its unfortunate that it generates a real 
> instruction, but I couldn't find a better alternative.

Ah, yes, you're right. If you want to make gcc aware of the fact that a
function never returns, it must recognize that there is no path to that
function's EXIT, otherwise it will emit a warning (which is pretty
annoying, indeed). Since we don't want to make a call/jmp to an external
noreturn function, so the no-return code must be directly visible to
gcc. Unfortunately, there is no way of telling gcc that an asm statement
does not return.

Actually, it would be a good thing if you could attach a special
__attribute__ to the asm to mark it that way. But it's not there, so gcc
_must_ generate the code itself. :|

I thought about putting the endless loop into a separate section which
would get discarded by the linker. But that doesn't work either, because
we need a .pushsection _and_ a .popsection afterwards, but there is no
way of emitting that .popsection, because gcc discards everything and
anything after the endless loop.

To recap:

 1. gcc must emit the code for the endless loop
 2. we do not want to have the code in the resulting binary
 3. gcc will not let us insert anything after the loop

So, the only method I could invent was using gas macros. It works but is
quite ugly, because it relies on the actual assembler instruction which
is generated by the compiler. Now, AFAIK gcc has always translated
"for(;;)" into a jump to self, and that with any conceivable compiler
options, but I don't know anything about Intel cc.

Anyway, if you think the ugliness is acceptable, you may take my patch.

Signed-off-by: Petr Tesarik <ptesarik@suse.cz>

diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index d9cf1cd..004093a 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -4,6 +4,14 @@
 #ifdef CONFIG_BUG
 #define HAVE_ARCH_BUG
 
+static inline __noreturn void discarded_jmp(void)
+{
+	asm volatile(".macro jmp target\n"
+		     "\t.purgem jmp\n"
+		     ".endm\n");
+	for (;;) ;
+}
+
 #ifdef CONFIG_DEBUG_BUGVERBOSE
 
 #ifdef CONFIG_X86_32
@@ -22,14 +30,14 @@ do {								\
 		     ".popsection"				\
 		     : : "i" (__FILE__), "i" (__LINE__),	\
 		     "i" (sizeof(struct bug_entry)));		\
-	for (;;) ;						\
+	discarded_jmp();					\
 } while (0)
 
 #else
 #define BUG()							\
 do {								\
 	asm volatile("ud2");					\
-	for (;;) ;						\
+	discarded_jmp();					\
 } while (0)
 #endif
 



       reply	other threads:[~2009-02-19 11:41 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1234975856.15053.16.camel@nathan.suse.cz>
     [not found] ` <499C4786.5010504@goop.org>
2009-02-19 11:40   ` Petr Tesarik [this message]
2009-02-19 12:10     ` Ingo Molnar
2009-02-19 12:19       ` Petr Tesarik
2009-02-19 12:22         ` Ingo Molnar
2009-02-19 12:38           ` Petr Tesarik
2009-02-19 12:47             ` Ingo Molnar
2009-02-19 13:02               ` Petr Tesarik
2009-02-19 14:49                 ` Ingo Molnar
2009-02-19 15:32                   ` Jeremy Fitzhardinge
2009-02-19 15:35                     ` Ingo Molnar
2009-02-19 16:11                       ` Petr Tesarik
2009-02-19 16:16                         ` Ingo Molnar
2009-02-19 16:34                           ` Jeremy Fitzhardinge
2009-02-19 16:41                             ` Ingo Molnar
2009-02-19 20:07                             ` H. Peter Anvin
2009-02-19 20:26                               ` Ingo Molnar
2009-02-19 16:55                           ` Petr Tesarik
2009-02-19 16:32                       ` Jeremy Fitzhardinge
2009-02-19 18:38     ` H. Peter Anvin

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=1235043648.15053.35.camel@nathan.suse.cz \
    --to=ptesarik@suse.cz \
    --cc=jeremy@goop.org \
    --cc=linux-kernel@vger.kernel.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
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

all inboxes | Powered by JetHome®