[lldb][test] Fix `LibCxxInternalsRecognizerTestCase` on clang <= 17 #114122

vogelsgesang · 2024-10-29T20:24:25Z

We had to disable the tests for libc++ <= 15 because the std::ranges functions were not available, yet.

Also, on libc++17 there was still an additional __fn struct withing ranges::__sort. The test expectation was updated to use a regular expression, so we can match both the old and the new name.

See https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-matrix/912/execution/node/107/log/

Because of a build failure with libc++17. See https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-matrix/912/execution/node/107/log/

llvmbot · 2024-10-29T20:25:00Z

@llvm/pr-subscribers-lldb

Author: Adrian Vogelsgesang (vogelsgesang)

Changes

Because of a build failure with libc++17.
See https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-matrix/912/execution/node/107/log/

Full diff: https://github.com/llvm/llvm-project/pull/114122.diff

1 Files Affected:

(modified) lldb/test/API/lang/cpp/libcxx-internals-recognizer/TestLibcxxInternalsRecognizer.py (+1)

diff --git a/lldb/test/API/lang/cpp/libcxx-internals-recognizer/TestLibcxxInternalsRecognizer.py b/lldb/test/API/lang/cpp/libcxx-internals-recognizer/TestLibcxxInternalsRecognizer.py
index ad48208f21e502..0270e78808c646 100644
--- a/lldb/test/API/lang/cpp/libcxx-internals-recognizer/TestLibcxxInternalsRecognizer.py
+++ b/lldb/test/API/lang/cpp/libcxx-internals-recognizer/TestLibcxxInternalsRecognizer.py
@@ -8,6 +8,7 @@ class LibCxxInternalsRecognizerTestCase(TestBase):
     NO_DEBUG_INFO_TESTCASE = True
 
     @add_test_categories(["libc++"])
+    @skipIf(compiler="clang", compiler_version=["<", "18.0"])
     def test_frame_recognizer(self):
         """Test that implementation details of libc++ are hidden"""
         self.build()

felipepiovezan

Thanks!

Michael137 · 2024-10-29T21:24:10Z

Note that for the other failure you'll have to make sure we account for inline namespaces in the test assertion (or at least that's what my impression was from the error log)

Michael137 · 2024-10-29T21:51:33Z

Sorry to clarify. I think based on @felipepiovezan comment here: #108870 (comment)

On Clang 17 we compile fine, just the assertion fails to find the expected function name (because of the std::__1 inline namespace):

FAIL: test_frame_recognizer (TestLibcxxInternalsRecognizer.LibCxxInternalsRecognizerTestCase)
   Test that implementation details of libc++ are hidden
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/llvm-project/lldb/test/API/lang/cpp/libcxx-internals-recognizer/TestLibcxxInternalsRecognizer.py", line 60, in test_frame_recognizer
    self.assertIn(
AssertionError: 'ranges::__sort::operator()' not found in 'std::__1::__wrap_iter<int*> std::__1::ranges::__sort::__fn::operator()[abi:ue170006]<std::__1::__wrap_iter<int*>, std::__1::__wrap_iter<int*>, bool (*)(int, int), std::__1::identity>(std::__1::__wrap_iter<int*>, std::__1::__wrap_iter<int*>, bool (*)(int, int), std::__1::identity) const'
Config=x86_64-/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/clang_1706_build/bin/clang

So really we should skip for <= 15. And fix the assertion for 17+

felipepiovezan · 2024-10-30T15:56:14Z

Sorry to clarify. I think based on @felipepiovezan comment here: #108870 (comment)

On Clang 17 we compile fine, just the assertion fails to find the expected function name (because of the std::__1 inline namespace):

FAIL: test_frame_recognizer (TestLibcxxInternalsRecognizer.LibCxxInternalsRecognizerTestCase)
   Test that implementation details of libc++ are hidden
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/llvm-project/lldb/test/API/lang/cpp/libcxx-internals-recognizer/TestLibcxxInternalsRecognizer.py", line 60, in test_frame_recognizer
    self.assertIn(
AssertionError: 'ranges::__sort::operator()' not found in 'std::__1::__wrap_iter<int*> std::__1::ranges::__sort::__fn::operator()[abi:ue170006]<std::__1::__wrap_iter<int*>, std::__1::__wrap_iter<int*>, bool (*)(int, int), std::__1::identity>(std::__1::__wrap_iter<int*>, std::__1::__wrap_iter<int*>, bool (*)(int, int), std::__1::identity) const'
Config=x86_64-/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/clang_1706_build/bin/clang

So really we should skip for <= 15. And fix the assertion for 17+

Yeah, I'm just now hitting this assertion on our downstream fork tests too

vogelsgesang · 2024-10-30T22:22:12Z

On Clang 17 we compile fine, just the assertion fails to find the expected function name

good catch! Thanks for staying vigilant

(because of the std::__1 inline namespace):

I don't think the inline namespace is the issue here.
We are looking for ranges::__sort::operator() in [...] std::__1::ranges::__sort::__fn::operator() [...].
The issue seems to be an additional __fn wrapper struct

vogelsgesang · 2024-10-30T23:29:39Z

Should be fixed the correct way, now.
@Michael137 @felipepiovezan Would appreciate if you could take another look

github-actions · 2024-10-30T23:30:42Z

⚠️ Python code formatter, darker found issues in your code. ⚠️

You can test this locally with the following command:

darker --check --diff -r 70af40ba74cf62fdaa3ae1d7db972c138655049f...9c98586e63151dc6165b6a0e4a529f2bc20d8056 lldb/test/API/lang/cpp/libcxx-internals-recognizer/TestLibcxxInternalsRecognizer.py

View the diff from darker here.

--- TestLibcxxInternalsRecognizer.py	2024-10-30 23:43:11.000000 +0000
+++ TestLibcxxInternalsRecognizer.py	2024-10-30 23:46:18.234374 +0000
@@ -61,11 +61,11 @@
                 # Expect the correct parent frame
                 func_name = thread.GetFrameAtIndex(frame_id).GetFunctionName()
                 if isinstance(expected_parent, re.Pattern):
                     self.assertTrue(
                         expected_parent.search(func_name) is not None,
-                        f"'{expected_parent}' not found in '{func_name}'"
+                        f"'{expected_parent}' not found in '{func_name}'",
                     )
                 else:
                     self.assertIn(expected_parent, func_name)
                 frame_id = frame_id + 1
             process.Continue()

Michael137

LGTM thx!

felipepiovezan

Awesome, thank you!

felipepiovezan · 2024-10-31T16:01:15Z

@vogelsgesang since I don't know if our timezones align, I am going to click the merge button here to see if we can get the bots green again!

felipepiovezan · 2024-11-01T15:49:38Z

It's interesting that the 17.0 bot is still not happy:

  File "/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 321, in check_value
    test_base.assertEqual(
AssertionError: '"Hello"' != None : Checking child with index 0:
(std::__lldb::optional<const char *>) maybe_string =  Has Value=true  {
  Value = 0x000000010a2f3f9e
}
Checking SBValue: (std::__1::remove_cv_t<value_type>) Value = 0x000000010a2f3f9e
Config=x86_64-/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/clang_1706_build/bin/clang

https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-matrix/925/execution/node/135/log/

Michael137 · 2024-11-01T16:49:57Z

It's interesting that the 17.0 bot is still not happy:


  File "/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 321, in check_value

    test_base.assertEqual(

AssertionError: '"Hello"' != None : Checking child with index 0:

(std::__lldb::optional<const char *>) maybe_string =  Has Value=true  {

  Value = 0x000000010a2f3f9e

}

Checking SBValue: (std::__1::remove_cv_t<value_type>) Value = 0x000000010a2f3f9e

Config=x86_64-/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/clang_1706_build/bin/clang

https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-matrix/925/execution/node/135/log/

i think that's a different test though (meant to look into it but havent yet :D )

felipepiovezan · 2024-11-01T16:56:10Z

It's interesting that the 17.0 bot is still not happy:


  File "/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 321, in check_value

    test_base.assertEqual(

AssertionError: '"Hello"' != None : Checking child with index 0:

(std::__lldb::optional<const char *>) maybe_string =  Has Value=true  {

  Value = 0x000000010a2f3f9e

}

Checking SBValue: (std::__1::remove_cv_t<value_type>) Value = 0x000000010a2f3f9e

Config=x86_64-/Users/ec2-user/jenkins/workspace/llvm.org/lldb-cmake-matrix/clang_1706_build/bin/clang

https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-matrix/925/execution/node/135/log/

i think that's a different test though (meant to look into it but havent yet :D )

oh oops, sorry, didn't notice that

…lvm#114122) We had to disable the tests for libc++ <= 15 because the `std::ranges` functions were not available, yet. Also, on libc++17 there was still an additional `__fn` struct withing `ranges::__sort`. The test expectation was updated to use a regular expression, so we can match both the old and the new name. See https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-matrix/912/execution/node/107/log/

[lldb][test] Skip LibCxxInternalsRecognizerTestCase on clang <= 17

00d136e

Because of a build failure with libc++17. See https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake-matrix/912/execution/node/107/log/

vogelsgesang requested a review from JDevlieghere as a code owner October 29, 2024 20:24

llvmbot added the lldb label Oct 29, 2024

vogelsgesang mentioned this pull request Oct 29, 2024

[lldb][libc++] Hide all libc++ implementation details from stacktraces #108870

Merged

vogelsgesang requested a review from felipepiovezan October 29, 2024 20:25

felipepiovezan approved these changes Oct 29, 2024

View reviewed changes

felipepiovezan self-requested a review October 30, 2024 15:55

Correctly fix the issue

f59335a

vogelsgesang changed the title ~~[lldb][test] Skip LibCxxInternalsRecognizerTestCase on clang <= 17~~ [lldb][test] Fix LibCxxInternalsRecognizerTestCase on clang <= 17 Oct 30, 2024

Format

9c98586

Michael137 approved these changes Oct 31, 2024

View reviewed changes

felipepiovezan approved these changes Oct 31, 2024

View reviewed changes

felipepiovezan merged commit 2aed0d9 into llvm:main Oct 31, 2024
4 of 7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[lldb][test] Fix `LibCxxInternalsRecognizerTestCase` on clang <= 17 #114122

[lldb][test] Fix `LibCxxInternalsRecognizerTestCase` on clang <= 17 #114122

Uh oh!

vogelsgesang commented Oct 29, 2024 •

edited

Loading

Uh oh!

llvmbot commented Oct 29, 2024

Uh oh!

felipepiovezan left a comment

Uh oh!

Michael137 commented Oct 29, 2024 •

edited

Loading

Uh oh!

Michael137 commented Oct 29, 2024

Uh oh!

felipepiovezan commented Oct 30, 2024

Uh oh!

vogelsgesang commented Oct 30, 2024

Uh oh!

vogelsgesang commented Oct 30, 2024

Uh oh!

github-actions bot commented Oct 30, 2024 •

edited

Loading

Uh oh!

Michael137 left a comment

Uh oh!

felipepiovezan left a comment

Uh oh!

felipepiovezan commented Oct 31, 2024 •

edited

Loading

Uh oh!

Uh oh!

felipepiovezan commented Nov 1, 2024

Uh oh!

Michael137 commented Nov 1, 2024

Uh oh!

felipepiovezan commented Nov 1, 2024

Uh oh!

Uh oh!

[lldb][test] Fix LibCxxInternalsRecognizerTestCase on clang <= 17 #114122

[lldb][test] Fix LibCxxInternalsRecognizerTestCase on clang <= 17 #114122

Uh oh!

Conversation

vogelsgesang commented Oct 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Oct 29, 2024

Uh oh!

felipepiovezan left a comment

Choose a reason for hiding this comment

Uh oh!

Michael137 commented Oct 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Michael137 commented Oct 29, 2024

Uh oh!

felipepiovezan commented Oct 30, 2024

Uh oh!

vogelsgesang commented Oct 30, 2024

Uh oh!

vogelsgesang commented Oct 30, 2024

Uh oh!

github-actions bot commented Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Michael137 left a comment

Choose a reason for hiding this comment

Uh oh!

felipepiovezan left a comment

Choose a reason for hiding this comment

Uh oh!

felipepiovezan commented Oct 31, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

felipepiovezan commented Nov 1, 2024

Uh oh!

Michael137 commented Nov 1, 2024

Uh oh!

felipepiovezan commented Nov 1, 2024

Uh oh!

Uh oh!

[lldb][test] Fix `LibCxxInternalsRecognizerTestCase` on clang <= 17 #114122

[lldb][test] Fix `LibCxxInternalsRecognizerTestCase` on clang <= 17 #114122

vogelsgesang commented Oct 29, 2024 •

edited

Loading

Michael137 commented Oct 29, 2024 •

edited

Loading

github-actions bot commented Oct 30, 2024 •

edited

Loading

felipepiovezan commented Oct 31, 2024 •

edited

Loading