[PATCH] x86: Disallow GOT memory as vector memory operand

Jan Beulich jbeulich@suse.com
Wed Feb 5 08:08:03 GMT 2025
On 04.02.2025 23:19, H.J. Lu wrote:
> GOT memory is 4-byte for i386 and 8-byte for x86-64.  Disallow GOT memory
> as vector memory operand.

This is going too far and not far enough at the same time.

Too far: Assembly programmers ought to be permitted to do whatever
they like. There may be an advanced mode where we warn about things
we deem bogus.

Not far enough: Why would non-vector operands of the wrong size be
okay? (Correcting this can then easily go too far, though: If
one is after a specific property of a GOT entry, TEST or CMP may,
for example, be used on just the high part [of whatever size] of
it.)

Overall: This is yet another arbitrary heuristic you're introducing.
I did complain about you doing such already in the past. Please
don't. Either there are firm, psABI-mandated criteria, or there
aren't. As said above, a default-off optional check may be okay.

Furthermore, why would you check the template for dword/qword?
Shouldn't this be the actual operand that we're processing? While
doing so has (afaict, without actually trying it out) the advantage
of still permitting broadcasts of the right size, imo that needs
dealing with separately.

> PR gas/32624
> * config/tc-i386.c (output_disp): Disallow GOT memory as vector
> memory operand.
> * testsuite/gas/i386/i386.exp: Run inval-got.
> * testsuite/gas/i386/inval-got.l: New file.
> * testsuite/gas/i386/inval-got.s: Likewise.
> * testsuite/gas/i386/x86-64-inval-got.l: Likewise.
> * testsuite/gas/i386/x86-64-inval-got.l: Likewise.
> * testsuite/gas/i386/x86-64.exp: Run x86-64-inval-got.

All you test is VPERMB. Imo for a code change like this, testing
needs to cover quite a few more insns / variants. Since it was
just the other day that you demanded that I add testcases to
patches, here's my (general) request: Testcases ought to be
providing good coverage for what needs testing. It is often not
sufficient to merely test the one special case that was pointed
out somewhere.

Jan


More information about the Binutils mailing list