Some API test failures are really opaque/could be improved

Subprojects LLDB
#1

Wondering if anyone else has encountered/dealt with debugging lldb test failures like the one shown at the end of this email (“AssertionError: 10 != 5” in “test.assertEqual(process.GetState(), lldb.eStateStopped)” while checking that a breakpoint was reached)

Is there anything that could be done to improve the debuggability of such failures? Logging standard output/error from the lldb process or any other logging it might have? At least for one of these I managed to figure it out by running lldb directly on the binary and finding that the binary couldn’t find libc++.so (that’s a story for another bug/email thread, looks like maybe lldb libc++ pretty printer tests are using the system installed libc++, not the just-built libc++ (& thus not running if there is no system installed libc++)). But my current failure like this seems a bit more inscrutible and I’m still looking into it.

Looks like dotest.py has some sense of logging (it has a --log-success option which says preserves the logs even on failure - though the output of dotest.py, at least for me, has no mention of logs, log files, or where they might be located, and looking at the source points to some sort of “.log” files… ah, found some)

So, yeah, there do seem to be some Failure.log, SkippedTest.log, etc - should dotest print something about the paths to those files when it exits with failure, maybe?

#2

I think https://reviews.llvm.org/D111978 ,
https://reviews.llvm.org/D111981 and the other patches Pavel & me put
up today should improve this situation IIUC.

- Raphael

#3

I think https://reviews.llvm.org/D111978 ,
https://reviews.llvm.org/D111981 and the other patches Pavel & me put
up today should improve this situation IIUC.

Thanks Raphael - really appreciate you & looking into this!

With https://reviews.llvm.org/D111981 I still seem to not have that cxx dependency (building/running the test, then building libcxx, then running the test again goes from unsupported → failing) - didn’t seem to work for me?

The diagnostic improvement sounds good to me (pity about whatever limitations it has re: remote or windows execution gathering the stdout). I guess gathering the logs in the remote execution case has the same problem (that the log is made on the remote machine/non-trivial to retrieve?)?

& yeah, have you got any patches/ideas about how to make the libcxx tests use the just-built libcxx library (using LD_LIBRARY_PATH, rpath, etc)? For now, even with both these fixes I’ll just be seeing (consistent, which is nice) failures, rather than being able to run these tests successfully. I’ll either have to get used to ignoring certain failures, or disable the tests by not building libcxx in that build tree, which would also be unfortunate. (or maybe there’s some other workarounds?) Any idea how this works for other folks?

  • Dave
#4

I just saw in your review comment that this is using
LLVM_ENABLE_RUNTIMES and not LLVM_ENABLE_PROJECTS for libcxx, so the
failure just comes from us setting the wrong RPATH due to the
different runtimes library directory (at least from what I can see).
Would it be possible to put libcxx in LLVM_ENABLE_PROJECTS for now? I
think this shouldn't be too hard to fix (famous last words?).

#5

Actually the RPATH theory is wrong, but the LLVM_ENABLE_PROJECT
workaround *should* still work.

#6

Actually the RPATH theory is wrong, but the LLVM_ENABLE_PROJECT
workaround should still work.

I’ll give that a go (it’s running at the moment) though I guess this is inconsistent with the direction libcxx is moving in for building, re: https://groups.google.com/g/llvm-dev/c/tpuLxk_ipLw

#7

Yep, it does work with LLVM_ENABLE_PROJECT rather than LLVM_ENABLE_RUNTIME.

Specifically the test binary is linked with an rpath to the just-built lib directory that ensures the just-built libc++.so is found:

/usr/local/google/home/blaikie/dev/llvm/build/release/bin/clang main.o -g -O0 -fno-builtin -m64 -I/usr/local/google/home/blaikie/dev/llvm/src/lldb/packages/Python/lldbsuite/test/make/…/…/…/…/…/include -I/usr/local/google/home/blaikie/dev/llvm/src/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx/list -I/usr/local/google/home/blaikie/dev/llvm/src/lldb/packages/Python/lldbsuite/test/make -include /usr/local/google/home/blaikie/dev/llvm/src/lldb/packages/Python/lldbsuite/test/make/test_common.h -fno-limit-debug-info -gsplit-dwarf -stdlib=libc++ -Wl,-rpath,/usr/local/google/home/blaikie/dev/llvm/build/release/./lib --driver-mode=g++ -o “a.out”

Oh, actually it passes the same rpath when using LLVM_ENABLE_RUNTIME, but the libc++.so.1 is in a different place: ./lib/x86_64-unknown-linux-gnu/libc++.so.1

Looks like this rpath setting happens here: (changing this to a junk argument causes the test to fail to build as expected)
https://github.com/llvm/llvm-project/blob/618583565687f5a494066fc902a977f6057fc93e/lldb/packages/Python/lldbsuite/test/make/Makefile.rules#L400

And it gets the LLVM_LIBS_DIR from here: https://github.com/llvm/llvm-project/blob/207998c242c8c8a270ff22a5136da87338546725/lldb/test/API/lit.cfg.py#L163

So maybe we need to pass down the default target triple too, since that seems to be how libc++ is deciding where to put the library? ( https://github.com/llvm/llvm-project/blob/207998c242c8c8a270ff22a5136da87338546725/libcxx/CMakeLists.txt#L424 ) at least on non-apple :confused: (or maybe there’s some way to make the connection between the two less brittle - for libc++'s build to export some variable that lldb can use, or for LLVM to provide something for both to use?)

Yeah, applying this change does work for me, but wouldn’t work on Apple for instance (where libcxx doesn’t add the default target triple to the path):

$ git diff

diff --git lldb/test/API/lit.site.cfg.py.in lldb/test/API/lit.site.cfg.py.in

index 987078a53edb…e327429b7ff9 100644

— lldb/test/API/lit.site.cfg.py.in

+++ lldb/test/API/lit.site.cfg.py.in

@@ -3,7 +3,7 @@

config.llvm_src_root = “@LLVM_SOURCE_DIR@”

config.llvm_obj_root = “@LLVM_BINARY_DIR@”

config.llvm_tools_dir = “@LLVM_TOOLS_DIR@”

-config.llvm_libs_dir = “@LLVM_LIBS_DIR@”

+config.llvm_libs_dir = “@LLVM_LIBS_DIR@/@LLVM_DEFAULT_TARGET_TRIPLE@”

config.llvm_shlib_dir = “@SHLIBDIR@”

config.llvm_build_mode = “@LLVM_BUILD_MODE@”

config.lit_tools_dir = “@LLVM_LIT_TOOLS_DIR@”

Thoughts?

#8

+Louis Dionne - perhaps the libcxx and lldb folks would be interesting in finding a suitable way to address this issue, since currently either option (using libcxx in ENABLE_PROJECTS or using it in ENABLE_RUNTIMES) is incomplete - if I use ENABLE_RUNTIMES I get the libcxx testing run against the just-built clang and generally this is the “supported” configuration, but then some lldb tests fail because they can’t find libcxx.so.1 (on Linux) - and using ENABLE_PROJECTS means not using the just-built clang for libcxx tests (so missing the libcxx breakages caused by my array name change) but do use the just-built libcxx in lldb tests and find failures there…

#9

I believe the issue is probably not related so much to LLVM_ENABLE_PROJECTS vs LLVM_ENABLE_RUNTIMES, but rather to the fact that LLVM_ENABLE_RUNTIMES uses per-target runtime directories now (hasn’t always been the case), which basically means that libc++ ends up in <prefix>/lib/<target-triple>/libc++.so instead of <prefix>/lib/libc++.so.

I think you either want to specify the per-target library dir when running against libc++, or you want to disable that and use LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF when configuring the runtimes. In all cases, you want to be using LLVM_ENABLE_RUNTIMES and not LLVM_ENABLE_PROJECTS, since the latter is deprecated now.

Cheers,
Louis

#10

I believe the issue is probably not related so much to LLVM_ENABLE_PROJECTS vs LLVM_ENABLE_RUNTIMES, but rather to the fact that LLVM_ENABLE_RUNTIMES uses per-target runtime directories now (hasn’t always been the case), which basically means that libc++ ends up in <prefix>/lib/<target-triple>/libc++.so instead of <prefix>/lib/libc++.so.

Ish, yes. It’s a bug in LLVM_ENABLE_RUNTIMES that isn’t present in LLVM_ENABLE_PROJECTS, so for now if I want to run the lldb pretty printer tests for libc++ on Linux it seems the only way I can is by using the deprecated functionality of libc++ in LLVM_ENABLE_PROJECTS.

Consider this a bug report (looks like a bug in the lldb CMake configuration, not in libc++'s build itself, but something to figure out if Linux lldb devs are going to use libc++ +.ENABLE_RUNTIMES path) on that deprecation?

I think you either want to specify the per-target library dir when running against libc++, or you want to disable that and use LLVM_ENABLE_PER_TARGET_RUNTIME_DIR=OFF when configuring the runtimes. In all cases, you want to be using LLVM_ENABLE_RUNTIMES and not LLVM_ENABLE_PROJECTS, since the latter is deprecated now.

I didn’t enable LLVM_ENABLE_PER_TARGET_RUNTIME_DIR myself/in the root cmake config. It looks like it’s hardcoded(?) into the ENABLE_RUNTIMES sub-build? https://github.com/llvm/llvm-project/blob/e5fb79b31424267704e9d2d9674089fd7316453e/llvm/runtimes/CMakeLists.txt#L76 I’m not sure there’s any way to override that from the root? And in any case I’d have thought the defaults would need to/be intended to work correctly on supported platforms?

So something in lldb’s dir handling (maybe some general infrastructure in LLVM could use some improvement to provide an LLVM_RUNTIME_LIBS_DIR, or similar? that could then be used from other places - rather than libc++, for instance, creating that directory for itself based on LLVM_LIBS_DIR and LLVM_ENABLE_PER_TARGET_RUNTIME_DIR, etc) needs some fixes to support the current defaults/hardcoded modes on Linux?

#11

Is someone currently working on fixing this? FWIW, I think David's
change seems to go in the right direction (when I originally looked at
this I also ended up on the wrong rpath but I thought it was some
other code that set the wrong value. Didn't realize we have two places
where this happens). I think David's diff is better than we currently
have so maybe we should just turn this into a review?

#12

I haven’t made any further progress on it - I think the actual git diff I posted, changing config.llvm_libs_dir wouldn’t quite be shippable as-is, because it’s only correct to add the “/@LLVM_DEFAULT_TARGET_TRIPLE@” if the runtimes were built with LLVM_ENABLE_PER_TARGET_RUNTIME_DIR ON (which is the default on Linux, but not on MacOS) - so some extra conditionality is needed, but I’m not sure of the best/right place to implement that.