PPC: undefweak dynamic relocs

Message ID 20210503050451.GE22624@bubble.grove.modra.org
State New
Headers show
Series
  • PPC: undefweak dynamic relocs
Related show

Commit Message

Jan Beulich via Binutils May 3, 2021, 5:04 a.m.
This makes the default for ppc to keep dynamic relocs on undefweak
symbols when the code won't cause DT_TEXTREL (for instance when -fPIE
or -fPIC).  If ld is given -z dynamic-undefined-weak then dynamic
relocations will be created for non-PIC at the expense of possibly
causing DT_TEXTREL to be set on ppc32.  Note that DT_TEXTREL and GNU
indirect functions are incompatible.

I'm still running tests on this one, so it might be a while before I
commit it.

	* elf32-ppc.c (allocate_dynrelocs): Keep dyn_relocs for undefweak
	symbols when -z dynamic-undefined-weak or when there are no
	dynamic relocs in read-only sections and -z nodynamic-undefined-weak
	is not given.
	* elf64-ppc.c (allocate_dynrelocs): Likewise.


-- 
Alan Modra
Australia Development Lab, IBM

Comments

Fangrui Song May 5, 2021, 8:13 p.m. | #1
On 2021-05-03, Alan Modra via Binutils wrote:
>This makes the default for ppc to keep dynamic relocs on undefweak

>symbols when the code won't cause DT_TEXTREL (for instance when -fPIE

>or -fPIC).  If ld is given -z dynamic-undefined-weak then dynamic

>relocations will be created for non-PIC at the expense of possibly

>causing DT_TEXTREL to be set on ppc32.  Note that DT_TEXTREL and GNU

>indirect functions are incompatible.

>

>I'm still running tests on this one, so it might be a while before I

>commit it.

>

>	* elf32-ppc.c (allocate_dynrelocs): Keep dyn_relocs for undefweak

>	symbols when -z dynamic-undefined-weak or when there are no

>	dynamic relocs in read-only sections and -z nodynamic-undefined-weak

>	is not given.

>	* elf64-ppc.c (allocate_dynrelocs): Likewise.

>

>diff --git a/bfd/elf32-ppc.c b/bfd/elf32-ppc.c

>index 2199465dbf8..b17954c7312 100644

>--- a/bfd/elf32-ppc.c

>+++ b/bfd/elf32-ppc.c

>@@ -5245,7 +5245,11 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)

>       /* For the non-pic case, discard space for relocs against

> 	 symbols which turn out to need copy relocs or are not

> 	 dynamic.  */

>-      if (h->dynamic_adjusted

>+      if ((h->dynamic_adjusted

>+	   || (h->ref_regular

>+	       && h->root.type == bfd_link_hash_undefweak

>+	       && (info->dynamic_undefined_weak > 0

>+		   || !_bfd_elf_readonly_dynrelocs (h))))

> 	  && !h->def_regular

> 	  && !ELF_COMMON_DEF_P (h)

> 	  && !(h->protected_def

>diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c

>index bc960bf8e9d..a4a05302a6a 100644

>--- a/bfd/elf64-ppc.c

>+++ b/bfd/elf64-ppc.c

>@@ -9808,7 +9808,11 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)

> 	 relocs against symbols which are not dynamic.  */

>       else if (h->type != STT_GNU_IFUNC)

> 	{

>-	  if (h->dynamic_adjusted

>+	  if ((h->dynamic_adjusted

>+	       || (h->ref_regular

>+		   && h->root.type == bfd_link_hash_undefweak

>+		   && (info->dynamic_undefined_weak > 0

>+		       || !_bfd_elf_readonly_dynrelocs (h))))

> 	      && !h->def_regular

> 	      && !ELF_COMMON_DEF_P (h))

> 	    {

>


This adds more complexity to the already complex logic "whether an undefined
weak needs dynamic relocation when .dynsym exists".

I have checked that x86-64 -no-pie a.o b.so doesn't use a dynamic
relocation in this case. With -z dynamic-undefined-weak, there is a
stray R_X86_64_NONE in .rela.dyn

I think a relocation type oblivious approach works equally well:
emit a dynamic relocation for -shared, suppress it for -no-pie and -pie.
(GNU ld doesn't have -no-pie yet)

https://maskray.me/blog/2021-04-25-weak-symbol#remarks
Jan Beulich via Binutils May 6, 2021, 2 a.m. | #2
On Wed, May 05, 2021 at 01:13:24PM -0700, Fangrui Song wrote:
> > @@ -9808,7 +9808,11 @@ allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)

> > 	 relocs against symbols which are not dynamic.  */

> >       else if (h->type != STT_GNU_IFUNC)

> > 	{

> > -	  if (h->dynamic_adjusted

> > +	  if ((h->dynamic_adjusted

> > +	       || (h->ref_regular

> > +		   && h->root.type == bfd_link_hash_undefweak

> > +		   && (info->dynamic_undefined_weak > 0

> > +		       || !_bfd_elf_readonly_dynrelocs (h))))

> > 	      && !h->def_regular

> > 	      && !ELF_COMMON_DEF_P (h))

> > 	    {

> > 

> 

> This adds more complexity to the already complex logic "whether an undefined

> weak needs dynamic relocation when .dynsym exists".


Yes.  Unfortunately can't be avoided without affecting other targets.
(A cleaner approach is to change _bfd_elf_adjust_dynamic_symbol so
that all undefined weak symbols are seen by
elf_backend_adjust_dynamic_symbol, but doing so blows up x86_64 and
likely other targets.  I tried.)

I spent a lot of time simplifying the powerpc logic over the years so,
believe me, I don't like complicating anything here.  But contrast
this code with the mind-boggling complexity in the equivalent x86 code
still using non_got_ref, a flag that changes its meaning part way
through the link process, and using a zero_undefweak flag set per
relocation type.

> I have checked that x86-64 -no-pie a.o b.so doesn't use a dynamic

> relocation in this case. With -z dynamic-undefined-weak, there is a

> stray R_X86_64_NONE in .rela.dyn

> 

> I think a relocation type oblivious approach works equally well:


That's what I have on powerpc

> emit a dynamic relocation for -shared, suppress it for -no-pie and -pie.


but not quite like this.  -shared and -pie are treated the same.

-- 
Alan Modra
Australia Development Lab, IBM

Patch

diff --git a/bfd/elf32-ppc.c b/bfd/elf32-ppc.c
index 2199465dbf8..b17954c7312 100644
--- a/bfd/elf32-ppc.c
+++ b/bfd/elf32-ppc.c
@@ -5245,7 +5245,11 @@  allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
       /* For the non-pic case, discard space for relocs against
 	 symbols which turn out to need copy relocs or are not
 	 dynamic.  */
-      if (h->dynamic_adjusted
+      if ((h->dynamic_adjusted
+	   || (h->ref_regular
+	       && h->root.type == bfd_link_hash_undefweak
+	       && (info->dynamic_undefined_weak > 0
+		   || !_bfd_elf_readonly_dynrelocs (h))))
 	  && !h->def_regular
 	  && !ELF_COMMON_DEF_P (h)
 	  && !(h->protected_def
diff --git a/bfd/elf64-ppc.c b/bfd/elf64-ppc.c
index bc960bf8e9d..a4a05302a6a 100644
--- a/bfd/elf64-ppc.c
+++ b/bfd/elf64-ppc.c
@@ -9808,7 +9808,11 @@  allocate_dynrelocs (struct elf_link_hash_entry *h, void *inf)
 	 relocs against symbols which are not dynamic.  */
       else if (h->type != STT_GNU_IFUNC)
 	{
-	  if (h->dynamic_adjusted
+	  if ((h->dynamic_adjusted
+	       || (h->ref_regular
+		   && h->root.type == bfd_link_hash_undefweak
+		   && (info->dynamic_undefined_weak > 0
+		       || !_bfd_elf_readonly_dynrelocs (h))))
 	      && !h->def_regular
 	      && !ELF_COMMON_DEF_P (h))
 	    {