x86: Incorrect vmgexit entry in prefix_table

Message ID 20200616141131.21396-1-hjl.tools@gmail.com
State New
Headers show
Series
  • x86: Incorrect vmgexit entry in prefix_table
Related show

Commit Message

Alan Modra via Binutils June 16, 2020, 2:11 p.m.
From: "Cui,Lili" <lili.cui@intel.com>


	* i386-dis.c (prefix_table): Delete the incorrect vmgexit.
---
 opcodes/i386-dis.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.26.2

Comments

Alan Modra via Binutils June 17, 2020, 2:02 p.m. | #1
On Tue, Jun 16, 2020 at 7:11 AM H.J. Lu via Binutils
<binutils@sourceware.org> wrote:
>

> From: "Cui,Lili" <lili.cui@intel.com>

>

>         * i386-dis.c (prefix_table): Delete the incorrect vmgexit.

> ---

>  opcodes/i386-dis.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c

> index 441866d6c97..55e595a4be0 100644

> --- a/opcodes/i386-dis.c

> +++ b/opcodes/i386-dis.c

> @@ -3577,7 +3577,7 @@ static const struct dis386 prefix_table[][4] = {

>      { "vmmcall",       { Skip_MODRM }, 0 },

>      { "vmgexit",       { Skip_MODRM }, 0 },

>      { Bad_Opcode },

> -    { "vmgexit",       { Skip_MODRM }, 0 },

> +    { Bad_Opcode },

>    },

>

>    /* PREFIX_0F01_REG_5_MOD_0 */

> --

> 2.26.2

>


This is the patch I am checking in.

-- 
H.J.
From 6fde587ff78a54b9e3bd70259de60cc5d7d8ced7 Mon Sep 17 00:00:00 2001
From: "Cui,Lili" <lili.cui@intel.com>
Date: Tue, 16 Jun 2020 07:11:31 -0700
Subject: [PATCH] x86: Delete incorrect vmgexit entry in prefix_table

	* i386-dis.c (prefix_table): Delete the incorrect vmgexit.
---
 opcodes/ChangeLog  | 4 ++++
 opcodes/i386-dis.c | 2 --
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog
index a6adf7126d..f604f6e3f7 100644
--- a/opcodes/ChangeLog
+++ b/opcodes/ChangeLog
@@ -1,3 +1,7 @@
+2020-06-17  Lili Cui  <lili.cui@intel.com>
+
+	* i386-dis.c (prefix_table): Delete the incorrect vmgexit.
+
 2020-06-14  H.J. Lu  <hongjiu.lu@intel.com>
 
 	PR gas/26115
diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
index 441866d6c9..6ac1d7416a 100644
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -3576,8 +3576,6 @@ static const struct dis386 prefix_table[][4] = {
   {
     { "vmmcall",	{ Skip_MODRM }, 0 },
     { "vmgexit",	{ Skip_MODRM }, 0 },
-    { Bad_Opcode },
-    { "vmgexit",	{ Skip_MODRM }, 0 },
   },
 
   /* PREFIX_0F01_REG_5_MOD_0 */
Jan Beulich June 17, 2020, 2:10 p.m. | #2
On 17.06.2020 16:02, H.J. Lu wrote:
> On Tue, Jun 16, 2020 at 7:11 AM H.J. Lu via Binutils

> <binutils@sourceware.org> wrote:

>>

>> From: "Cui,Lili" <lili.cui@intel.com>

>>

>>         * i386-dis.c (prefix_table): Delete the incorrect vmgexit.

>> ---

>>  opcodes/i386-dis.c | 2 +-

>>  1 file changed, 1 insertion(+), 1 deletion(-)

>>

>> diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c

>> index 441866d6c97..55e595a4be0 100644

>> --- a/opcodes/i386-dis.c

>> +++ b/opcodes/i386-dis.c

>> @@ -3577,7 +3577,7 @@ static const struct dis386 prefix_table[][4] = {

>>      { "vmmcall",       { Skip_MODRM }, 0 },

>>      { "vmgexit",       { Skip_MODRM }, 0 },

>>      { Bad_Opcode },

>> -    { "vmgexit",       { Skip_MODRM }, 0 },

>> +    { Bad_Opcode },

>>    },

>>

>>    /* PREFIX_0F01_REG_5_MOD_0 */

>> --

>> 2.26.2

>>

> 

> This is the patch I am checking in.


But both patches are wrong - as per the PM there are two encodings,
i.e. either prefix is valid. Afaics from the quoted text above (I
didn't get to see the original mail, presumably due to email issues
over here) there's no justification here at all why the second
entry would be invalid. Please revert.

Jan
Alan Modra via Binutils June 17, 2020, 2:11 p.m. | #3
On Wed, Jun 17, 2020 at 7:10 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> On 17.06.2020 16:02, H.J. Lu wrote:

> > On Tue, Jun 16, 2020 at 7:11 AM H.J. Lu via Binutils

> > <binutils@sourceware.org> wrote:

> >>

> >> From: "Cui,Lili" <lili.cui@intel.com>

> >>

> >>         * i386-dis.c (prefix_table): Delete the incorrect vmgexit.

> >> ---

> >>  opcodes/i386-dis.c | 2 +-

> >>  1 file changed, 1 insertion(+), 1 deletion(-)

> >>

> >> diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c

> >> index 441866d6c97..55e595a4be0 100644

> >> --- a/opcodes/i386-dis.c

> >> +++ b/opcodes/i386-dis.c

> >> @@ -3577,7 +3577,7 @@ static const struct dis386 prefix_table[][4] = {

> >>      { "vmmcall",       { Skip_MODRM }, 0 },

> >>      { "vmgexit",       { Skip_MODRM }, 0 },

> >>      { Bad_Opcode },

> >> -    { "vmgexit",       { Skip_MODRM }, 0 },

> >> +    { Bad_Opcode },

> >>    },

> >>

> >>    /* PREFIX_0F01_REG_5_MOD_0 */

> >> --

> >> 2.26.2

> >>

> >

> > This is the patch I am checking in.

>

> But both patches are wrong - as per the PM there are two encodings,

> i.e. either prefix is valid. Afaics from the quoted text above (I

> didn't get to see the original mail, presumably due to email issues

> over here) there's no justification here at all why the second

> entry would be invalid. Please revert.


Please add a testcase to verify it.

-- 
H.J.
Jan Beulich June 17, 2020, 2:17 p.m. | #4
On 17.06.2020 16:11, H.J. Lu wrote:
> On Wed, Jun 17, 2020 at 7:10 AM Jan Beulich <jbeulich@suse.com> wrote:

>>

>> On 17.06.2020 16:02, H.J. Lu wrote:

>>> On Tue, Jun 16, 2020 at 7:11 AM H.J. Lu via Binutils

>>> <binutils@sourceware.org> wrote:

>>>>

>>>> From: "Cui,Lili" <lili.cui@intel.com>

>>>>

>>>>         * i386-dis.c (prefix_table): Delete the incorrect vmgexit.

>>>> ---

>>>>  opcodes/i386-dis.c | 2 +-

>>>>  1 file changed, 1 insertion(+), 1 deletion(-)

>>>>

>>>> diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c

>>>> index 441866d6c97..55e595a4be0 100644

>>>> --- a/opcodes/i386-dis.c

>>>> +++ b/opcodes/i386-dis.c

>>>> @@ -3577,7 +3577,7 @@ static const struct dis386 prefix_table[][4] = {

>>>>      { "vmmcall",       { Skip_MODRM }, 0 },

>>>>      { "vmgexit",       { Skip_MODRM }, 0 },

>>>>      { Bad_Opcode },

>>>> -    { "vmgexit",       { Skip_MODRM }, 0 },

>>>> +    { Bad_Opcode },

>>>>    },

>>>>

>>>>    /* PREFIX_0F01_REG_5_MOD_0 */

>>>> --

>>>> 2.26.2

>>>>

>>>

>>> This is the patch I am checking in.

>>

>> But both patches are wrong - as per the PM there are two encodings,

>> i.e. either prefix is valid. Afaics from the quoted text above (I

>> didn't get to see the original mail, presumably due to email issues

>> over here) there's no justification here at all why the second

>> entry would be invalid. Please revert.

> 

> Please add a testcase to verify it.


I didn't at the time I added support for it primarily because
there's no way to sensibly express this to gas. And I'm very
hesitant to add .byte-based tests (except when wanting to test
the disassembler against invalid encodings) ... In any event
the bad commit needs reverting first, or any such test added
would fail. And also in any event it would have been prudent
to check why things are the way they are, before "fixing" them.
At that point one of the two of you could have decided to add
such a test to your liking.

Jan
Alan Modra via Binutils June 17, 2020, 2:32 p.m. | #5
On Wed, Jun 17, 2020 at 7:17 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> On 17.06.2020 16:11, H.J. Lu wrote:

> > On Wed, Jun 17, 2020 at 7:10 AM Jan Beulich <jbeulich@suse.com> wrote:

> >>

> >> On 17.06.2020 16:02, H.J. Lu wrote:

> >>> On Tue, Jun 16, 2020 at 7:11 AM H.J. Lu via Binutils

> >>> <binutils@sourceware.org> wrote:

> >>>>

> >>>> From: "Cui,Lili" <lili.cui@intel.com>

> >>>>

> >>>>         * i386-dis.c (prefix_table): Delete the incorrect vmgexit.

> >>>> ---

> >>>>  opcodes/i386-dis.c | 2 +-

> >>>>  1 file changed, 1 insertion(+), 1 deletion(-)

> >>>>

> >>>> diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c

> >>>> index 441866d6c97..55e595a4be0 100644

> >>>> --- a/opcodes/i386-dis.c

> >>>> +++ b/opcodes/i386-dis.c

> >>>> @@ -3577,7 +3577,7 @@ static const struct dis386 prefix_table[][4] = {

> >>>>      { "vmmcall",       { Skip_MODRM }, 0 },

> >>>>      { "vmgexit",       { Skip_MODRM }, 0 },

> >>>>      { Bad_Opcode },

> >>>> -    { "vmgexit",       { Skip_MODRM }, 0 },

> >>>> +    { Bad_Opcode },

> >>>>    },

> >>>>

> >>>>    /* PREFIX_0F01_REG_5_MOD_0 */

> >>>> --

> >>>> 2.26.2

> >>>>

> >>>

> >>> This is the patch I am checking in.

> >>

> >> But both patches are wrong - as per the PM there are two encodings,

> >> i.e. either prefix is valid. Afaics from the quoted text above (I

> >> didn't get to see the original mail, presumably due to email issues

> >> over here) there's no justification here at all why the second

> >> entry would be invalid. Please revert.

> >

> > Please add a testcase to verify it.

>

> I didn't at the time I added support for it primarily because

> there's no way to sensibly express this to gas. And I'm very

> hesitant to add .byte-based tests (except when wanting to test

> the disassembler against invalid encodings) ... In any event

> the bad commit needs reverting first, or any such test added

> would fail. And also in any event it would have been prudent

> to check why things are the way they are, before "fixing" them.

> At that point one of the two of you could have decided to add

> such a test to your liking.


All entries in dis-i386.c should have at least one test.  Currently
"make check" is clean.


-- 
H.J.
Jan Beulich June 17, 2020, 3:22 p.m. | #6
On 17.06.2020 16:32, H.J. Lu wrote:
> On Wed, Jun 17, 2020 at 7:17 AM Jan Beulich <jbeulich@suse.com> wrote:

>>

>> On 17.06.2020 16:11, H.J. Lu wrote:

>>> On Wed, Jun 17, 2020 at 7:10 AM Jan Beulich <jbeulich@suse.com> wrote:

>>>>

>>>> On 17.06.2020 16:02, H.J. Lu wrote:

>>>>> On Tue, Jun 16, 2020 at 7:11 AM H.J. Lu via Binutils

>>>>> <binutils@sourceware.org> wrote:

>>>>>>

>>>>>> From: "Cui,Lili" <lili.cui@intel.com>

>>>>>>

>>>>>>         * i386-dis.c (prefix_table): Delete the incorrect vmgexit.

>>>>>> ---

>>>>>>  opcodes/i386-dis.c | 2 +-

>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)

>>>>>>

>>>>>> diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c

>>>>>> index 441866d6c97..55e595a4be0 100644

>>>>>> --- a/opcodes/i386-dis.c

>>>>>> +++ b/opcodes/i386-dis.c

>>>>>> @@ -3577,7 +3577,7 @@ static const struct dis386 prefix_table[][4] = {

>>>>>>      { "vmmcall",       { Skip_MODRM }, 0 },

>>>>>>      { "vmgexit",       { Skip_MODRM }, 0 },

>>>>>>      { Bad_Opcode },

>>>>>> -    { "vmgexit",       { Skip_MODRM }, 0 },

>>>>>> +    { Bad_Opcode },

>>>>>>    },

>>>>>>

>>>>>>    /* PREFIX_0F01_REG_5_MOD_0 */

>>>>>> --

>>>>>> 2.26.2

>>>>>>

>>>>>

>>>>> This is the patch I am checking in.

>>>>

>>>> But both patches are wrong - as per the PM there are two encodings,

>>>> i.e. either prefix is valid. Afaics from the quoted text above (I

>>>> didn't get to see the original mail, presumably due to email issues

>>>> over here) there's no justification here at all why the second

>>>> entry would be invalid. Please revert.

>>>

>>> Please add a testcase to verify it.

>>

>> I didn't at the time I added support for it primarily because

>> there's no way to sensibly express this to gas. And I'm very

>> hesitant to add .byte-based tests (except when wanting to test

>> the disassembler against invalid encodings) ... In any event

>> the bad commit needs reverting first, or any such test added

>> would fail. And also in any event it would have been prudent

>> to check why things are the way they are, before "fixing" them.

>> At that point one of the two of you could have decided to add

>> such a test to your liking.

> 

> All entries in dis-i386.c should have at least one test.  Currently

> "make check" is clean.


And a test like what you ask for can't be added without breaking
it. Please undo the breakage, independent of the possible test
case addition (to be honest, I have better uses of my time).

Jan
Alan Modra via Binutils June 17, 2020, 3:42 p.m. | #7
On Wed, Jun 17, 2020 at 8:22 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> On 17.06.2020 16:32, H.J. Lu wrote:

> > On Wed, Jun 17, 2020 at 7:17 AM Jan Beulich <jbeulich@suse.com> wrote:

> >>

> >> On 17.06.2020 16:11, H.J. Lu wrote:

> >>> On Wed, Jun 17, 2020 at 7:10 AM Jan Beulich <jbeulich@suse.com> wrote:

> >>>>

> >>>> On 17.06.2020 16:02, H.J. Lu wrote:

> >>>>> On Tue, Jun 16, 2020 at 7:11 AM H.J. Lu via Binutils

> >>>>> <binutils@sourceware.org> wrote:

> >>>>>>

> >>>>>> From: "Cui,Lili" <lili.cui@intel.com>

> >>>>>>

> >>>>>>         * i386-dis.c (prefix_table): Delete the incorrect vmgexit.

> >>>>>> ---

> >>>>>>  opcodes/i386-dis.c | 2 +-

> >>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)

> >>>>>>

> >>>>>> diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c

> >>>>>> index 441866d6c97..55e595a4be0 100644

> >>>>>> --- a/opcodes/i386-dis.c

> >>>>>> +++ b/opcodes/i386-dis.c

> >>>>>> @@ -3577,7 +3577,7 @@ static const struct dis386 prefix_table[][4] = {

> >>>>>>      { "vmmcall",       { Skip_MODRM }, 0 },

> >>>>>>      { "vmgexit",       { Skip_MODRM }, 0 },

> >>>>>>      { Bad_Opcode },

> >>>>>> -    { "vmgexit",       { Skip_MODRM }, 0 },

> >>>>>> +    { Bad_Opcode },

> >>>>>>    },

> >>>>>>

> >>>>>>    /* PREFIX_0F01_REG_5_MOD_0 */

> >>>>>> --

> >>>>>> 2.26.2

> >>>>>>

> >>>>>

> >>>>> This is the patch I am checking in.

> >>>>

> >>>> But both patches are wrong - as per the PM there are two encodings,

> >>>> i.e. either prefix is valid. Afaics from the quoted text above (I

> >>>> didn't get to see the original mail, presumably due to email issues

> >>>> over here) there's no justification here at all why the second

> >>>> entry would be invalid. Please revert.

> >>>

> >>> Please add a testcase to verify it.

> >>

> >> I didn't at the time I added support for it primarily because

> >> there's no way to sensibly express this to gas. And I'm very

> >> hesitant to add .byte-based tests (except when wanting to test

> >> the disassembler against invalid encodings) ... In any event

> >> the bad commit needs reverting first, or any such test added

> >> would fail. And also in any event it would have been prudent

> >> to check why things are the way they are, before "fixing" them.

> >> At that point one of the two of you could have decided to add

> >> such a test to your liking.

> >

> > All entries in dis-i386.c should have at least one test.  Currently

> > "make check" is clean.

>

> And a test like what you ask for can't be added without breaking

> it. Please undo the breakage, independent of the possible test

> case addition (to be honest, I have better uses of my time).

>


Testcase does 2 things:

1. Verify implementation.
2. Prevent accidental change.

Without a testcase indicates it isn't important.  You can take your time.

-- 
H.J.
Jan Beulich June 17, 2020, 3:50 p.m. | #8
On 17.06.2020 17:42, H.J. Lu wrote:
> On Wed, Jun 17, 2020 at 8:22 AM Jan Beulich <jbeulich@suse.com> wrote:

>>

>> On 17.06.2020 16:32, H.J. Lu wrote:

>>> On Wed, Jun 17, 2020 at 7:17 AM Jan Beulich <jbeulich@suse.com> wrote:

>>>>

>>>> On 17.06.2020 16:11, H.J. Lu wrote:

>>>>> On Wed, Jun 17, 2020 at 7:10 AM Jan Beulich <jbeulich@suse.com> wrote:

>>>>>>

>>>>>> On 17.06.2020 16:02, H.J. Lu wrote:

>>>>>>> On Tue, Jun 16, 2020 at 7:11 AM H.J. Lu via Binutils

>>>>>>> <binutils@sourceware.org> wrote:

>>>>>>>>

>>>>>>>> From: "Cui,Lili" <lili.cui@intel.com>

>>>>>>>>

>>>>>>>>         * i386-dis.c (prefix_table): Delete the incorrect vmgexit.

>>>>>>>> ---

>>>>>>>>  opcodes/i386-dis.c | 2 +-

>>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)

>>>>>>>>

>>>>>>>> diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c

>>>>>>>> index 441866d6c97..55e595a4be0 100644

>>>>>>>> --- a/opcodes/i386-dis.c

>>>>>>>> +++ b/opcodes/i386-dis.c

>>>>>>>> @@ -3577,7 +3577,7 @@ static const struct dis386 prefix_table[][4] = {

>>>>>>>>      { "vmmcall",       { Skip_MODRM }, 0 },

>>>>>>>>      { "vmgexit",       { Skip_MODRM }, 0 },

>>>>>>>>      { Bad_Opcode },

>>>>>>>> -    { "vmgexit",       { Skip_MODRM }, 0 },

>>>>>>>> +    { Bad_Opcode },

>>>>>>>>    },

>>>>>>>>

>>>>>>>>    /* PREFIX_0F01_REG_5_MOD_0 */

>>>>>>>> --

>>>>>>>> 2.26.2

>>>>>>>>

>>>>>>>

>>>>>>> This is the patch I am checking in.

>>>>>>

>>>>>> But both patches are wrong - as per the PM there are two encodings,

>>>>>> i.e. either prefix is valid. Afaics from the quoted text above (I

>>>>>> didn't get to see the original mail, presumably due to email issues

>>>>>> over here) there's no justification here at all why the second

>>>>>> entry would be invalid. Please revert.

>>>>>

>>>>> Please add a testcase to verify it.

>>>>

>>>> I didn't at the time I added support for it primarily because

>>>> there's no way to sensibly express this to gas. And I'm very

>>>> hesitant to add .byte-based tests (except when wanting to test

>>>> the disassembler against invalid encodings) ... In any event

>>>> the bad commit needs reverting first, or any such test added

>>>> would fail. And also in any event it would have been prudent

>>>> to check why things are the way they are, before "fixing" them.

>>>> At that point one of the two of you could have decided to add

>>>> such a test to your liking.

>>>

>>> All entries in dis-i386.c should have at least one test.  Currently

>>> "make check" is clean.

>>

>> And a test like what you ask for can't be added without breaking

>> it. Please undo the breakage, independent of the possible test

>> case addition (to be honest, I have better uses of my time).

>>

> 

> Testcase does 2 things:

> 

> 1. Verify implementation.

> 2. Prevent accidental change.

> 

> Without a testcase indicates it isn't important.


Do you really want me to go through and point out what else has
no testcase, yet quite certainly _is_ important?

Jan
Alan Modra via Binutils June 17, 2020, 3:53 p.m. | #9
On Wed, Jun 17, 2020 at 8:50 AM Jan Beulich <jbeulich@suse.com> wrote:
>

> On 17.06.2020 17:42, H.J. Lu wrote:

> > On Wed, Jun 17, 2020 at 8:22 AM Jan Beulich <jbeulich@suse.com> wrote:

> >>

> >> On 17.06.2020 16:32, H.J. Lu wrote:

> >>> On Wed, Jun 17, 2020 at 7:17 AM Jan Beulich <jbeulich@suse.com> wrote:

> >>>>

> >>>> On 17.06.2020 16:11, H.J. Lu wrote:

> >>>>> On Wed, Jun 17, 2020 at 7:10 AM Jan Beulich <jbeulich@suse.com> wrote:

> >>>>>>

> >>>>>> On 17.06.2020 16:02, H.J. Lu wrote:

> >>>>>>> On Tue, Jun 16, 2020 at 7:11 AM H.J. Lu via Binutils

> >>>>>>> <binutils@sourceware.org> wrote:

> >>>>>>>>

> >>>>>>>> From: "Cui,Lili" <lili.cui@intel.com>

> >>>>>>>>

> >>>>>>>>         * i386-dis.c (prefix_table): Delete the incorrect vmgexit.

> >>>>>>>> ---

> >>>>>>>>  opcodes/i386-dis.c | 2 +-

> >>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)

> >>>>>>>>

> >>>>>>>> diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c

> >>>>>>>> index 441866d6c97..55e595a4be0 100644

> >>>>>>>> --- a/opcodes/i386-dis.c

> >>>>>>>> +++ b/opcodes/i386-dis.c

> >>>>>>>> @@ -3577,7 +3577,7 @@ static const struct dis386 prefix_table[][4] = {

> >>>>>>>>      { "vmmcall",       { Skip_MODRM }, 0 },

> >>>>>>>>      { "vmgexit",       { Skip_MODRM }, 0 },

> >>>>>>>>      { Bad_Opcode },

> >>>>>>>> -    { "vmgexit",       { Skip_MODRM }, 0 },

> >>>>>>>> +    { Bad_Opcode },

> >>>>>>>>    },

> >>>>>>>>

> >>>>>>>>    /* PREFIX_0F01_REG_5_MOD_0 */

> >>>>>>>> --

> >>>>>>>> 2.26.2

> >>>>>>>>

> >>>>>>>

> >>>>>>> This is the patch I am checking in.

> >>>>>>

> >>>>>> But both patches are wrong - as per the PM there are two encodings,

> >>>>>> i.e. either prefix is valid. Afaics from the quoted text above (I

> >>>>>> didn't get to see the original mail, presumably due to email issues

> >>>>>> over here) there's no justification here at all why the second

> >>>>>> entry would be invalid. Please revert.

> >>>>>

> >>>>> Please add a testcase to verify it.

> >>>>

> >>>> I didn't at the time I added support for it primarily because

> >>>> there's no way to sensibly express this to gas. And I'm very

> >>>> hesitant to add .byte-based tests (except when wanting to test

> >>>> the disassembler against invalid encodings) ... In any event

> >>>> the bad commit needs reverting first, or any such test added

> >>>> would fail. And also in any event it would have been prudent

> >>>> to check why things are the way they are, before "fixing" them.

> >>>> At that point one of the two of you could have decided to add

> >>>> such a test to your liking.

> >>>

> >>> All entries in dis-i386.c should have at least one test.  Currently

> >>> "make check" is clean.

> >>

> >> And a test like what you ask for can't be added without breaking

> >> it. Please undo the breakage, independent of the possible test

> >> case addition (to be honest, I have better uses of my time).

> >>

> >

> > Testcase does 2 things:

> >

> > 1. Verify implementation.

> > 2. Prevent accidental change.

> >

> > Without a testcase indicates it isn't important.

>

> Do you really want me to go through and point out what else has

> no testcase, yet quite certainly _is_ important?


There are many old entries without testcases.  I have been
adding them over time.  But all new entries should have at least
one testcase.

-- 
H.J.

Patch

diff --git a/opcodes/i386-dis.c b/opcodes/i386-dis.c
index 441866d6c97..55e595a4be0 100644
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -3577,7 +3577,7 @@  static const struct dis386 prefix_table[][4] = {
     { "vmmcall",	{ Skip_MODRM }, 0 },
     { "vmgexit",	{ Skip_MODRM }, 0 },
     { Bad_Opcode },
-    { "vmgexit",	{ Skip_MODRM }, 0 },
+    { Bad_Opcode },
   },
 
   /* PREFIX_0F01_REG_5_MOD_0 */