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

[H.J. Lu]

On Tue, Feb 4, 2025 at 3:58 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 04.02.2025 08:26, H.J. Lu wrote:
> > 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.
>
> And I did, in the 64-bit counterpart. The code here is exactly the same
> as the 64-bit one (up to and including the wrong [but benign] use of
> 0x3c as a mask).

I also requested tests for the 64-bit change.

>
> >> 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.
>
> Ehem. Are you really asking to hold back bugfixes?
>

No test, no bug, no change.

-- 
H.J.