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

[H.J. Lu]

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.  I think it
is a good compromise between usability and error checking.

> Ideally we'd have annotations indicating the boundaries of 16-bit
> code, e.g. along the lines of Arm's Arm/Thumb/data markers. An
> intermediate workaround (which I still wouldn't like, but I
> thought I'd mention it) would be to have a flag indicating use of
> the problematic relocation _in 16-bit code_. (I don't like this
> because it's rather likely for the flag to get set, as an %ip-
> relative JMP or CALL is pretty likely to be present in such code,
> and hence the original purpose of the earlier change would still
> be undermined.)
>
> I can't easily tell whether the SeaBIOS code triggering the issue
> Martin has reported is _pure_ 16-bit code. If it is, for this
> sub-case an almost-clean solution would be possible: Mark objects
> containing _only_ 16-bit code as such, and AND together the
> individual flags when linking. This is only "almost-clean" since
> data sections could still be meant for 32-bit consumption. But I
> view chances of such actually happening as relatively slim.
>
> On the whole I view your suggestion (in the bugzilla entry) to
> extend -z noreloc-overflow as the more viable one, at least for
> now. This would express a developer's explicit consent to be
> responsible themselves for avoiding any overflows.

-z noreloc-overflow isn't transparent.  We shouldn't error PC16
relocation overflow in 16-bit codes when there are none.

Here is the v2 patch to fix compilation errors.

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