Re: [PATCH] x86: Initialize broadcast_op.bytes to 0

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Thu, Jul 26, 2018 at 9:12 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 26.07.18 at 18:01, <hjl.tools@gmail.com> wrote:
>> On Thu, Jul 26, 2018 at 8:57 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 26.07.18 at 17:51, <hjl.tools@gmail.com> wrote:
>>>> On Thu, Jul 26, 2018 at 8:02 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>> Also, wouldn't you better clear ->bytes again in case the function later
>>>>>> returns an error, leading to the next template to be looked at?
>>>>>
>>>>> Good point.  I will take a look.
>>>>>
>>>>
>>>> This is what I checked in.
>>>
>>> Is that enough? This sets the value to zero once while parsing aiui,
>>> but not between multiple attempts to match the parsing result
>>> against templates.
>>>
>>
>> It should be sufficient. Do you have a testcase to show otherwise?
>
> I don't have a testcase, I'm more afraid of this being a latent bug (i.e.
> for someone to hit with future changes, just like I've recently had to
> fix a long standing issue with found_reverse_match; patch yet to be
> posted).
>
>> broadcast_op is referenced only by
>>
>>               broadcast_op.type = bcst_type;
>>               broadcast_op.operand = this_operand;
>>               broadcast_op.bytes = 0;
>>               i.broadcast = &broadcast_op;
>>
>> broadcast_op.bytes is initialized to 0 when broadcast_op.type
>> is set.
>
> That's all fine, but not to the point. If you fail matching against
> a template after having set the field to non-zero already, the
> matching attempt against the next template (if any) is going to
> have this value already set, even if perhaps the template does
> not allow for broadcast.

I don't believe it will happen.  i.broadcast is initialized to zero
before parsing each instruction.  It is set to &broadcast_op only
when there is a broadcast operand.


-- 
H.J.