This is the mail archive of the mailing list for the binutils project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [ARM] PR ld/21402, only override the symbol dynamic decision on undefined weak symbol

Hi Jiong,

Thanks for your review! Here is my explanation below.

diff --git a/ld/testsuite/ld-arm/tls-app.r b/ld/testsuite/ld-arm/tls-app.r
index b156d52..518c18c 100644
--- a/ld/testsuite/ld-arm/tls-app.r
+++ b/ld/testsuite/ld-arm/tls-app.r
@@ -3,8 +3,5 @@

 OFFSET   TYPE              VALUE
-[0-9a-f]+ R_ARM_TLS_DTPMOD32  app_gd
-[0-9a-f]+ R_ARM_TLS_DTPOFF32  app_gd
 [0-9a-f]+ R_ARM_TLS_DTPMOD32  lib_gd
 [0-9a-f]+ R_ARM_TLS_DTPOFF32  lib_gd
-[0-9a-f]+ R_ARM_TLS_TPOFF32  app_ie

For this test case
GOT_TLS_GD relocation type against app_gd.
GOT_TLS_IE relocation type against app_ie.

The relocations for those two symbols in GOT are omitted because they are not dynamic symbols and no relocation are needed.

For example for app_gd case. The following code snippet shows the result.
	    if ((bfd_link_pic (info) || indx != 0)
		&& (h == NULL
		    || ELF_ST_VISIBILITY (h->other) == STV_DEFAULT
		    || h->root.type != bfd_link_hash_undefweak))
		need_relocs = TRUE;
		BFD_ASSERT (srelgot != NULL);
need_relocs is FALSE in this case, and
		    /* If we are not emitting relocations for a
		       general dynamic reference, then we must be in a
		       static link or an executable link with the
		       symbol binding locally.  Mark it as belonging
		       to module 1, the executable.  */
		    bfd_put_32 (output_bfd, 1,
				sgot->contents + cur_off);
		    bfd_put_32 (output_bfd, value - dtpoff_base (info),
				sgot->contents + cur_off + 4);

That is the same case for aarch64.

diff --git a/ld/testsuite/ld-arm/unresolved-1-dyn.d b/ld/testsuite/ld-arm/unresolved-1-dyn.d
index 21cd959..529da37 100644
--- a/ld/testsuite/ld-arm/unresolved-1-dyn.d
+++ b/ld/testsuite/ld-arm/unresolved-1-dyn.d
@@ -5,4 +5,4 @@

 Relocation section '\.rel\.dyn' .*
  Offset .*
-.* R_ARM_GLOB_DAT +00000000 +foo
+^.*  00000000 R_ARM_NONE.+

Initially, symbol FOO will request a slot in GOT table.
Once the dynamic section is created, it will reserve a slot for GOT entry's R_ARM_GLOB_DAT relocation. It's initialized to zero. And this is the same value for R_ARM_NONE relocation.

At the moment, the srelgot->reloc_count is not synchronized with srelgot->size.
In this particular case:
srelgot->reloc_count == 0
srelgot->size == 8

Originally, when the symbol FOO is recored as dynamic symbol. A R_ARM_GLOB_DAT will be create against its GOT slot. And this relocation will use the space reserved above as the srelgot->reloc_count is zero. That's why there is only a R_ARM_GLOB_DAT relocation for foo in the relocation table before the change here.

After the change, FOO is not put into the dynamic symbol table. And there is no R_ARM_RELATIVE elocation created for it as we don't create this for non-pic case.

And I think, when readelf dump the relocation table, the only the size is used. And 0s are dumped as R_ARM_NONE.

It looks this part could be cleaned a little bit.


On 27/09/17 10:07, Jiong Wang wrote:
     * testsuite/ld-arm/tls-app.d: Update address.
     * testsuite/ld-arm/tls-app.r: Remove relocations.

Hi Renlin,

   I have concerns on these testcases changes.

   They look not like something straightforward to understand.

   Could you explain why some dynamic TLS relocations are removed? This
patch doesn't touch TLS relaxation, so how could some dynamic TLS
relocations been resolved at static linking time?

   My concern is this patch has break some internal logic that caused
runtime TLS relocations missing, this will cause wrong tls address

  >     * testsuite/ld-arm/unresolved-1-dyn.d: Update.

    This looks to me is redundant dynamic relocation that the linker
should not generate it at all.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]