Skip to content
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

[test] Avoid writing to a potentially write-protected dir #102073

Merged
merged 1 commit into from
Aug 6, 2024

Conversation

karka228
Copy link
Collaborator

@karka228 karka228 commented Aug 5, 2024

The test length-errors.hlsl don't check the output written to the current directory. The current directory may be write protected e.g. in a sandboxed environment.

This patch simply remove the -emit-llvm option as this testcase don't care about the outputed llvm IR.

@llvmbot llvmbot added clang Clang issues not falling into any other category backend:X86 HLSL HLSL Language Support labels Aug 5, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Aug 5, 2024

@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-hlsl

@llvm/pr-subscribers-clang

Author: Karl-Johan Karlsson (karka228)

Changes

The tests avx10_2_512minmax-error.c and length-errors.hlsl don't check the output written to the current directory. The current directory may be write protected e.g. in a sandboxed environment.

This patch replace the -emit-llvm option with -emit-llvm-only as these testcases don't care about the outputed llvm IR.


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

2 Files Affected:

  • (modified) clang/test/CodeGen/X86/avx10_2_512minmax-error.c (+2-2)
  • (modified) clang/test/SemaHLSL/BuiltIns/length-errors.hlsl (+1-1)
diff --git a/clang/test/CodeGen/X86/avx10_2_512minmax-error.c b/clang/test/CodeGen/X86/avx10_2_512minmax-error.c
index 15ed7a0b35d82..7a5f6b5b2345c 100644
--- a/clang/test/CodeGen/X86/avx10_2_512minmax-error.c
+++ b/clang/test/CodeGen/X86/avx10_2_512minmax-error.c
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 %s -flax-vector-conversions=none -ffreestanding -triple=x86_64 -target-feature +avx10.2-512 \
-// RUN: -Wno-invalid-feature-combination -emit-llvm -verify
+// RUN: -Wno-invalid-feature-combination -emit-llvm-only -verify
 // RUN: %clang_cc1 %s -flax-vector-conversions=none -ffreestanding -triple=i386 -target-feature +avx10.2-512 \
-// RUN: -Wno-invalid-feature-combination -emit-llvm -verify
+// RUN: -Wno-invalid-feature-combination -emit-llvm-only -verify
 
 #include <immintrin.h>
 
diff --git a/clang/test/SemaHLSL/BuiltIns/length-errors.hlsl b/clang/test/SemaHLSL/BuiltIns/length-errors.hlsl
index fe0046a2ab5a1..8153726367637 100644
--- a/clang/test/SemaHLSL/BuiltIns/length-errors.hlsl
+++ b/clang/test/SemaHLSL/BuiltIns/length-errors.hlsl
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -verify-ignore-unexpected
+// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm-only -disable-llvm-passes -verify -verify-ignore-unexpected
 
 void test_too_few_arg()
 {

@@ -1,4 +1,4 @@
// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm -disable-llvm-passes -verify -verify-ignore-unexpected
// RUN: %clang_cc1 -finclude-default-header -triple dxil-pc-shadermodel6.6-library %s -fnative-half-type -emit-llvm-only -disable-llvm-passes -verify -verify-ignore-unexpected
Copy link
Collaborator

Choose a reason for hiding this comment

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

This probably shouldn't have the -emit-llvm flag at all since it isn't actually checking that output.

Copy link
Collaborator

@llvm-beanz llvm-beanz left a comment

Choose a reason for hiding this comment

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

Looks like neither of these tests should be passing -emit-llvm at all.

@@ -1,7 +1,7 @@
// RUN: %clang_cc1 %s -flax-vector-conversions=none -ffreestanding -triple=x86_64 -target-feature +avx10.2-512 \
// RUN: -Wno-invalid-feature-combination -emit-llvm -verify
// RUN: -Wno-invalid-feature-combination -emit-llvm-only -verify
Copy link
Contributor

Choose a reason for hiding this comment

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

I just realized `-emit-llvm is not required here. You can directly removed it. Sorry for mistake.

The test length-errors.hlsl don't check the output written to the current
directory. The current directory may be write protected e.g. in a sandboxed
environment.

This patch remove the -emit-llvm option as these testcases don't care about the
outputed llvm IR.

Co-authored-by: Chris B <cbieneman@microsoft.com>
@karka228
Copy link
Collaborator Author

karka228 commented Aug 6, 2024

When rebasing the patch I removed the fix in avx10_2_512minmax-error.c as it was already fixed in:

commit 1b9d7dd9eb31974077b64d66404193bd7c4ad65e
Author: Augie Fackler <augie@google.com>
Date:   Mon Aug 5 17:08:06 2024 -0400

    tests: fix clang flags in avx10_2_512minmax-error test
    
    Introduced in 3d5cc7e1e632b74119af13824dabc346bd248c93, caught by a
    sandboxed test run.

@karka228 karka228 merged commit b7730a2 into llvm:main Aug 6, 2024
7 checks passed
banach-space pushed a commit to banach-space/llvm-project that referenced this pull request Aug 7, 2024
The test length-errors.hlsl don't check the output written to the
current directory. The current directory may be write protected e.g. in
a sandboxed environment.

This patch simply remove the -emit-llvm option as this testcase don't
care about the outputed llvm IR.

Co-authored-by: Chris B <cbieneman@microsoft.com>
kstoimenov pushed a commit to kstoimenov/llvm-project that referenced this pull request Aug 15, 2024
The test length-errors.hlsl don't check the output written to the
current directory. The current directory may be write protected e.g. in
a sandboxed environment.

This patch simply remove the -emit-llvm option as this testcase don't
care about the outputed llvm IR.

Co-authored-by: Chris B <cbieneman@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 clang Clang issues not falling into any other category HLSL HLSL Language Support
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

5 participants