From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 109108 invoked by alias); 30 Apr 2019 06:44:19 -0000 Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org Received: (qmail 109096 invoked by uid 89); 30 Apr 2019 06:44:18 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=scatter, H*i:sk:dpXrPBR, H*f:sk:dpXrPBR, claim X-HELO: prv1-mh.provo.novell.com Received: from prv1-mh.provo.novell.com (HELO prv1-mh.provo.novell.com) (137.65.248.33) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 30 Apr 2019 06:44:17 +0000 Received: from INET-PRV1-MTA by prv1-mh.provo.novell.com with Novell_GroupWise; Tue, 30 Apr 2019 00:44:14 -0600 Message-Id: <5CC7EEBC020000780022A3BA@prv1-mh.provo.novell.com> Date: Tue, 30 Apr 2019 06:44:00 -0000 From: "Jan Beulich" To: "H.J. Lu" Cc: Subject: Re: [PATCH] i386: Don't add 0x66 prefix to IRET for .code16gcc References: <20190426172207.13838-1-hjl.tools@gmail.com> <5CC6A1280200007800229D48@prv1-mh.provo.novell.com> <5CC7174A020000780022A168@prv1-mh.provo.novell.com> <5CC72236020000780022A212@prv1-mh.provo.novell.com> In-Reply-To: Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Content-Disposition: inline X-SW-Source: 2019-04/txt/msg00262.txt.bz2 >>> On 29.04.19 at 19:13, wrote: > On Mon, Apr 29, 2019 at 9:11 AM Jan Beulich wrote: >> >> >>> On 29.04.19 at 18:02, wrote: >> > On Mon, Apr 29, 2019 at 8:25 AM Jan Beulich wrote: >> >> >> >> >>> On 29.04.19 at 17:09, wrote: >> >> > On Mon, Apr 29, 2019 at 12:01 AM Jan Beulich wr= ote: >> >> >> >>> On 26.04.19 at 19:22, wrote: >> >> >> > The .code16gcc directive supports 16bit mode with 32-bit address= . Since >> >> >> > IRET (opcode 0xcf) in 16bit mode returns from an interrupt in 16= bit mode, >> >> >> > we shouldn't add 0x66 prefix for IRET. >> >> >> > >> >> >> > PR gas/24485 >> >> >> > * config/tc-i386.c (process_suffix): Don't add DATA_PREFIX= _OPCODE >> >> >> > to IRET for .code16gcc. >> >> >> >> >> >> This, at the very least, needs to be accompanied by a warning: >> >> > >> >> > This patch fixes: >> >> >[...] >> >> >> As the bug report validly says, the changed behavior is what is >> >> >> wanted only "almost always". The report even mentions the >> >> >> (supposedly uncommon) case: Code manually building a frame >> >> >> and IRETing to it will now be silently(!) broken. >> >> > >> >> > The .code16gcc directive is to support "gcc -m16". Any other purp= oses >> >> > are not supported. >> >> >> >> But you realize that people may use inline assembly? >> > >> > Inline assembly with the .code16gcc directive in an interrupt >> > handler? It is a supported usage? >> >> I don't know, but I see no reasons why it would not be. Note >> that I didn't mention "in an interrupt handler" - I can see uses >> for manually created frames to IRET to elsewhere. >> >> >> >> In fact I think the better solution would be to reject ambiguous >> >> >> code by demanding a suffix in all cases in .code16gcc mode. >> >> > >> >> > This may break existing codes. >> >> >> >> Of course, but breaking things at build time (with a proper >> >> diagnostic) that's better than silently breaking things at >> >> runtime. At the very least you can't claim it would break the >> >> supposedly common case, as that was already broken (and >> >> hence your fix). So the difference between suggested >> >> and current behavior is that right now there's silent latent >> >> breakage, whereas otherwise people would be made aware >> >> of there being a problem they need to address by changing >> >> some of their code. >> > >> > Assembler has no way to know if an assembly sequence is >> > correct and it shouldn't issue a warning for "gcc -m16" just >> > because the same instruction may be incorrect. >> >> I disagree: In this case, the assembler simply can't decide >> whether adding an operand size override is correct. Instead >> of silently doing the opposite of what has been done for >> many years, it should point out that it needs programmer >> guidance. >> >=20 > So the specific case is >=20 > 1. Programming in 16-bit mode with GCC using "gcc -m16". > 2. Manually create a 32-bit stack frame for a function with 32-bit iret. > 3. Implement such a function with .code16gcc and "iret". > 4. Jump to such a function. Yes. And note that the PUSHes potentially involved in step 2 default to 32-bit operand size, i.e. there's then a mixture of requirements as to whether suffixes are needed on the insns. In the end, just like for your address size override fix for scatter/gather insns a few weeks back, I think this is again a case which would better be fixed in the compiler (making it emit the needed suffix) than in the assembler. Jan