Re: Definition of BUG on x86

Re: Definition of BUG on x86

[Petr Tesarik]

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
 

Re: Definition of BUG on x86

[Ingo Molnar]

* Petr Tesarik <ptesarik@suse.cz> wrote:

> 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.

> +static inline __noreturn void discarded_jmp(void)
> +{
> +	asm volatile(".macro jmp target\n"
> +		     "\t.purgem jmp\n"
> +		     ".endm\n");
> +	for (;;) ;
> +}

hm, that's very fragile.

Why not just:

 static inline __noreturn void x86_u2d(void)
 {
	asm volatile("u2d\n");
 }

If GCC emits a bogus warning about _that_, then it's a bug in 
the compiler that should be fixed.

	Ingo

Re: Definition of BUG on x86

[Petr Tesarik]

Ingo Molnar píše v Čt 19. 02. 2009 v 13:10 +0100:
> * Petr Tesarik <ptesarik@suse.cz> wrote:
> 
> > 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.
> 
> > +static inline __noreturn void discarded_jmp(void)
> > +{
> > +	asm volatile(".macro jmp target\n"
> > +		     "\t.purgem jmp\n"
> > +		     ".endm\n");
> > +	for (;;) ;
> > +}
> 
> hm, that's very fragile.
> 
> Why not just:
> 
>  static inline __noreturn void x86_u2d(void)
>  {
> 	asm volatile("u2d\n");
>  }
> 
> If GCC emits a bogus warning about _that_, then it's a bug in 
> the compiler that should be fixed.

I wouldn't call it a bug. The compiler has no idea about what the inline
assembly actualy does. So it cannot recognize that the ud2 instruction
does not return (which BTW might not even be the case, depending on the
implementation of the Invalid Opcode exception).

So, yes, it would be very nice if we had a way of telling the compiler
that a particular asm() statement is in fact a jump somewhere else. I
don't care about the syntax, I could even imagine something like:

  asm("some code" : : : "ip");  /* clobbers the instruction pointer */

Alternatively, if we could turn that particular warning off, I'd go for
it, but having a build spit warnings like:

/usr/src/linux-2.6/arch/x86/include/asm/bug.h:10: warning: 'noreturn' function does return

for each file that includes <asm/bug.h> in some way is _TOO_ annoying,
sorry.

Petr Tesarik

Re: Definition of BUG on x86

[Ingo Molnar]

* Petr Tesarik <ptesarik@suse.cz> wrote:

> Ingo Molnar píše v Čt 19. 02. 2009 v 13:10 +0100:
> > * Petr Tesarik <ptesarik@suse.cz> wrote:
> > 
> > > 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.
> > 
> > > +static inline __noreturn void discarded_jmp(void)
> > > +{
> > > +	asm volatile(".macro jmp target\n"
> > > +		     "\t.purgem jmp\n"
> > > +		     ".endm\n");
> > > +	for (;;) ;
> > > +}
> > 
> > hm, that's very fragile.
> > 
> > Why not just:
> > 
> >  static inline __noreturn void x86_u2d(void)
> >  {
> > 	asm volatile("u2d\n");
> >  }
> > 
> > If GCC emits a bogus warning about _that_, then it's a bug in 
> > the compiler that should be fixed.
> 
> I wouldn't call it a bug. The compiler has no idea about what 
> the inline assembly actualy does. So it cannot recognize that 
> the ud2 instruction does not return (which BTW might not even 
> be the case, depending on the implementation of the Invalid 
> Opcode exception).

No, i'm not talking about the inline assembly.

I'm talking about the x86_u2d() _inline function_, which has 
the __noreturn attribute.

Shouldnt that be enough to tell the compiler that it ... wont 
return?

	Ingo

Re: Definition of BUG on x86

[Petr Tesarik]

Ingo Molnar píše v Čt 19. 02. 2009 v 13:22 +0100:
> * Petr Tesarik <ptesarik@suse.cz> wrote:
> 
> > Ingo Molnar píše v Čt 19. 02. 2009 v 13:10 +0100:
> > > * Petr Tesarik <ptesarik@suse.cz> wrote:
> > > 
> > > > 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.
> > > 
> > > > +static inline __noreturn void discarded_jmp(void)
> > > > +{
> > > > +	asm volatile(".macro jmp target\n"
> > > > +		     "\t.purgem jmp\n"
> > > > +		     ".endm\n");
> > > > +	for (;;) ;
> > > > +}
> > > 
> > > hm, that's very fragile.
> > > 
> > > Why not just:
> > > 
> > >  static inline __noreturn void x86_u2d(void)
> > >  {
> > > 	asm volatile("u2d\n");
> > >  }
> > > 
> > > If GCC emits a bogus warning about _that_, then it's a bug in 
> > > the compiler that should be fixed.
> > 
> > I wouldn't call it a bug. The compiler has no idea about what 
> > the inline assembly actualy does. So it cannot recognize that 
> > the ud2 instruction does not return (which BTW might not even 
> > be the case, depending on the implementation of the Invalid 
> > Opcode exception).
> 
> No, i'm not talking about the inline assembly.
> 
> I'm talking about the x86_u2d() _inline function_, which has 
> the __noreturn attribute.
> 
> Shouldnt that be enough to tell the compiler that it ... wont 
> return?

Nope, that's not how it works.

You _may_ specify a noreturn attribute to any function (and GCC will
honour it AFAICS), but if GCC _thinks_ that the function does return, it
will issue the above-mentioned warning:

/usr/src/linux-2.6/arch/x86/include/asm/bug.h:10: warning: 'noreturn' function does return

And that's what your function will do. :-(

Yes, I also thinks that this behaviour is counter-intuitive. Besides, I
haven't found a gcc switch to turn this warning off, which would be my
next recommendation, since the GCC heuristics is broken, of course.

Petr Tesarik

Re: Definition of BUG on x86

[Ingo Molnar]

* Petr Tesarik <ptesarik@suse.cz> wrote:

> Ingo Molnar píše v Čt 19. 02. 2009 v 13:22 +0100:
> > * Petr Tesarik <ptesarik@suse.cz> wrote:
> > 
> > > Ingo Molnar píše v Čt 19. 02. 2009 v 13:10 +0100:
> > > > * Petr Tesarik <ptesarik@suse.cz> wrote:
> > > > 
> > > > > 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.
> > > > 
> > > > > +static inline __noreturn void discarded_jmp(void)
> > > > > +{
> > > > > +	asm volatile(".macro jmp target\n"
> > > > > +		     "\t.purgem jmp\n"
> > > > > +		     ".endm\n");
> > > > > +	for (;;) ;
> > > > > +}
> > > > 
> > > > hm, that's very fragile.
> > > > 
> > > > Why not just:
> > > > 
> > > >  static inline __noreturn void x86_u2d(void)
> > > >  {
> > > > 	asm volatile("u2d\n");
> > > >  }
> > > > 
> > > > If GCC emits a bogus warning about _that_, then it's a bug in 
> > > > the compiler that should be fixed.
> > > 
> > > I wouldn't call it a bug. The compiler has no idea about what 
> > > the inline assembly actualy does. So it cannot recognize that 
> > > the ud2 instruction does not return (which BTW might not even 
> > > be the case, depending on the implementation of the Invalid 
> > > Opcode exception).
> > 
> > No, i'm not talking about the inline assembly.
> > 
> > I'm talking about the x86_u2d() _inline function_, which has 
> > the __noreturn attribute.
> > 
> > Shouldnt that be enough to tell the compiler that it ... wont 
> > return?
> 
> Nope, that's not how it works.
> 
> You _may_ specify a noreturn attribute to any function (and 
> GCC will honour it AFAICS), but if GCC _thinks_ that the 
> function does return, it will issue the above-mentioned 
> warning:
> 
> /usr/src/linux-2.6/arch/x86/include/asm/bug.h:10: warning: 'noreturn' function does return
> 
> And that's what your function will do. :-(
> 
> Yes, I also thinks that this behaviour is counter-intuitive. 
> Besides, I haven't found a gcc switch to turn this warning 
> off, which would be my next recommendation, since the GCC 
> heuristics is broken, of course.

so GCC should be fixed and improved here, on several levels.

	Ingo

Re: Definition of BUG on x86

[Petr Tesarik]

Ingo Molnar píše v Čt 19. 02. 2009 v 13:47 +0100:
> * Petr Tesarik <ptesarik@suse.cz> wrote:
> 
> > Ingo Molnar píše v Čt 19. 02. 2009 v 13:22 +0100:
> > > * Petr Tesarik <ptesarik@suse.cz> wrote:
> > > 
> > > > Ingo Molnar píše v Čt 19. 02. 2009 v 13:10 +0100:
> > > > > * Petr Tesarik <ptesarik@suse.cz> wrote:
> > > > > 
> > > > > > 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.
> > > > > 
> > > > > > +static inline __noreturn void discarded_jmp(void)
> > > > > > +{
> > > > > > +	asm volatile(".macro jmp target\n"
> > > > > > +		     "\t.purgem jmp\n"
> > > > > > +		     ".endm\n");
> > > > > > +	for (;;) ;
> > > > > > +}
> > > > > 
> > > > > hm, that's very fragile.
> > > > > 
> > > > > Why not just:
> > > > > 
> > > > >  static inline __noreturn void x86_u2d(void)
> > > > >  {
> > > > > 	asm volatile("u2d\n");
> > > > >  }
> > > > > 
> > > > > If GCC emits a bogus warning about _that_, then it's a bug in 
> > > > > the compiler that should be fixed.
> > > > 
> > > > I wouldn't call it a bug. The compiler has no idea about what 
> > > > the inline assembly actualy does. So it cannot recognize that 
> > > > the ud2 instruction does not return (which BTW might not even 
> > > > be the case, depending on the implementation of the Invalid 
> > > > Opcode exception).
> > > 
> > > No, i'm not talking about the inline assembly.
> > > 
> > > I'm talking about the x86_u2d() _inline function_, which has 
> > > the __noreturn attribute.
> > > 
> > > Shouldnt that be enough to tell the compiler that it ... wont 
> > > return?
> > 
> > Nope, that's not how it works.
> > 
> > You _may_ specify a noreturn attribute to any function (and 
> > GCC will honour it AFAICS), but if GCC _thinks_ that the 
> > function does return, it will issue the above-mentioned 
> > warning:
> > 
> > /usr/src/linux-2.6/arch/x86/include/asm/bug.h:10: warning: 'noreturn' function does return
> > 
> > And that's what your function will do. :-(
> > 
> > Yes, I also thinks that this behaviour is counter-intuitive. 
> > Besides, I haven't found a gcc switch to turn this warning 
> > off, which would be my next recommendation, since the GCC 
> > heuristics is broken, of course.
> 
> so GCC should be fixed and improved here, on several levels.

Agree.

But it takes some time, even if we start pushing right now. What's your
suggestion for the meantime? Keep the dummy jmp? And in case anybody is
concerned about saving every byte in the text section, they can apply my
dirty patch?

Actually, this doesn't sound too bad.

Petr Tesarik

Re: Definition of BUG on x86

[Ingo Molnar]

* Petr Tesarik <ptesarik@suse.cz> wrote:

> Ingo Molnar píše v Čt 19. 02. 2009 v 13:47 +0100:
> > * Petr Tesarik <ptesarik@suse.cz> wrote:
> > 
> > > Ingo Molnar píše v Čt 19. 02. 2009 v 13:22 +0100:
> > > > * Petr Tesarik <ptesarik@suse.cz> wrote:
> > > > 
> > > > > Ingo Molnar píše v Čt 19. 02. 2009 v 13:10 +0100:
> > > > > > * Petr Tesarik <ptesarik@suse.cz> wrote:
> > > > > > 
> > > > > > > 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.
> > > > > > 
> > > > > > > +static inline __noreturn void discarded_jmp(void)
> > > > > > > +{
> > > > > > > +	asm volatile(".macro jmp target\n"
> > > > > > > +		     "\t.purgem jmp\n"
> > > > > > > +		     ".endm\n");
> > > > > > > +	for (;;) ;
> > > > > > > +}
> > > > > > 
> > > > > > hm, that's very fragile.
> > > > > > 
> > > > > > Why not just:
> > > > > > 
> > > > > >  static inline __noreturn void x86_u2d(void)
> > > > > >  {
> > > > > > 	asm volatile("u2d\n");
> > > > > >  }
> > > > > > 
> > > > > > If GCC emits a bogus warning about _that_, then it's a bug in 
> > > > > > the compiler that should be fixed.
> > > > > 
> > > > > I wouldn't call it a bug. The compiler has no idea about what 
> > > > > the inline assembly actualy does. So it cannot recognize that 
> > > > > the ud2 instruction does not return (which BTW might not even 
> > > > > be the case, depending on the implementation of the Invalid 
> > > > > Opcode exception).
> > > > 
> > > > No, i'm not talking about the inline assembly.
> > > > 
> > > > I'm talking about the x86_u2d() _inline function_, which has 
> > > > the __noreturn attribute.
> > > > 
> > > > Shouldnt that be enough to tell the compiler that it ... wont 
> > > > return?
> > > 
> > > Nope, that's not how it works.
> > > 
> > > You _may_ specify a noreturn attribute to any function (and 
> > > GCC will honour it AFAICS), but if GCC _thinks_ that the 
> > > function does return, it will issue the above-mentioned 
> > > warning:
> > > 
> > > /usr/src/linux-2.6/arch/x86/include/asm/bug.h:10: warning: 'noreturn' function does return
> > > 
> > > And that's what your function will do. :-(
> > > 
> > > Yes, I also thinks that this behaviour is counter-intuitive. 
> > > Besides, I haven't found a gcc switch to turn this warning 
> > > off, which would be my next recommendation, since the GCC 
> > > heuristics is broken, of course.
> > 
> > so GCC should be fixed and improved here, on several levels.
> 
> Agree.
> 
> But it takes some time, even if we start pushing right now. 
> What's your suggestion for the meantime? Keep the dummy jmp? 
> And in case anybody is concerned about saving every byte in 
> the text section, they can apply my dirty patch?
> 
> Actually, this doesn't sound too bad.

yeah. Please forward the problem to the appropriate GCC list in 
any case.

	Ingo

Re: Definition of BUG on x86

[Jeremy Fitzhardinge]

Ingo Molnar wrote:
> * Petr Tesarik <ptesarik@suse.cz> wrote:
>
>   
>> Ingo Molnar píše v Čt 19. 02. 2009 v 13:47 +0100:
>>     
>>> * Petr Tesarik <ptesarik@suse.cz> wrote:
>>>
>>>       
>>>> Ingo Molnar píše v Čt 19. 02. 2009 v 13:22 +0100:
>>>>         
>>>>> * Petr Tesarik <ptesarik@suse.cz> wrote:
>>>>>
>>>>>           
>>>>>> Ingo Molnar píše v Čt 19. 02. 2009 v 13:10 +0100:
>>>>>>             
>>>>>>> * Petr Tesarik <ptesarik@suse.cz> wrote:
>>>>>>>
>>>>>>>               
>>>>>>>> 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.
>>>>>>>>                 
>>>>>>>> +static inline __noreturn void discarded_jmp(void)
>>>>>>>> +{
>>>>>>>> +	asm volatile(".macro jmp target\n"
>>>>>>>> +		     "\t.purgem jmp\n"
>>>>>>>> +		     ".endm\n");
>>>>>>>> +	for (;;) ;
>>>>>>>> +}
>>>>>>>>                 
>>>>>>> hm, that's very fragile.
>>>>>>>
>>>>>>> Why not just:
>>>>>>>
>>>>>>>  static inline __noreturn void x86_u2d(void)
>>>>>>>  {
>>>>>>> 	asm volatile("u2d\n");
>>>>>>>  }
>>>>>>>
>>>>>>> If GCC emits a bogus warning about _that_, then it's a bug in 
>>>>>>> the compiler that should be fixed.
>>>>>>>               
>>>>>> I wouldn't call it a bug. The compiler has no idea about what 
>>>>>> the inline assembly actualy does. So it cannot recognize that 
>>>>>> the ud2 instruction does not return (which BTW might not even 
>>>>>> be the case, depending on the implementation of the Invalid 
>>>>>> Opcode exception).
>>>>>>             
>>>>> No, i'm not talking about the inline assembly.
>>>>>
>>>>> I'm talking about the x86_u2d() _inline function_, which has 
>>>>> the __noreturn attribute.
>>>>>
>>>>> Shouldnt that be enough to tell the compiler that it ... wont 
>>>>> return?
>>>>>           
>>>> Nope, that's not how it works.
>>>>
>>>> You _may_ specify a noreturn attribute to any function (and 
>>>> GCC will honour it AFAICS), but if GCC _thinks_ that the 
>>>> function does return, it will issue the above-mentioned 
>>>> warning:
>>>>
>>>> /usr/src/linux-2.6/arch/x86/include/asm/bug.h:10: warning: 'noreturn' function does return
>>>>
>>>> And that's what your function will do. :-(
>>>>
>>>> Yes, I also thinks that this behaviour is counter-intuitive. 
>>>> Besides, I haven't found a gcc switch to turn this warning 
>>>> off, which would be my next recommendation, since the GCC 
>>>> heuristics is broken, of course.
>>>>         
>>> so GCC should be fixed and improved here, on several levels.
>>>       
>> Agree.
>>
>> But it takes some time, even if we start pushing right now. 
>> What's your suggestion for the meantime? Keep the dummy jmp? 
>> And in case anybody is concerned about saving every byte in 
>> the text section, they can apply my dirty patch?
>>
>> Actually, this doesn't sound too bad.
>>     
>
> yeah. Please forward the problem to the appropriate GCC list in 
> any case.
>
>   

I think the official answer for this case is to use __builtin_trap.  But:

 -- Built-in Function: void __builtin_trap (void)
     This function causes the program to exit abnormally.  GCC
     implements this function by using a target-dependent mechanism
     (such as intentionally executing an illegal instruction) or by
     calling `abort'.  ***The mechanism used may vary from release to
     release so you should not rely on any particular implementation.***

which in principle is hard for us to make use of.  In practice I think 
it has always been ud2a on x86.

http://gcc.gnu.org/ml/gcc-patches/2000-01/msg00190.html

    J

Re: Definition of BUG on x86

[Ingo Molnar]

* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> Ingo Molnar wrote:
>> * Petr Tesarik <ptesarik@suse.cz> wrote:
>>
>>   
>>> Ingo Molnar píše v Čt 19. 02. 2009 v 13:47 +0100:
>>>     
>>>> * Petr Tesarik <ptesarik@suse.cz> wrote:
>>>>
>>>>       
>>>>> Ingo Molnar píše v Čt 19. 02. 2009 v 13:22 +0100:
>>>>>         
>>>>>> * Petr Tesarik <ptesarik@suse.cz> wrote:
>>>>>>
>>>>>>           
>>>>>>> Ingo Molnar píše v Čt 19. 02. 2009 v 13:10 +0100:
>>>>>>>             
>>>>>>>> * Petr Tesarik <ptesarik@suse.cz> wrote:
>>>>>>>>
>>>>>>>>               
>>>>>>>>> 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.
>>>>>>>>>                 +static inline __noreturn void 
>>>>>>>>> discarded_jmp(void)
>>>>>>>>> +{
>>>>>>>>> +	asm volatile(".macro jmp target\n"
>>>>>>>>> +		     "\t.purgem jmp\n"
>>>>>>>>> +		     ".endm\n");
>>>>>>>>> +	for (;;) ;
>>>>>>>>> +}
>>>>>>>>>                 
>>>>>>>> hm, that's very fragile.
>>>>>>>>
>>>>>>>> Why not just:
>>>>>>>>
>>>>>>>>  static inline __noreturn void x86_u2d(void)
>>>>>>>>  {
>>>>>>>> 	asm volatile("u2d\n");
>>>>>>>>  }
>>>>>>>>
>>>>>>>> If GCC emits a bogus warning about _that_, then it's a bug 
>>>>>>>> in the compiler that should be fixed.
>>>>>>>>               
>>>>>>> I wouldn't call it a bug. The compiler has no idea about what 
>>>>>>> the inline assembly actualy does. So it cannot recognize that 
>>>>>>> the ud2 instruction does not return (which BTW might not even 
>>>>>>> be the case, depending on the implementation of the Invalid  
>>>>>>> Opcode exception).
>>>>>>>             
>>>>>> No, i'm not talking about the inline assembly.
>>>>>>
>>>>>> I'm talking about the x86_u2d() _inline function_, which has  
>>>>>> the __noreturn attribute.
>>>>>>
>>>>>> Shouldnt that be enough to tell the compiler that it ... wont  
>>>>>> return?
>>>>>>           
>>>>> Nope, that's not how it works.
>>>>>
>>>>> You _may_ specify a noreturn attribute to any function (and GCC 
>>>>> will honour it AFAICS), but if GCC _thinks_ that the function 
>>>>> does return, it will issue the above-mentioned warning:
>>>>>
>>>>> /usr/src/linux-2.6/arch/x86/include/asm/bug.h:10: warning: 'noreturn' function does return
>>>>>
>>>>> And that's what your function will do. :-(
>>>>>
>>>>> Yes, I also thinks that this behaviour is counter-intuitive.  
>>>>> Besides, I haven't found a gcc switch to turn this warning off, 
>>>>> which would be my next recommendation, since the GCC heuristics 
>>>>> is broken, of course.
>>>>>         
>>>> so GCC should be fixed and improved here, on several levels.
>>>>       
>>> Agree.
>>>
>>> But it takes some time, even if we start pushing right now. What's 
>>> your suggestion for the meantime? Keep the dummy jmp? And in case 
>>> anybody is concerned about saving every byte in the text section, 
>>> they can apply my dirty patch?
>>>
>>> Actually, this doesn't sound too bad.
>>>     
>>
>> yeah. Please forward the problem to the appropriate GCC list in any 
>> case.
>>
>>   
>
> I think the official answer for this case is to use __builtin_trap.  But:
>
> -- Built-in Function: void __builtin_trap (void)
>     This function causes the program to exit abnormally.  GCC
>     implements this function by using a target-dependent mechanism
>     (such as intentionally executing an illegal instruction) or by
>     calling `abort'.  ***The mechanism used may vary from release to
>     release so you should not rely on any particular implementation.***
>
> which in principle is hard for us to make use of.  In practice I think  
> it has always been ud2a on x86.

could we just do:

	__builtin_trap();
	for (;;);

and _now_ GCC would optimize away the infinite loop? And if it 
does something silly in a future release, we'd either get a 
build error or we'd run into the infinite loop for sure.

	Ingo

Re: Definition of BUG on x86

[Petr Tesarik]

Ingo Molnar píše v Čt 19. 02. 2009 v 16:35 +0100:
>[...]
> * Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> 
> > Ingo Molnar wrote:
> >> * Petr Tesarik <ptesarik@suse.cz> wrote:
> >>
> >>   
> >>> Ingo Molnar píše v Čt 19. 02. 2009 v 13:47 +0100:
> >>>     
> >>>> so GCC should be fixed and improved here, on several levels.
> >>>>       
> >>> Agree.
> >>>
> >>> But it takes some time, even if we start pushing right now. What's 
> >>> your suggestion for the meantime? Keep the dummy jmp? And in case 
> >>> anybody is concerned about saving every byte in the text section, 
> >>> they can apply my dirty patch?
> >>>
> >>> Actually, this doesn't sound too bad.
> >>>     
> >>
> >> yeah. Please forward the problem to the appropriate GCC list in any 
> >> case.
> >>
> >>   
> >
> > I think the official answer for this case is to use __builtin_trap.  But:
> >
> > -- Built-in Function: void __builtin_trap (void)
> >     This function causes the program to exit abnormally.  GCC
> >     implements this function by using a target-dependent mechanism
> >     (such as intentionally executing an illegal instruction) or by
> >     calling `abort'.  ***The mechanism used may vary from release to
> >     release so you should not rely on any particular implementation.***
> >
> > which in principle is hard for us to make use of.  In practice I think  
> > it has always been ud2a on x86.
> 
> could we just do:
> 
> 	__builtin_trap();
> 	for (;;);

I'm afraid that's not the point of the exercise. I'm trying to trim
BUG() to two bytes, while still making sure that the Illegal Opcode
exception is generated at the exact code point, so we can track it down
using the info in __bug_table. If __builtin_trap() ever translates to
anything else than ud2a in the above code snippet, there will be no BUG
reported. Instead, the CPU that encountered the BUG() will burn CPU
cycles forever without any apparent reason.

NAK,
Petr Tesarik

Re: Definition of BUG on x86

[Ingo Molnar]

* Petr Tesarik <ptesarik@suse.cz> wrote:

> Ingo Molnar píše v Čt 19. 02. 2009 v 16:35 +0100:
> >[...]
> > * Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> > 
> > > Ingo Molnar wrote:
> > >> * Petr Tesarik <ptesarik@suse.cz> wrote:
> > >>
> > >>   
> > >>> Ingo Molnar píše v Čt 19. 02. 2009 v 13:47 +0100:
> > >>>     
> > >>>> so GCC should be fixed and improved here, on several levels.
> > >>>>       
> > >>> Agree.
> > >>>
> > >>> But it takes some time, even if we start pushing right now. What's 
> > >>> your suggestion for the meantime? Keep the dummy jmp? And in case 
> > >>> anybody is concerned about saving every byte in the text section, 
> > >>> they can apply my dirty patch?
> > >>>
> > >>> Actually, this doesn't sound too bad.
> > >>>     
> > >>
> > >> yeah. Please forward the problem to the appropriate GCC list in any 
> > >> case.
> > >>
> > >>   
> > >
> > > I think the official answer for this case is to use __builtin_trap.  But:
> > >
> > > -- Built-in Function: void __builtin_trap (void)
> > >     This function causes the program to exit abnormally.  GCC
> > >     implements this function by using a target-dependent mechanism
> > >     (such as intentionally executing an illegal instruction) or by
> > >     calling `abort'.  ***The mechanism used may vary from release to
> > >     release so you should not rely on any particular implementation.***
> > >
> > > which in principle is hard for us to make use of.  In practice I think  
> > > it has always been ud2a on x86.
> > 
> > could we just do:
> > 
> > 	__builtin_trap();
> > 	for (;;);
> 
> I'm afraid that's not the point of the exercise. I'm trying to 
> trim BUG() to two bytes, while still making sure that the 
> Illegal Opcode exception is generated at the exact code point, 
> so we can track it down using the info in __bug_table. If 
> __builtin_trap() ever translates to anything else than ud2a in 
> the above code snippet, there will be no BUG reported. 
> Instead, the CPU that encountered the BUG() will burn CPU 
> cycles forever without any apparent reason.

Well, the important question is thatGCC will optimize out 
whatever comes after the __builtin_trap(), right? To guarantee 
an assert we can do something like:

 	__builtin_trap();
	panic("should never get here");

to guarantee a message. (But realistically GCC will at most 
generate a build error.)

	Ingo

Re: Definition of BUG on x86

[Jeremy Fitzhardinge]

Ingo Molnar wrote:
> Well, the important question is thatGCC will optimize out 
> whatever comes after the __builtin_trap(), right? To guarantee 
> an assert we can do something like:
>
>  	__builtin_trap();
> 	panic("should never get here");
>
> to guarantee a message. (But realistically GCC will at most 
> generate a build error.)
>   

Ah, right, I remember the problem.  There's no guaranteed way of getting 
the address of the ud2a instruction __builtin_trap generates to put it 
into the bug table.

    J

Re: Definition of BUG on x86

[Ingo Molnar]

* Jeremy Fitzhardinge <jeremy@goop.org> wrote:

> Ingo Molnar wrote:
>> Well, the important question is thatGCC will optimize out whatever 
>> comes after the __builtin_trap(), right? To guarantee an assert we can 
>> do something like:
>>
>>  	__builtin_trap();
>> 	panic("should never get here");
>>
>> to guarantee a message. (But realistically GCC will at most generate a 
>> build error.)
>
> Ah, right, I remember the problem.  There's no guaranteed way 
> of getting the address of the ud2a instruction __builtin_trap 
> generates to put it into the bug table.

hm, indeed.

So that 2 byte overhead will be there for now, until gcc (or 
some other compiler) is improved.

	Ingo

Re: Definition of BUG on x86

[H. Peter Anvin]

Jeremy Fitzhardinge wrote:
> Ingo Molnar wrote:
>> Well, the important question is thatGCC will optimize out whatever 
>> comes after the __builtin_trap(), right? To guarantee an assert we can 
>> do something like:
>>
>>      __builtin_trap();
>>     panic("should never get here");
>>
>> to guarantee a message. (But realistically GCC will at most generate a 
>> build error.)
>>   
> 
> Ah, right, I remember the problem.  There's no guaranteed way of getting 
> the address of the ud2a instruction __builtin_trap generates to put it 
> into the bug table.
> 

Did we actually run into any instance where that failed?

It's true that it's not guaranteed, but it seems highly unlikely that it 
would happen in real life.  We *could* do a forward search at that 
point, that should catch the vast majority of the failing cases, again, 
but once again there are no guarantees.

I guess I should ask the gcc people...

	-hpa

Re: Definition of BUG on x86

[Ingo Molnar]

* H. Peter Anvin <hpa@zytor.com> wrote:

> Jeremy Fitzhardinge wrote:
>> Ingo Molnar wrote:
>>> Well, the important question is thatGCC will optimize out whatever  
>>> comes after the __builtin_trap(), right? To guarantee an assert we 
>>> can do something like:
>>>
>>>      __builtin_trap();
>>>     panic("should never get here");
>>>
>>> to guarantee a message. (But realistically GCC will at most generate 
>>> a build error.)
>>>   
>>
>> Ah, right, I remember the problem.  There's no guaranteed way of 
>> getting the address of the ud2a instruction __builtin_trap generates to 
>> put it into the bug table.
>>
>
> Did we actually run into any instance where that failed?
>
> It's true that it's not guaranteed, but it seems highly 
> unlikely that it would happen in real life.  We *could* do a 
> forward search at that point, that should catch the vast 
> majority of the failing cases, again, but once again there are 
> no guarantees.
>
> I guess I should ask the gcc people...

The whole thing is borderline anyway (the win is small), and the 
combination of relying on __builtin_trap() [which is documented 
as a non-stable interface], and the reliance on basic block 
non-ordering.

Another complication is that this is _debug_ code - i.e. if 
there's a rare bug here we'll only see it if a bug triggers 
there - which is very rare in itself.

So i'm rather uneasy to rely on GCC to this level. They should 
allow to pass __noreturn to asm()s - that's a far cleaner 
approach.

	Ingo

Re: Definition of BUG on x86

[Petr Tesarik]

Ingo Molnar píše v Čt 19. 02. 2009 v 17:16 +0100:
> * Petr Tesarik <ptesarik@suse.cz> wrote:
> 
> > Ingo Molnar píše v Čt 19. 02. 2009 v 16:35 +0100:
> > >[...]
> > > * Jeremy Fitzhardinge <jeremy@goop.org> wrote:
> > > 
> > > > Ingo Molnar wrote:
> > > >> * Petr Tesarik <ptesarik@suse.cz> wrote:
> > > >>
> > > >>   
> > > >>> Ingo Molnar píše v Čt 19. 02. 2009 v 13:47 +0100:
> > > >>>     
> > > >>>> so GCC should be fixed and improved here, on several levels.
> > > >>>>       
> > > >>> Agree.
> > > >>>
> > > >>> But it takes some time, even if we start pushing right now. What's 
> > > >>> your suggestion for the meantime? Keep the dummy jmp? And in case 
> > > >>> anybody is concerned about saving every byte in the text section, 
> > > >>> they can apply my dirty patch?
> > > >>>
> > > >>> Actually, this doesn't sound too bad.
> > > >>>     
> > > >>
> > > >> yeah. Please forward the problem to the appropriate GCC list in any 
> > > >> case.
> > > >>
> > > >>   
> > > >
> > > > I think the official answer for this case is to use __builtin_trap.  But:
> > > >
> > > > -- Built-in Function: void __builtin_trap (void)
> > > >     This function causes the program to exit abnormally.  GCC
> > > >     implements this function by using a target-dependent mechanism
> > > >     (such as intentionally executing an illegal instruction) or by
> > > >     calling `abort'.  ***The mechanism used may vary from release to
> > > >     release so you should not rely on any particular implementation.***
> > > >
> > > > which in principle is hard for us to make use of.  In practice I think  
> > > > it has always been ud2a on x86.
> > > 
> > > could we just do:
> > > 
> > > 	__builtin_trap();
> > > 	for (;;);
> > 
> > I'm afraid that's not the point of the exercise. I'm trying to 
> > trim BUG() to two bytes, while still making sure that the 
> > Illegal Opcode exception is generated at the exact code point, 
> > so we can track it down using the info in __bug_table. If 
> > __builtin_trap() ever translates to anything else than ud2a in 
> > the above code snippet, there will be no BUG reported. 
> > Instead, the CPU that encountered the BUG() will burn CPU 
> > cycles forever without any apparent reason.
> 
> Well, the important question is thatGCC will optimize out 
> whatever comes after the __builtin_trap(), right? To guarantee 
> an assert we can do something like:
> 
>  	__builtin_trap();
> 	panic("should never get here");

Yes, perfect, then I vote for this:

	__builtin_trap();
	printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __func__);
	panic("BUG!");

(copied from include/asm-generic/bug.h)

Then the worst thing GCC folks might invent would be using the ud2b
opcode instead of ud2a, but I think the bug table is searched for every
die() invocation nowadays, so it doesn't depend on seeing the
0f 0b sequence at the point of the failure, does it?

And even if GCC decides to insert one or more _valid_ instructions
before the invalid one, we'll start getting reports with 
"[verbose debug info unavailable]". If, instead, they start calling
abort(), we'll get build errors.

Last thing -- what about Intel cc?

Petr Tesarik

Re: Definition of BUG on x86

[Jeremy Fitzhardinge]

Ingo Molnar wrote:
> could we just do:
>
> 	__builtin_trap();
> 	for (;;);
>
> and _now_ GCC would optimize away the infinite loop? And if it 
> does something silly in a future release, we'd either get a 
> build error or we'd run into the infinite loop for sure.
>   

I guess that would work.  And we have plenty of infrastructure and 
precedent for compiler-dependent defines like this.

    J

Re: Definition of BUG on x86

[H. Peter Anvin]

Petr Tesarik wrote:
> 
> 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.
> 

At Transmeta, we had an internal hack to gcc that added
__builtin_not_reached(); for this particular reason.  This would
probably be the best way to handle this.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.