[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Bug#844357: binutils: mips* mesa libGL.so.1 contains an invalid symbol table



Hi,

On 16/11/16 17:15, James Cowgill wrote:
> Control: forwarded -1 https://sourceware.org/bugzilla/show_bug.cgi?id=20828
> 
> Hi,
> 
> I've submitted the bug upstream with a reduced testcase and a bisection.
> 
> As the bug requires -Wl,--gc-sections to occur, it may be possible to
> workaround in mesa by recompiling without it.

I have attached two patches:

binutils-pr20828-v1.patch attempts to fix the underlying issue in
binutils where local symbols appear intermixed with global symbols. It
does this by applying another pass over the symbol table to assign all
the local symbol indexes first, then ignoring local symbols in the
second pass. This seems to fix my testcase and mesa, although I am not
100% sure of it and I would like to get some feedback from upstream as
well since I don't really do much work with binutils.

mesa-mips-844227-workaround.patch is a workaround for mesa I was
investigating in the meantime. It turns out that removing
--Wl,gc-sections triggers some more undefined references so you need to
link libnir.a in a few extra places.

Thanks,
James
--- a/bfd/elfxx-mips.c
+++ b/bfd/elfxx-mips.c
@@ -743,6 +743,8 @@ static struct mips_got_entry *mips_elf_create_local_got_entry
    struct mips_elf_link_hash_entry *, int);
 static bfd_boolean mips_elf_sort_hash_table_f
   (struct mips_elf_link_hash_entry *, void *);
+static bfd_boolean mips_elf_sort_hash_table_local_f
+  (struct mips_elf_link_hash_entry *, void *);
 static bfd_vma mips_elf_high
   (bfd_vma);
 static bfd_boolean mips_elf_create_dynamic_relocation
@@ -3850,6 +3852,11 @@ mips_elf_sort_hash_table (bfd *abfd, struct bfd_link_info *info)
     = hsd.min_got_dynindx
     = (elf_hash_table (info)->dynsymcount - g->reloc_only_gotno);
   hsd.max_non_got_dynindx = count_section_dynsyms (abfd, info) + 1;
+
+  mips_elf_link_hash_traverse (((struct mips_elf_link_hash_table *)
+                    elf_hash_table (info)),
+                    mips_elf_sort_hash_table_local_f,
+                    &hsd);
   mips_elf_link_hash_traverse (((struct mips_elf_link_hash_table *)
 				elf_hash_table (info)),
 			       mips_elf_sort_hash_table_f,
@@ -3879,28 +3886,40 @@ mips_elf_sort_hash_table_f (struct mips_elf_link_hash_entry *h, void *data)
 {
   struct mips_elf_hash_sort_data *hsd = data;
 
-  /* Symbols without dynamic symbol table entries aren't interesting
-     at all.  */
-  if (h->root.dynindx == -1)
+  /* Only interested in global symbols with dynamic symbol table entries */
+  if (h->root.dynindx == -1 || h->root.forced_local)
     return TRUE;
 
-  switch (h->global_got_area)
-    {
-    case GGA_NONE:
+  if (h->global_got_area == GGA_NONE) {
       h->root.dynindx = hsd->max_non_got_dynindx++;
-      break;
+  } else {
+      if (h->global_got_area == GGA_NORMAL)
+          h->root.dynindx = --hsd->min_got_dynindx;
+      else
+          h->root.dynindx = hsd->max_unref_got_dynindx++;
 
-    case GGA_NORMAL:
-      h->root.dynindx = --hsd->min_got_dynindx;
-      hsd->low = (struct elf_link_hash_entry *) h;
-      break;
+      if (hsd->low == NULL || h->root.dynindx < hsd->low->dynindx)
+          hsd->low = (struct elf_link_hash_entry *) h;
+  }
 
-    case GGA_RELOC_ONLY:
-      if (hsd->max_unref_got_dynindx == hsd->min_got_dynindx)
-	hsd->low = (struct elf_link_hash_entry *) h;
-      h->root.dynindx = hsd->max_unref_got_dynindx++;
-      break;
-    }
+  return TRUE;
+}
+
+/* All local symbols must be appear before global symbols in the dynamic symbol
+   table so they're assigned indexs first. */
+
+static bfd_boolean
+mips_elf_sort_hash_table_local_f (struct mips_elf_link_hash_entry *h, void *data)
+{
+  struct mips_elf_hash_sort_data *hsd = data;
+
+  /* Only interested in local symbols with dynamic symbol table entries */
+  if (h->root.dynindx == -1 || !h->root.forced_local)
+    return TRUE;
+
+  h->root.dynindx = hsd->max_non_got_dynindx++;
+  if (h->global_got_area != GGA_NONE && hsd->low == NULL)
+      hsd->low = (struct elf_link_hash_entry *) h;
 
   return TRUE;
 }

--- a/configure.ac
+++ b/configure.ac
@@ -543,17 +543,7 @@ AC_SUBST([BSYMBOLIC])
 dnl
 dnl Check if linker supports garbage collection
 dnl
-save_LDFLAGS=$LDFLAGS
-LDFLAGS="$LDFLAGS -Wl,--gc-sections"
-AC_MSG_CHECKING([whether ld supports --gc-sections])
-AC_LINK_IFELSE(
-    [AC_LANG_SOURCE([static char UnusedFunc() { return 5; } int main() { return 0;}])],
-    [AC_MSG_RESULT([yes])
-        GC_SECTIONS="-Wl,--gc-sections";],
-    [AC_MSG_RESULT([no])
-        GC_SECTIONS="";])
-LDFLAGS=$save_LDFLAGS
-
+GC_SECTIONS=""
 AC_SUBST([GC_SECTIONS])
 
 dnl
--- a/src/gallium/targets/va/Makefile.am
+++ b/src/gallium/targets/va/Makefile.am
@@ -29,6 +29,7 @@ gallium_drv_video_la_LIBADD = \
 	$(top_builddir)/src/gallium/auxiliary/libgalliumvl.la \
 	$(top_builddir)/src/gallium/auxiliary/libgallium.la \
 	$(top_builddir)/src/util/libmesautil.la \
+	$(top_builddir)/src/compiler/nir/libnir.la \
 	$(VL_LIBS) \
 	$(LIBDRM_LIBS) \
 	$(GALLIUM_COMMON_LIB_DEPS)
--- a/src/gallium/targets/vdpau/Makefile.am
+++ b/src/gallium/targets/vdpau/Makefile.am
@@ -34,6 +34,7 @@ libvdpau_gallium_la_LIBADD = \
 	$(top_builddir)/src/gallium/auxiliary/libgalliumvl.la \
 	$(top_builddir)/src/gallium/auxiliary/libgallium.la \
 	$(top_builddir)/src/util/libmesautil.la \
+	$(top_builddir)/src/compiler/nir/libnir.la \
 	$(VL_LIBS) \
 	$(LIBDRM_LIBS) \
 	$(GALLIUM_COMMON_LIB_DEPS)

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: