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

[Jan Beulich]

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.

>  I think it
> is a good compromise between usability and error checking.

Maybe it is; I'm not convinced though as of yet.

Jan