[PATCH v2] x86: Restore PC16 relocation overflow check

Michael Matz matz@suse.de
Mon May 31 13:04:07 GMT 2021


Hello,

On Mon, 31 May 2021, H.J. Lu wrote:

> > > > We can change the psABI.  But changes should make sense in the
> > > > abstract as well as being demanded by reality.  I think we have good
> > > > cause to make PC16 conforming now, but I don't think we have good
> > > > cause to make its semantic anything else from the obvious.  FWIW, my
> > > > patch would be
> > > >   https://gitlab.com/x86-psABIs/x86-64-ABI/-/merge_requests/22
> > > >
> > > > Do you also want _8 and _16 (zero-extended I guess)?  _PC8
> > > > (sign-extended)?
> > > >
> > >
> > > Since there are 16-bit applications which expect PC16 behavior in
> > > binutils 2.36, we can't change it without breaking them.
> >
> > Which change specifically would break them?  AFAICS any change in binutils
> > will still result in the same binary output, so can you please explain?
> 
> https://sourceware.org/bugzilla/show_bug.cgi?id=27905
> 
> > (I thought we are only talking about giving a meaning to PC16 in order to
> > be able to make the linker depend on and hence check certain invariants)
> >
> > > We can update psABI to add a new PC16 relocation and leave the current
> > > PC16 alone.
> >
> > Well, the current psABI makes PC16 use non-conforming, so strictly
> > speaking we can do there whatever we want.  And even if that note of
> > non-conformity were simply removed without any other change, it would
> > still suggest a signed number (because, well, it's PC- _relative_, hence
> > offset, hence signed).
> >
> > So, again, what will break if we make it explicit that it's a
> > sign-extended number?
> 
> It is not about the sign extension.

I will say it is, in so far as when sign-extending it overflows, whereas 
with zero-extension it doesn't.  So we have the typical case of a 1.5 
range value indeed, and can't require just sign-extension.

> It is about overflow behavior.  In 16-bit mode, PC16 can wrap around.

So, let's look at the example you had in the above PR:

[hjl@gnu-cfl-2 pr27905]$ cat rom.s 
	.code16gcc
	.text
	.section	.text.default_process_op.isra.0,"ax",@progbits
default_process_op.isra.0:
	ret
	.section	.text.mpt_scsi_process_op,"ax",@progbits
mpt_scsi_process_op:
	jmp	default_process_op.isra.0
...
.text.default_process_op.isra.0 0x737c : { *(.text.default_process_op.isra.0) }
.text.mpt_scsi_process_op 0xf869 : { *(.text.mpt_scsi_process_op) }

The offset between both sections is 0x84ed, which indeed can't be 
sign-extended to the same value in 16 bit.  But with wrap-around (and 
hence non-extension) it of course fits.  A real overflow would occur 
when the sections would be, say, 0x12345 bytes apart.

So, what we can say in the psABI is that the value should match the 
original value when either sign- or zero-extended.  That still rules out 
"real" overflows (and checking for that makes sense, because also at 
runtime this won't work in any mode), but allows for this vague-extension 
to be relied upon.  Would that work for you?

(FWIW: I don't think we should add a new PC16 variant, if it only allows 
for stricter checking; we should clarify what we have and make it allow 
exactly what we need but not more; an psABI change with it's usual 
repercussions just for checking purposes seems to be too much).


Ciao,
Michael.


More information about the Binutils mailing list