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

[Jan Beulich]

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?

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.

Jan