This is an archive of the discontinued LLVM Phabricator instance.

[llvm-zorg] Use ndk21 for buildbot
ClosedPublic

Authored by oontvoo on Oct 21 2020, 8:33 PM.

Diff Detail

Event Timeline

oontvoo created this revision.Oct 21 2020, 8:33 PM
oontvoo requested review of this revision.Oct 21 2020, 8:33 PM
vitalybuka added inline comments.Oct 21 2020, 8:46 PM
zorg/buildbot/builders/sanitizers/buildbot_android_functions.sh
82–83

we don't want to do that, the point of this bot to test clang and compiler_rt from master

vitalybuka added inline comments.Oct 21 2020, 8:49 PM
zorg/buildbot/builders/sanitizers/buildbot_android_functions.sh
66–70

maybe link errors because it includes libc++, but then sets -stdlib=libstdc++ ?

oontvoo added inline comments.Oct 22 2020, 7:02 AM
zorg/buildbot/builders/sanitizers/buildbot_android_functions.sh
66–70

Without the include, I was getting missing header errors (eg., <vector>, <limits>, etc)
(I have also tried setting stdlib=libc++ )

oontvoo added inline comments.Oct 22 2020, 12:53 PM
zorg/buildbot/builders/sanitizers/buildbot_android_functions.sh
66–70

Oh, I see. compiler-rt passed stdlib=libstdc++, and that overrode libc++.

********************
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90
FAIL: AddressSanitizer-aarch64-android :: TestCases/uar_and_exceptions.cpp (458 of 485)
******************** TEST 'AddressSanitizer-aarch64-android :: TestCases/uar_and_exceptions.cpp' FAILED ********************
Script:
--
: 'RUN: at line 2';     /mnt/ssd/repo/buildbot/d_emulated_tls/run_dir/llvm-project/compiler-rt/test/sanitizer_common/android_commands/android_compile.py  /mnt/ssd/repo/buildbot/d_emulated_tls/run_dir/llvm_build64/bin/clang  --driver-mode=g++ -stdlib=libstdc++ -fsanitize=address -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only  -pie -fuse-ld=gold --target=aarch64-linux-android --sysroot=/mnt/ssd/repo/buildbot/d_emulated_tls/run_dir/android_ndk/standalone-aarch64/sysroot -B/mnt/ssd/repo/buildbot/d_emulated_tls/run_dir/android_ndk/standalone-aarch64  -shared-libasan -O0 /mnt/ssd/repo/buildbot/d_emulated_tls/run_dir/llvm-project/compiler-rt/test/asan/TestCases/uar_and_exceptions.cpp -o /mnt/ssd/repo/buildbot/d_emulated_tls/run_dir/compiler_rt_build_android_aarch64/test/asan/AARCH64AndroidConfig/TestCases/Output/uar_and_exceptions.cpp.tmp
: 'RUN: at line 3';   env ASAN_OPTIONS=abort_on_error=0:detect_stack_use_after_return=1  /mnt/ssd/repo/buildbot/d_emulated_tls/run_dir/compiler_rt_build_android_aarch64/test/asan/AARCH64AndroidConfig/TestCases/Output/uar_and_exceptions.cpp.tmp
oontvoo updated this revision to Diff 300108.Oct 22 2020, 2:57 PM

updated to use the right API, depending on the device and use libc++

oontvoo retitled this revision from [wip][llvm-zorg] Use ndk21 for buildbot to [llvm-zorg] Use ndk21 for buildbot.Oct 22 2020, 2:58 PM
oontvoo marked 2 inline comments as done.
srhines added inline comments.
zorg/buildbot/builders/sanitizers/buildbot_android_functions.sh
60

@danalbert Should we be adding this kind of autodetection (to look in $_arch-linux-android) to Clang directly? How does this work for NDK users today?

Thank you!
I tried yesterday myself but didn't figureout "-stdlib=libc++ -I/$ANDROID_TOOLCHAIN/sysroot/usr/include/c++/v1"" part.

May I ask you to move ANDROID_HAS_ELF_TLS and ANDROID_HAS_ELF_TLS into a separate patch so we can keep ndk upgrade only to see how it works for the bot?

zorg/buildbot/builders/sanitizers/buildbot_android_functions.sh
60

double spaces?

60

not sure about this part
we build ndk and all this for particular platform no mater what devices are available
the script is going to run all tests on all available devices

64

"-lt 21" for consistency?

71

can we use cmake rule to detect that? like link trivial program?

111

check buildbot_android.sh
ANDROID_HAS_ELF_TLS here is from the last arch and can mismatch

Can be solved by reordering of
build_android_ndk aarch64 arm64
configure_android aarch64 aarch64-linux-android

but I would prefer if we detect with ANDROID_HAS_ELF_TLS
isn't this crt property so we can test from cmake if it links?

actually I see a lot of test failing with v21 on 93953d411a0fc240e10237fea34aaff43e8f0ff5 on x86 emulator

-
Exit Code: 1

Command Output (stderr):
--
/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/test/ubsan/TestCases/ImplicitConversion/integer-arithmetic-value-change.c:160:14: error: CHECK-V0: expected string not found in input
// CHECK-V0: {{.*}}integer-arithmetic-value-change.c:900:10: runtime error: implicit conversion from type '{{.*}}' (aka 'unsigned int') of value 4294967295 (32-bit, unsigned) to type '{{.*}}' (aka 'int') changed the value to -1 (32-bit, signed)
             ^
<stdin>:1:1: note: scanning from here
CANNOT LINK EXECUTABLE "/data/local/tmp/Output/usr/local/google/home/vitalybuka/slow/bbot/compiler_rt_build_android_i686/test/ubsan/AddressSanitizer-i386/TestCases/ImplicitConversion/Output/integer-arithmetic-value-change.c.tmp": cannot locate symbol "_ZTIN10__cxxabiv117__class_type_infoE" referenced by "/data/local/tmp/Output/libclang_rt.asan-i686-android.so"...
^

Input file: <stdin>
Check file: /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/test/ubsan/TestCases/ImplicitConversion/integer-arithmetic-value-change.c

-dump-input=help explains the following input dump.

Input was:
<<<<<<
           1: CANNOT LINK EXECUTABLE "/data/local/tmp/Output/usr/local/google/home/vitalybuka/slow/bbot/compiler_rt_build_android_i686/test/ubsan/AddressSanitizer-i386/TestCases/ImplicitConversion/Output/integer-arithmetic-value-change.c.tmp": cannot locate symbol "_ZTIN10__cxxabiv117__class_type_infoE" referenced by "/data/local/tmp/Output/libclang_rt.asan-i686-android.so"...
check:160     X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
           2: Aborted 
check:160     ~~~~~~~~
>>>>>>

Do they work for you?

actually I see a lot of test failing with v21 on 93953d411a0fc240e10237fea34aaff43e8f0ff5 on x86 emulator

Sorry, forgot to mention compiler-rt needs to set stdlib=libc++ too. Right now, they are using stdlibc++. (Or better yet, change the default value of the cmake variable ANDROID_NDK_VERSION=21)
(This is why you're getting these undefined symbols).

I'll send a patch for that soon.

oontvoo updated this revision to Diff 300143.Oct 22 2020, 7:13 PM
oontvoo marked 3 inline comments as done.

updated

May I ask you to move ANDROID_HAS_ELF_TLS and ANDROID_HAS_ELF_TLS into a separate patch so we can keep ndk upgrade only to see how it works for the bot?

Done - removed the ANDROID_HAS_ELF_TLS part.

zorg/buildbot/builders/sanitizers/buildbot_android_functions.sh
60

It should match the platform/arch to the right device and its libc API level??

I'm concerned that if you say want to build for 29 (with elf-tls), then run it on 25 (without TLS support) it won't load.

oontvoo updated this revision to Diff 300285.Oct 23 2020, 7:20 AM

Use the libc++_shared.so under sysroot rather than searching for it.
(In the new NDKs, there are a bunch of libc++_shared.so for different archs)

oontvoo updated this revision to Diff 300440.Oct 23 2020, 5:16 PM

Set cmake ndk-version variable

BTW zorg repo is frozen right now for buildbot migration, we need to wait week or so before able to commit.

zorg/buildbot/builders/sanitizers/buildbot_android.sh
26

let's remove r21 here and set global ANDROID_NDK_VERSION=21 in buildbot_android_functions

zorg/buildbot/builders/sanitizers/buildbot_android_functions.sh
38

here we build compiler for the host and we don't use NDK at all

60

That is a problem but I'd like focus this patch only on 16->21 update keeping as much as possible unchanged, including ANDROID_API,
please move entire check_android_api into a separate patch

60

I suspect they each clang-* from NDK has hard coded value.
Here we share single clang binary for all platforms

111

it would be nice to have single place for "21"

oontvoo updated this revision to Diff 300698.Oct 26 2020, 9:29 AM
oontvoo marked 6 inline comments as done.

updated diff

oontvoo added inline comments.Oct 26 2020, 9:29 AM
zorg/buildbot/builders/sanitizers/buildbot_android_functions.sh
111

but I would prefer if we detect with ANDROID_HAS_ELF_TLS
isn't this crt property so we can test from cmake if it links?

I've moved the ANDROID_HAS_ELF_TLS to a different patch. So will close this for now.

danalbert added inline comments.Oct 27 2020, 12:28 PM
zorg/buildbot/builders/sanitizers/buildbot_android_functions.sh
6

Don't you want the fixed version? r21d is the latest.

56–57

This isn't needed any more. You can just use clang directly with the appropriate target (e.g. aarch64-linux-android24) and it works. I'm guessing it's the case that you're only using the NDK for its sysroot and using your own clang/linker/ar/etc here, in which case you just need to make sure you pass --sysroot $NDK/toolchains/llvm/prebuilt/linux-x86_64/sysroot when invoking clang.

60

Looks unnecessary to me. make_standalone_toolchain.py is no longer the common approach, but we do have tests that say it should work fine without this.

64

These values are in $NDK/meta/platforms.json if you want to remove the hard coding.

The minimum isn't right either. The minimum for current NDKs is 16. It's only the LP64 ABIs that have a minimum of 21. Don't know if that matters here. *We* certainly care about testing API 16, but we also aren't using zorg to do that.

oontvoo marked 2 inline comments as done.Oct 27 2020, 8:33 PM
oontvoo added inline comments.
zorg/buildbot/builders/sanitizers/buildbot_android_functions.sh
60

I suspect they each clang-* from NDK has hard coded value.

seems plausible

I'm guessing it's the case that you're only using the NDK for its sysroot and using your own clang/linker/ar/etc here, in which case you >just need to make sure you pass --sysroot $NDK/toolchains/llvm/prebuilt/linux-x86_64/sysroot when invoking clang.

I've tried using the prebuilt sysroot but unfortunately it didn't work. (Can't find -lc++ and -lgcc)

eg., this failed during Cmake compiler check

[2/2] Linking C executable cmTC_1a555
FAILED: cmTC_1a555 
: && /mnt/ssd/repo/buildbot/d_emulated_tls/run_dir/llvm_build64/bin/clang --target=i686-linux-android --sysroot=/mnt/ssd/repo/buildbot/d_emulated_tls/run_dir/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot -B/mnt/ssd/repo/buildbot/d_emulated_tls/run_dir/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib  -pie CMakeFiles/cmTC_1a555.dir/testCCompiler.c.o  -o cmTC_1a555   && :
/usr/bin/ld: cannot find -lgcc
oontvoo updated this revision to Diff 301340.Oct 28 2020, 10:58 AM
oontvoo marked an inline comment as done.

download r21d package (instead of r21)

danalbert added inline comments.Oct 28 2020, 1:16 PM
zorg/buildbot/builders/sanitizers/buildbot_android_functions.sh
60

It can't find libc++ because you didn't specify an API level. -target i686-linux-android21 finds libc++.

libgcc is part of the toolchain rather than the Android system. For that you need to add -gcc-toolchain $NDK/toolchains/llvm/prebuilt/linux-x86_64. Sorry, forgot that step :)

(you could also try using libclangrt instead of libgcc, but we haven't finished that migration, so unless that's what you're trying to test I wouldn't recommend that just yet)

oontvoo marked 2 inline comments as done.Oct 28 2020, 4:54 PM
oontvoo added inline comments.
zorg/buildbot/builders/sanitizers/buildbot_android_functions.sh
60

Thanks. Specifying the -gcc-toolchain in addition to adding the API level to target got it to work.

oontvoo updated this revision to Diff 301491.Oct 28 2020, 5:44 PM
oontvoo marked an inline comment as done.

rebase

oontvoo updated this revision to Diff 301504.Oct 28 2020, 7:16 PM

updated diff

vitalybuka added a comment.EditedOct 29 2020, 12:57 AM

updated diff

does this version pass tests with emulator on master?
I see a lot of CANNOT LINK EXECUTABLE

******************** TEST 'AddressSanitizer-i386-android :: TestCases/strcspn-1.c' FAILED ********************
Script:
--
: 'RUN: at line 2';     /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/test/sanitizer_common/android_commands/android_compile.py  /usr/local/google/home/vitalybuka/slow/bbot/llvm_build64/bin/clang  -fsanitize=address -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only  -pie -fuse-ld=gold --target=i686-linux-android24 --sysroot=/usr/local/google/home/vitalybuka/slow/bbot/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot -gcc-toolchain /usr/local/google/home/vitalybuka/slow/bbot/android_ndk/toolchains/llvm/prebuilt/linux-x86_64  -B/usr/local/google/home/vitalybuka/slow/bbot/android_ndk/toolchains/llvm/prebuilt/linux-x86_64  -shared-libasan /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/test/asan/TestCases/strcspn-1.c -o /usr/local/google/home/vitalybuka/slow/bbot/compiler_rt_build_android_i686/test/asan/I386AndroidConfig/TestCases/Output/strcspn-1.c.tmp && env ASAN_OPTIONS=abort_on_error=0:strict_string_checks=true not  /usr/local/google/home/vitalybuka/slow/bbot/compiler_rt_build_android_i686/test/asan/I386AndroidConfig/TestCases/Output/strcspn-1.c.tmp 2>&1 | FileCheck /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/test/asan/TestCases/strcspn-1.c
: 'RUN: at line 5';   env ASAN_OPTIONS=abort_on_error=0:intercept_strspn=false  /usr/local/google/home/vitalybuka/slow/bbot/compiler_rt_build_android_i686/test/asan/I386AndroidConfig/TestCases/Output/strcspn-1.c.tmp 2>&1
--
Exit Code: 1

Command Output (stderr):
--
/usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/test/asan/TestCases/strcspn-1.c:17:11: error: CHECK: expected string not found in input
 // CHECK:'s1'{{.*}} <== Memory access at offset {{[0-9]+}} partially overflows this variable
          ^
<stdin>:1:1: note: scanning from here
CANNOT LINK EXECUTABLE "/data/local/tmp/Output/usr/local/google/home/vitalybuka/slow/bbot/compiler_rt_build_android_i686/test/asan/I386AndroidConfig/TestCases/Output/strcspn-1.c.tmp": cannot locate symbol "_ZTIN10__cxxabiv117__class_type_infoE" referenced by "/data/local/tmp/Output/libclang_rt.asan-i686-android.so"...
^

Input file: <stdin>
Check file: /usr/local/google/home/vitalybuka/src/llvm.git/llvm-project/compiler-rt/test/asan/TestCases/strcspn-1.c

-dump-input=help explains the following input dump.

Input was:
<<<<<<
          1: CANNOT LINK EXECUTABLE "/data/local/tmp/Output/usr/local/google/home/vitalybuka/slow/bbot/compiler_rt_build_android_i686/test/asan/I386AndroidConfig/TestCases/Output/strcspn-1.c.tmp": cannot locate symbol "_ZTIN10__cxxabiv117__class_type_infoE" referenced by "/data/local/tmp/Output/libclang_rt.asan-i686-android.so"...
check:17     X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
          2: Aborted 
check:17     ~~~~~~~~
          3: error: Aborted
check:17     ~~~~~~~~~~~~~~
>>>>>>

does this version pass tests with emulator on master?

Yes, worked for me.
Though i'm not sure you meant llvm-project master? or emulator with aosp-master image?
I've tested it with latest llvm-project on a Q image.

CANNOT LINK EXECUTABLE "/data/local/tmp/Output/usr/local/google/home/vitalybuka/slow/bbot/compiler_rt_build_android_i686/test/asan/I386AndroidConfig/TestCases/Output/strcspn-1.c.tmp": cannot locate symbol "_ZTIN10cxxabiv117class_type_infoE" referenced by "/data/local/tmp/Output/libclang_rt.asan-i686-android.so"...

This seems strange. The test is C. Why is it looking for _ZTIN10cxxabiv117class_type_infoE ?

oontvoo added a comment.EditedOct 29 2020, 10:32 AM

P.S: For completeness, I've re-tested with aosp-master. I didn't see the link errors you saw, but did see 16 Scudo test failures. Looked a like unrelated issues, because same errors also seen with the old zorg script
https://pastebin.ubuntu.com/p/bTDc7Vj9TD/

Failed Tests (16):
  AddressSanitizer-i386-android :: TestCases/throw_invoke_test.cpp
  SanitizerCommon-asan-i386-Android :: Linux/mallopt.cpp
  Scudo-i386 :: aligned-new.cpp
  Scudo-i386 :: alignment.c
  Scudo-i386 :: double-free.cpp
  Scudo-i386 :: fsanitize.c
  Scudo-i386 :: interface.cpp
  Scudo-i386 :: memalign.c
  Scudo-i386 :: mismatch.cpp
  Scudo-i386 :: options.cpp
  Scudo-i386 :: overflow.c
  Scudo-i386 :: quarantine.c
  Scudo-i386 :: rss.c
  Scudo-i386 :: sized-delete.cpp
  Scudo-i386 :: sizes.cpp
  Scudo-i386 :: threads.c
vitalybuka added a comment.EditedOct 29 2020, 2:31 PM

I use internal emulator, I've sent you how to run it.
master means llvm-project master, no other patches (20a3931f8fadb9c13b4868d1b0124498b014b1d5)
zorg:
237d0b4da0c0df5ffec99ce30e62b3acc7d15c3c no errors
237d0b4da0c0df5ffec99ce30e62b3acc7d15c3c+this patch: can't link

I use internal emulator, I've sent you how to run it.

Thanks! was able to repro the errors.

So what I've got so far. This only occurs for 25 or older. (Didn't get the errors when testing this patch with Pie or newer images)

A closer. look at the error, cannot locate symbol "_ZTIN10cxxabiv117class_type_infoE" referenced by "/data/local/tmp/Output/libclang_rt.asan-i686-android.so"...
(note that this symbol is defined in libc++_shared.so ) So maybe somewhere it was confusing libc++ and libstdc++ ?

@srhines @danalbert Is there any subtlety we don't know about?

oontvoo updated this revision to Diff 302054.Oct 30 2020, 6:10 PM

Update compiler-rt build to set -DSANITIZER_CXX_ABl=libcxxabi (which is similar to what the ndk build does).
This should fix the undefined symbols issue.

small changes and disable failing tests

vitalybuka accepted this revision.Oct 31 2020, 4:00 AM
This revision is now accepted and ready to land.Oct 31 2020, 4:00 AM
This revision was automatically updated to reflect the committed changes.