Re: [PATCH] D14858: Support building tsan on android.
eugenis added a comment. In http://reviews.llvm.org/D14858#293651, @danalbert wrote: > I assume we're going to actually want to go the other direction on this and > build a shared library for Android's TSAN (see eugenis' comment on > https://android-review.googlesource.com/#/c/120507/1/core/config_sanitizers.mk@68) Yes, we should use shared runtime library on Android. See AsanSharedRuntime in SanitizerArgs.h, we need something similar for TSan. http://reviews.llvm.org/D14858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14858: Support building tsan on android.
kcc added a comment. performance is a very strong reason to have tsan linked statically. every memory access in the app is instrumented with a function call, if we make this call go through PLT we'll get significant drop in performance. This is not a blocker, but I want to explicitly mention it. http://reviews.llvm.org/D14858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14858: Support building tsan on android.
dvyukov added a comment. > Yes, we should use shared runtime library on Android. Note that tsan is different from all of asan/msan/ubsan in that it does zillions of calls into runtime, so the indirection will have non-zero runtime cost. Is it possible to statically link it into something that has the bulk of verified code? http://reviews.llvm.org/D14858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14858: Support building tsan on android.
eugenis added a comment. I vaguely recall that android runtime loader had some symbol lookup differencies with glibc and that prevented interceptors (when statically linked into the main executable) from working. Maybe it is not the case now. http://reviews.llvm.org/D14858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14858: Support building tsan on android.
dimitry added a subscriber: dimitry. dimitry added a comment. In http://reviews.llvm.org/D14858#294082, @eugenis wrote: > I vaguely recall that android runtime loader had some symbol lookup > differencies with glibc and that prevented interceptors (when statically > linked into the main executable) from working. Maybe it is not the case now. This is no longer the case... There should be no difference in symbol lookup order between glibc and android linker. If you find any - please let me know. :) http://reviews.llvm.org/D14858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14858: Support building tsan on android.
eugenis added a comment. OK, it sounds like static runtime would work fine. We would still need a way to switch to the shared runtime for the apps (the workflow when we LD_PRELOAD the runtime into the Zygote to run instrumented apps on a non-instrumented device). Something like -shared-libasan flag but for TSan. Does not have to be done now. LGTM http://reviews.llvm.org/D14858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D14858: Support building tsan on android.
dimitry added a comment. The only difference is that main executable group is not RTLD_GLOBAL in android (it is RTLD_GLOBAL in glibc) - so the symbols of DT_NEEDED libraries are not visible by default; the way to enforce this for you library on android is to use -z global ld option http://reviews.llvm.org/D14858 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
