[PATCH] i386: Allow GOT32 relocations against ABS symbols

[PATCH] i386: Allow GOT32 relocations against ABS symbols

[H.J. Lu]

I am checking this into master branch and backporting it to 2.37/2.38
branches.

H.J.
---
GOT32 relocations are allowed since absolute value + addend is stored in
the GOT slot.

Tested on glibc 2.35 build with GCC 11.2 and -Os.

bfd/

	PR ld/28870
	* elfxx-x86.c (_bfd_elf_x86_valid_reloc_p): Also allow GOT32
	relocations.

ld/

	PR ld/28870
	* testsuite/ld-i386/i386.exp: Run pr28870.
	* testsuite/ld-i386/pr28870.d: New file.
	* testsuite/ld-i386/pr28870.s: Likewise.
---
 bfd/elfxx-x86.c                | 10 ++++++----
 ld/testsuite/ld-i386/i386.exp  |  1 +
 ld/testsuite/ld-i386/pr28870.d | 10 ++++++++++
 ld/testsuite/ld-i386/pr28870.s |  6 ++++++
 4 files changed, 23 insertions(+), 4 deletions(-)
 create mode 100644 ld/testsuite/ld-i386/pr28870.d
 create mode 100644 ld/testsuite/ld-i386/pr28870.s

diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
index 7ac2411fc80..d00dc45677b 100644
--- a/bfd/elfxx-x86.c
+++ b/bfd/elfxx-x86.c
@@ -1942,9 +1942,9 @@ _bfd_elf_x86_valid_reloc_p (asection *input_section,
       irel = *rel;
 
       /* Only allow relocations against absolute symbol, which can be
-	 resolved as absolute value + addend.  GOTPCREL relocations
-	 are allowed since absolute value + addend is stored in the
-	 GOT slot.  */
+	 resolved as absolute value + addend.  GOTPCREL and GOT32
+	 relocations are allowed since absolute value + addend is
+	 stored in the GOT slot.  */
       if (bed->target_id == X86_64_ELF_DATA)
 	{
 	  r_type &= ~R_X86_64_converted_reloc_bit;
@@ -1965,7 +1965,9 @@ _bfd_elf_x86_valid_reloc_p (asection *input_section,
       else
 	valid_p = (r_type == R_386_32
 		   || r_type == R_386_16
-		   || r_type == R_386_8);
+		   || r_type == R_386_8
+		   || r_type == R_386_GOT32
+		   || r_type == R_386_GOT32X);
 
       if (valid_p)
 	*no_dynreloc_p = true;
diff --git a/ld/testsuite/ld-i386/i386.exp b/ld/testsuite/ld-i386/i386.exp
index fe36b0fb533..82e14ab38d0 100644
--- a/ld/testsuite/ld-i386/i386.exp
+++ b/ld/testsuite/ld-i386/i386.exp
@@ -509,6 +509,7 @@ run_dump_test "pr27491-3"
 run_dump_test "pr27491-4"
 run_dump_test "dt-relr-1a"
 run_dump_test "dt-relr-1b"
+run_dump_test "pr28870"
 
 if { !([istarget "i?86-*-linux*"]
        || [istarget "i?86-*-gnu*"]
diff --git a/ld/testsuite/ld-i386/pr28870.d b/ld/testsuite/ld-i386/pr28870.d
new file mode 100644
index 00000000000..8e9b9fb82eb
--- /dev/null
+++ b/ld/testsuite/ld-i386/pr28870.d
@@ -0,0 +1,10 @@
+#as: --32
+#ld: --no-dynamic-linker -m elf_i386 -pie
+#readelf: -s --wide
+
+#...
+Symbol table '.symtab' contains [0-9]+ entries:
+   Num:    Value  Size Type    Bind   Vis      Ndx Name
+#...
+ +[a-f0-9]+: 00000002     0 NOTYPE  LOCAL  DEFAULT  ABS foo
+#pass
diff --git a/ld/testsuite/ld-i386/pr28870.s b/ld/testsuite/ld-i386/pr28870.s
new file mode 100644
index 00000000000..8e320470e2d
--- /dev/null
+++ b/ld/testsuite/ld-i386/pr28870.s
@@ -0,0 +1,6 @@
+	.text
+	.globl _start
+_start:
+	addl	foo@GOT(%ebx), %eax
+	cmpl	$0, foo@GOT(%ebx)
+foo = 2
-- 
2.34.1

Re: [PATCH] i386: Allow GOT32 relocations against ABS symbols

[Jan Beulich]

On 08.02.2022 01:03, H.J. Lu via Binutils wrote:
> GOT32 relocations are allowed since absolute value + addend is stored in
> the GOT slot.

Before permitting this and with you specifically mentioning addends
here, I think it needs to be clarified what ...

> --- /dev/null
> +++ b/ld/testsuite/ld-i386/pr28870.s
> @@ -0,0 +1,6 @@
> +	.text
> +	.globl _start
> +_start:
> +	addl	foo@GOT(%ebx), %eax
> +	cmpl	$0, foo@GOT(%ebx)
> +foo = 2

... missing (here) variants thereof actually mean (and whether the use
of addends actually works as intended):

	mov	$v@got+3, %eax				# bogus, meaningless operand
	mov	$(v+3)@got, %eax			# wrong (gets treated same as above)

Comments are my own, preliminary judgement.

Jan

Re: [PATCH] i386: Allow GOT32 relocations against ABS symbols

[H.J. Lu]

On Tue, Feb 8, 2022 at 12:37 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 08.02.2022 01:03, H.J. Lu via Binutils wrote:
> > GOT32 relocations are allowed since absolute value + addend is stored in
> > the GOT slot.
>
> Before permitting this and with you specifically mentioning addends
> here, I think it needs to be clarified what ...
>
> > --- /dev/null
> > +++ b/ld/testsuite/ld-i386/pr28870.s
> > @@ -0,0 +1,6 @@
> > +     .text
> > +     .globl _start
> > +_start:
> > +     addl    foo@GOT(%ebx), %eax
> > +     cmpl    $0, foo@GOT(%ebx)
> > +foo = 2
>
> ... missing (here) variants thereof actually mean (and whether the use
> of addends actually works as intended):
>
>         mov     $v@got+3, %eax                          # bogus, meaningless operand
>         mov     $(v+3)@got, %eax                        # wrong (gets treated same as above)
>
> Comments are my own, preliminary judgement.

GOT relocations should only be used as @got or @got(%reg).
Other variants are undefined.

-- 
H.J.

Re: [PATCH] i386: Allow GOT32 relocations against ABS symbols

[Jan Beulich]

On 08.02.2022 14:02, H.J. Lu wrote:
> On Tue, Feb 8, 2022 at 12:37 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 08.02.2022 01:03, H.J. Lu via Binutils wrote:
>>> GOT32 relocations are allowed since absolute value + addend is stored in
>>> the GOT slot.
>>
>> Before permitting this and with you specifically mentioning addends
>> here, I think it needs to be clarified what ...
>>
>>> --- /dev/null
>>> +++ b/ld/testsuite/ld-i386/pr28870.s
>>> @@ -0,0 +1,6 @@
>>> +     .text
>>> +     .globl _start
>>> +_start:
>>> +     addl    foo@GOT(%ebx), %eax
>>> +     cmpl    $0, foo@GOT(%ebx)
>>> +foo = 2
>>
>> ... missing (here) variants thereof actually mean (and whether the use
>> of addends actually works as intended):
>>
>>         mov     $v@got+3, %eax                          # bogus, meaningless operand
>>         mov     $(v+3)@got, %eax                        # wrong (gets treated same as above)
>>
>> Comments are my own, preliminary judgement.
> 
> GOT relocations should only be used as @got or @got(%reg).
> Other variants are undefined.

Which raises two questions:

1) Why did you say "absolute value + addend is stored in the GOT slot" in
the description? Where would an addend come from if it can't be specified
in the assembly source?

2) Why isn't the assembler at least warning about such undefined uses?

Jan

Re: [PATCH] i386: Allow GOT32 relocations against ABS symbols

[H.J. Lu]

On Tue, Feb 8, 2022 at 5:11 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 08.02.2022 14:02, H.J. Lu wrote:
> > On Tue, Feb 8, 2022 at 12:37 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 08.02.2022 01:03, H.J. Lu via Binutils wrote:
> >>> GOT32 relocations are allowed since absolute value + addend is stored in
> >>> the GOT slot.
> >>
> >> Before permitting this and with you specifically mentioning addends
> >> here, I think it needs to be clarified what ...
> >>
> >>> --- /dev/null
> >>> +++ b/ld/testsuite/ld-i386/pr28870.s
> >>> @@ -0,0 +1,6 @@
> >>> +     .text
> >>> +     .globl _start
> >>> +_start:
> >>> +     addl    foo@GOT(%ebx), %eax
> >>> +     cmpl    $0, foo@GOT(%ebx)
> >>> +foo = 2
> >>
> >> ... missing (here) variants thereof actually mean (and whether the use
> >> of addends actually works as intended):
> >>
> >>         mov     $v@got+3, %eax                          # bogus, meaningless operand
> >>         mov     $(v+3)@got, %eax                        # wrong (gets treated same as above)
> >>
> >> Comments are my own, preliminary judgement.
> >
> > GOT relocations should only be used as @got or @got(%reg).
> > Other variants are undefined.
>
> Which raises two questions:
>
> 1) Why did you say "absolute value + addend is stored in the GOT slot" in
> the description? Where would an addend come from if it can't be specified
> in the assembly source?

addend came from GOTPCREL relocation.   There is no addend for
GOT32.

> 2) Why isn't the assembler at least warning about such undefined uses?

A warning is useful.

-- 
H.J.