This is an archive of the discontinued LLVM Phabricator instance.

Illustrate a redirector using the example of round function from math.h.
ClosedPublic

Authored by sivachandra on Oct 15 2019, 11:12 PM.

Details

Summary

Setup demonstrated is only for ELF-ish platforms.

Also note:

  1. Use of redirectors is a temporary scheme. They will be removed once LLVM-libc has implementations for the redirected functions.
  2. Redirectors are optional. One can choose to not include them in the LLVM-libc build for their platform.
  3. Even with redirectors used, we want to link to the system libc dynamically.

Diff Detail

Event Timeline

sivachandra created this revision.Oct 15 2019, 11:12 PM

This patch illustrates one the desired "features" as proposed in http://llvm.org/docs/Proposals/LLVMLibC.html

MaskRay added inline comments.
libc/cmake/modules/LLVMLibCRules.cmake
270

You may need something like -nostdlib

281

-lc is implicit unless -nolibc (clang-only).

-nolibc is implied by -nodefaultlibs. -nodefaultlibs is implied by -nostdlib.

libc/src/math/round/round.h
15

Delete x

I've missed some emails and the round table at llvm. Could you describe more about why this mechanism for redirecting is used? It seems to me that there should be a way for LLVM_LIBC_ENTRYPOINT(round) to be an alias for ::round rather than requiring two (likely tail) calls here. Could you motivate this mechanism?

libc/docs/redirectors.rst
34

"...to see how it's..." should be "...to see what it's..."

I've missed some emails and the round table at llvm. Could you describe more about why this mechanism for redirecting is used? It seems to me that there should be a way for LLVM_LIBC_ENTRYPOINT(round) to be an alias for ::round rather than requiring two (likely tail) calls here. Could you motivate this mechanism?

Yes. Agreed that there is an added 2-call overhead, so I do not see this being used for all functions. Further, this is only an optional feature and not a necessary feature in the sense that one can choose to not include it in their libc build if they do not want to.

About what is it used for: we want to use LLVM-libc as a drop in replacement for the system libc as early as possible. So, having this redirector mechanism is one way to do it. One can argue that if static linking is all we care about, then such a mechanism is not required. However, having wrappers for system libc functions (when LLVM-libc does not provide the implementation) gives us a place to apply compiler based and other security mitigations.

[I now realize that the diagram is not viewable on phabricator, so I have put it on my github account: https://github.com/sivachandra/llvm-libc/blob/master/docs/redirectors.md ]

I guess my question is more, whats the evidence that there isn't a way to avoid the two call overhead? The use case makes sense to me but my impression is that __round_redirector could be declared to have extern "C" linkage and then llvm_libc::round can be an alias for it. That gets rid of 1 layer of indirection. Getting rid of the next call seems more gross to me but I think some additional objcopy magic can be done to make __round_redirector an alias for ::round then I think this would all link correctly, right?

We can do this for now perhaps and then we can add a feature to llvm-objcopy called --add-symbol-alias that would allow adding symbol aliases like this.

If we can't make __round_redirector live outside of llvm_libc and have extern "C" linkage then I suppose there's no good way but we should explain that we have that requirement

sivachandra added a comment.EditedOct 28 2019, 2:49 PM

I guess my question is more, whats the evidence that there isn't a way to avoid the two call overhead? The use case makes sense to me but my impression is that __round_redirector could be declared to have extern "C" linkage and then llvm_libc::round can be an alias for it. That gets rid of 1 layer of indirection. Getting rid of the next call seems more gross to me but I think some additional objcopy magic can be done to make __round_redirector an alias for ::round then I think this would all link correctly, right?

I have probably not listed out some of the constraints clearly. One of them is this: Even with LLVM-libc redirectors used, we want link to the system libc dynamically. This is because: 1. On most platforms, one links to the system libc dynamically, 2. If the system-libc were glibc, then we cannot unconditionally link to it statically anyway. Hence, I am not sure if we can eliminate the second call at all.

About the scheme you suggest to eliminate the first additional call:

Let me simplify it further. When redirectors are involved, we do not really need a llvm_libc::round. We can just have extern "C" __round_redirector and add an alias to it using llvm-objcopy or put it in source. Look at this example: https://github.com/sivachandra/redirectors

The problem with this scheme is that, since the alias round and the aliasee __round_redirector both live in the same object file, linkers will end up wiring a recursive call even if round is a hidden symbol.

Did I misunderstand your suggestion?

We can do this for now perhaps and then we can add a feature to llvm-objcopy called --add-symbol-alias that would allow adding symbol aliases like this.

If we can come up with schemes which require help from tools like llvm-objcopy (even if they have to be extended), I think that would be acceptable. In this case though, unless I have totally misunderstood your proposal, it does not seem like we can pull off without some form special treatment from the linker.

sivachandra edited the summary of this revision. (Show Details)Oct 28 2019, 3:01 PM

Personally this looks good to me!

jakehehrlich accepted this revision.Oct 29 2019, 3:46 PM

Please wait on another reviewer to give the ok and respond to remaining comments however.

This revision is now accepted and ready to land.Oct 29 2019, 3:46 PM
sivachandra marked 3 inline comments as done.

Address comments

sivachandra added inline comments.Oct 31 2019, 11:11 AM
libc/cmake/modules/LLVMLibCRules.cmake
281

I tried all the combinations I could think of (and also explicitly setting the linker language below.) But, CMake's link step is still adding -lstdc++ to the driver command line. So, I am leaving it now as you suggest here. Will fix this up in later, more focused, round.

dlj accepted this revision.Oct 31 2019, 1:07 PM
This revision was automatically updated to reflect the committed changes.