[5/5] ix86: restrict use of GOT32X relocs

Message ID aa3ba66c-cedb-4dc4-9cab-cabed4770e2b@suse.com
State New
Headers
Series x86: further GOT{,PCREL} related adjustments |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Jan Beulich Feb. 3, 2025, 11:42 a.m. UTC
  The linker rejects use of this reloc type without a base register for
PIC code. Suppress its use by gas in such cases.
---
The linker also rejects use of GOT32, but that's an issue the programmer
has to deal with. In the assembler we need to avoid doing something
wrong that the programmer has not explicitly asked for.
  

Comments

H.J. Lu Feb. 3, 2025, 10:41 p.m. UTC | #1
On Mon, Feb 3, 2025 at 7:42 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> The linker rejects use of this reloc type without a base register for
> PIC code. Suppress its use by gas in such cases.
> ---
> The linker also rejects use of GOT32, but that's an issue the programmer
> has to deal with. In the assembler we need to avoid doing something
> wrong that the programmer has not explicitly asked for.
>
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -12990,7 +12990,8 @@ output_disp (fragS *insn_start_frag, off
>                         }
>                     }
>                   else if (generate_relax_relocations
> -                          || (i.rm.mode == 0 && i.rm.regmem == 5))
> +                          ? (!shared || i.rm.mode != 0 || i.rm.regmem != 5)
> +                          : (!shared && i.rm.mode == 0 && i.rm.regmem == 5))
>                     fixP->fx_tcbit2 = 1;
>                 }
>             }
>

Please add a testcase to show it makes a difference.

Thanks.
  
Jan Beulich Feb. 4, 2025, 7:24 a.m. UTC | #2
On 03.02.2025 23:41, H.J. Lu wrote:
> On Mon, Feb 3, 2025 at 7:42 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> The linker rejects use of this reloc type without a base register for
>> PIC code. Suppress its use by gas in such cases.
>> ---
>> The linker also rejects use of GOT32, but that's an issue the programmer
>> has to deal with. In the assembler we need to avoid doing something
>> wrong that the programmer has not explicitly asked for.
>>
>> --- a/gas/config/tc-i386.c
>> +++ b/gas/config/tc-i386.c
>> @@ -12990,7 +12990,8 @@ output_disp (fragS *insn_start_frag, off
>>                         }
>>                     }
>>                   else if (generate_relax_relocations
>> -                          || (i.rm.mode == 0 && i.rm.regmem == 5))
>> +                          ? (!shared || i.rm.mode != 0 || i.rm.regmem != 5)
>> +                          : (!shared && i.rm.mode == 0 && i.rm.regmem == 5))
>>                     fixP->fx_tcbit2 = 1;
>>                 }
>>             }
> 
> Please add a testcase to show it makes a difference.

Such a testcase wouldn't fit my quality criteria, I'm afraid: It would
likely need to look for the specific diagnostic text, and that's what
I'm pretty sure you know I argue against when others try to add such
tests.

It'll also be in the linker testsuite, despite testing gas behavior.

All this said, I guess I'll think about making a testcase nevertheless.

Jan
  
H.J. Lu Feb. 4, 2025, 7:27 a.m. UTC | #3
On Tue, Feb 4, 2025 at 3:24 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 03.02.2025 23:41, H.J. Lu wrote:
> > On Mon, Feb 3, 2025 at 7:42 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> The linker rejects use of this reloc type without a base register for
> >> PIC code. Suppress its use by gas in such cases.
> >> ---
> >> The linker also rejects use of GOT32, but that's an issue the programmer
> >> has to deal with. In the assembler we need to avoid doing something
> >> wrong that the programmer has not explicitly asked for.
> >>
> >> --- a/gas/config/tc-i386.c
> >> +++ b/gas/config/tc-i386.c
> >> @@ -12990,7 +12990,8 @@ output_disp (fragS *insn_start_frag, off
> >>                         }
> >>                     }
> >>                   else if (generate_relax_relocations
> >> -                          || (i.rm.mode == 0 && i.rm.regmem == 5))
> >> +                          ? (!shared || i.rm.mode != 0 || i.rm.regmem != 5)
> >> +                          : (!shared && i.rm.mode == 0 && i.rm.regmem == 5))
> >>                     fixP->fx_tcbit2 = 1;
> >>                 }
> >>             }
> >
> > Please add a testcase to show it makes a difference.
>
> Such a testcase wouldn't fit my quality criteria, I'm afraid: It would
> likely need to look for the specific diagnostic text, and that's what
> I'm pretty sure you know I argue against when others try to add such
> tests.
>
> It'll also be in the linker testsuite, despite testing gas behavior.

That is true for this kind of changes.

> All this said, I guess I'll think about making a testcase nevertheless.
>
> Jan
  

Patch

--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -12990,7 +12990,8 @@  output_disp (fragS *insn_start_frag, off
 			}
 		    }
 		  else if (generate_relax_relocations
-			   || (i.rm.mode == 0 && i.rm.regmem == 5))
+			   ? (!shared || i.rm.mode != 0 || i.rm.regmem != 5)
+			   : (!shared && i.rm.mode == 0 && i.rm.regmem == 5))
 		    fixP->fx_tcbit2 = 1;
 		}
 	    }