Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable TLS/C++ tests with some unsupported compilers #442

Merged
merged 2 commits into from
Aug 20, 2020

Conversation

jforissier
Copy link
Contributor

@jforissier jforissier changed the title DIsable TLS/C++ tests with some unsupported compilers Disable TLS/C++ tests with some unsupported compilers Aug 19, 2020
@jenswi-linaro
Copy link
Contributor

Acked-by: Jens Wiklander <[email protected]>

res = TEEC_InvokeCommand(&session, TA_OS_TEST_CMD_TLS_TEST_MAIN, NULL,
&ret_orig);
if (res == TEEC_ERROR_NOT_SUPPORTED)
printf("TA returned TEEC_ERROR_NOT_SUPPORTED\n");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do_ADBG_Log() instead of printf().

For consistency with other skipped tested:
Do_ADBG_Log(" - 1029 - skip test, Thread Local Storage test not supported");

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I will use Do_ADBG_Log() and adjust the format for more consistency. But I'd rather not repeat "Thread Local Storage" which is printed immediately before. It's better to inform the user that the cause is with the TA I think.

@jforissier
Copy link
Contributor Author

Update

@etienne-lms
Copy link
Contributor

Acked-by: Etienne Carriere <[email protected]>

The Thread Local Storage test (__thread attribute) is modified to accept
TEE_ERROR_NOT_SUPPORTED without failing. This return value will be used
in a later commit in case an unsupported compiler is used to build the
TA.

Signed-off-by: Jerome Forissier <[email protected]>
Acked-by: Jens Wiklander <[email protected]>
Acked-by: Etienne Carriere <[email protected]>
In 64-bit mode, the Thead Local Storage tests currently pass only with
Clang, or GCC 8 or later. Earlier GCC versions (6.2, 7.3) generate an
unsupported relocation type:

 * regression_1006 Test Basic OS features
 E/LD:  e64_relocate:538 Unknown relocation type 1031
 E/TC:?? 0 init_with_ldelf:232 ldelf failed with res: 0xffff0005
 [...]
   regression_1006 FAILED

TLS is not currently useful in itself, since OP-TEE doesn't have multi-
threaded TAs. It was implemented primarily to support C++ because the
GNU C++ runtime depends on it. To avoid failures in the regression
tests of people who upgrade their OP-TEE environment and who possibly
use older toolchains, the TLS tests should be ignored unless a "known
good" compiler is used.

Similarly, since the C++ tests depend on TLS, they need to be disabled
too with Aarch64 GCC < 8 (note they are already disabled with Clang).

Signed-off-by: Jerome Forissier <[email protected]>
Tested-by: Jerome Forissier <[email protected]> (QEMU/QEMUv8, GCC 8.3/GCC 6.2/Clang 10)
Acked-by: Jens Wiklander <[email protected]>
Acked-by: Etienne Carriere <[email protected]>
@jforissier jforissier force-pushed the tls-and-c++-check-compiler branch from 7a9e17a to 158be32 Compare August 19, 2020 15:59
@jforissier jforissier merged commit 30efcbe into OP-TEE:master Aug 20, 2020
@jforissier jforissier deleted the tls-and-c++-check-compiler branch August 20, 2020 07:33
@vchong
Copy link

vchong commented Aug 22, 2020

@jforissier I'm getting below build errors with aosp clang 11.0.2 if variables in os_test.c has __thread type modifier. Any ideas? Note that in aosp there's no libpthread. pthread functionalities are included in libc (bionic).

  LD      aosp/master/out/target/product/hikey960/optee/ta/external_optee_test_ta_os_test/5b9e0e40-2636-11e1-ad9e-0002a5d5c51b.elfld.lld: error: undefined symbol: pthread_getspecific
>>> referenced by emutls.c
>>>               emutls.c.o:(__emutls_get_address) in archive aosp/master/prebuilts/clang/host/linux-x86/clang-r383902b/lib64/clang/11.0.2/lib/linux/libclang_rt.builtins-aarch64-android.a
>>> referenced by emutls.c
>>>               emutls.c.o:(__emutls_get_address) in archive aosp/master/prebuilts/clang/host/linux-x86/clang-r383902b/lib64/clang/11.0.2/lib/linux/libclang_rt.builtins-aarch64-android.a

ld.lld: error: undefined symbol: pthread_once
>>> referenced by emutls.c
>>>               emutls.c.o:(__emutls_get_address) in archive aosp/master/prebuilts/clang/host/linux-x86/clang-r383902b/lib64/clang/11.0.2/lib/linux/libclang_rt.builtins-aarch64-android.a

ld.lld: error: undefined symbol: pthread_mutex_lock
>>> referenced by emutls.c
>>>               emutls.c.o:(__emutls_get_address) in archive aosp/master/prebuilts/clang/host/linux-x86/clang-r383902b/lib64/clang/11.0.2/lib/linux/libclang_rt.builtins-aarch64-android.a

ld.lld: error: undefined symbol: pthread_mutex_unlock
>>> referenced by emutls.c
>>>               emutls.c.o:(__emutls_get_address) in archive aosp/master/prebuilts/clang/host/linux-x86/clang-r383902b/lib64/clang/11.0.2/lib/linux/libclang_rt.builtins-aarch64-android.a

ld.lld: error: undefined symbol: pthread_setspecific
>>> referenced by emutls.c
>>>               emutls.c.o:(__emutls_get_address) in archive aosp/master/prebuilts/clang/host/linux-x86/clang-r383902b/lib64/clang/11.0.2/lib/linux/libclang_rt.builtins-aarch64-android.a
>>> referenced by emutls.c
>>>               emutls.c.o:(emutls_key_destructor) in archive aosp/master/prebuilts/clang/host/linux-x86/clang-r383902b/lib64/clang/11.0.2/lib/linux/libclang_rt.builtins-aarch64-android.a

ld.lld: error: undefined symbol: pthread_key_delete
>>> referenced by emutls.c
>>>               emutls.c.o:(__emutls_unregister_key) in archive aosp/master/prebuilts/clang/host/linux-x86/clang-r383902b/lib64/clang/11.0.2/lib/linux/libclang_rt.builtins-aarch64-android.a

ld.lld: error: undefined symbol: pthread_key_create
>>> referenced by emutls.c
>>>               emutls.c.o:(emutls_init) in archive aosp/master/prebuilts/clang/host/linux-x86/clang-r383902b/lib64/clang/11.0.2/lib/linux/libclang_rt.builtins-aarch64-android.a
make: *** [aosp/master/out/target/product/hikey960/optee/arm-plat-hikey/export-ta_arm64/mk/link.mk:110: aosp/master/out/target/product/hikey960/optee/ta/external_optee_test_ta_os_test/5b9e0e40-2636-11e1-ad9e-0002a5d5c51b.elf] Error 1
make: Leaving directory 'aosp/master/external/optee_test/ta/os_test'

@jforissier
Copy link
Contributor Author

Hi @vchong,

This __thread thing is only needed for C++ , and we support only GCC for C++. So we should just disable the test IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants