Include Python binding tests in CMake rules

[mgorny]

Summary

Add a new CMake rule check-clang-python to run the Python bindings'
test suite, and include it in check-all.

[steveire]

I didn't run it, but CMake-wise this looks fine. (I trust you did)

I notice some tests use

from tests.cindex.util import foo

while others use

from .util import foo

You might want to make that consistent in a follow-up.

[mgorny]

Yes, I've tested it on Linux. I'll commit it once the dep is approved, and hopefully build bots will answer if they work across all platforms.

[Szelethus]

I'm afraid this may have broken many buildbots, I'm having issues building from scratch. Can you please take a look?

[mgorny]

v2: fixed appending to check-all to make it compatible both with in-tree and standalone builds

[mgorny]

v3: one more correction for stand-alone builds.

[mgorny]

Please review the new version, addressing the issue caught by buildbots.

[aaron.ballman]

I'm not a CMake expert, but when I apply this patch locally, I'm able to run cmake to completion without errors, so LGTM!

[uweigand]

This causes check-all to abort for me on SystemZ in Release mode (and never actually run the lit tests):

[40/365] cd /home/uweigand/llvm/llvm-head/tools/clang/bindings/python && /usr/bin/cmake -E...BRARY_PATH=/home/uweigand/llvm/build/llvm-head/lib /usr/bin/python2.7 -m unittest discover
FAILED: tools/clang/bindings/python/tests/CMakeFiles/check-clang-python 
cd /home/uweigand/llvm/llvm-head/tools/clang/bindings/python && /usr/bin/cmake -E env CLANG_LIBRARY_PATH=/home/uweigand/llvm/build/llvm-head/lib /usr/bin/python2.7 -m unittest discover
Invalid CXChildVisitResult!
UNREACHABLE executed at /home/uweigand/llvm/llvm-head/tools/clang/tools/libclang/CIndex.cpp:233!
Child aborted

When running in Debug mode I see instead:

[165/536] cd /home/uweigand/llvm/llvm-head/tools/clang/bindings/python && /usr/bin/cmake -...PATH=/home/uweigand/llvm/build/llvm-head-debug/lib /usr/bin/python2.7 -m unittest discover
..............................................................LIBCLANG TOOLING ERROR: fixed-compilation-database: Error while opening fixed database: No such file or directory
json-compilation-database: Error while opening JSON database: No such file or directory

..........................................................
----------------------------------------------------------------------
Ran 120 tests in 8.522s

OK

which apparently doesn't count as failure, but still doesn't look quite right to me.

Is this some environment setup problem, or does this expose a real bug? It's certainly not good that running the remaining tests is completely aborted in Release mode ...

[mgorny]

The first one seems to indicate that your libclang.so is broken in release mode (optimization error?). The second one is correct (some of the tests test for errors, and apparently don't silence the messages).

[uweigand]

In D52840#1265615, @mgorny wrote:

The first one seems to indicate that your libclang.so is broken in release mode (optimization error?). The second one is correct (some of the tests test for errors, and apparently don't silence the messages).

Ok, thanks for the clarification on the second point.

As to the first issue, it turned out to be more complicated. The wrong value results from this call in CursorVisitor::Visit

switch (Visitor(Cursor, Parent, ClientData)) {

This is a callback routine passed in from the caller, which in the case of the Python bindings means that an FFI closure is being called.

Now, due to a historical quirk in the FFI ABI, return values of integral type smaller than the register word size must be passed as the special "ffi_arg" type when using FFI (either for calls or for closures). It seems the Python 2.7 code that interfaces with FFI does not correctly respect this. As a result, it seems that when the closure returns a 32-bit value, it is not properly extended and the high bits of the return register contain garbage.

But that violates our ABI, which requires extension to full register size. And it seems the clang code, when built with optimization, does indeed rely on that ABI guarantee.

So there's a bug in Python (or depending on how you want to look at it, in libffi -- that part of the interface has also long been under-documented unfortunately), which hits on our platform. I'll see if I can do anything about that, but for now we'll probably want to disable those tests on SystemZ.

[yroux]

Hi Michal,

AArch64 bots are broken as well, logs are available at:
http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/15717/steps/ninja%20check%201/logs/stdio

Cheers,
Yvan

[kparzysz]

Hexagon clang builder has been failing since around the time of this commit as well:
http://lab.llvm.org:8011/builders/clang-hexagon-elf/builds/20617/steps/ninja%20check%201/logs/stdio

[mgorny]

Thanks for all your reports. I have filed https://reviews.llvm.org/D53326 to disable the tests on all three known-broken arches.

cfe/trunk/CMakeLists.txt

add_lit_target(check-all
       "Running all regression tests"
       ${LLVM_LIT_TESTSUITES}
       PARAMS ${LLVM_LIT_PARAMS}
       DEPENDS ${LLVM_LIT_DEPENDS}
       ARGS ${LLVM_LIT_EXTRA_ARGS}
       )
   endif()
   add_subdirectory(utils/perf-training)
+  add_subdirectory(bindings/python/tests)
 endif()
 
 option(CLANG_INCLUDE_DOCS "Generate build targets for the Clang docs."
   ${LLVM_INCLUDE_DOCS})
 if( CLANG_INCLUDE_DOCS )
   add_subdirectory(docs)
 endif()
 

cfe/trunk/bindings/python/tests/CMakeLists.txt

+# Test target to run Python test suite from main build.
+
+add_custom_target(check-clang-python
+  COMMAND CLANG_LIBRARY_PATH=$<TARGET_FILE_DIR:libclang> ${PYTHON_EXECUTABLE} -m unittest discover
+  DEPENDS libclang
+  WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/..)
+add_dependencies(check-all check-clang-python)