This is an archive of the discontinued LLVM Phabricator instance.

[ELF2] - implemented default enterpteter for X86, X86_64, PPC and AArch64
AbandonedPublic

Authored by grimar on Oct 12 2015, 1:38 AM.

Details

Summary

Before this patch .interp section was never created (if -dynamic-linker was not specified) because there was no default dynamic linker assigned.

Diff Detail

Event Timeline

grimar updated this revision to Diff 37082.Oct 12 2015, 1:38 AM
grimar retitled this revision from to [ELF2] - implemented default enterpteter for X86, X86_64, PPC and AArch64.
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added subscribers: grimar, llvm-commits.

You're missing a test case.

Target.cpp
204 ↗(On Diff #37082)

Please make this /lib64/ld64.so.1

ruiu edited edge metadata.Oct 12 2015, 6:49 AM

Target does not seems to be the right place to put OS-specific things. It is just architecture-specific (e.g. ELF on x86-64). I'd to write this in Driver.cpp.

emaste requested changes to this revision.Oct 12 2015, 8:00 AM
emaste added a reviewer: emaste.
emaste added a subscriber: emaste.

As @ruiu says Linux-specific settings aren't appropriate there.
For reference on FreeBSD it should be /libexec/ld-elf.so.1.

This revision now requires changes to proceed.Oct 12 2015, 8:00 AM
grimar updated this revision to Diff 37271.Oct 13 2015, 10:50 AM
grimar edited edge metadata.

Added test.
getDefaultInterp() is in Driver.cpp as was discussed via llvm-mails.

ruiu added inline comments.Oct 13 2015, 11:02 AM
ELF/Driver.cpp
70–72
The empty string is not a valid value for .interp, so let's error out.

error("--dynamic-linker is not specified");
216–217

I'd write

Config->DynamicLinker = getString(Args, OPT_dynamic_linker);
if (Config->DynamicLinker.empty())
  Config->DynamicLinker = getDefaultInterp();

so that we don't call the function if --dynamic-linker is givne.

emaste added inline comments.Oct 13 2015, 12:24 PM
ELF/Driver.cpp
60

This is not valid on non-Linux

emaste requested changes to this revision.Oct 13 2015, 12:25 PM
emaste edited edge metadata.
This revision now requires changes to proceed.Oct 13 2015, 12:25 PM
ruiu added inline comments.Oct 13 2015, 12:27 PM
ELF/Driver.cpp
60

What's the value for FreeBSD?

We don't have to make this function complete from the beginning. I'm fine with this as long as we can add new entries to this function.

grimar added inline comments.Oct 13 2015, 12:49 PM
ELF/Driver.cpp
60

Yes, it is not valid on non-linux. But currently it looks there is no good way to find which OS we are targeting. So it is just basic implementation to support .interp section.

rafael edited edge metadata.Oct 13 2015, 1:53 PM

What depends on this?

Both gcc and clang pass -dynamic_linker to ld.

Having ld itself guess the dynamic linker when not given that option seems like an historical artifact.

ruiu added a comment.Oct 13 2015, 2:04 PM

Hm, that's an interesting point. Are you suggesting we always set the same value to .interp or make --dynamic_linker mandatory?

grimar abandoned this revision.Oct 14 2015, 10:34 AM

According to discussions in mail list.