[PATCH 2/5] ix86: tighten convert-load-reloc checking

[H.J. Lu]

On Tue, Feb 4, 2025 at 3:21 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 03.02.2025 23:34, H.J. Lu wrote:
> > On Mon, Feb 3, 2025 at 7:40 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> Just like was done recently for x86-64 (commit 4998f9ea9d35): Even if
> >> the assembler avoids using the relaxable relocation for inapplicable
> >> insns, the relocation type can still appear for other reasons. Be more
> >> thorough in the opcode checking we do, to avoid bogusly altering other
> >> insns.
> >>
> >> Furthermore correct an opcode mask (even if with the added condition
> >> that's now fully benign).
> >>
> >> --- a/bfd/elf32-i386.c
> >> +++ b/bfd/elf32-i386.c
> >> @@ -1453,15 +1453,16 @@ elf_i386_convert_load_reloc (bfd *abfd,
> >>                   modrm = 0xc0 | (modrm & 0x38) >> 3;
> >>                   opcode = 0xf7;
> >>                 }
> >> -             else
> >> +             else if ((opcode | 0x38) == 0x3b)
> >>                 {
> >>                   /* Convert "binop foo@GOT(%reg1), %reg2" to
> >>                      "binop $foo, %reg2".  */
> >> -                 modrm = (0xc0
> >> -                          | (modrm & 0x38) >> 3
> >> -                          | (opcode & 0x3c));
> >> +                 modrm = 0xc0 | ((modrm & 0x38) >> 3) | (opcode & 0x38);
> >>                   opcode = 0x81;
> >>                 }
> >> +             else
> >> +               return true;
> >> +
> >>               bfd_put_8 (abfd, modrm, contents + roff - 1);
> >>               r_type = R_386_32;
> >>             }
> >>
> >
> > Please add a testcase to show it makes a difference.
>
> The 64-bit counterpart has got a testcase demonstrating that it does.
> Counter request: Please could you have added an exhaustive testcase
> back at the time, demonstrating that no undue adjustments could ever
> be done?

If you find issues, please show them with tests.

>
> Underlying point: Experience tells me that adding (sensible) testcases
> usually takes much more time than making the actual code change. Hence
> why in a case like this I opted for not adding (another) one.
>

Then please postpone your change until you find time to write
some tests.


-- 
H.J.