This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[PATCH] i386: Add .code16gcc fldenv tests
On Thu, Oct 31, 2019 at 10:26 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Oct 31, 2019 at 2:24 AM Jan Beulich <jbeulich@suse.com> wrote:
> >
> > On 31.10.2019 00:57, H.J. Lu wrote:
> > > On Wed, Oct 30, 2019 at 12:59 AM Jan Beulich <jbeulich@suse.com> wrote:
> > >>
> > >> On 29.10.2019 18:55, H.J. Lu wrote:
> > >>> On Mon, Oct 28, 2019 at 1:05 AM Jan Beulich <jbeulich@suse.com> wrote:
> > >>>>
> > >>>> Commit b76bc5d54e ("x86: don't default variable shift count insns to
> > >>>> 8-bit operand size") pointed out a very bad case, but the underlying
> > >>>> problem is, as mentioned on various occasions, much larger: Silently
> > >>>> selecting a (nowhere documented afaict) certain default operand size
> > >>>> when there's no "sizing" suffix and no suitable register operand(s) is
> > >>>> simply dangerous (for the programmer to make mistakes).
> > >>>>
> > >>>> While in Intel syntax mode such mistakes already lead to an error (which
> > >>>> is going to remain that way), AT&T syntax mode now gains warnings in
> > >>>> such cases by default, which can be suppressed or promoted to an error
> > >>>> if so desired by the programmer. Furthermore at least general purpose
> > >>>> insns now consistently have a default applied (alongside the warning
> > >>>> emission), rather than accepting some and refusing others.
> > >>>>
> > >>>> No warnings are (as before) to be generated for "DefaultSize" insns as
> > >>>> well as ones acting on selector and other fixed-width values. The set of
> > >>>> "DefaultSize" ones gets slightly widened for the purposes here.
> > >>>
> > >>> What is the advantage to add DefaultSize vs the alternative?
> > >>
> > >> I don't know what alternative you refer to; if you mean some
> > >> hypothetical one, then the advantage of simply adding
> > >> DefaultSize as done here is likely that it allows to not add or
> > >> further complicate logic in tc-i386*.c. Furthermore the ones which
> > >> get the attribute added should have had it already before, if the
> > >> comment "default insn size depends on mode" is to be trusted.
> > >>
> > >
> > > DefaultSize is added to some instructions and then they are excluded:
> > >
> > > + /* exclude jmp/ljmp */
> > > + && strcmp (i.tm.name, "jmp") && strcmp (i.tm.name, "ljmp")
> > > + /* exclude byte-displacement jumps */
> > > + && !i.tm.opcode_modifier.jumpbyte
> > > + /* exclude lgdt/lidt/sgdt/sidt */
> > > + && (i.tm.base_opcode != 0x0f01 || i.tm.extension_opcode > 3)
> > > /* exclude fldenv/frstor/fsave/fstenv */
> > > && i.tm.opcode_modifier.no_ssuf)
> > >
> > > It looks odd.
> >
> > But this isn't the only place where defaultsize gets evaluated.
> > See how lgdt/lidt/sgdt/sidt already have the attribute in the
> > opcode table, but need exclusion here now too. The alternative
> > would be two independent attributes - one to be evaluated here,
> > and the other to be evaluated further down in the function. Yet
> > again - this dual use has been there before, and just needs
> > suitable extending of the logic now.
> >
>
> Normally instructions with DefaultSize have i.suffix unset. Except with
> .code16gcc, which is used to support 16-bit mode with 32-bit address,
> i.suffix is set to 'l' for 32-bit address. However,
> iret/fldenv/frstor/fsave/fstenv
> are exceptions since they need 16-bit variants. So we need 2 different
> DefaultSize behaviors for .code16gcc, one uses LONG_MNEM_SUFFIX
> and the other uses WORD_MNEM_SUFFIX. We should update
> DefaultSize to properly encode iret/fldenv/frstor/fsave/fstenv for
> .code16gcc, instead of checking i.tm.opcode_modifier.no_ssuf.
> Something like this.
>
I checked in this patch to add .code16gcc fldenv tests.
--
H.J.
From f78d04905a457abde48c8f521ec2303e84683100 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 31 Oct 2019 10:42:04 -0700
Subject: [PATCH] i386; Add .code16gcc fldenv tests
* testsuite/gas/i386/general.s: Add .code16gcc fldenv tests.
* testsuite/gas/i386/general.l: Updated.
---
gas/ChangeLog | 5 +++++
gas/testsuite/gas/i386/general.l | 11 +++++++++--
gas/testsuite/gas/i386/general.s | 6 ++++++
3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/gas/ChangeLog b/gas/ChangeLog
index 25a504b374..fc99adce7f 100644
--- a/gas/ChangeLog
+++ b/gas/ChangeLog
@@ -1,3 +1,8 @@
+2019-10-31 H.J. Lu <hongjiu.lu@intel.com>
+
+ * testsuite/gas/i386/general.s: Add .code16gcc fldenv tests.
+ * testsuite/gas/i386/general.l: Updated.
+
2019-10-31 Mihail Ionescu <mihail.ionescu@arm.com>
* config/tc-arm.c (selected_ctx_ext_table) New static variable.
diff --git a/gas/testsuite/gas/i386/general.l b/gas/testsuite/gas/i386/general.l
index 17bf88f12d..ac17096020 100644
--- a/gas/testsuite/gas/i386/general.l
+++ b/gas/testsuite/gas/i386/general.l
@@ -282,5 +282,12 @@
216 0226 660FB6F8 movzb %al,%di
217 022a 0FB6C8 movzb %al,%ecx
218
- 219 # Force a good alignment.
- 220 022d 000000 .p2align 4,0
+ 219 .code16gcc
+ 220 # Use 16-bit layout by default for fldenv.
+ 221 022d 67D920 fldenv \(%eax\)
+ 222 0230 67D920 fldenvs \(%eax\)
+ 223 0233 6766D920 fldenvl \(%eax\)
+ 224
+ 225 # Force a good alignment.
+ 226 0237 00000000 00000000 .p2align 4,0
+ 226 00
diff --git a/gas/testsuite/gas/i386/general.s b/gas/testsuite/gas/i386/general.s
index a0ea660842..e4b2530e0d 100644
--- a/gas/testsuite/gas/i386/general.s
+++ b/gas/testsuite/gas/i386/general.s
@@ -216,5 +216,11 @@
movzb %al,%di
movzb %al,%ecx
+.code16gcc
+# Use 16-bit layout by default for fldenv.
+ fldenv (%eax)
+ fldenvs (%eax)
+ fldenvl (%eax)
+
# Force a good alignment.
.p2align 4,0
--
2.21.0