Page MenuHomePhabricator

Fix lld detection in standalone compiler-rt.
ClosedPublic

Authored by eugenis on Fri, Oct 18, 2:33 PM.

Details

Summary

Right now all hwasan tests on Android are silently disabled because they
require "has_lld" and standalone compiler-rt can not (and AFAIK was
never able to) set it.

Diff Detail

Event Timeline

eugenis created this revision.Fri, Oct 18, 2:33 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFri, Oct 18, 2:33 PM
Herald added subscribers: Restricted Project, mgorny, dberris. · View Herald Transcript
eugenis marked an inline comment as done.Fri, Oct 18, 2:37 PM

As it turns out, hwasan has not been tested on Android for the past two months.
It was broken by this change (which went in without code review, btw, not cool):

http://llvm.org/viewvc/llvm-project?rev=368242&view=rev

The change may look trivial, but nothing in CMake ever is.

Now, this can not go in right now, because, naturally, most of the stuff we have committed over the past two months does not pass on Android.
It looks like it is almost all Peter.
One thing that I've noticed is that __hwasan_personality_wrapper has neither extern"C" or public visibility, so it is not exported from the shared runtime library.

compiler-rt/CMakeLists.txt
500

I believe all the stuff above this line is necessary for non-standalone case, when the linker flag check would run before the linker itself is built.

pcc added inline comments.Fri, Oct 18, 3:17 PM
compiler-rt/CMakeLists.txt
500

Shouldn't this be ${COMPILER_RT_HAS_FUSE_LD_LLD_FLAG}?

dyung added a comment.Fri, Oct 18, 3:22 PM

Sorry for breaking this, I thought the change would be trivial, guess it was not. From what I can remember, Peter's change was causing our internal testing to break because we don't build/use LLD for our platform.

eugenis updated this revision to Diff 225704.Fri, Oct 18, 3:23 PM
eugenis marked an inline comment as done.

Add missing ${}

compiler-rt/CMakeLists.txt
500

It definitely should!

pcc accepted this revision.Fri, Oct 18, 3:30 PM

LGTM

This revision is now accepted and ready to land.Fri, Oct 18, 3:30 PM
dyung accepted this revision.Mon, Oct 21, 10:59 AM

I tried applying this to our internal sources and it worked, so LGTM as well.

I've applied a workaround for the last remaining regression in r375471.
Time to land this change.

This revision was automatically updated to reflect the committed changes.

This appears to have broken the standalone compiler-rt build: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/23986

CMake Error at /b/sanitizer-x86_64-linux/build/llvm_build64/lib/cmake/llvm/AddLLVM.cmake:1422 (add_dependencies):
  The dependency target "lld" of target "check-compiler-rt" does not exist.
Call Stack (most recent call first):
  test/CMakeLists.txt:86 (add_lit_target)


CMake Error at /b/sanitizer-x86_64-linux/build/llvm_build64/lib/cmake/llvm/AddLLVM.cmake:1422 (add_dependencies):
  The dependency target "lld" of target "check-ubsan" does not exist.
Call Stack (most recent call first):
  /b/sanitizer-x86_64-linux/build/llvm_build64/lib/cmake/llvm/AddLLVM.cmake:1443 (add_lit_target)
  test/ubsan/CMakeLists.txt:131 (add_lit_testsuite)


CMake Error at /b/sanitizer-x86_64-linux/build/llvm_build64/lib/cmake/llvm/AddLLVM.cmake:1422 (add_dependencies):
  The dependency target "lld" of target "check-msan" does not exist.
Call Stack (most recent call first):
  /b/sanitizer-x86_64-linux/build/llvm_build64/lib/cmake/llvm/AddLLVM.cmake:1443 (add_lit_target)
  test/msan/CMakeLists.txt:56 (add_lit_testsuite)

I've just taken over buildbot monitoring duty and it looks like our http://lab.llvm.org:8011/builders/clang-cmake-thumbv7-full-sh/builds/3006 is still failing even with the latest fix.

I can reproduce on an x86_64 machine. It looks like specific problem is when the compiler has -fuse-ld=lld, but lld is not in the LLVM_ENABLE_PROJECTS list. If I add it then the cmake step will complete.

CC=/path/to/clang CXX=/path/to/clang++ \
  cmake -GNinja ../../llvm \
      -DLLVM_ENABLE_PROJECTS="clang;compiler-rt" \
      -DCMAKE_BUILD_TYPE=Release

Can you take a look?

Hi Peter, it appears that I've missed some of the compiler-rt subprojects. This commit should fix it:
https://github.com/llvm/llvm-project/commit/3f345732b4f88e8d0d302470929cbe33d65a7435