From owner-svn-src-all@FreeBSD.ORG Wed Oct 15 12:08:45 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 18FC4106568E for ; Wed, 15 Oct 2008 12:08:45 +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 6E54A8FC1E for ; Wed, 15 Oct 2008 12:08:44 +0000 (UTC) (envelope-from christoph.mallon@gmx.de) Received: (qmail invoked by alias); 15 Oct 2008 12:08:42 -0000 Received: from p54A3ECE9.dip.t-dialin.net (EHLO tron.homeunix.org) [84.163.236.233] by mail.gmx.net (mp002) with SMTP; 15 Oct 2008 14:08:42 +0200 X-Authenticated: #1673122 X-Provags-ID: V01U2FsdGVkX199mHI1MfV92CRgXxkjaP8bQpuo+dAUMi/UN16Yqp CAp6jfSx46Siif Message-ID: <48F5DD49.6040003@gmx.de> Date: Wed, 15 Oct 2008 14:08:41 +0200 From: Christoph Mallon User-Agent: Thunderbird 2.0.0.17 (X11/20080927) MIME-Version: 1.0 To: Kostik Belousov References: <200810131245.m9DCjIsR076490@svn.freebsd.org> <48F48B7F.5030009@gmx.de> <20081015112810.GN7782@deviant.kiev.zoral.com.ua> In-Reply-To: <20081015112810.GN7782@deviant.kiev.zoral.com.ua> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit X-Y-GMX-Trusted: 0 X-FuHaFi: 0.48 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: Wed, 15 Oct 2008 12:08:45 -0000 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