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: BFD internal errors in *_finish_dynamic_symbol

On 07/03/2017 04:13 PM, Egeyar Bagcioglu wrote:
On 07/03/2017 03:24 PM, Jiong Wang wrote:
On 30/06/17 15:11, Jiong Wang wrote:
On 29/06/17 22:23, Joseph Myers wrote:

Building the glibc testsuite, test elf/ifuncmain1staticpie, I see the
following BFD internal errors on AArch64 and SPARC64:

BFD (GNU Binutils) internal error, aborting at
/scratch/jmyers/glibc-bot/src/binutils/bfd/elfnn-aarch64.c:8900 in

BFD (GNU Binutils) internal error, aborting at
/scratch/jmyers/glibc-bot/src/binutils/bfd/elfxx-sparc.c:4510 in

These are caused by the commits:

commit ec1acaba1381d0196c45965a7db9942b67fbd88d
Author: Egeyar Bagcioglu <>
Date:   Thu Jun 29 04:28:27 2017 -0700

bfd: prevent all but undef weak syms from becoming dynamic in sparc.

commit ff07562f1e369b6e37eafb2a888dc48fa2453e86
Author: Jiong Wang <>
Date:   Thu Jun 29 11:47:43 2017 +0100

[AArch64] Only override the symbol dynamic decision on undefined weak symbol

(as verified by: building with binutils commit
ec1acaba1381d0196c45965a7db9942b67fbd88d both internal errors appear;
building two commits earlier, commit
2aff25ba76035d2f1f48ea8a6c4b7e498ee31790, neither error occurs).

The new assertion added in *_finish_dynamic_symbol missed IFUNC type.

A local defined IFUNC in an executable still requires some finalization code in
this finish_dynamic_symbol hook to setup PLT + IRELATIVE.

The fix might be either follow the X86-64 approach to only assert on entries
that were exactly touched by that patch.

Or to relax the check on IFUNC. Searching the code in bfd/elflink.c, there are similar checks done at the call site of backend *_finish_dynamic_symbol already to make sure it's only called when necessary, so I fell this assertion could also be

I incline to remove the assertion at the start of *finish_dynamic_symbol as this assertion brings nothing more guarantee on the correctness. I guess it's the
same on x86 and SPARC.

The outer caller elf_link_output_extsym in elflink.c is a traverse function on all external symbol, and it will only call *finish_dynamic_symbol if some
conditions is meet.  It is executed conditionally.

If the condition to trigger that assertion is satisified, it then won't satify the outer check in finish_dynamic_symbol, so *finish_dynamic_symbol won't be
called that the assertion is expected to be dead code.

If elf_link_output_extsym is a traverse function that unconditionally called on external symbols decided to be exported, then an assertion to make sure these
symbols are in sane status might make sense.

H.J,  Egeyar, what's your opinion here?

Hello Jiong,

I agree with your point. This assertion does not contribute to correctness. It was not to skip any unforseen problems about the core change of the patch we submitted. I would expect it to help detecting an issue regarding the changed relocation. Instead, it detected an issue with the condition to assert. Under these circumstances, I see no advantage of keeping it. If a problem occurs regarding the core change of our previous patches, it can easily be traced back to us via git bisect.

Unless someone objects by then, I'll upload the corresponding mini-patch after receiving internal approval from my team.

The attached patch fixes the issue for sparc.
>From 234604426719d6696a41ef7b37837632ceb7c03e Mon Sep 17 00:00:00 2001
From: Egeyar Bagcioglu <>
Date: Sun, 2 Jul 2017 19:51:37 -0700
Subject: [PATCH] sparc: Remove 'abort' statement in finish_dynamic_symbol

The check is incomplete and caused issues. Maintaining it is not necessary as there is no sign of *_finish_dynamic_symbol being called unintentionally.

This patch is tested on sparc64-unknown-linux-gnu, no regressions are found.


2017-07-03  Egeyar Bagcioglu  <>

        * elfxx-sparc.c (_bfd_sparc_elf_finish_dynamic_symbol): Remove the
        abort statement that was put for symbols that are not dynamic.
 bfd/elfxx-sparc.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/bfd/elfxx-sparc.c b/bfd/elfxx-sparc.c
index 1fd2141..b50d34b 100644
--- a/bfd/elfxx-sparc.c
+++ b/bfd/elfxx-sparc.c
@@ -4502,13 +4502,6 @@ _bfd_sparc_elf_finish_dynamic_symbol (bfd *output_bfd,
   eh = (struct _bfd_sparc_elf_link_hash_entry *) h;
-  /* Abort if the symbol is not dynamic in PIC */
-  if (h->dynindx == -1
-      && !h->forced_local
-      && h->root.type != bfd_link_hash_undefweak
-      && bfd_link_pic (info))
-    abort();
   /* We keep PLT/GOT entries without dynamic PLT/GOT relocations for
      resolved undefined weak symbols in executable so that their
      references have value 0 at run-time.  */

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