From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 46236 invoked by alias); 15 Dec 2018 15:38:57 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 46190 invoked by uid 89); 15 Dec 2018 15:38:55 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-1.4 required=5.0 tests=BAYES_00,KAM_NUMSUBJECT,RCVD_IN_DNSWL_NONE autolearn=no version=3.3.2 spammy=H*f:sk:87woohs, pr52813, PR52813, restoring X-HELO: gate.crashing.org Received: from gate.crashing.org (HELO gate.crashing.org) (63.228.1.57) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 15 Dec 2018 15:38:53 +0000 Received: from gate.crashing.org (localhost.localdomain [127.0.0.1]) by gate.crashing.org (8.14.1/8.14.1) with ESMTP id wBFFcaTH015063; Sat, 15 Dec 2018 09:38:36 -0600 Received: (from segher@localhost) by gate.crashing.org (8.14.1/8.14.1/Submit) id wBFFcYNM015062; Sat, 15 Dec 2018 09:38:34 -0600 Date: Sat, 15 Dec 2018 15:38:00 -0000 From: Segher Boessenkool To: Christophe Lyon , Thomas Preudhomme , dimitar@dinux.eu, gcc Patches , "Thomas Preud'homme" , richard.sandiford@arm.com Subject: Re: [PATCH] [RFC] PR target/52813 and target/11807 Message-ID: <20181215153833.GU3803@gate.crashing.org> References: <20181209100856.14051-1-dimitar@dinux.eu> <87woohsk32.fsf@arm.com> <2807771.CbC4dySGB1@tpdeb> <87woog9i32.fsf@arm.com> <87ftv0nrpn.fsf@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87ftv0nrpn.fsf@arm.com> User-Agent: Mutt/1.4.2.3i X-IsSubscribed: yes X-SW-Source: 2018-12/txt/msg01149.txt.bz2 On Fri, Dec 14, 2018 at 01:49:40PM +0000, Richard Sandiford wrote: > > OK, I read PR52813 too, but I'm not sure to fully understand the new status. > > My understanding is that since this patch was committed, if an asm statement > > clobbers sp, it is now allowed to actually declare it as clobber (this patch > > generates an error in such a case). > > So the user is now expected to lie to the compiler when writing to > > this kind of register (sp, pic register), by not declaring it as "clobber"? > > The user isn't expected to lie. The point is that GCC simply doesn't > support asms that leave the stack pointer with a different value from > before, and IMO never has. If that happened to work in some cases then > it was purely an accident. Yup. It now errors for void f(void) { asm("ohmy" ::: "sp"); } but not for void f(void) { register long x asm("sp"); asm("ohmy %0" : "=r"(x)); } which is the same problem. (I would be happier if it was a warning instead of an error btw, since there apparently is existing code that uses a clobber of sp, and GCC has always worked with that, accidentally or not). > The PRs also show disagreement about what GCC should do for an asm like > that. The asm in PR52813 temporarily changed the stack pointer and the > bug was that GCC didn't restore the original value afterwards. The asm > in PR77904 was trying to set the stack pointer to an entirely new value > and the bug was the GCC did restore the original value afterwards, > defeating the point. > > This wouldn't be the first time that there's disagreement about what > the behaviour should be. But IMO we can't support either reliably. > Spilling sp is dangerous in general because we might need the stack > for the reload, or we might accidentally try to reload something else > before restoring the stack pointer. And continuing with a new sp > provided by the asm could lead to all sorts of problems. (AIUI, the > point of PR77904 was that it would also be wrong for GCC to set up a > frame pointer and restore the sp from that frame pointer on function > exit. The new sp value was supposed to survive. So the answer isn't > simply "use a frame pointer".) Inline asm cannot do things that are not allowed by the ABI, or that touch other things in the execution environment you shouldn't touch. Apparently some people really try to clobber sp, and this new error will help them a bit. I don't know how useful that is, is it better to scare people away from using inline asm? It might well be safer for everyone... Segher