xtest: gate the GP socket tests at compile time

[andrew2311]

Always compile regression_2000.c, sock_server.c and rand_stream.c and guard
their contents with CFG_GP_SOCKETS taken from conf.h, instead of selecting
them into the source list with CFG_GP_SOCKETS taken from the TA dev kit conf.mk

On a clean AOSP build Android.mk is parsed long before ninja builds the TA dev
kit, so the -include of conf.mk finds nothing, CFG_GP_SOCKETS is empty and
regression_2000.c, sock_server.c and rand_stream.c are dropped from the source
list with no diagnostic: xtest ships without the GP socket tests even though the
TEE it is built against has CFG_GP_SOCKETS=y, and the missing cases are not even
reported as skipped. A second build picks them up, so the contents of xtest
depend on whether the output directory happens to be warm. This cannot be fixed
by adding a build dependency, because kati generates the very ninja graph that
would build conf.mk. So the patch drops the build-time gate and guards the file
contents with CFG_GP_SOCKETS from conf.h instead, which is force included by all
three build systems and, unlike conf.mk, is guaranteed to be there by the time
the compiler runs - the same fix 44ec72b applied to install_ta.c.

This is issue 275, reported in
2018 and closed without a fix ("known issue but no good solution has been
proposed yet"); integrations workaround it by exporting the flags before the build -
see optee_android_manifest ("we need to set an OP-TEE's CFG_* flag ONLY IF
it's used in an Android.mk somewhere!")

[jforissier]

@andrew2311 thanks for the patch, and thanks for digging out the root cause.

It is unfortunate that the CFG_ flags currently cannot be used in the Android Makefiles, and I would certainly like to have a more fundamental fix for that. Indeed, firstly CFG_GP_SOCKETS is not the only problematic flag in Android.mk ; and secondly, if we accept that Android can't test the CFG_ flags in the build then the other build systems should not test them either, and we should unify things.

$ find . -name \*.mk -o -name Makefile -o -name CMake\* | xargs grep -n 'if.*CFG'
./CMakeLists.txt:27:if(CFG_WERROR)
./CMakeLists.txt:29:endif(CFG_WERROR)
./Android.mk:33:ifeq ($(CFG_GP_SOCKETS),y)
./Android.mk:64:ifeq ($(CFG_SECURE_PARTITION)-$(CFG_SPMC_TESTS),y-y)
./Android.mk:68:ifeq ($(CFG_SECURE_DATA_PATH),y)
./Android.mk:72:ifeq ($(CFG_PKCS11_TA),y)
./host/xtest/CMakeLists.txt:68:if(CFG_SECURE_PARTITION AND CFG_SPMC_TESTS)
./host/xtest/CMakeLists.txt:97:if (CFG_GP_SOCKETS)
./host/xtest/CMakeLists.txt:105:if (CFG_SECURE_DATA_PATH)
./host/xtest/CMakeLists.txt:109:if (CFG_PKCS11_TA)
./host/xtest/CMakeLists.txt:114:if (CFG_CRYPTO_SE05X)
./host/xtest/Makefile:52:ifeq ($(CFG_GP_SOCKETS),y)
./host/xtest/Makefile:83:ifeq ($(CFG_SECURE_PARTITION)-$(CFG_SPMC_TESTS),y-y)
./host/xtest/Makefile:87:ifeq ($(CFG_SECURE_DATA_PATH),y)
./host/xtest/Makefile:91:ifeq ($(CFG_PKCS11_TA),y)
./host/xtest/Makefile:97:ifeq ($(CFG_PKCS11_TA),y)
./host/xtest/Makefile:101:ifeq ($(CFG_CRYPTO_SE05X),y)
./host/xtest/Makefile:154:ifeq ($(CFG_WERROR),y)
./host/xtest/Makefile:161:ifeq ($(CFG_PKCS11_TA),y)
./host/xtest/Makefile:185:ifeq ($(CFG_GCM_NIST_VECTORS),y)
./host/xtest/Makefile:207:		$(if $(filter y,$(CFG_GCM_NIST_VECTORS_LIMITED)),--limited)
./ta/Makefile:38:ifeq ($(CFG_SECURE_DATA_PATH),y)

I propose a different path. I think we should make the dev kit a required input, that is run make ta_dev_kit ... in optee_os prior to entering optee_test. Then we can do include instead of -include and everything should work. Does that sound reasonable?

[andrew2311]

Thanks, yes, I saw CFG_GP_SOCKETS is just a part of the issue.
Your more general approach seems working fine, already wrote it and tested it,
just need to prepare it properly for upstream