Skip to content

[libc++][CI] Tests LLDB libc++ data formatters. #88312

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.

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

Conversation

mordante
Copy link
Member

@mordante mordante commented Apr 10, 2024

This enables testing of the LLDB libc++ specific data formatters.
This is enabled in the bootstrap build since building LLDB requires Clang and this
is quite expensive. Adding this test changes the build time from 31 to 34 minutes.

@@ -751,6 +751,8 @@ def setUpCommands(cls):
"settings set symbols.enable-external-lookup false",
# Inherit the TCC permissions from the inferior's parent.
"settings set target.inherit-tcc true",
# Based on https://discourse.llvm.org/t/running-lldb-in-a-container/76801/4
"settings set target.disable-aslr false",
Copy link
Member Author

Choose a reason for hiding this comment

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

The reason for change is in the Discourse link. This probably needs to be done in a separate commit.

@mordante mordante marked this pull request as ready for review April 12, 2024 16:55
@mordante mordante requested review from JDevlieghere and a team as code owners April 12, 2024 16:55
@llvmbot llvmbot added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. lldb labels Apr 12, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 12, 2024

@llvm/pr-subscribers-lldb

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

This enables testing of the LLDB libc++ specific data formatters.
This is enabled in the bootstrap build since building LLDB requires Clang and this
is quite expensive. Adding this test changes the build time from 31 to 34 minutes.


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

2 Files Affected:

  • (modified) libcxx/utils/ci/run-buildbot (+7-3)
  • (modified) lldb/packages/Python/lldbsuite/test/lldbtest.py (+2)
diff --git a/libcxx/utils/ci/run-buildbot b/libcxx/utils/ci/run-buildbot
index a6f3eb174308b4..e6240a829b0c73 100755
--- a/libcxx/utils/ci/run-buildbot
+++ b/libcxx/utils/ci/run-buildbot
@@ -376,18 +376,22 @@ bootstrapping-build)
           -DCMAKE_CXX_COMPILER_LAUNCHER="ccache" \
           -DCMAKE_BUILD_TYPE=Release \
           -DCMAKE_INSTALL_PREFIX="${INSTALL_DIR}" \
-          -DLLVM_ENABLE_PROJECTS="clang" \
+          -DLLVM_ENABLE_PROJECTS="clang;lldb" \
           -DLLVM_ENABLE_RUNTIMES="libcxx;libcxxabi;libunwind" \
           -DLLVM_RUNTIME_TARGETS="$(${CXX} --print-target-triple)" \
+          -DLLVM_HOST_TRIPLE="$(${CXX} --print-target-triple)" \
           -DLLVM_TARGETS_TO_BUILD="host" \
           -DRUNTIMES_BUILD_ALLOW_DARWIN=ON \
           -DLLVM_ENABLE_ASSERTIONS=ON \
           -DLLVM_LIT_ARGS="-sv --xunit-xml-output test-results.xml --timeout=1500 --time-tests"
 
-    echo "+++ Running the libc++ and libc++abi tests"
+    echo "+++ Running the LLDB libc++ data formatter tests"
+    ${NINJA} -vC "${BUILD_DIR}" check-lldb-api-functionalities-data-formatter-data-formatter-stl-libcxx
+
+    echo "--- Running the libc++ and libc++abi tests"
     ${NINJA} -vC "${BUILD_DIR}" check-runtimes
 
-    echo "--- Installing libc++ and libc++abi to a fake location"
+    echo "+++ Installing libc++ and libc++abi to a fake location"
     ${NINJA} -vC "${BUILD_DIR}" install-runtimes
 
     ccache -s
diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index c28a78a2c4a27a..7a7afec7345707 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -751,6 +751,8 @@ def setUpCommands(cls):
             "settings set symbols.enable-external-lookup false",
             # Inherit the TCC permissions from the inferior's parent.
             "settings set target.inherit-tcc true",
+            # Based on https://discourse.llvm.org/t/running-lldb-in-a-container/76801/4
+            "settings set target.disable-aslr false",
             # Kill rather than detach from the inferior if something goes wrong.
             "settings set target.detach-on-error false",
             # Disable fix-its by default so that incorrect expressions in tests don't

@Michael137
Copy link
Member

Awesome, thanks for doing this!!

@jasonmolenda
Copy link
Collaborator

The lldb change part of this PR looks good to me.

@medismailben
Copy link
Member

LGTM! Thanks!

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for picking this up!

@mordante mordante merged commit b4776b8 into llvm:main Apr 15, 2024
@mordante mordante deleted the review/libcxx_ci_test_lldb_dataformatters branch April 15, 2024 15:58
@adrian-prantl
Copy link
Collaborator

@mordante This broke the green dragon bot

https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake/1069/

This change seems to break the frame diagnose test:

  •        # Based on https://discourse.llvm.org/t/running-lldb-in-a-container/76801/4
    
  •        "settings set target.disable-aslr false",
    

@adrian-prantl
Copy link
Collaborator

I worked around the problem in 466017c.

aniplcc pushed a commit to aniplcc/llvm-project that referenced this pull request Apr 15, 2024
This enables testing of the LLDB libc++ specific data formatters.
This is enabled in the bootstrap build since building LLDB requires
Clang and this
is quite expensive. Adding this test changes the build time from 31 to
34 minutes.
@mordante
Copy link
Member Author

Thanks for the fix!

Michael137 added a commit to Michael137/llvm-project that referenced this pull request Oct 24, 2024
When running in constrained environments like docker,
disable ASLR might fail with errors like:
```
AssertionError: False is not true : launch failed (Cannot launch
'/__w/.../lldb-dap/stackTrace/subtleFrames/TestDAP_subtleFrames.test_subtleFrames/a.out':
personality set failed: Operation not permitted)
```
E.g., llvm#110303

Hence we already run `settings set target.disable-aslr false`
as part of the init-commands for the non-DAP tests (see
llvm#88312 and
https://discourse.llvm.org/t/running-lldb-in-a-container/76801).

But we never adjusted it for the DAP tests. Hence we get conflicting
test houtput like:
```
 {
    "arguments": {
      "commandEscapePrefix": null,
      "disableASLR": true,
     ....
      "initCommands": [
        ...
        "settings set target.disable-aslr false",
```

Disabling ASLR by default in tests isn't useulf (it's only really
a debugging aid for users). So this patch sets `disableASLR=False`
by default.
Michael137 added a commit that referenced this pull request Oct 25, 2024
When running in constrained environments like docker, disabling ASLR
might fail with errors like:
```
AssertionError: False is not true : launch failed (Cannot launch
'/__w/.../lldb-dap/stackTrace/subtleFrames/TestDAP_subtleFrames.test_subtleFrames/a.out':
personality set failed: Operation not permitted)
```
E.g., #110303

Hence we already run `settings set target.disable-aslr false` as part of
the init-commands for the non-DAP tests (see
#88312 and
https://discourse.llvm.org/t/running-lldb-in-a-container/76801).

But we never adjusted it for the DAP tests. As a result we get
conflicting test logs like:
```
 {
    "arguments": {
      "commandEscapePrefix": null,
      "disableASLR": true,
     ....
      "initCommands": [
        ...
        "settings set target.disable-aslr false",
```

Disabling ASLR by default in tests isn't useulf (it's only really a
debugging aid for users). So this patch sets `disableASLR=False` by
default.
NoumanAmir657 pushed a commit to NoumanAmir657/llvm-project that referenced this pull request Nov 4, 2024
When running in constrained environments like docker, disabling ASLR
might fail with errors like:
```
AssertionError: False is not true : launch failed (Cannot launch
'/__w/.../lldb-dap/stackTrace/subtleFrames/TestDAP_subtleFrames.test_subtleFrames/a.out':
personality set failed: Operation not permitted)
```
E.g., llvm#110303

Hence we already run `settings set target.disable-aslr false` as part of
the init-commands for the non-DAP tests (see
llvm#88312 and
https://discourse.llvm.org/t/running-lldb-in-a-container/76801).

But we never adjusted it for the DAP tests. As a result we get
conflicting test logs like:
```
 {
    "arguments": {
      "commandEscapePrefix": null,
      "disableASLR": true,
     ....
      "initCommands": [
        ...
        "settings set target.disable-aslr false",
```

Disabling ASLR by default in tests isn't useulf (it's only really a
debugging aid for users). So this patch sets `disableASLR=False` by
default.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. lldb
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants