-
Notifications
You must be signed in to change notification settings - Fork 15.2k
Reapply "[compiler-rt] Default to Lit's Internal Shell" #168232
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
Reapply "[compiler-rt] Default to Lit's Internal Shell" #168232
Conversation
|
@llvm/pr-subscribers-compiler-rt-sanitizer Author: Aiden Grossman (boomanaiden154) ChangesThis reverts commit 8cc49fb. This was causing failures on two specific buildbots that have since been fixed. Full diff: https://github.com/llvm/llvm-project/pull/168232.diff 2 Files Affected:
diff --git a/compiler-rt/test/asan/TestCases/Linux/allocator_oom_test.cpp b/compiler-rt/test/asan/TestCases/Linux/allocator_oom_test.cpp
index f60a6a4ef79e6..1daf56d9d0a52 100644
--- a/compiler-rt/test/asan/TestCases/Linux/allocator_oom_test.cpp
+++ b/compiler-rt/test/asan/TestCases/Linux/allocator_oom_test.cpp
@@ -23,9 +23,11 @@
// RUN: | FileCheck %s --check-prefixes=CHECK-REALLOC,CHECK-CRASH
// RUN: %env_asan_opts=allocator_may_return_null=1 %run %t realloc 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK-REALLOC,CHECK-NULL
-// RUN: %env_asan_opts=allocator_may_return_null=0 not %run %t realloc-after-malloc 2>&1 \
+// RUN: %export_asan_opts=allocator_may_return_null=0
+// RUN: not %run %t realloc-after-malloc 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK-MALLOC-REALLOC,CHECK-CRASH
-// RUN: %env_asan_opts=allocator_may_return_null=1 %run %t realloc-after-malloc 2>&1 \
+// RUN: %export_asan_opts=allocator_may_return_null=1
+// RUN: %run %t realloc-after-malloc 2>&1 \
// RUN: | FileCheck %s --check-prefixes=CHECK-MALLOC-REALLOC,CHECK-NULL
// ASan shadow memory on s390 is too large for this test.
diff --git a/compiler-rt/test/lit.common.cfg.py b/compiler-rt/test/lit.common.cfg.py
index 3f7dd8e402b78..0f0f87915bafe 100644
--- a/compiler-rt/test/lit.common.cfg.py
+++ b/compiler-rt/test/lit.common.cfg.py
@@ -113,6 +113,9 @@ def push_dynamic_library_lookup_path(config, new_path):
config.environment[dynamic_library_lookup_var] = new_ld_library_path_64
+# TODO: Consolidate the logic for turning on the internal shell by default for all LLVM test suites.
+# See https://github.com/llvm/llvm-project/issues/106636 for more details.
+#
# Choose between lit's internal shell pipeline runner and a real shell. If
# LIT_USE_INTERNAL_SHELL is in the environment, we use that as an override.
use_lit_shell = os.environ.get("LIT_USE_INTERNAL_SHELL")
@@ -120,9 +123,8 @@ def push_dynamic_library_lookup_path(config, new_path):
# 0 is external, "" is default, and everything else is internal.
execute_external = use_lit_shell == "0"
else:
- # Otherwise we default to internal on Windows and external elsewhere, as
- # bash on Windows is usually very slow.
- execute_external = not sys.platform in ["win32"]
+ # Otherwise we default to internal everywhere.
+ execute_external = False
# Allow expanding substitutions that are based on other substitutions
config.recursiveExpansionLimit = 10
|
This reverts commit 8cc49fb. This was causing failures on two specific buildbots that have since been fixed.
0c1a374 to
909cc56
Compare
|
It looks like the ppc64le failures were never real in the first place. The bot seems to fail ~50% of its runs on some compiler-rt test or another. |
|
I think this commit has caused a number of test failures on Green Dragon (Apple OS CI testing): https://green.lab.llvm.org/job/llvm.org/job/clang-stage1-RA/5985/. |
|
Our internal linux builder is also hitting issues in a bunch of XRay tests that start passing when I set |
|
@boomanaiden154 I can see some of the Darwin tests for sanitizers using |
No. They need to be rewritten to not use subshells. I'm working on a fix and hope to have patches submitted soon. Working on the XRay failures first that I expect to have done in ~30 minutes. |
|
Reverted for now in cf0909e. I'll hopefully have patches up to get the tests fixed later today. |
|
Hi @boomanaiden154, it looks like a good number of these tests failing are specific to Darwin - if you need any help or have a patch to review for these could you please tag me, @ndrewh, @wrotki or @thetruestblue? Thanks |
|
Minus the XRay tests, all the failures reported here are on Darwin. Hoping to put patches up later today to get things working (will ping you all for review) and will then reland the enablement once those land. |
|
We also see test issues after this patch landed: Failed build task: https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8698056561481521121/overview Could we revert this PR? |
|
It was reverted four hours ago. cf0909e |
Hm? The commit that lands this PR is still the most recent one to touch the affected file: https://github.com/llvm/llvm-project/commits/main/compiler-rt/test/lit.common.cfg.py |
)" This reverts commit bde9062. This caused failures on Darwin that were not caught by upstream buildbots. Reverting for now to give myself some time to fix.
|
Ah, yeah. Looks like I only pushed that one to a branch instead of main. 🙃 Actually pushed now as eb20b53. |
|
Thanks! |
This reverts commit 8cc49fb.
This was causing failures on two specific buildbots that have since been fixed.