From owner-svn-src-all@FreeBSD.ORG Tue Oct 14 12:34:11 2008 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 98E321065694 for ; Tue, 14 Oct 2008 12:34:11 +0000 (UTC) (envelope-from christoph.mallon@gmx.de) Received: from mail.gmx.net (mail.gmx.net [213.165.64.20]) by mx1.freebsd.org (Postfix) with SMTP id F16D38FC1E for ; Tue, 14 Oct 2008 12:34:10 +0000 (UTC) (envelope-from christoph.mallon@gmx.de) Received: (qmail invoked by alias); 14 Oct 2008 12:07:28 -0000 Received: from p54A3EEED.dip.t-dialin.net (EHLO tron.homeunix.org) [84.163.238.237] by mail.gmx.net (mp003) with SMTP; 14 Oct 2008 14:07:28 +0200 X-Authenticated: #1673122 X-Provags-ID: V01U2FsdGVkX1/cUwx6X8nc72azS9F4XQmpmaCgxWgYPXzyYcuXo5 8u2h5m4ed7rmkF Message-ID: <48F48B7F.5030009@gmx.de> Date: Tue, 14 Oct 2008 14:07:27 +0200 From: Christoph Mallon User-Agent: Thunderbird 2.0.0.17 (X11/20080927) MIME-Version: 1.0 To: Konstantin Belousov References: <200810131245.m9DCjIsR076490@svn.freebsd.org> In-Reply-To: <200810131245.m9DCjIsR076490@svn.freebsd.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Y-GMX-Trusted: 0 X-FuHaFi: 0.42 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 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Oct 2008 12:34:11 -0000 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. > > 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 > ============================================================================== > --- stable/7/sys/i386/include/profile.h Mon Oct 13 12:28:33 2008 (r183817) > +++ stable/7/sys/i386/include/profile.h Mon Oct 13 12:45:18 2008 (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("" : "=c" (ecx)); \ > /* \ > * Find the return address for mcount, \ > * and the return address for mcount's caller. \ > @@ -132,6 +140,7 @@ mcount() \ > __asm("movl (%%ebp),%0" : "=r" (frompc)); \ > frompc = ((uintfptr_t *)frompc)[1]; \ > _mcount(frompc, selfpc); \ > + __asm("" : : "c" (ecx)); \ > } > #else /* !__GNUCLIKE_ASM */ > #define MCOUNT This fix is conceptually broken and an accident waiting to happen. There is no way to prevent the compiler from shuffling instructions and clobbering %ecx. Here is a simple example, which demonstrates this problem: unsigned f(unsigned a) { unsigned ecx; asm("nop" : "=c" (ecx)); a = 1 << a; asm("nop" : : "c" (ecx)); return a; } GCC compiles this to: f: #APP nop nop #NO_APP movl 4(%esp), %ecx movl $1, %eax sall %cl, %eax ret 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(). On a related note, I have submitted PR i386/127387 with patch (http://www.freebsd.org/cgi/query-pr.cgi?pr=i386/127387) about a similar problem in _start() in crt1.c for x86. Regards Christoph