From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x42c.google.com (mail-pf1-x42c.google.com [IPv6:2607:f8b0:4864:20::42c]) by sourceware.org (Postfix) with ESMTPS id B08F13858D3C for ; Mon, 27 Sep 2021 14:40:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B08F13858D3C Received: by mail-pf1-x42c.google.com with SMTP id s16so16131820pfk.0 for ; Mon, 27 Sep 2021 07:40:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=yHLTtHa32oMh25R3iY9C5REpE6pkkLh0DseIEtK518Y=; b=fx1iLJAWkKmBvZKEU24/hJWccFlQRb3hKx4B0WJ9+uPf0si0E1JiPT8QOqjD/hUU+L kKhIL7eRIOyHsCF7S7MVetU7LoZrX9C/P14ZSY0sDF08Zewcmq2TM38nr0Nca7qeipmc QcrvvtzisAND+C3ap8WIsedxgfcQd7NeFXtGqwBotdY7M+56O2ihSgHDKK9x0z38qNQb NQgSoC6DkPx/qeJPrqhBLbAztgwMzelYBUernYbs07sYzhg3mi8OhdN+Ufm6QP7v0YB1 lMHhPPIZGLVB5WZYn+XU+wXSFIwuRYx/Pa/ZYClMDA6K1f+J3oM2FgMfE8a2/PdBCtvY D1Ew== X-Gm-Message-State: AOAM533VCR9d3BfLguvKOYfwFO7OvYk0P+pXqRPU+42XgB81RhOU0i9l v79eiKNFSRXV7sR6TCsEjvXFq9/M69hpMGHYAp+uprqs5pM= X-Google-Smtp-Source: ABdhPJxKVNtdkekBfT/jQxIuZtgeeKslwkzku+e2SV6wu7igRokO+xWO3jaebv6atPmCDp8GdsOVlREJQTVfv9pX/yQ= X-Received: by 2002:a62:2507:0:b0:447:a583:5a88 with SMTP id l7-20020a622507000000b00447a5835a88mr253307pfl.43.1632753619604; Mon, 27 Sep 2021 07:40:19 -0700 (PDT) MIME-Version: 1.0 References: <20210927093905.22496-1-lili.cui@intel.com> In-Reply-To: From: "H.J. Lu" Date: Mon, 27 Sep 2021 07:39:43 -0700 Message-ID: Subject: Re: [PATCH] x86: Print {bad} on invalid broadcast in OP_E_memory To: "Cui, Lili" Cc: Jan Beulich , "binutils@sourceware.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-3030.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Sep 2021 14:40:22 -0000 On Mon, Sep 27, 2021 at 4:28 AM Cui, Lili wrote: > > > -----Original Message----- > > From: Jan Beulich > > Sent: Monday, September 27, 2021 6:12 PM > > To: Cui, Lili > > Cc: binutils@sourceware.org; hjl.tools@gmail.com > > Subject: Re: [PATCH] x86: Print {bad} on invalid broadcast in OP_E_memo= ry > > > I've indicated so in the past already (perhaps not to you, but in > > general): I consider it wrong to add test cases with bogus expectations= . > > In the case here it's not only the broadcast that's invalid, but also t= he {%k7}. > > For your purpose you don't need the masking part, so I'd like to ask th= at you > > drop it. If and when we properly mark such bad uses of masking, a separ= ate > > test case covering that aspect will want adding, but the one you add no= w > > would then better not require adjusting. > > > Yes, we don=E2=80=99t need masking part here, I dropped it. Thanks. > > > Jan > > Subject: [PATCH] x86: Print {bad} on invalid broadcast in OP_E_memory > > Don't print broadcast for scalar_mode, and print {bad} for invalid broadc= ast. > > gas/ > > PR binutils/28381 > * testsuite/gas/i386/bad-bcast.s: Add a new testcase. > * testsuite/gas/i386/bad-bcast.d: Likewise. > > opcodes/ > > PR binutils/28381 > * i386-dis.c (static struct): Add no_broadcast. > (OP_E_memory): Mark invalid broadcast with no_broadcast=3D1 and P= rint "{bad}"for it. > (intel_operand_size): mark invalid broadcast with no_broadcast=3D= 1. > (OP_XMM): Mark scalar_mode with no_broadcast=3D1. > --- > gas/testsuite/gas/i386/bad-bcast.d | 10 +- > gas/testsuite/gas/i386/bad-bcast.s | 2 + > opcodes/i386-dis.c | 155 +++++++++++++++-------------- > 3 files changed, 88 insertions(+), 79 deletions(-) > > diff --git a/gas/testsuite/gas/i386/bad-bcast.d b/gas/testsuite/gas/i386/= bad-bcast.d > index 9fc474a42f..4f82925999 100644 > --- a/gas/testsuite/gas/i386/bad-bcast.d > +++ b/gas/testsuite/gas/i386/bad-bcast.d > @@ -7,8 +7,8 @@ > Disassembly of section .text: > > 0+ <.text>: > - +[a-f0-9]+: 62 .byte 0x62 > - +[a-f0-9]+: c3 ret > - +[a-f0-9]+: 8c 1d 66 90 66 90 mov %ds,0x90669066 > - +[a-f0-9]+: 66 90 xchg %ax,%ax > -#pass > + +[a-f0-9]+: 62 c3 8c 1d 66\s+\(bad\) > + +[a-f0-9]+: 90\s+nop > + +[a-f0-9]+: 66 90\s+xchg %ax,%ax > + +[a-f0-9]+: 66 90\s+xchg %ax,%ax > + +[a-f0-9]+: 62 c1 ff 38 2a 20\s+vcvtsi2sd \(%eax\){bad},%xmm0,%xmm4 > diff --git a/gas/testsuite/gas/i386/bad-bcast.s b/gas/testsuite/gas/i386/= bad-bcast.s > index 3e49b2238e..6c55dcbbbd 100644 > --- a/gas/testsuite/gas/i386/bad-bcast.s > +++ b/gas/testsuite/gas/i386/bad-bcast.s > @@ -1,3 +1,5 @@ > .text > # Invalid 16-bit broadcast with EVEX.W =3D=3D 1. > .byte 0x62, 0xc3, 0x8c, 0x1d, 0x66, 0x90, 0x66, 0x90, 0x66, 0x90 > +# Invalid vcvtsi2sd with EVEX.b =3D=3D 1. > + .byte 0x62,0xc1,0xff,0x38,0x2a,0x20 The bug report is for -Mintel. But there are no tests for it. Please add -Mintel tests for bad casts. > diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c > index aa292233d4..926f776de8 100644 > --- a/opcodes/i386-dis.c > +++ b/opcodes/i386-dis.c > @@ -2422,6 +2422,7 @@ static struct > int zeroing; > int ll; > int b; > + int no_broadcast; > } > vex; > static unsigned char need_vex; > @@ -11059,23 +11060,25 @@ intel_operand_size (int bytemode, int sizeflag) > { > if (vex.b) > { > - switch (bytemode) > - { > - case x_mode: > - case evex_half_bcst_xmmq_mode: > - if (vex.w) > - oappend ("QWORD PTR "); > - else > - oappend ("DWORD PTR "); > - break; > - case xh_mode: > - case evex_half_bcst_xmmqh_mode: > - case evex_half_bcst_xmmqdh_mode: > - oappend ("WORD PTR "); > - break; > - default: > - abort (); > - } > + if (!vex.no_broadcast) > + switch (bytemode) > + { > + case x_mode: > + case evex_half_bcst_xmmq_mode: > + if (vex.w) > + oappend ("QWORD PTR "); > + else > + oappend ("DWORD PTR "); > + break; > + case xh_mode: > + case evex_half_bcst_xmmqh_mode: > + case evex_half_bcst_xmmqdh_mode: > + oappend ("WORD PTR "); > + break; > + default: > + vex.no_broadcast =3D 1; > + break; > + } > return; > } > switch (bytemode) > @@ -11908,69 +11911,71 @@ OP_E_memory (int bytemode, int sizeflag) > if (vex.b) > { > evex_used |=3D EVEX_b_used; > - if (bytemode =3D=3D xh_mode) > - { > - if (vex.w) > - { > - oappend ("{bad}"); > - } > - else > - { > - switch (vex.length) > - { > - case 128: > - oappend ("{1to8}"); > - break; > - case 256: > - oappend ("{1to16}"); > - break; > - case 512: > - oappend ("{1to32}"); > - break; > - default: > - abort (); > - } > - } > - } > - else if (vex.w > - || bytemode =3D=3D evex_half_bcst_xmmqdh_mode > - || bytemode =3D=3D evex_half_bcst_xmmq_mode) > + if (!vex.no_broadcast) > { > - switch (vex.length) > + if (bytemode =3D=3D xh_mode) > { > - case 128: > - oappend ("{1to2}"); > - break; > - case 256: > - oappend ("{1to4}"); > - break; > - case 512: > - oappend ("{1to8}"); > - break; > - default: > - abort (); > + if (vex.w) > + oappend ("{bad}"); > + else > + { > + switch (vex.length) > + { > + case 128: > + oappend ("{1to8}"); > + break; > + case 256: > + oappend ("{1to16}"); > + break; > + case 512: > + oappend ("{1to32}"); > + break; > + default: > + abort (); > + } > + } > } > - } > - else if (bytemode =3D=3D x_mode > - || bytemode =3D=3D evex_half_bcst_xmmqh_mode) > - { > - switch (vex.length) > + else if (vex.w > + || bytemode =3D=3D evex_half_bcst_xmmqdh_mode > + || bytemode =3D=3D evex_half_bcst_xmmq_mode) > { > - case 128: > - oappend ("{1to4}"); > - break; > - case 256: > - oappend ("{1to8}"); > - break; > - case 512: > - oappend ("{1to16}"); > - break; > - default: > - abort (); > + switch (vex.length) > + { > + case 128: > + oappend ("{1to2}"); > + break; > + case 256: > + oappend ("{1to4}"); > + break; > + case 512: > + oappend ("{1to8}"); > + break; > + default: > + abort (); > + } > + } > + else if (bytemode =3D=3D x_mode > + || bytemode =3D=3D evex_half_bcst_xmmqh_mode) > + { > + switch (vex.length) > + { > + case 128: > + oappend ("{1to4}"); > + break; > + case 256: > + oappend ("{1to8}"); > + break; > + case 512: > + oappend ("{1to16}"); > + break; > + default: > + abort (); > + } > } > + else > + vex.no_broadcast =3D 1; > } > - else > - /* If operand doesn't allow broadcast, vex.b should be 0. */ > + if (vex.no_broadcast) > oappend ("{bad}"); > } > } > @@ -12685,6 +12690,8 @@ OP_XMM (int bytemode, int sizeflag ATTRIBUTE_UNUS= ED) > > if (bytemode =3D=3D tmm_mode) > modrm.reg =3D reg; > + else if (bytemode =3D=3D scalar_mode) > + vex.no_broadcast =3D 1; > > print_vector_reg (reg, bytemode); > } > -- > 2.17.1 > > Lili. > > > --=20 H.J.