This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] x86: Initialize broadcast_op.bytes to 0
>>> 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.
Jan