Re: [PATCH] x86-64: always use unsigned 32-bit relocation for 32-bit addressing

["Jan Beulich" <JBeulich at suse dot com>]

>>> On 13.11.17 at 17:09, <hjl.tools@gmail.com> wrote:
> On Mon, Nov 13, 2017 at 8:01 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 13.11.17 at 16:44, <hjl.tools@gmail.com> wrote:
>>> On Mon, Nov 13, 2017 at 6:30 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>> On 13.11.17 at 14:35, <hjl.tools@gmail.com> wrote:
>>>>> On Mon, Nov 13, 2017 at 3:50 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> Except for %eip-relative addressing, where we don't have a suitable
>>>>>> relocation type silently wrapping at the 4G boundary, consistently
>>>>>> force use of R_X86_64_32 (in ELF terms) instead of its sign-extending
>>>>>> counterpart. This wasn't right in case there was no base register in
>>>>>> the addressing expression.
>>>>>
>>>>> Since displacement is signed, won't it generate different code?  Is there
>>>>> a bug report to show it is necessary?
>>>>
>>>> I'm afraid I don't understand the question: Signed-ness of
>>>> 32-bit quantities doesn't matter when talking of 32-bit addressing.
>>>> If you think something is wrong here, may I ask that you take the
>>>> tests being added and state what you would want to see be the
>>>> expected result(s)?
>>>>
>>>
>>> We currently have
>>>
>>> (BASE + INDEX * SCALE + DISP) & 0xffffffff
>>>
>>> You want to change it to
>>>
>>> (BASE + INDEX * SCALE + (DISP & 0xffffffff)) & 0xffffffff
>>>
>>> Am I correct?
>>
>> Yes, that's a way to put it. And it's another way to show that
>> there's no dependency on the sign bit here. IOW I still don't
>> see how you would see different (as in: wrong) code to be
>> generated. The point of the change merely is to prevent
>> overflow errors from the linker when processing relocations,
>> which signed relocations could trigger, but unsigned ones
>> can't (unless the target is above 4Gb of course).
> 
> Do you have a testcase to trigger linker error?

I could see to put one together.

> Have you tested this change to bootstrap/test GCC

Not yet.

> and build/test glibc in x32?

I don't think I'm going to do anything like this.

In the end I'm not going to have a very high threshold for
dropping the ball here and just keeping the change in my
private patch set. As in quite a few cases in the past - if you'd
rather keep the bug in the tree, then so be it. If 32-bit
addressing had been thought through properly when x86-64
support was introduced (or the latest when x32 support was
added), testcases pointing out this bug would have existed
for quite some time already.

I continue to dislike to attitude of making patch acceptance
dependent on the correction of past omissions. I can see this
being a nice-to-have, but it shouldn't be a hard requirement.

Jan