Page MenuHomePhabricator

[runtimes] Use add_lit_testsuite to register lit testsuites
ClosedPublic

Authored by ldionne on Mar 3 2021, 10:42 PM.

Details

Summary

The runtimes build uses variables set by add_lit_testsuite to collect
testsuites from all the runtimes.

Diff Detail

Event Timeline

phosek created this revision.Mar 3 2021, 10:42 PM
phosek requested review of this revision.Mar 3 2021, 10:42 PM
Herald added projects: Restricted Project, Restricted Project, Restricted Project. · View Herald TranscriptMar 3 2021, 10:42 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added subscribers: libcxx-commits, Restricted Project. · View Herald Transcript
ldionne accepted this revision.Mar 4 2021, 4:42 AM

Lgtm pending ci

This revision is now accepted and ready to land.Mar 4 2021, 4:42 AM
ldionne requested changes to this revision.Mar 4 2021, 7:29 AM

Actually, I had another look at this, and I'm not sure it's right. It used to be add_lit_testsuite, but I changed it to add_lit_target in

commit 6f69318c7248275b509ecf0f88eb2ba725aaeb82
Author: Louis Dionne <ldionne@apple.com>
Date:   Thu Jul 9 11:54:09 2020 -0400

    [runtimes] Allow passing Lit parameters through CMake
    
    This allows passing parameters to the test suites without using
    LLVM_LIT_ARGS. The problem is that we sometimes want to set some
    Lit arguments on the CMake command line, but the Lit parameters in
    a CMake cache file. If the only knob to do that is LLVM_LIT_ARGS,
    the command-line entry overrides the cache one, and the parameters
    set by the cache are ignored.
    
    This fixes a current issue with the build bots that they completely
    ignore the 'std' param set by Lit, because other Lit arguments are
    provided via LLVM_LIT_ARGS on the CMake command-line.

IIRC, the issue was that add_lit_testsuite sets several parameters globally, i.e. for all the test suites:

if(NOT ARG_EXCLUDE_FROM_CHECK_ALL)
    # Register the testsuites, params and depends for the global check rule.
    set_property(GLOBAL APPEND PROPERTY LLVM_LIT_TESTSUITES ${ARG_UNPARSED_ARGUMENTS})
    set_property(GLOBAL APPEND PROPERTY LLVM_LIT_PARAMS ${ARG_PARAMS})
    set_property(GLOBAL APPEND PROPERTY LLVM_LIT_DEPENDS ${ARG_DEPENDS})
    set_property(GLOBAL APPEND PROPERTY LLVM_LIT_EXTRA_ARGS ${ARG_ARGS})
  endif()

This is wrong because we don't want e.g. the libc++ parameters to be used when running the Clang test suite. I agree it's probably better to fix the root cause instead of just *not* using add_lit_testsuite though.

This revision now requires changes to proceed.Mar 4 2021, 7:29 AM
phosek added a comment.Mar 4 2021, 3:10 PM

That explains why this used to work before. The runtimes build uses these global properties to gather the set of the tests and arguments across different runtimes, see https://github.com/llvm/llvm-project/blob/2b957ed4ff3344d8f761a053566e307277a1cdeb/runtimes/CMakeLists.txt#L130, so if we don't want to go with this change, then we'll need to come up with a different mechanism.

ldionne accepted this revision.Mar 4 2021, 3:28 PM

From the CI job, it looks like we are still picking up the standard:

llvm-lit: /home/libcxx-builder/.buildkite-agent/builds/332e75e1566a-1/llvm-project/libcxx-ci/libcxx/utils/libcxx/test/newconfig.py:25: note: Applied 'add Lit feature c++14' as a result of parameter 'std=c++14'

I don't fully remember what's the issue I was running into, but I just did some archeology and I don't think it can be an issue anymore. Basically, since the CMake caches we use to run the buildbots look like

set(LIBCXX_TEST_PARAMS "std=c++14" CACHE STRING "")
set(LIBCXXABI_TEST_PARAMS "${LIBCXX_TEST_PARAMS}" CACHE STRING "")

we're not reusing LLVM_LIT_ARGS, and I don't think the issue I was explaining in the commit message can happen anymore. So let's give this a shot. Sorry for the back and forth, this happened a while ago.

This revision is now accepted and ready to land.Mar 4 2021, 3:28 PM
jsji added a subscriber: jsji.Mar 5 2021, 3:11 PM

Looks like this will cause runtime build to fail sanitizer tests. https://lab.llvm.org/buildbot/#/builders/57/builds/4922

phosek added a comment.Mar 5 2021, 3:26 PM

Looks like this will cause runtime build to fail sanitizer tests. https://lab.llvm.org/buildbot/#/builders/57/builds/4922

Thanks for the heads up, I've reverted the compiler-rt part so I can further investigate this.

jsji added a comment.Mar 5 2021, 6:24 PM

Looks like this will cause runtime build to fail sanitizer tests. https://lab.llvm.org/buildbot/#/builders/57/builds/4922

Thanks for the heads up, I've reverted the compiler-rt part so I can further investigate this.

Thank you!

Hello. I have an auto-bisecting cron job that has identified this change as the source of a regression. Specifically, after this change, the following libcxx tests start failing. Given that this patch is partially reverted already, I'm going to revert the libcxx part too. If people need help debugging this, I'm happy to help.

Failed Tests (31):

libc++ :: libcxx/experimental/memory/memory.resource.global/global_memory_resource_lifetime.pass.cpp
libc++ :: libcxx/experimental/memory/memory.resource.global/new_delete_resource_lifetime.pass.cpp
libc++ :: libcxx/gdb/gdb_pretty_printer_test.sh.cpp
libc++ :: std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.ctor/default.pass.cpp
libc++ :: std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.eq/equal.pass.cpp
libc++ :: std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.eq/not_equal.pass.cpp
libc++ :: std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/allocate.pass.cpp
libc++ :: std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair.pass.cpp
libc++ :: std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_const_lvalue_pair.pass.cpp
libc++ :: std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_rvalue.pass.cpp
libc++ :: std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_pair_values.pass.cpp
libc++ :: std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp
libc++ :: std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair_evil.pass.cpp
libc++ :: std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_types.pass.cpp
libc++ :: std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/deallocate.pass.cpp
libc++ :: std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/destroy.pass.cpp
libc++ :: std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/resource.pass.cpp
libc++ :: std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/select_on_container_copy_construction.pass.cpp
libc++ :: std/experimental/memory/memory.resource.aliases/header_deque_synop.pass.cpp
libc++ :: std/experimental/memory/memory.resource.aliases/header_forward_list_synop.pass.cpp
libc++ :: std/experimental/memory/memory.resource.aliases/header_list_synop.pass.cpp
libc++ :: std/experimental/memory/memory.resource.aliases/header_map_synop.pass.cpp
libc++ :: std/experimental/memory/memory.resource.aliases/header_regex_synop.pass.cpp
libc++ :: std/experimental/memory/memory.resource.aliases/header_set_synop.pass.cpp
libc++ :: std/experimental/memory/memory.resource.aliases/header_string_synop.pass.cpp
libc++ :: std/experimental/memory/memory.resource.aliases/header_unordered_map_synop.pass.cpp
libc++ :: std/experimental/memory/memory.resource.aliases/header_unordered_set_synop.pass.cpp
libc++ :: std/experimental/memory/memory.resource.aliases/header_vector_synop.pass.cpp
libc++ :: std/experimental/memory/memory.resource.global/default_resource.pass.cpp
libc++ :: std/experimental/memory/memory.resource.global/new_delete_resource.pass.cpp
libc++ :: std/experimental/memory/memory.resource.global/null_memory_resource.pass.cpp

And here is one failure, if it helps:

FAIL: libc++ :: std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp (72395 of 81491)

  • TEST 'libc++ :: std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp' FAILED ****

Script:

: 'COMPILED WITH'; /usr/bin/clang++ /home/dave/ro_s/lp/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp -v --target=x86_64-unknown-linux-gnu -include /tmp/_update_lc/r/projects/libcxx/__config_site -include /home/dave/ro_s/lp/libcxx/test/support/nasty_macros.h -nostdinc++ -I/home/dave/ro_s/lp/libcxx/include -I/tmp/_update_lc/r/projects/libcxx/include/c++build -DSTDC_FORMAT_MACROS -DSTDC_LIMIT_MACROS -D__STDC_CONSTANT_MACROS -I/home/dave/ro_s/lp/libcxx/test/support -D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -std=c++2a -Werror -Wall -Wextra -Wshadow -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-user-defined-literals -Wno-noexcept-type -Wno-atomic-alignment -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -D_LIBCPP_DISABLE_AVAILABILITY -fcoroutines-ts -Werror=thread-safety -Wuser-defined-warnings -fmodules-cache-path=/tmp/_update_lc/r/projects/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/Output/construct_piecewise_pair.pass.cpp.dir/t.tmp/ModuleCache -Wno-macro-redefined -D_LIBCPP_HAS_THREAD_API_PTHREAD -Wno-macro-redefined -D_LIBCPP_ABI_VERSION=1 -L/tmp/_update_lc/r/./lib -Wl,-rpath,/tmp/_update_lc/r/./lib -nodefaultlibs /tmp/_update_lc/r/./lib/libc++.a -lc++abi -lm -lgcc_s -lgcc -lpthread -lrt -lc -lgcc_s -lgcc -latomic -lc++experimental -o /tmp/_update_lc/r/projects/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/Output/construct_piecewise_pair.pass.cpp.dir/t.tmp.exe

: 'EXECUTED AS'; "/usr/bin/python3.9" /home/dave/ro_s/lp/libcxx/test/../utils/run.py --execdir /tmp/_update_lc/r/projects/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/Output/construct_piecewise_pair.pass.cpp.dir --codesign_identity "" --env -- /tmp/_update_lc/r/projects/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/Output/construct_piecewise_pair.pass.cpp.dir/t.tmp.exe

Exit Code: 1

Command Output (stderr):

clang version 11.0.0 (Fedora 11.0.0-2.fc33)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-redhat-linux/10
Found candidate GCC installation: /usr/lib/gcc/x86_64-redhat-linux/10
Selected GCC installation: /usr/lib/gcc/x86_64-redhat-linux/10
Candidate multilib: .;@m64
Candidate multilib: 32;@m32
Selected multilib: .;@m64
"/usr/bin/clang-11" -cc1 -triple x86_64-unknown-linux-gnu -emit-obj -mrelax-all -disable-free -disable-llvm-verifier -discard-value-names -main-file-name construct_piecewise_pair.pass.cpp -mrelocation-model static -mframe-pointer=all -fmath-errno -fno-rounding-math -mconstructor-aliases -munwind-tables -target-cpu x86-64 -fno-split-dwarf-inlining -debugger-tuning=gdb -v -nostdinc++ -resource-dir /usr/lib64/clang/11.0.0 -include /tmp/_update_lc/r/projects/libcxx/__config_site -include /home/dave/ro_s/lp/libcxx/test/support/nasty_macros.h -I /home/dave/ro_s/lp/libcxx/include -I /tmp/_update_lc/r/projects/libcxx/include/c++build -D STDC_FORMAT_MACROS -D STDC_LIMIT_MACROS -D STDC_CONSTANT_MACROS -I /home/dave/ro_s/lp/libcxx/test/support -D _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -D _LIBCPP_DISABLE_AVAILABILITY -D _LIBCPP_HAS_THREAD_API_PTHREAD -D _LIBCPP_ABI_VERSION=1 -internal-isystem /usr/local/include -internal-isystem /usr/lib64/clang/11.0.0/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -Werror -Wall -Wextra -Wshadow -Wno-unused-command-line-argument -Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions -Wno-user-defined-literals -Wno-noexcept-type -Wno-atomic-alignment -Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code -Wno-unused-local-typedef -Werror=thread-safety -Wuser-defined-warnings -Wno-macro-redefined -Wno-macro-redefined -std=c++2a -fdeprecated-macro -fdebug-compilation-dir /tmp/_update_lc/r/projects/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem -ferror-limit 19 -fcoroutines-ts -fgnuc-version=4.2.1 -fno-implicit-modules -fcxx-exceptions -fexceptions -faddrsig -o /tmp/construct_piecewise_pair-00f260.o -x c++ /home/dave/ro_s/lp/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/construct_piecewise_pair.pass.cpp
clang -cc1 version 11.0.0 based upon LLVM 11.0.0 default target x86_64-unknown-linux-gnu
ignoring nonexistent directory "/include"
#include "..." search starts here:
#include <...> search starts here:
/home/dave/ro_s/lp/libcxx/include
/tmp/_update_lc/r/projects/libcxx/include/c++build
/home/dave/ro_s/lp/libcxx/test/support
/usr/local/include
/usr/lib64/clang/11.0.0/include
/usr/include
End of search list.
"/usr/bin/ld" --hash-style=gnu --build-id --eh-frame-hdr -m elf_x86_64 -dynamic-linker /lib64/ld-linux-x86-64.so.2 -o /tmp/_update_lc/r/projects/libcxx/test/std/experimental/memory/memory.polymorphic.allocator.class/memory.polymorphic.allocator.mem/Output/construct_piecewise_pair.pass.cpp.dir/t.tmp.exe /usr/lib/gcc/x86_64-redhat-linux/10/../../../../lib64/crt1.o /usr/lib/gcc/x86_64-redhat-linux/10/../../../../lib64/crti.o /usr/lib/gcc/x86_64-redhat-linux/10/crtbegin.o -L/tmp/_update_lc/r/./lib -L/usr/lib/gcc/x86_64-redhat-linux/10 -L/usr/lib/gcc/x86_64-redhat-linux/10/../../../../lib64 -L/usr/bin/../lib64 -L/lib/../lib64 -L/usr/lib/../lib64 -L/usr/lib/gcc/x86_64-redhat-linux/10/../../.. -L/usr/bin/../lib -L/lib -L/usr/lib /tmp/construct_piecewise_pair-00f260.o -rpath /tmp/_update_lc/r/./lib /tmp/_update_lc/r/./lib/libc++.a -lc++abi -lm -lgcc_s -lgcc -lpthread -lrt -lc -lgcc_s -lgcc -latomic -lc++experimental /usr/lib/gcc/x86_64-redhat-linux/10/crtend.o /usr/lib/gcc/x86_64-redhat-linux/10/../../../../lib64/crtn.o
/usr/bin/ld: /tmp/_update_lc/r/./lib/libc++experimental.a(memory_resource.cpp.o): in function `std::experimental::fundamentals_v1::pmr::
null_memory_resource_imp::do_allocate(unsigned long, unsigned long)':
memory_resource.cpp:(.text._ZNSt12experimental15fundamentals_v13pmr26null_memory_resource_imp11do_allocateEmm[_ZNSt12experimental15fundamentals_v13pmr26null_memory_resource_imp11do_allocateEmm]+0x2): undefined reference to `std::__throw_bad_alloc()'
clang-11: error: linker command failed with exit code 1 (use -v to see invocation)

phosek added a comment.Mar 6 2021, 1:07 PM

What's your build setup? Do you use regular, runtimes or standalone build? Do you have instructions I could use to reproduce the failure?

From my build script:

-DCMAKE_C_COMPILER=clang \
-DCMAKE_CXX_COMPILER=clang++ \
-DCMAKE_LINKER=ld.lld \
-DLLVM_TARGETS_TO_BUILD="X86;AArch64;PowerPC;RISCV;AMDGPU" \
-DLLVM_INCLUDE_GO_TESTS=FALSE \
-DLLVM_TOOL_LTO_BUILD=FALSE \
-DLLVM_ENABLE_PROJECTS="clang;lld;libcxxabi;libcxx;lldb" \
-DLLVM_INCLUDE_GO_TESTS=FALSE \
-DLLVM_USE_LINKER=lld \
-DLLVM_ENABLE_LIBCXX=TRUE \
-DCLANG_DEFAULT_LINKER=lld \
-DCLANG_DEFAULT_CXX_STDLIB="libc++" \
-DLIBCXX_ENABLE_SHARED=FALSE \
-DLIBCXXABI_ENABLE_SHARED=FALSE \
-DLIBUNWIND_ENABLE_SHARED=FALSE \
-DLLVM_LIBC_ENABLE_LINTING=FALSE \

From my build script:

-DCMAKE_C_COMPILER=clang \
-DCMAKE_CXX_COMPILER=clang++ \
-DCMAKE_LINKER=ld.lld \
-DLLVM_TARGETS_TO_BUILD="X86;AArch64;PowerPC;RISCV;AMDGPU" \
-DLLVM_INCLUDE_GO_TESTS=FALSE \
-DLLVM_TOOL_LTO_BUILD=FALSE \
-DLLVM_ENABLE_PROJECTS="clang;lld;libcxxabi;libcxx;lldb" \
-DLLVM_INCLUDE_GO_TESTS=FALSE \
-DLLVM_USE_LINKER=lld \
-DLLVM_ENABLE_LIBCXX=TRUE \
-DCLANG_DEFAULT_LINKER=lld \
-DCLANG_DEFAULT_CXX_STDLIB="libc++" \
-DLIBCXX_ENABLE_SHARED=FALSE \
-DLIBCXXABI_ENABLE_SHARED=FALSE \
-DLIBUNWIND_ENABLE_SHARED=FALSE \
-DLLVM_LIBC_ENABLE_LINTING=FALSE \

Why do you use these settings?

-DLIBCXX_ENABLE_SHARED=FALSE
-DLIBCXXABI_ENABLE_SHARED=FALSE
-DLIBUNWIND_ENABLE_SHARED=FALSE

What is the intent of your build job?

Why do you use these settings?

-DLIBCXX_ENABLE_SHARED=FALSE
-DLIBCXXABI_ENABLE_SHARED=FALSE
-DLIBUNWIND_ENABLE_SHARED=FALSE

What is the intent of your build job?

I try to ensure that building/testing works with PIC disabled, which (of course) requires disabling shared libraries.

Why do you use these settings?

-DLIBCXX_ENABLE_SHARED=FALSE
-DLIBCXXABI_ENABLE_SHARED=FALSE
-DLIBUNWIND_ENABLE_SHARED=FALSE

What is the intent of your build job?

I try to ensure that building/testing works with PIC disabled, which (of course) requires disabling shared libraries.

@davezarzycki The problem here is that this change enabled the libc++ and libc++abi tests in your job, which were not being run before. I think there might be issues with the configuration you're using and this change only makes those issues apparent.

Why do you use these settings?

-DLIBCXX_ENABLE_SHARED=FALSE
-DLIBCXXABI_ENABLE_SHARED=FALSE
-DLIBUNWIND_ENABLE_SHARED=FALSE

What is the intent of your build job?

I try to ensure that building/testing works with PIC disabled, which (of course) requires disabling shared libraries.

@davezarzycki The problem here is that this change enabled the libc++ and libc++abi tests in your job, which were not being run before. I think there might be issues with the configuration you're using and this change only makes those issues apparent.

Can you please elaborate on what those issues might be?

Can you please elaborate on what those issues might be?

Things should work after https://reviews.llvm.org/D99268.

ldionne reopened this revision.Mar 24 2021, 1:32 PM
This revision is now accepted and ready to land.Mar 24 2021, 1:32 PM
ldionne commandeered this revision.Mar 24 2021, 1:32 PM
ldionne edited reviewers, added: phosek; removed: ldionne.
This revision now requires review to proceed.Mar 24 2021, 1:32 PM
ldionne updated this revision to Diff 333111.Mar 24 2021, 1:46 PM

Re-apply the libcxx part

This revision was not accepted when it landed; it landed in state Needs Review.Mar 24 2021, 1:47 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.