[LLDB] Make internal shell the default for running LLDB lit tests. #156729
+12
−6
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch updates the lld lit test config to use the internal shell by default. This has some performance advantages (~10-15%) and also produces nicer failure output. It also updates the two LLDB tests to not require shell (so that they run under the internal shell), after first verifying that they run and pass using the internal shell; and it fixes one test that was not passing under the internal shell.
|
@llvm/pr-subscribers-lldb Author: None (cmtice) ChangesThis patch updates the lld lit test config to use the internal shell by default. This has some performance advantages (~10-15%) and also produces nicer failure output. It also updates the two LLDB tests to not require shell (so that they run under the internal shell), after first verifying that they run and pass using the internal shell; and it fixes one test that was not passing under the internal shell. Full diff: https://github.com/llvm/llvm-project/pull/156729.diff 4 Files Affected:
diff --git a/lldb/test/Shell/Process/Optimization.test b/lldb/test/Shell/Process/Optimization.test
index c189d505ef5d7..d2d02a74f621a 100644
--- a/lldb/test/Shell/Process/Optimization.test
+++ b/lldb/test/Shell/Process/Optimization.test
@@ -1,5 +1,5 @@
Test warnings.
-REQUIRES: shell, system-darwin
+REQUIRES: system-darwin
RUN: %clang_host -O3 %S/Inputs/true.c -std=c99 -g -o %t.exe
RUN: %lldb -o "b main" -o r -o q -b %t.exe 2>&1 | FileCheck %s
diff --git a/lldb/test/Shell/Process/UnsupportedLanguage.test b/lldb/test/Shell/Process/UnsupportedLanguage.test
index d7e6e5de77512..b7bbd860d0cac 100644
--- a/lldb/test/Shell/Process/UnsupportedLanguage.test
+++ b/lldb/test/Shell/Process/UnsupportedLanguage.test
@@ -1,7 +1,5 @@
Test unsupported language warning
-REQUIRES: shell
-
RUN: %clang_host %S/Inputs/true.c -std=c99 -g -c -S -emit-llvm -o - \
RUN: | sed -e 's/DW_LANG_C99/DW_LANG_Mips_Assembler/g' >%t.ll
RUN: %clang_host %t.ll -g -o %t.exe
diff --git a/lldb/test/Shell/SymbolFile/DWARF/dwo-missing-error.test b/lldb/test/Shell/SymbolFile/DWARF/dwo-missing-error.test
index 2805bbb5df7de..72315e8284745 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/dwo-missing-error.test
+++ b/lldb/test/Shell/SymbolFile/DWARF/dwo-missing-error.test
@@ -11,12 +11,12 @@
# "a.out-dwo-missing-error.dwo".
# RUN: rm -rf %t.compdir/
# RUN: mkdir -p %t.compdir/a/b/
-# RUN: cd %t.compdir/a/b/
+# RUN: pushd %t.compdir/a/b/
# RUN: %clang_host %S/Inputs/dwo-missing-error.c -glldb -gdwarf-5 \
# RUN: -gsplit-dwarf -fdebug-prefix-map=%t.compdir=. -o a.out
# RUN: rm *.dwo
# RUN: %lldb a.out -s %s -o exit 2>&1 | FileCheck %s
-# RUN: cd -
+# RUN: popd
# Test the error message with an absolute DW_AT_comp_dir and DW_AT_dwo_name.
# RUN: rm -rf %t.compdir/
diff --git a/lldb/test/lit.cfg.py b/lldb/test/lit.cfg.py
index eefc32aabd16d..9982ab442ba5d 100644
--- a/lldb/test/lit.cfg.py
+++ b/lldb/test/lit.cfg.py
@@ -13,3 +13,15 @@
config.name = "lldb"
config.test_source_root = os.path.dirname(__file__)
config.test_exec_root = os.path.join(config.lldb_obj_root, "test")
+
+# We prefer the lit internal shell which provides a better user experience on
+# failures and is faster unless the user explicitly disables it with
+# LIT_USE_INTERNAL_SHELL=0 env var.
+
+use_lit_shell = True
+lit_shell_env = os.environ.get("LIT_USE_INTERNAL_SHELL")
+if lit_shell_env:
+ use_lit_shell = lit.util.pythonize_bool(lit_shell_env)
+
+if use_lit_shell:
+ os.environ["LIT_USE_INTERNAL_SHELL"] = "1"
|
lldb/test/lit.cfg.py
Outdated
| use_lit_shell = lit.util.pythonize_bool(lit_shell_env) | ||
|
|
||
| if use_lit_shell: | ||
| os.environ["LIT_USE_INTERNAL_SHELL"] = "1" |
There was a problem hiding this comment.
I'm thinking we can avoid setting the env variable if we set config.test_format to
config.test_format = lit.formats.ShTest(execute_external=not use_lit_shell) like in the other tests. I'm assuming that's already the default, but not sure.
There was a problem hiding this comment.
I tried that originally, but for some reason the LLDB lit tests ignored that.
There was a problem hiding this comment.
Looks like that's because all of the shell tests override it in lldb/test/Shell/lit.cfg.py. I think you want to make this modification there.
boomanaiden154
approved these changes
Sep 3, 2025
ckoparkar
added a commit
to ckoparkar/llvm-project
that referenced
this pull request
Sep 4, 2025
* main: (1483 commits) [clang] fix error recovery for invalid nested name specifiers (llvm#156772) Revert "[lldb] Add count for errors of DWO files in statistics and combine DWO file count functions" (llvm#156777) AMDGPU: Add agpr variants of multi-data DS instructions (llvm#156420) [libc][NFC] disable localtime on aarch64/baremetal (llvm#156776) [win/asan] Improve SharedReAlloc with HEAP_REALLOC_IN_PLACE_ONLY. (llvm#132558) [LLDB] Make internal shell the default for running LLDB lit tests. (llvm#156729) [lldb][debugserver] Max response size for qSpeedTest (llvm#156099) [AMDGPU] Define 1024 VGPRs on gfx1250 (llvm#156765) [flang] Check for BIND(C) name conflicts with alternate entries (llvm#156563) [RISCV] Add exhausted_gprs_fprs test to calling-conv-half.ll. NFC (llvm#156586) [NFC] Remove trailing whitespaces from `clang/include/clang/Basic/AttrDocs.td` [lldb] Mark scripted frames as synthetic instead of artificial (llvm#153117) [docs] Refine some of the wording in the quality developer policy (llvm#156555) [MLIR] Apply clang-tidy fixes for readability-identifier-naming in TransformOps.cpp (NFC) [MLIR] Add LDBG() tracing to VectorTransferOpTransforms.cpp (NFC) [NFC] Apply clang-format to PPCInstrFutureMMA.td (llvm#156749) [libc] implement template functions for localtime (llvm#110363) [llvm-objcopy][COFF] Update .symidx values after stripping (llvm#153322) Add documentation on debugging LLVM. [lldb] Add count for errors of DWO files in statistics and combine DWO file count functions (llvm#155023) ...
|
I suspect this broke Though don't have a machine to confirm that. |
|
Same on Windows on Arm: https://lab.llvm.org/buildbot/#/builders/141/builds/11277 If the sed is not doing anything, the first check could pass because it's expecting not to find a message. The second check requires that the sed actually modified the IR file. |
boomanaiden154
added a commit
that referenced
this pull request
Sep 4, 2025
This is to fix buildbot fallout post #156729 without needing to revert the original patch.
|
Ok, I will revert this and try to work out how to test/fix the issues on windows (I wonder why it passed the windows CI premerge tests). |
Maybe not? The failing test has already been updated and marked as 'Unsupported' on windows? |
It might be picking up a different
Probably still good to fix the TODO there if possible. |
|
Although this patch just let the test additionally run on Windows, so if it doesn't get fixed it's not like we're losing test coverage. |
|
@cmtice I suspect that this may have broken three tests: https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/lldb-cmake/14836/ Could you revert/investigate? Let me know if you need any data or help reproducing. |
@adrian-prantl Could you please test #156931 and see if it fixes the problem? |
It turns out that that PR fixes 2 of the 3 failing tests. We're working on another PR to fix the 3rd test (hope to have it in an hour or two). |
|
#156939 should fix the last remaining issue. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch updates the lld lit test config to use the internal shell by default. This has some performance advantages (~10-15%) and also produces nicer failure output. It also updates the two LLDB tests to not require shell (so that they run under the internal shell), after first verifying that they run and pass using the internal shell; and it fixes one test that was not passing under the internal shell.