[PATCH] x86: don't wrongly scale non-8-bit displacements

[H.J. Lu]

On Mon, Jul 30, 2018 at 8:26 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 30.07.18 at 16:40, <hjl.tools@gmail.com> wrote:
>> On Mon, Jul 30, 2018 at 6:37 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 30.07.18 at 15:19, <hjl.tools@gmail.com> wrote:
>>>> On Mon, Jul 30, 2018 at 6:13 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>>> On 30.07.18 at 15:02, <hjl.tools@gmail.com> wrote:
>>>>>> On Mon, Jul 30, 2018 at 12:41 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>> In commit b5014f7af2 I've removed (instead of replaced) a conditional,
>>>>>>> resulting in addressing forms not allowing 8-bit displacements to now
>>>>>>> get their displacements scaled under certain circumstances. Re-add the
>>>>>>> missing conditional.
>>>>>>>
>>>>>>> gas/
>>>>>>> 2018-07-30  Jan Beulich  <jbeulich@suse.com>
>>>>>>>
>>>>>>>         * config/tc-i386.c (output_disp): Restrict scaling.
>>>>>>>         * testsuite/gas/i386/evex-no-scale.s,
>>>>>>>           testsuite/gas/i386/evex-no-scale-32.d
>>>>>>>           testsuite/gas/i386/evex-no-scale-64.d: New.
>>>>>>>         * testsuite/gas/i386/i386.exp: Run new tests.
>>>>>>>
>>>>>>> --- a/gas/config/tc-i386.c
>>>>>>> +++ b/gas/config/tc-i386.c
>>>>>>> @@ -7965,7 +7965,8 @@ output_disp (fragS *insn_start_frag, off
>>>>>>>               int size = disp_size (n);
>>>>>>>               offsetT val = i.op[n].disps->X_add_number;
>>>>>>>
>>>>>>> -             val = offset_in_range (val >> i.memshift, size);
>>>>>>> +             val = offset_in_range (val >> (size == 1 ? i.memshift : 0),
>>>>>>> +                                    size);
>>>>>>>               p = frag_more (size);
>>>>>>>               md_number_to_chars (p, val, size);
>>>>>>>             }
>>>>>>
>>>>>> Please open a binutils bug against binutils 2.31 first  and mention
>>>>>> it in ChangeLog.   OK with that change.
>>>>>
>>>>> I'm sorry, but no - I'm not going to waste time writing bug reports
>>>>> just for them to be closed again right away. I also don't think bug
>>>>> fixes should be blocked upon such a requirement.
>>>>>
>>>>
>>>> Your patch changes assembler behavior to fix a regression in
>>>> the released binutils.  All regressions should be tracked in
>>>>
>>>> https://sourceware.org/bugzilla/
>>>>
>>>> It isn't a waste of time.
>>>
>>> If there are people who care about such tracking, then they're fine
>>> creating an entry. Doing so is certainly a waste of _my_ time, and I
>>> find it (once again) strange that bug fixes get blocked by non-
>>> technical side issues. In the end you'd force me to hold back fixes
>>> like this (until whoever else runs into it and it gets fixed it after
>>> reporting through bugzilla), including the reporting of them.
>>>
>>
>> Please add "PR gas/23465" to ChangeLog entry. OK with that change
>> for master.
>
> Done.

You should have added "PR gas/23465" to commit log so that

https://sourceware.org/bugzilla/show_bug.cgi?id=23465

will be updated automatically.  It is too late for master branch.  But
please do it when you backport it to 2.31 branch.

>> Please also backport it to 2.31 branch.
>
> Nick, is that okay?
>
> Jan
>
>



-- 
H.J.