: Fix ld pr11138 FAILures on mips*.

The pr11138 testcase links an executable with a version script. On mips64-linux the presence of a version script was causing the MIPS_RLD_MAP dynamic tag to be populated with a NULL value. When such an executable was run ld.so would try to dereference this and receive SIGSEGV, thus killing the process.

The root cause of this is that the mips linker synthesizes a special symbol "__RLD_MAP", and then sets MIPS_RLD_MAP to point to it. When a version script is present, this symbol gets versioned along with all the rest, and when it is time to take its address, the symbol can no longer be found as it has had version information appended to its name.

Since "__RLD_MAP" is really part of the ABI, we want to exclude it from symbol versioning. To this end, I introduced a new symbol flag 'no_sym_version' to tag this type of symbol. When the "__RLD_MAP" symbol is created, we set this flag.

In _bfd_elf_link_assign_sym_version, we then skip all symbols that have 'no_sym_version' set, and everything now works.

This problem has also been reported in the wild when linking the firefox executable.

Tested on mips64-linux-gnu and x86_64-linux-gnu

Ok to commit?

2011-12-05 David Daney <[email protected]>

* elf-bfd.h (elf_link_hash_entry): Add no_sym_version field.
* elflink.c (_bfd_elf_link_assign_sym_version): Don't assign a version if no_sym_version is set.
* elfxx-mips.c (_bfd_mips_elf_create_dynamic_sections): Set no_sym_version for "__RLD_MAP".

Index: bfd/elf-bfd.h
===================================================================
RCS file: /cvs/src/src/bfd/elf-bfd.h,v
retrieving revision 1.329

diff -u -p -r1.329 elf-bfd.h
@@ -198,6 +198,8 @@ struct elf_link_hash_entry
   unsigned int pointer_equality_needed : 1;
   /* Symbol is a unique global symbol.  */
   unsigned int unique_global : 1;
+  /* Symbol should not be versioned.  It is part of the ABI */
+  unsigned int no_sym_version : 1;
 
   /* String table index in .dynstr if this is a dynamic symbol.  */
   unsigned long dynstr_index;

Index: bfd/elflink.c
===================================================================
RCS file: /cvs/src/src/bfd/elflink.c,v
retrieving revision 1.430

diff -u -p -r1.430 elflink.c
@@ -1946,6 +1946,9 @@ _bfd_elf_link_assign_sym_version (struct
   if (!h->def_regular)
     return TRUE;
 
+  if (h->no_sym_version)
+    return TRUE;
+
   bed = get_elf_backend_data (info->output_bfd);
   p = strchr (h->root.root.string, ELF_VER_CHR);
   if (p != NULL && h->verinfo.vertree == NULL)

Index: bfd/elfxx-mips.c
===================================================================
RCS file: /cvs/src/src/bfd/elfxx-mips.c,v
retrieving revision 1.296

diff -u -p -r1.296 elfxx-mips.c
@@ -7260,6 +7260,7 @@ _bfd_mips_elf_create_dynamic_sections (b
 	  h = (struct elf_link_hash_entry *) bh;
 	  h->non_elf = 0;
 	  h->def_regular = 1;
+	  h->no_sym_version = 1;
 	  h->type = STT_OBJECT;
 
 	  if (! bfd_elf_link_record_dynamic_symbol (info, h))

Why not just change

	  && (strcmp (name, "__rld_map") == 0
	      || strcmp (name, "__RLD_MAP") == 0))

to

	  && (strncmp (name, "__rld_map", 9) == 0
	      || strncmp (name, "__RLD_MAP", 9) == 0))

in _bfd_mips_elf_finish_dynamic_symbol? Perhaps the same for other syms there too?

Alan Modra
Australia Development Lab, IBM

Because that doesn't work. Perhpas I should have been a bit more detailed in my description of what is happening (at least in one case).

If the version script contains something like:

{
        global: main;
        local: *;
};

Then "__RLD_MAP" gets hidden and we never see it in _bfd_mips_elf_finish_dynamic_symbol().

This hiding gets done precisely in _bfd_elf_link_assign_sym_version() after the version information is calculated. So as the patch stands, we bail out of _bfd_elf_link_assign_sym_version() before the symbol is hidden (or modified in any way).

It is possible that 'no_sym_version' is not the best name for the flag, but I think we really some sort of flag to exclude ABI symbols from being mangled in _bfd_elf_link_assign_sym_version().

David Daney

I see. Well, the real question is whether this symbol needs to be dynamic. If it does, then you can't allow it to be hidden. That would best be accomplished by writing a mips elf_backend_hide_symbol function, which is nicer than adding another field used by only one target to struct elf_link_hash_entry.

Alan Modra
Australia Development Lab, IBM

Showing my ignorance here, but is that the usual behaviour for this kind of thing? I wouldn't have expected versions to apply to internally-created symbols.

There again, is this symbol (as opposed to the DT_MIPS_RLD_MAP tag) actually part of the ABI? I can't find any reference to it in the original psABI, the SGI ELF64 spec, gdb or glibc. If it's just an internal thing, maybe we could get rid of it altogether, or at least make it bind locally rather than globally.

Richard

I don't buy it, you are probably the most knowledgeable about this.

Yes, that is what I was trying to accomplish.

That is an option too I suppose. I would say that it is part of a de facto ABI if nothing else. The question of weather anybody uses it it a different question. I thought boehm-gc may have used it, but I cannot find it there now.

I don't know for sure why the symbol was created, but it seems like it may just be for the side effect of having _bfd_mips_elf_finish_dynamic_symbol() called. This lets us determine mips_elf_hash_table(info)->rld_value at a time when the output sections have already been laid out.

It might be possible to #define elf_backend_output_arch_local_syms and then handle calculation of the rld_value value there instead.

If this seems like a good approach, I can prepare and test a patch that does that.

David Daney

Yeah, good point. It occured to me rather belatedly that if wasn't part of the de facto ABI, it wouldn't have two distinct names...

So if we were just doing this for the tag, the simplest way would have been to find the .rld_map section in _bfd_mips_elf_finish_dynamic_sections. But I agree that we should keep the symbol Just In Case. So...

...how about caching the hash table entry for __rld_map/__RLD_MAP/ __rld_obj_head in mips_elf_link_hash_entry, instead of rld_value. (One field shared by all three should be enough.) We can then use that in _bfd_mips_elf_finish_dynamic_sections.

The code to set sym->st_value in _bfd_mips_elf_finish_dynamic_symbol should already be redundant: the symbol is defined as being at the start of .rld_map by:

	  s = bfd_get_section_by_name (abfd, ".rld_map");
	  BFD_ASSERT (s != NULL);

	  name = SGI_COMPAT (abfd) ? "__rld_map" : "__RLD_MAP";
	  bh = NULL;
	  if (!(_bfd_generic_link_add_one_symbol
		(info, abfd, name, BSF_GLOBAL, s, 0, NULL, FALSE,
		 get_elf_backend_data (abfd)->collect, &bh)))
	    return FALSE;

And the code to clear the first word should be redundant too, since _bfd_mips_elf_size_dynamic_sections uses bfd_zalloc to allocate the section's contents. So if we do cache the hash table entry, the whole _bfd_mips_elf_finish_dynamic_symbol handling should become dead.

Richard

That's right, also elsewhere we set the size of the section to 4, which is not correct for ELF64 targets.

Thanks Richard,

Those are all good ideas, so I think I will hack up a new patch to try and implement this plan. Some care also needs to be taken for the SGI case as there the symbol seems to come from one of the input objects rather than being synthesized by the linker.

David Daney

Normally this sort of symbol isn't dynamic, at least nowadays. Earlier versions of GNU ld made many symbols like _GLOBAL_OFFSET_TABLE_ dynamic unnecessarily, so it might be just a case of old mips code following even older practice.

Alan Modra
Australia Development Lab, IBM