svn commit: r183818 - in stable/7/sys: . i386/include

[Christoph Mallon]

Kostik Belousov wrote:
> On Tue, Oct 14, 2008 at 02:07:27PM +0200, Christoph Mallon wrote:
>> Hi Konstantin
>>
>> Konstantin Belousov wrote:
>>> Author: kib
>>> Date: Mon Oct 13 12:45:18 2008
>>> New Revision: 183818
>>> URL: http://svn.freebsd.org/changeset/base/183818
>>>
>>> Log:
>>>  MFC r180756 (by luoqi):
>>>  Unbreak cc -pg support on i386 by changing mcount() to always preserve 
>>>  %ecx.

[...]

>> As you can see, %ecx gets destroyed (GCC does not emit the #APP marker 
>> for empty asm statements, so I added "nop" for clarity. Even then GCC 
>> merged the two #APP blocks!). In mcount() the compiler could choose to 
>> place selfpc or frompc into %ecx and change the order of statements, 
>> which would destroy the contents of %ecx. In fact, if -mstackrealign is 
>> used, the stack realignment in mcount() destroys %ecx before any of the 
>> inline assembler statements is executed for sure. The only safe way is 
>> to implement mcount() using a global asm statement:
>>
>> #define _MCOUNT_DECL static __attribute((cdecl,noinline)) void _mcount
>>
>> #define MCOUNT                                         \
>> asm(                                                   \
>> 	".globl mcount\n\t"                            \
>> 	".type	mcount, @function\n"                   \
>> 	"mcount:\n\t"                                  \
>> 	"pushl %ecx\n\t"                               \
>> 	"pushl 4(%esp)\n\t" // my return address       \
>> 	"pushl 4(%ebp)\n\t" // caller's return address \
>> 	"call  _mcount\n\t"                            \
>> 	"addl  $8, %esp\n\t"                           \
>> 	"pop   %ecx\n\t"                               \
>> 	"ret\n\t"                                      \
>> 	".size   mcount, .-mcount");
>>
>> Considering the whole issue, I think this is a bug/misfeature of GCC. It 
>> could easily restore %ecx after calling mcount(), which it does for any 
>> normal (i.e. non-pg-induced) function call().
> I was worried too about suspiciously looking direct asm manipulations of
> the registers that could interfere with optimizer, when I have seen the
> patch committed to the HEAD. On the other hand, it magically works for
> the present version of the gcc and used compiler flags.

No, it does not work. I exactly described the scenario in which it plain 
breaks: If you use -mstackrealign, then mcount() gets realignment code 
too, which destroys the contents of %ecx *before* any of the inline 
assembler statemets get executed. The first thing the compiler inserts 
into mcount() is this line:

	leal    4(%esp), %ecx

This immediately destroys %ecx. And even if you do not use 
-mstackrealign - as I explained - you have *no* guarantee that you get 
the contents of %ecx from before the function call. It depends on the 
exact switches (which GCC has dozens of) to not explode - have you 
tested *all* combinations?

> We cannot have any other fix for 7.1, I suspect.

We should, for the reasons explained above.

> Feel free to submit more accurate patch.

Um, I *did*. See the code above.

Regards
	Christoph