This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] x86: don't wrongly scale non-8-bit displacements
On Mon, Jul 30, 2018 at 9:30 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 30.07.18 at 17:32, <hjl.tools@gmail.com> wrote:
>> 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
>
> Hmm, it's there in my local git tree, and it's also there according to
> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=commitdiff;h=629cfaf1b0fbb32a985607c774bd8e7870b9fa94
> Did I typo it somehow, albeit I'm sure I've simple copy-n-pasted
> what you wrote in your mail?
>
"PR gas/23465" must be in COMMIT message like:
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=1286ab788188bb8f0b0da2a3def4641fb3817f10
When git daemon saw "PR gas/23453" in commit log, it updated
https://sourceware.org/bugzilla/show_bug.cgi?id=23453
--
H.J.