Bug 196918 - [PATCH] Add R_X86_64_PC64 to sys/sys/elf_common.h
Summary: [PATCH] Add R_X86_64_PC64 to sys/sys/elf_common.h
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords: patch, toolchain
Depends on:
Blocks:
 
Reported: 2015-01-20 04:08 UTC by Yuri Victorovich
Modified: 2015-04-24 16:14 UTC (History)
3 users (show)

See Also:


Attachments
patch (493 bytes, patch)
2015-01-20 04:08 UTC, Yuri Victorovich
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yuri Victorovich freebsd_committer freebsd_triage 2015-01-20 04:08:21 UTC
Created attachment 151876 [details]
patch

This is the the relatively new relocation type, that is already supported by LLVM and binutils, but not yet used in FreeBSD base system.

Please MFC to 10-STABLE.
Comment 1 Ed Maste freebsd_committer freebsd_triage 2015-01-20 16:16:19 UTC
Will also need to update contrib/elftoolchain/readelf/readelf.c
Comment 2 Yuri Victorovich freebsd_committer freebsd_triage 2015-01-20 19:28:14 UTC
With the next contrib/llvm/tools/clang update clang will probably begin to insert these into elfs, so libexec/rtld-elf/amd64 and sys/amd64/amd64/elf_machdep.c will also need to be updated.
Comment 3 Dimitry Andric freebsd_committer freebsd_triage 2015-01-20 20:09:09 UTC
This elf_common.h is our 'own' elf header, right?  E.g. not from contrib'd libelf?  If so, just adding this define should be fine.  Otherwise we should probably look at libelf upstream first.

Btw, yuri, when you say "supported by LLVM and binutils", do you mean the versions in FreeBSD head, or the upstream versions?
Comment 4 Ed Maste freebsd_committer freebsd_triage 2015-01-20 20:13:57 UTC
> This elf_common.h is our 'own' elf header, right?

Yes, we do not use ELF Tool Chain (libelf)'s header. We can just go ahead and add the defines, although we should pick up the full set of additional relocation types and add the corresponding entries to readelf.c.
Comment 5 Yuri Victorovich freebsd_committer freebsd_triage 2015-01-20 21:00:08 UTC
> Btw, yuri, when you say "supported by LLVM and binutils", do you mean the versions in FreeBSD head, or the upstream versions?

binutils version in FreeBSD tree supports it.
llvm version in FreeBSD tree mentions it, but apparently clang doesn't insert it into executables yet. But the current llvm trunk does insert it into executables, I saw it myself.
Comment 6 commit-hook freebsd_committer freebsd_triage 2015-01-21 01:08:25 UTC
A commit references this bug:

Author: emaste
Date: Wed Jan 21 01:08:00 UTC 2015
New revision: 277462
URL: https://svnweb.freebsd.org/changeset/base/277462

Log:
  redelf: Add missing R_X86_64_ relocation types

  PR:		196918
  Reviewed by:	dim
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D1570

Changes:
  head/contrib/elftoolchain/common/elfdefinitions.h
  head/contrib/elftoolchain/readelf/readelf.c
Comment 7 commit-hook freebsd_committer freebsd_triage 2015-01-21 01:12:30 UTC
A commit references this bug:

Author: emaste
Date: Wed Jan 21 01:12:22 UTC 2015
New revision: 277464
URL: https://svnweb.freebsd.org/changeset/base/277464

Log:
  Add missing R_X86_64_ constants to elf_common.h

  PR:		196918
  MFC after:	2 weeks
  Sponsored by:	The FreeBSD Foundation

Changes:
  head/sys/sys/elf_common.h
Comment 8 Konstantin Belousov freebsd_committer freebsd_triage 2015-01-23 11:13:50 UTC
Clang cannot 'insert' any type of relocation into an executable, since clang, being a compiler, only produce object files.  That said, I believe that PC32 and PC64 relocations are intended for the object files, and should not be left in the final linked object.

Note the comment in the libexec/rtld-elf/amd64/reloc.c at line 218 about R_X86_64_PC32.

If there is a linker bug which causes similar problem for PC64, we would handle this in rtld, but not before somebody provides me the affected binary.
Comment 9 Yuri Victorovich freebsd_committer freebsd_triage 2015-01-23 19:56:05 UTC
Current llvm trunk inserts R_X86_64_PC64 relocation elements into binary object modules it produces (which are relocable ELFs). There is no bug in linker that I observed.
Comment 10 Konstantin Belousov freebsd_committer freebsd_triage 2015-01-24 10:20:13 UTC
(In reply to yuri from comment #9)

The '-P' part in the description of the relocation in the ABI spec provides a hint that the relocation is for pc-relative addressing, and to be applied to the (part of) the instruction. The presence of the pc-relative relocations in the object files is expected.  This is how assembler represents references which cannot be resolved at the assembler time, but should be finished by linker.

On the other hand, such relocations must not appear in the final linking objects, both executables and shared libraries, but for different reasons.  For executables, the symbol must be resolved.  For shared libs, undefined external symbols must be referenced through the GOT/PLT, and not by a relocation of the text segment.  Allowing the relocations in the final object would require patching of the read-only text segment, i.e. DT_TEXTREL and all ill consequences of it.

From your reply, it is not clear what you mean by 'no bug in linker'.  Is it mean that linker does not emit such relocations for the final link result, or that it does emit, but you deny that this is a bug ?
Comment 11 Yuri Victorovich freebsd_committer freebsd_triage 2015-01-24 10:32:21 UTC
No, linker doesn't emit R_X86_64_PC64 into executables or shared libraries, only llvm does into the object modules, which wasn't the case before. I used the wrong term, should have said "object modules".

Various tools still should show such relocations correctly, and they were missing from headers, etc.
Comment 12 commit-hook freebsd_committer freebsd_triage 2015-04-24 15:59:33 UTC
A commit references this bug:

Author: emaste
Date: Fri Apr 24 15:58:42 UTC 2015
New revision: 281937
URL: https://svnweb.freebsd.org/changeset/base/281937

Log:
  MFC r277464: Add missing R_X86_64_ constants to elf_common.h

  PR:		196918
  Sponsored by:	The FreeBSD Foundation

Changes:
_U  stable/10/
  stable/10/sys/sys/elf_common.h
Comment 13 commit-hook freebsd_committer freebsd_triage 2015-04-24 16:12:35 UTC
A commit references this bug:

Author: emaste
Date: Fri Apr 24 16:12:31 UTC 2015
New revision: 281939
URL: https://svnweb.freebsd.org/changeset/base/281939

Log:
  MFC r277464: Add missing R_X86_64_ constants to elf_common.h

  PR:		196918
  Sponsored by:	The FreeBSD Foundation

Changes:
_U  stable/9/sys/
_U  stable/9/sys/sys/
  stable/9/sys/sys/elf_common.h
Comment 14 Ed Maste freebsd_committer freebsd_triage 2015-04-24 16:14:01 UTC
Merged to HEAD, stable/10 and stable/9.