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

[H.J. Lu]

On Tue, May 25, 2021 at 9:31 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, May 25, 2021 at 9:27 AM Jan Beulich <jbeulich@suse.com> wrote:
> >
> > 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).
>
> The odd behavior is correct for 16-bit codes.  My patch restores the
> old behavior only if input has 16-bit codes.
>

This is the patch I am going to check in.

-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v3-0001-x86-Propery-check-PC16-reloc-overflow-in-16-bit-m.patch
Type: text/x-patch
Size: 19302 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20210526/d6fdac2c/attachment-0001.bin>