Skip to content

Conversation

@kuilpd
Copy link
Collaborator

@kuilpd kuilpd commented Dec 19, 2024

Add unit tests ported from lldb-eval.

TODO:

  • Fix errors in XFailed tests
  • Fix segfault errors in GTEST_SKIP tests
  • Implement interfaces for scoped, context, and separate parsing and re-enable DISABLED_ tests

@kuilpd kuilpd requested a review from cmtice December 19, 2024 16:39
@kuilpd
Copy link
Collaborator Author

kuilpd commented Dec 19, 2024

Since not all test cases were moved to python tests, I added unit tests from lldb-eval to see better what functionality is missing. Notably, a lot of expressions are not making it past the lexer now, many of which can be fixed by solving issue #4

This pull request is just to show the branch. My question is, should we maintain these unit tests or move them one by one to python suite as we fix them?

@kuilpd kuilpd marked this pull request as ready for review January 24, 2025 18:53
@kuilpd
Copy link
Collaborator Author

kuilpd commented Jan 24, 2025

@cmtice
Cleaned up the unit tests:

  • Marked currently failing tests as XFail and they will pass until the issue is fixed (every test suite will print the number of these tests in the summary)
  • Skipped 3 tests that cause a segfault, marked by GTEST_SKIP
  • Added placeholders for currently unimplemented functionality: scope evaluation, evaluation with context, separate parsing. They return an error when running a relevant test
  • Disabled all the test that use unimplemented functionality, marked by DISABLED_ and printed out at the end of unit test summary

Haven't figured out yet how to build the test binary from source and pass it to the unit tests, so for now it's still a pre-built binary.
If it doesn't work on your PC, clang++ -O0 -g -stdlib=libc++ test_binary.cc -o test_binary_libc++.bin in lldb/unittests/DIL/Inputs directory will rebuild it for your system.

Please try launching the tests and tell me how it goes :)
If it works decent enough, we can merge it to main and then fix the tests along with bugs in other pull requests.

@kuilpd
Copy link
Collaborator Author

kuilpd commented Jan 27, 2025

@cmtice
Added a way to compile the test binary with the in-tree clang. It's kind of hacky, requires clang and lld projects enabled, and libc++ installed in the system (some of the tests depend specifically on being linked against libc++, I will probably adjust that later)

get_target_property(EXE_PATH LLDBDILTests RUNTIME_OUTPUT_DIRECTORY)
add_custom_command(
OUTPUT test_binary.bin
COMMAND $<TARGET_FILE:clang> test_binary.cc -std=c++17 -fuse-ld=lld -B$<TARGET_FILE:lld>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
COMMAND $<TARGET_FILE:clang> test_binary.cc -std=c++17 -fuse-ld=lld -B$<TARGET_FILE:lld>
COMMAND $<TARGET_FILE:clang> ${CMAKE_CURRENT_SOURCE_DIR}/test_binary.cc -std=c++17 -fuse-ld=lld -B$<TARGET_FILE:lld>

Copy link
Collaborator

Choose a reason for hiding this comment

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

I found that I needed to tell clang what directory to look in, for building test_binary.cc

@cmtice
Copy link
Collaborator

cmtice commented Jan 28, 2025

I found that with the small change I've shown above, the unittest builds & runs for me. I get 2 failures:
[ FAILED ] 2 tests, listed below:
[ FAILED ] EvalTest.TestCxxReinterpretCast
[ FAILED ] EvalTest.TestUniquePtr

@kuilpd
Copy link
Collaborator Author

kuilpd commented Jan 28, 2025

Hmm, these are the tests that fail when I link against libstdc++.
I should add checks for which library is used anyway, will do that next.

@kuilpd
Copy link
Collaborator Author

kuilpd commented Jan 29, 2025

@cmtice
Added linking against in-tree libc++ and adjusted tests accordingly, so that there is no ambiguity about which library is used.
Just need to enable runtimes libcxx;libcxxabi;libunwind in the project.

@kuilpd
Copy link
Collaborator Author

kuilpd commented Feb 5, 2025

@cmtice Could you please check if the tests work after the last changes?

@kuilpd kuilpd changed the title Tests and fuzzer from lldb-eval Add unit tests from lldb-eval Feb 5, 2025
@cmtice
Copy link
Collaborator

cmtice commented Feb 5, 2025

@cmtice Could you please check if the tests work after the last changes?

The tests build just fine. When I run them I get the following results:
[ PASSED ] 60 tests.
[ SKIPPED ] 3 tests, listed below:
[ SKIPPED ] EvalTest.TestCStyleCastPointer
[ SKIPPED ] EvalTest.TestCxxStaticCast
[ SKIPPED ] EvalTest.TestTernaryOperator
[ FAILED ] 1 test, listed below:
[ FAILED ] EvalTest.TestCxxReinterpretCast

1 FAILED TEST
YOU HAVE 23 DISABLED TESTS

@kuilpd
Copy link
Collaborator Author

kuilpd commented Feb 5, 2025

Okay, disabled and skipped tests are supposed to be there, but nothing should fail. What are the errors?

@cmtice
Copy link
Collaborator

cmtice commented Feb 5, 2025

Okay, disabled and skipped tests are supposed to be there, but nothing should fail. What are the errors?

[ RUN ] EvalTest.TestCxxReinterpretCast
/usr/local/google3/cmtice/access-softek-dil/lldb/unittests/DIL/DILTests.cpp:1779: Failure
Value of: Eval("reinterpret_cast<nullptr_t>(ptr)")
Expected: evaluates with an error: 'reinterpret_cast from 'int *' to 'nullptr_t' (canonically referred to as 'std::nullptr_t') is not allowed'
Actual: { DIL: NULL, lldb: NULL }, error:
:1:18: unknown type name 'nullptr_t'
reinterpret_cast<nullptr_t>(ptr)
^

/usr/local/google3/cmtice/access-softek-dil/lldb/unittests/DIL/DILTests.cpp:1783: Failure
Value of: Eval("reinterpret_cast<nullptr_t>(0)")
Expected: evaluates with an error: 'reinterpret_cast from 'int' to 'nullptr_t' (canonically referred to as 'std::nullptr_t') is not allowed'
Actual: { DIL: NULL, lldb: NULL }, error:
:1:18: unknown type name 'nullptr_t'
reinterpret_cast<nullptr_t>(0)
^

/usr/local/google3/cmtice/access-softek-dil/lldb/unittests/DIL/DILTests.cpp:1787: Failure
Value of: Eval("reinterpret_cast<nullptr_t>(nullptr)")
Expected: evaluates with an error: 'reinterpret_cast from 'std::nullptr_t' to 'nullptr_t' (canonically referred to as 'std::nullptr_t') is not allowed'
Actual: { DIL: NULL, lldb: NULL }, error:
:1:18: unknown type name 'nullptr_t'
reinterpret_cast<nullptr_t>(nullptr)
^

@kuilpd
Copy link
Collaborator Author

kuilpd commented Feb 5, 2025

I skipped these tests for now, it's just the error message not matching exactly, will figure it out later why do they match on my build, but not on yours.
If none of the tests fail now, I guess we can merge?

@cmtice
Copy link
Collaborator

cmtice commented Feb 5, 2025

I skipped these tests for now, it's just the error message not matching exactly, will figure it out later why do they match on my build, but not on yours. If none of the tests fail now, I guess we can merge?

SGTM!

@kuilpd
Copy link
Collaborator Author

kuilpd commented Feb 6, 2025

I misunderstood how GTEST_SKIP() works, it skips the rest of the test fixture and marks it skipped, so I moved the tests I want to skip to the end of their fixture.

@kuilpd kuilpd merged commit d1ea418 into dil-main Feb 6, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants