Skip to content

[NFC][CFI] Avoid clang error in CFI tests #135981

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
merged 3 commits into from
Apr 16, 2025

Conversation

vitalybuka
Copy link
Collaborator

@vitalybuka vitalybuka commented Apr 16, 2025

In these tests we test correct linking flags set,
and it's confusing that command fails because of
other missing required flags.

Clang diagnostics opportunistically proceed after
error report on required flags, but there is no
guaranty that processing of tested flags are the same
in supported and erroneous flag sets.

Created using spr 1.3.4
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Apr 16, 2025
@vitalybuka vitalybuka requested review from fmayer and thurstond April 16, 2025 16:09
@llvmbot
Copy link
Member

llvmbot commented Apr 16, 2025

@llvm/pr-subscribers-clang-driver

@llvm/pr-subscribers-clang

Author: Vitaly Buka (vitalybuka)

Changes

In these tests we test correct linking flags set,
and it' confusing that command fails because of
other missing required flags.


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

1 Files Affected:

  • (modified) clang/test/Driver/sanitizer-ld.c (+12-6)
diff --git a/clang/test/Driver/sanitizer-ld.c b/clang/test/Driver/sanitizer-ld.c
index 67ca33d676d20..a00ec029d3d46 100644
--- a/clang/test/Driver/sanitizer-ld.c
+++ b/clang/test/Driver/sanitizer-ld.c
@@ -840,7 +840,8 @@
 // CHECK-CFI-PREREQ-LINUX: '-fsanitize=cfi' only allowed with '-fvisibility='
 
 // CFI by itself does not link runtime libraries.
-// RUN: not %clang -fsanitize=cfi \
+// RUN: %clang -fsanitize=cfi \
+// RUN:     -flto -fvisibility=hidden \
 // RUN:     --target=x86_64-unknown-linux -fuse-ld=ld -rtlib=platform \
 // RUN:     -resource-dir=%S/Inputs/resource_dir \
 // RUN:     --sysroot=%S/Inputs/basic_linux_tree \
@@ -849,7 +850,8 @@
 // CHECK-CFI-LINUX: "{{.*}}ld{{(.exe)?}}"
 
 // CFI with diagnostics links the UBSan runtime.
-// RUN: not %clang -fsanitize=cfi -fno-sanitize-trap=cfi -fsanitize-recover=cfi \
+// RUN: %clang -fsanitize=cfi -fno-sanitize-trap=cfi -fsanitize-recover=cfi \
+// RUN:     -flto -fvisibility=hidden \
 // RUN:     --target=x86_64-unknown-linux -fuse-ld=ld \
 // RUN:     -resource-dir=%S/Inputs/resource_dir \
 // RUN:     --sysroot=%S/Inputs/basic_linux_tree \
@@ -859,7 +861,8 @@
 // CHECK-CFI-DIAG-LINUX: "--whole-archive" "{{[^"]*}}libclang_rt.ubsan_standalone.a" "--no-whole-archive"
 
 // Cross-DSO CFI links the CFI runtime.
-// RUN: not %clang -fsanitize=cfi -fsanitize-cfi-cross-dso \
+// RUN: %clang -fsanitize=cfi -fsanitize-cfi-cross-dso \
+// RUN:     -flto -fvisibility=hidden \
 // RUN:     --target=x86_64-unknown-linux -fuse-ld=ld \
 // RUN:     -resource-dir=%S/Inputs/resource_dir \
 // RUN:     --sysroot=%S/Inputs/basic_linux_tree \
@@ -870,7 +873,8 @@
 // CHECK-CFI-CROSS-DSO-LINUX: -export-dynamic
 
 // Cross-DSO CFI with diagnostics links just the CFI runtime.
-// RUN: not %clang -fsanitize=cfi -fsanitize-cfi-cross-dso \
+// RUN: %clang -fsanitize=cfi -fsanitize-cfi-cross-dso \
+// RUN:     -flto -fvisibility=hidden \
 // RUN:     -fno-sanitize-trap=cfi -fsanitize-recover=cfi \
 // RUN:     --target=x86_64-unknown-linux -fuse-ld=ld \
 // RUN:     -resource-dir=%S/Inputs/resource_dir \
@@ -882,7 +886,8 @@
 // CHECK-CFI-CROSS-DSO-DIAG-LINUX: -export-dynamic
 
 // Cross-DSO CFI on Android does not link runtime libraries.
-// RUN: not %clang -fsanitize=cfi -fsanitize-cfi-cross-dso \
+// RUN: %clang -fsanitize=cfi -fsanitize-cfi-cross-dso \
+// RUN:     -flto -fvisibility=hidden \
 // RUN:     --target=aarch64-linux-android -fuse-ld=ld \
 // RUN:     -resource-dir=%S/Inputs/resource_dir \
 // RUN:     --sysroot=%S/Inputs/basic_android_tree \
@@ -891,7 +896,8 @@
 // CHECK-CFI-CROSS-DSO-ANDROID: "{{.*}}ld{{(.exe)?}}"
 
 // Cross-DSO CFI with diagnostics on Android links just the UBSAN runtime.
-// RUN: not %clang -fsanitize=cfi -fsanitize-cfi-cross-dso \
+// RUN: %clang -fsanitize=cfi -fsanitize-cfi-cross-dso \
+// RUN:     -flto -fvisibility=hidden \
 // RUN:     -fno-sanitize-trap=cfi -fsanitize-recover=cfi \
 // RUN:     --target=aarch64-linux-android -fuse-ld=ld \
 // RUN:     -resource-dir=%S/Inputs/resource_dir \

@vitalybuka vitalybuka changed the base branch from users/vitalybuka/spr/main.nfccfi-avoid-failing-cfi-tests to main April 16, 2025 16:30
Created using spr 1.3.4
@vitalybuka vitalybuka changed the title [NFC][CFI] Avoid failing CFI tests [NFC][CFI] Avoid clang error in CFI tests Apr 16, 2025
@vitalybuka vitalybuka merged commit 726a5c2 into main Apr 16, 2025
8 of 11 checks passed
@vitalybuka vitalybuka deleted the users/vitalybuka/spr/nfccfi-avoid-failing-cfi-tests branch April 16, 2025 16:54
@mysterymath
Copy link
Contributor

This is causing a test failure in our clang CI bots: https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-host-linux-x64/b8717414717471782065/overview

It doesn't seem that any error output is produced for the test though; do you have an intuition for what's happening? This is the only change in the blamelist, so it's definitely a result of this PR.

@vitalybuka
Copy link
Collaborator Author

This is causing a test failure in our clang CI bots: https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-host-linux-x64/b8717414717471782065/overview

It doesn't seem that any error output is produced for the test though; do you have an intuition for what's happening? This is the only change in the blamelist, so it's definitely a result of this PR.

#136002 to get output

if it gives no clue, I'll revert both

vitalybuka added a commit that referenced this pull request Apr 16, 2025
vitalybuka added a commit that referenced this pull request Apr 16, 2025
Reverts #135981

Fails with 'clang: error: --rtlib=libgcc requires --unwindlib=libgcc' on
some bots.
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Apr 16, 2025
Reverts llvm/llvm-project#135981

Fails with 'clang: error: --rtlib=libgcc requires --unwindlib=libgcc' on
some bots.
vitalybuka added a commit that referenced this pull request Apr 16, 2025
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Apr 16, 2025
var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025
In these tests we test correct linking flags set,
and it's confusing that command fails because of
other missing required flags.

Clang diagnostics opportunistically proceed after
error report on required flags, but there is no
guaranty that processing of tested flags are the same
in supported and erroneous flag sets.
var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025
Reverts llvm#135981

Fails with 'clang: error: --rtlib=libgcc requires --unwindlib=libgcc' on
some bots.
var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants