Skip to content

[lldb][test] Fix TestStdCXXDisassembly test #112357

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 15, 2024
Merged

[lldb][test] Fix TestStdCXXDisassembly test #112357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
29326de
[lldb][test] Fix TestStdCXXDisassembly test
slydiman Oct 15, 2024
bd0c917
Skip test if libstdc++.so is missing.
slydiman Oct 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion lldb/test/API/lang/cpp/stl/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
CXX_SOURCES := main.cpp

USE_SYSTEM_STDLIB := 1
ifneq ($(OS),Darwin)
USE_LIBSTDCPP := 1
Copy link
Contributor

@zeroomega zeroomega Oct 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This will break every test under lldb/test/API/lang/cpp/stl on non darwin machines if GNU libstdc++ is not installed.

I don't think it is a correct to assume the host machine has libstdc++. Plenty of bots are set up in a way to avoid accidentally (and implicitly) introducing GNU toolchains in a LLVM build by removing the entire GNU toolchain from the host machine. It appears the upstream LLDB bots are not set up in this way.

If libstdc++ is required to run this test (and it is the indended behavior), the correct way is to introduce LLVM cmake flag, e.g. LLDB_STDCPP_PATH to explicitly specify the path to the libstdc++, and disable the tests that depends on it in the build system.

But from what I see in the patch history. I believe both tests are not bound to a specific c++ lib. It just need either libc++ or libstdc++. And specifically, if libcxx is linked statically, the test runner somehow needs the libc++ shared object, but it may be not present. If that is the case, the way this patch addresses the test failure on upstream llvm bot is wrong. The correct way it to fix the test runner 1> make sure it builds and works properly when libc++ is linked statically instead of depending on the presence of shared object 2> skip the tests if libcxx and libstd++ is not present instead of forcing using libstdc++ regardless of host environment.

Looking at logics at

llvm-project/lldb/test/API/lang/cpp/stl/TestStdCXXDisassembly.py

Line 34 in 4ddea29

for i in range(target.GetNumModules()):
I think what contributed to the failure on our bot is that it found the libstdc++.so or libc++.so in the modules. Since we don't have gcc installed, the header files for libstdc++ doesn't exist, but clang was invoked with -stdlib=libstdc++ so it is forced to look for gcc's c++ header files. They do not exist. I am not 100% sure if this logic will skip the test if both so files are absent (it looks to me the test will fail with "FAILHORRIBLYHERE"). This logic is also not present in TestSTL.py. So it will fail consistently if none of the so files are present.

else
USE_SYSTEM_STDLIB := 1
endif

include Makefile.rules
5 changes: 4 additions & 1 deletion lldb/test/API/lang/cpp/stl/TestStdCXXDisassembly.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@ def test_stdcxx_disasm(self):
# At this point, lib_stdcxx is the full path to the stdc++ library and
# module is the corresponding SBModule.

self.expect(lib_stdcxx, "Library StdC++ is located", exe=False, substrs=["lib"])
if "lib" not in lib_stdcxx:
self.skipTest(
"This test requires libstdc++.so or libc++.dylib in the target's module list."
)

self.runCmd("image dump symtab '%s'" % lib_stdcxx)
raw_output = self.res.GetOutput()
Expand Down
Loading