This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


>>> 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.

Jan



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]