[PATCH v2] x86: Propery check PC16 reloc overflow in 16-bit mode instructions

[Jan Beulich]

On 25.05.2021 18:11, H.J. Lu wrote:
> On Tue, May 25, 2021 at 9:08 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 25.05.2021 17:40, H.J. Lu wrote:
>>> On Tue, May 25, 2021 at 8:34 AM Jan Beulich <jbeulich@suse.com> wrote:
>>>>
>>>> On 25.05.2021 15:41, H.J. Lu wrote:
>>>>> On Mon, May 24, 2021 at 11:40 PM Jan Beulich <jbeulich@suse.com> wrote:
>>>>>>
>>>>>> On 25.05.2021 01:42, H.J. Lu via Binutils wrote:
>>>>>>> commit a7664973b24a242cd9ea17deb5eaf503065fc0bd
>>>>>>> Author: Jan Beulich <jbeulich@suse.com>
>>>>>>> Date:   Mon Apr 26 10:41:35 2021 +0200
>>>>>>>
>>>>>>>     x86: correct overflow checking for 16-bit PC-relative relocs
>>>>>>>
>>>>>>> caused linker failure when building 16-bit program in a 32-bit ELF
>>>>>>> container.  Update GNU_PROPERTY_X86_FEATURE_2_USED with
>>>>>>>
>>>>>>>  #define GNU_PROPERTY_X86_FEATURE_2_CODE16 (1U << 12)
>>>>>>>
>>>>>>> as in
>>>>>>>
>>>>>>> https://groups.google.com/g/x86-64-abi/c/UvvXWeHIGMA
>>>>>>>
>>>>>>> to indicate that 16-bit mode instructions are used in the object to
>>>>>>> allow linker to properly perform relocation overflow check for 16-bit
>>>>>>> PC-relative relocations in 16-bit mode instructions.
>>>>>>
>>>>>> I certainly don't mind the introduction of this flag; I think its
>>>>>> introduction wants to be separated from the specific use in the
>>>>>> linker, not the lease because ...
>>>>>>
>>>>>>> 1. Update x86 assembler to always generate the GNU property note with
>>>>>>> GNU_PROPERTY_X86_FEATURE_2_CODE16 for .code16 in ELF object.
>>>>>>> 2. Update i386 and x86-64 linkers to use 16-bit PC16 relocations if
>>>>>>> input object is marked with GNU_PROPERTY_X86_FEATURE_2_CODE16.
>>>>>>
>>>>>> ... I don't think this is an appropriate step to take. The majority
>>>>>> of cases of 16-bit code use that I know of is in projects where this
>>>>>> is just a small portion of code, and the rest of the code is 32-
>>>>>> and/or 64-bit. By taking mere presence of a tiny bit of 16-bit code
>>>>>> as indication to relax overflow checking, you undermine the main
>>>>>> goal of the earlier change.
>>>>>
>>>>> commit a7664973b24a242cd9ea17deb5eaf503065fc0bd
>>>>> Author: Jan Beulich <jbeulich@suse.com>
>>>>> Date:   Mon Apr 26 10:41:35 2021 +0200
>>>>>
>>>>>     x86: correct overflow checking for 16-bit PC-relative relocs
>>>>>
>>>>> is technically correct according to psABIs.  But GNU assembler
>>>>> only generates PC16 relocations for 16-bit codes.  That is why
>>>>> we never ran into any PC16 relocation overflow before.  My change
>>>>> restores the old behavior only when input has 16-bit codes.
>>>>
>>>> When paying specific attention to resulting code size, people may
>>>> want to encode XBEGIN with 16-bit operand size in 32- or 64-bit
>>>> code. This not having worked properly is what initially triggered
>>>> me touching this area. And of course assembler programmers could
>>>> even cause data to have PC16 relocations attached to it.
>>>
>>> As long as it doesn't .code16, PC16 relocation overflow check won't be
>>> changed.
>>
>> Of course, but I expect typical use cases (kernels, hypervisors) to
>> contain small pieces of 16-bit code.
> 
> And we never ran into any issues before.  My patch just restores
> the old behavior for them.

And my point is that the old behavior was wrong (and will be again if
we go with your change).

Jan