Date: Wed, 15 Oct 2008 14:28:10 +0300 From: Kostik Belousov <kostikbel@gmail.com> To: Christoph Mallon <christoph.mallon@gmx.de> Cc: svn-src-stable@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, svn-src-stable-7@freebsd.org Subject: Re: svn commit: r183818 - in stable/7/sys: . i386/include Message-ID: <20081015112810.GN7782@deviant.kiev.zoral.com.ua> In-Reply-To: <48F48B7F.5030009@gmx.de> References: <200810131245.m9DCjIsR076490@svn.freebsd.org> <48F48B7F.5030009@gmx.de>
next in thread | previous in thread | raw e-mail | index | archive | help
--0f/2aphqXmKkUnNA Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Oct 14, 2008 at 02:07:27PM +0200, Christoph Mallon wrote: > Hi Konstantin >=20 > 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= =20 > > %ecx. > > =20 > > Approved by: re (kensmith) > > > >Modified: > > stable/7/sys/ (props changed) > > stable/7/sys/i386/include/profile.h > > > >Modified: stable/7/sys/i386/include/profile.h > >=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > >--- stable/7/sys/i386/include/profile.h Mon Oct 13 12:28:33 2008=20 > >(r183817) > >+++ stable/7/sys/i386/include/profile.h Mon Oct 13 12:45:18 2008=20 > >(r183818) > >@@ -115,7 +115,15 @@ void user(void); > > void \ > > mcount() \ > > { \ > >- uintfptr_t selfpc, frompc; \ > >+ uintfptr_t selfpc, frompc, ecx; \ > >+ /* \ > >+ * In gcc 4.2, ecx might be used in the caller as the arg \ > >+ * pointer if the stack realignment option is set (-mstackrealign) \ > >+ * or if the caller has the force_align_arg_pointer attribute \ > >+ * (stack realignment is ALWAYS on for main). Preserve ecx \ > >+ * here. \ > >+ */ \ > >+ __asm("" : "=3Dc" (ecx)); \ > > /* \ > > * Find the return address for mcount, \ > > * and the return address for mcount's caller. \ > >@@ -132,6 +140,7 @@ mcount() \ > > __asm("movl (%%ebp),%0" : "=3Dr" (frompc)); \ > > frompc =3D ((uintfptr_t *)frompc)[1]; \ > > _mcount(frompc, selfpc); \ > >+ __asm("" : : "c" (ecx)); \ > > } > > #else /* !__GNUCLIKE_ASM */ > > #define MCOUNT >=20 > This fix is conceptually broken and an accident waiting to happen. There= =20 > is no way to prevent the compiler from shuffling instructions and=20 > clobbering %ecx. Here is a simple example, which demonstrates this proble= m: >=20 > unsigned f(unsigned a) > { > unsigned ecx; > asm("nop" : "=3Dc" (ecx)); > a =3D 1 << a; > asm("nop" : : "c" (ecx)); > return a; > } >=20 > GCC compiles this to: > f: > #APP > nop > nop > #NO_APP > movl 4(%esp), %ecx > movl $1, %eax > sall %cl, %eax > ret >=20 > As you can see, %ecx gets destroyed (GCC does not emit the #APP marker=20 > for empty asm statements, so I added "nop" for clarity. Even then GCC=20 > merged the two #APP blocks!). In mcount() the compiler could choose to=20 > place selfpc or frompc into %ecx and change the order of statements,=20 > which would destroy the contents of %ecx. In fact, if -mstackrealign is= =20 > used, the stack realignment in mcount() destroys %ecx before any of the= =20 > inline assembler statements is executed for sure. The only safe way is=20 > to implement mcount() using a global asm statement: >=20 > #define _MCOUNT_DECL static __attribute((cdecl,noinline)) void _mcount >=20 > #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"); >=20 > Considering the whole issue, I think this is a bug/misfeature of GCC. It= =20 > could easily restore %ecx after calling mcount(), which it does for any= =20 > 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. We cannot have any other fix for 7.1, I suspect. Feel free to submit more accurate patch. And, as was stated in the original commit message, saving of the whole register file may be right thing to do. >=20 >=20 > On a related note, I have submitted PR i386/127387 with patch=20 > (http://www.freebsd.org/cgi/query-pr.cgi?pr=3Di386/127387) about a simila= r=20 > problem in _start() in crt1.c for x86. >=20 > Regards > Christoph --0f/2aphqXmKkUnNA Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (FreeBSD) iEYEARECAAYFAkj108kACgkQC3+MBN1Mb4iGYgCgxFKXeArQboGFoV2RS1COGYYx re8AoPJta5Sl5346jnCxnbHU1TPVFB5s =no6b -----END PGP SIGNATURE----- --0f/2aphqXmKkUnNA--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20081015112810.GN7782>