On 21.08.2024 17:10, Andrew Cooper wrote:
> On 21/08/2024 4:03 pm, Jan Beulich wrote:
>> On 21.08.2024 15:37, Andrew Cooper wrote:
>>> I think it would be helpful to at least note the fuzzing aspect in the
>>> commit message.
>> I've added "This way even for insns not covered by the test harness
>> fuzzers will have a chance of noticing issues, should any still exist
>> or new ones be introduced" to the 2nd paragraph.
>
> Acked-by: Andrew Cooper <[email protected]>
Thanks.
>>>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>>>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>>>> @@ -8114,13 +8114,13 @@ x86_emulate(
>>>> }
>>>> else if ( state->simd_size != simd_none )
>>>> {
>>>> - generate_exception_if(!op_bytes, X86_EXC_UD);
>>>> generate_exception_if((vex.opcx && (d & TwoOp) &&
>>>> (vex.reg != 0xf || (evex_encoded() &&
>>>> !evex.RX))),
>>>> X86_EXC_UD);
>>>>
>>>> - if ( !opc )
>>>> - BUG();
>>>> + EXPECT(op_bytes);
>>>> + EXPECT(opc);
>>> This is the only BUG() in x86_emulate.c, and it's right to get rid of it
>>> IMO.
>>>
>>> Therefore, we should have a hunk removing it from
>>> tools/tests/x86_emulator/x86-emulate.h too, which will prevent
>>> reintroduction.
>>>
>>> Maybe even undef BUG somewhere in x86_emulate/private.h?
>> Both of these actions can only be taken if the other BUG() in decode.c
>> also goes away. But yes, what you suggest is probably the best course of
>> action. I guess I'll do that in yet another patch, though.
>
> Is that BUG() local to your tree? I cant see it in staging.
I first thought it would be when you mentioned you found only one, but it's
been there for a long time[1], in VEX/EVEX prefix decoding. With a comment
added by you[2].
Jan
[1]
https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=11c35f84b53622c429071049b830bb9b7a880eff
[2]
https://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;h=933acddf74213f77a5adbb2c10ccd3f72634d456
