Bug 19516 - microblaze: invalid symbol indices in GLOB_DAT relocs
Summary: microblaze: invalid symbol indices in GLOB_DAT relocs
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-24 00:30 UTC by Rich Felker
Modified: 2016-02-23 10:39 UTC (History)
2 users (show)

See Also:
Host:
Target: microblaze*-*-*
Build:
Last reconfirmed:


Attachments
proposed fix (324 bytes, patch)
2016-02-11 23:36 UTC, Rich Felker
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rich Felker 2016-01-24 00:30:37 UTC
Current git versions of musl libc (since commit ad1cd43a86) are producing a libc.so that crashes during startup due to a bogus GLOB_DAT relocations with symbol index 0xffffff in libc's dynamic symbol table. I don't have a minimal test case to reproduce it yet, but the problem seems to be an interaction of several features including --gc-sections, -Bsymbolic-functions, and possibly hidden-visibility weak references.

I've identified the code that's producing these invalid relocations:

https://sourceware.org/git/?p=binutils-gdb.git;a=blob;f=bfd/elf32-microblaze.c;h=b9c32a4b2e9e09988e2a9e8898ccd888207f8794;hb=HEAD#l3293

The 'if' code path has some exclusions so that the 'else' path can be taken even when h->dynindx==-1, and in this case, it becomes 0xffffff when limited to the 24-bit symbol index field of r_info.

IMO microblaze_elf_output_dynamic_relocation should have an assertion to check for index -1 when producing a relocation that references a symbol, since this is malformed. But I'm not sure whether fixing the above logic is sufficient to fix the bug. It's possible that these GOT slots referencing symbol index -1 should have been removed much earlier in the linking process.

I'll try to follow up with a minimal test case or see if one of our users affected by the issue can do so.
Comment 1 Rich Felker 2016-01-24 00:32:05 UTC
For reference here is the link to the first version of musl affected which can be used to reproduce the bug:

http://git.musl-libc.org/cgit/musl/commit/?id=ad1cd43a86645ba2d4f7c8747240452a349d6bc1
Comment 2 Rich Felker 2016-02-11 23:36:29 UTC
Created attachment 8978 [details]
proposed fix

Producing a symbolic GLOB_DAT relocation for a symbol that has been made local/hidden by setting dynindx to -1 cannot possibly work. It appears the binding of and/or conditions for using a relative relocation rather than a symbolic relocation was just wrong: either symbolic&&def_regular or already-hidden (dynindx==-1) should be sufficient for using a relative relocation, rather than demanding that the symbol be def_regular even if it was already hidden.
Comment 3 cvs-commit@gcc.gnu.org 2016-02-23 10:38:30 UTC
The master branch has been updated by Nick Clifton <nickc@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=47993b4af18c6ef1cad300f6393bf896d3cb5e5c

commit 47993b4af18c6ef1cad300f6393bf896d3cb5e5c
Author: Rich Felker <bugdal@aerifal.cx>
Date:   Tue Feb 23 10:37:24 2016 +0000

    Fix the genetation of GOT entries for the Microblaze target.
    
    	PR target/19516
    	* elf32-microblaze.c (microblaze_elf_finish_dynamic_symbol):
    	Always produce a RELATIVE reloc for a local symbol.
Comment 4 Nick Clifton 2016-02-23 10:39:26 UTC
Hi Rich,

  Thanks for reporting and fixing this problem. :-)

  I have checked in your patch along with this changelog entry.

Cheers
  Nick

bfd/ChangeLog
2016-02-23  Rich Felker  <bugdal@aerifal.cx>

	PR target/19516
	* elf32-microblaze.c (microblaze_elf_finish_dynamic_symbol):
	Always produce a RELATIVE reloc for a local symbol.