Page MenuHomePhabricator

Add tsan shared library in Android version
Needs ReviewPublic

Authored by ZijunZhao on Dec 3 2021, 11:26 AM.

Details

Summary

Adding tsan shared library in Android version will cause these undefined symbol errors:

  1. ld.lld: error: undefined symbol: fileno_unlocked
  2. ld.lld: error: undefined symbol: stdout
  3. ld.lld: error: undefined symbol: stderr

So do a separate CMake build that only builds tsan when minSdkVersion is above 23.

Tests: None
Bugs: https://github.com/android/ndk/issues/1041

Change-Id: Idb28c10d4d246867c7e364ff153acf12f6fe162e

Diff Detail

Event Timeline

ZijunZhao created this revision.Dec 3 2021, 11:26 AM
ZijunZhao requested review of this revision.Dec 3 2021, 11:26 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 3 2021, 11:26 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript
vitalybuka added inline comments.Dec 3 2021, 12:06 PM
compiler-rt/cmake/config-ix.cmake
747

I guess for anything not Android ANDROID_PLATFORM_LEVEL is undefined so this disables TSAN everywhere?

"Tests: None" tag looks unusual for LLVM
Change-Id: also but I see it's used by some folks.

vitalybuka added inline comments.Dec 3 2021, 12:19 PM
compiler-rt/cmake/config-ix.cmake
747

Also compiler-rt uses ANDROID_API_LEVEL

danalbert added inline comments.Dec 3 2021, 12:22 PM
compiler-rt/cmake/config-ix.cmake
747

I guess for anything not Android ANDROID_PLATFORM_LEVEL is undefined so this disables TSAN everywhere?

I think so? Probably most readable to just add an elif branch for Android.

ZijunZhao added inline comments.Dec 6 2021, 11:03 AM
compiler-rt/cmake/config-ix.cmake
747

Should I add this minSdkVersion requirement for Linux, too? What I mean is that COMPILER_RT_HAS_TSAN should be true on Linux only when minSdkVersion is above 23.

danalbert added inline comments.Dec 10 2021, 1:25 PM
compiler-rt/cmake/config-ix.cmake
747

minSdkVersion is an Android-specific concept. Nothing to do for the other OS's.