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

[compiler-rt][NFC] Replace environment variable with %t #102197

Merged
merged 2 commits into from
Aug 9, 2024

Conversation

Harini0924
Copy link
Contributor

@Harini0924 Harini0924 commented Aug 6, 2024

This patch addresses a "command not found" issue encountered in certain tests within the compiler-rt subproject when using lit's internal shell. Specifically, tests like sanitizer_coverage_trace_pc_guard-init.cpp failed with an exit code of 127 because the DIR environment variable was incorrectly treated as a command. The error occurred when running the command LIT_USE_INTERNAL_SHELL=1 ninja check-compiler-rt, where the lit internal shell did not handle environment variable assignments as expected.

The issues were resolved by removing the use of the DIR environment variable and simplifying the tests to directly utilize %t_workdir for managing temporary directories. The clang command arguments have also been directly included in the test scripts to avoid complications with environment variable expansion under lit's internal shell.

This cleanup ensures that the tests run smoothly with lit's internal shell and prevents "command not found" errors, thereby improving the reliability of the test suite when executed in this environment.

fixes: #106598
This is part of the test cleanups proposed in the RFC: [RFC] Enabling the Lit Internal Shell by Default

Resolved Command Not Found Errors in compiler-rt tests. Some tests when using
lit's internal shell found command not found error when calling `DIR`
environment variable. I removed the use of the `DIR` enviornment
variable to prevent command no found errors. Simplified the test to
directly use `%t_workdir` for managing temporary directories.
@Harini0924
Copy link
Contributor Author

CC: @ilovepi @petrhosek

@llvmbot
Copy link
Member

llvmbot commented Aug 6, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: None (Harini0924)

Changes

Resolved Command Not Found Errors in compiler-rt tests. Some tests when using lit's internal shell found command not found errors when calling the DIR environment variable. I removed the use of the DIR environment variable and simplified the test to directly use %t_workdir for managing temporary directories. Also, ensured that the clang command arguments are directly included in the test to avoid issues with environment variable expansion.


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

5 Files Affected:

  • (modified) compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_allowlist_ignorelist.cpp (+4-5)
  • (modified) compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_symbolize.cpp (+4-5)
  • (modified) compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-dso.cpp (+7-9)
  • (modified) compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-init.cpp (+7-9)
  • (modified) compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard.cpp (+4-5)
diff --git a/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_allowlist_ignorelist.cpp b/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_allowlist_ignorelist.cpp
index 2fbace52696a9e..6fdd23b84432f9 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_allowlist_ignorelist.cpp
+++ b/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_allowlist_ignorelist.cpp
@@ -7,10 +7,9 @@
 // XFAIL: ubsan,tsan
 // XFAIL: android && asan
 
-// RUN: DIR=%t_workdir
-// RUN: rm -rf $DIR
-// RUN: mkdir -p $DIR
-// RUN: cd $DIR
+// RUN: rm -rf %t_workdir
+// RUN: mkdir -p %t_workdir
+// RUN: cd %t_workdir
 
 // RUN: echo -e "src:*\nfun:*"     > al_all.txt
 // RUN: echo -e ""                 > al_none.txt
@@ -82,7 +81,7 @@
 // RUN: %clangxx -O0 %s -S -o - -emit-llvm -fsanitize-coverage=inline-8bit-counters,indirect-calls,trace-cmp,pc-table -fsanitize-coverage-allowlist=al_bar.txt  -fsanitize-coverage-ignorelist=bl_bar.txt   2>&1 | not grep -f patterns.txt
 
 // RUN: cd -
-// RUN: rm -rf $DIR
+// RUN: rm -rf %t_workdir
 
 // foo has 3 instrumentation points, 0 indirect call, 1 comparison point
 
diff --git a/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_symbolize.cpp b/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_symbolize.cpp
index b168954a1c92cf..ba59e28f688a71 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_symbolize.cpp
+++ b/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_symbolize.cpp
@@ -3,15 +3,14 @@
 // REQUIRES: x86_64-linux
 // XFAIL: tsan
 //
-// RUN: DIR=%t_workdir
-// RUN: rm -rf $DIR
-// RUN: mkdir -p $DIR
-// RUN: cd $DIR
+// RUN: rm -rf %t_workdir
+// RUN: mkdir -p %t_workdir
+// RUN: cd %t_workdir
 /// In glibc 2.39+, fprintf has a nonnull attribute. Disable nonnull-attribute,
 /// which would increase counters for ubsan.
 // RUN: %clangxx -O0 -fsanitize-coverage=trace-pc-guard -fno-sanitize=nonnull-attribute %s -o %t
 // RUN: %env_tool_opts=coverage=1 %t 2>&1 | FileCheck %s
-// RUN: rm -rf $DIR
+// RUN: rm -rf %t_workdir
 
 #include <stdio.h>
 
diff --git a/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-dso.cpp b/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-dso.cpp
index e27d552f3c3184..9a27bc89a5fae1 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-dso.cpp
+++ b/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-dso.cpp
@@ -5,19 +5,17 @@
 // XFAIL: tsan,darwin
 // XFAIL: android && asan
 
-// RUN: DIR=%t_workdir
-// RUN: CLANG_ARGS="-O0 -fsanitize-coverage=trace-pc-guard"
-// RUN: rm -rf $DIR
-// RUN: mkdir -p $DIR
-// RUN: cd $DIR
-// RUN: %clangxx -DSHARED1 $CLANG_ARGS -shared %s -o %t_1.so -fPIC
-// RUN: %clangxx -DSHARED2 $CLANG_ARGS -shared %s -o %t_2.so -fPIC
-// RUN: %clangxx -DMAIN $CLANG_ARGS %s -o %t %t_1.so %t_2.so
+// RUN: rm -rf %t_workdir
+// RUN: mkdir -p %t_workdir
+// RUN: cd %t_workdir
+// RUN: %clangxx -DSHARED1 -O0 -fsanitize-coverage=trace-pc-guard -shared %s -o %t_1.so -fPIC
+// RUN: %clangxx -DSHARED2 -O0 -fsanitize-coverage=trace-pc-guard -shared %s -o %t_2.so -fPIC
+// RUN: %clangxx -DMAIN -O0 -fsanitize-coverage=trace-pc-guard %s -o %t %t_1.so %t_2.so
 // RUN: %env_tool_opts=coverage=1 %t 2>&1 | FileCheck %s
 // RUN: %sancovcc  -covered-functions -strip_path_prefix=TestCases/ *.sancov \
 // RUN:            %t %t_1.so %t_2.so 2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-SANCOV %s
-// RUN: rm -rf $DIR
+// RUN: rm -rf %t_workdir
 
 #include <stdio.h>
 
diff --git a/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-init.cpp b/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-init.cpp
index 0b2da9aebac8e2..cd5ea30713a578 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-init.cpp
+++ b/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard-init.cpp
@@ -2,16 +2,14 @@
 //
 // REQUIRES: has_sancovcc,stable-runtime,x86_64-linux
 //
-// RUN: DIR=%t_workdir
-// RUN: CLANG_ARGS="-O0 -fsanitize-coverage=trace-pc-guard"
-// RUN: rm -rf $DIR
-// RUN: mkdir -p $DIR
-// RUN: cd $DIR
-// RUN: %clangxx -DSHARED1 $CLANG_ARGS -shared %s -o %t_1.so -fPIC
-// RUN: %clangxx -DSTATIC1 $CLANG_ARGS %s -c -o %t_2.o
-// RUN: %clangxx -DMAIN $CLANG_ARGS %s -o %t %t_1.so %t_2.o
+// RUN: rm -rf %t_workdir
+// RUN: mkdir -p %t_workdir
+// RUN: cd %t_workdir
+// RUN: %clangxx -DSHARED1 -O0 -fsanitize-coverage=trace-pc-guard -shared %s -o %t_1.so -fPIC
+// RUN: %clangxx -DSTATIC1 -O0 -fsanitize-coverage=trace-pc-guard %s -c -o %t_2.o
+// RUN: %clangxx -DMAIN -O0 -fsanitize-coverage=trace-pc-guard %s -o %t %t_1.so %t_2.o
 // RUN: %env_tool_opts=coverage=1 %t 2>&1 | FileCheck %s
-// RUN: rm -rf $DIR
+// RUN: rm -rf %t_workdir
 
 #include <stdio.h>
 #include <stdint.h>
diff --git a/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard.cpp b/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard.cpp
index eea92d0ba9e838..b4b491455c390a 100644
--- a/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard.cpp
+++ b/compiler-rt/test/sanitizer_common/TestCases/sanitizer_coverage_trace_pc_guard.cpp
@@ -7,16 +7,15 @@
 // XFAIL: tsan
 // XFAIL: android && asan
 
-// RUN: DIR=%t_workdir
-// RUN: rm -rf $DIR
-// RUN: mkdir -p $DIR
-// RUN: cd $DIR
+// RUN: rm -rf %t_workdir
+// RUN: mkdir -p %t_workdir
+// RUN: cd %t_workdir
 // RUN: %clangxx -O0 -fsanitize-coverage=trace-pc-guard %s -o %t
 // RUN: %env_tool_opts=coverage=1 %t 2>&1 | FileCheck %s
 // RUN: %sancovcc  -covered-functions -strip_path_prefix=TestCases/ *.sancov %t 2>&1 | \
 // RUN:   FileCheck --check-prefix=CHECK-SANCOV %s
 // RUN: %env_tool_opts=coverage=0 %t 2>&1 | FileCheck --check-prefix=CHECK-NOCOV %s
-// RUN: rm -rf $DIR
+// RUN: rm -rf %t_workdir
 // Make some room to stabilize line numbers
 
 #include <stdio.h>

@ilovepi ilovepi requested review from yln and ilovepi August 9, 2024 21:26
Copy link
Contributor

@ilovepi ilovepi left a comment

Choose a reason for hiding this comment

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

This seems like a straight forward cleanup. I'd drop the full error message from the description (a summary describing the error is fine). I'd also avoid the use of "I" in the patch, and just describe what you've done. Alternatively, you can use "we". Also, replace the link to RFC text with the title, and mention that this is in part of the test cleanups proposed in that RFC.

I'd also retitle to [compiler-rt][NFC] Replace environment variable with %t

@Harini0924 Harini0924 changed the title [llvm-lit] Fix Command Not Found Errors in compiler-rt when using lit internal shell [compiler-rt][NFC] Replace environment variable with %t Aug 9, 2024
@vitalybuka vitalybuka merged commit c69b8c4 into llvm:main Aug 9, 2024
7 checks passed
kutemeikito added a commit to kutemeikito/llvm-project that referenced this pull request Aug 10, 2024
* 'main' of https://github.com/llvm/llvm-project: (700 commits)
  [SandboxIR][NFC] SingleLLVMInstructionImpl class (llvm#102687)
  [ThinLTO]Clean up 'import-assume-unique-local' flag. (llvm#102424)
  [nsan] Make #include more conventional
  [SandboxIR][NFC] Use Tracker.emplaceIfTracking()
  [libc]  Moved range_reduction_double ifdef statement (llvm#102659)
  [libc] Fix CFP long double and add tests (llvm#102660)
  [TargetLowering] Handle vector types in expandFixedPointMul (llvm#102635)
  [compiler-rt][NFC] Replace environment variable with %t (llvm#102197)
  [UnitTests] Convert a test to use opaque pointers (llvm#102668)
  [CodeGen][NFCI] Don't re-implement parts of ASTContext::getIntWidth (llvm#101765)
  [SandboxIR] Clean up tracking code with the help of emplaceIfTracking() (llvm#102406)
  [mlir][bazel] remove extra blanks in mlir-tblgen test
  [NVPTX][NFC] Update tests to use bfloat type (llvm#101493)
  [mlir] Add support for parsing nested PassPipelineOptions (llvm#101118)
  [mlir][bazel] add missing td dependency in mlir-tblgen test
  [flang][cuda] Fix lib dependency
  [libc] Clean up remaining use of *_WIDTH macros in printf (llvm#102679)
  [flang][cuda] Convert cuf.alloc for box to fir.alloca in device context (llvm#102662)
  [SandboxIR] Implement the InsertElementInst class (llvm#102404)
  [libc] Fix use of cpp::numeric_limits<...>::digits (llvm#102674)
  [mlir][ODS] Verify type constraints in Types and Attributes (llvm#102326)
  [LTO] enable `ObjCARCContractPass` only on optimized build  (llvm#101114)
  [mlir][ODS] Consistent `cppType` / `cppClassName` usage (llvm#102657)
  [lldb] Move definition of SBSaveCoreOptions dtor out of header (llvm#102539)
  [libc] Use cpp::numeric_limits in preference to C23 <limits.h> macros (llvm#102665)
  [clang] Implement -fptrauth-auth-traps. (llvm#102417)
  [LLVM][rtsan] rtsan transform to preserve CFGAnalyses (llvm#102651)
  Revert "[AMDGPU] Move `AMDGPUAttributorPass` to full LTO post link stage (llvm#102086)"
  [RISCV][GISel] Add missing tests for G_CTLZ/CTTZ instruction selection. NFC
  Return available function types for BindingDecls. (llvm#102196)
  [clang] Wire -fptrauth-returns to "ptrauth-returns" fn attribute. (llvm#102416)
  [RISCV] Remove riscv-experimental-rv64-legal-i32. (llvm#102509)
  [RISCV] Move PseudoVSET(I)VLI expansion to use PseudoInstExpansion. (llvm#102496)
  [NVPTX] support switch statement with brx.idx (reland) (llvm#102550)
  [libc][newhdrgen]sorted function names in yaml (llvm#102544)
  [GlobalIsel] Combine G_ADD and G_SUB with constants (llvm#97771)
  Suppress spurious warnings due to R_RISCV_SET_ULEB128
  [scudo] Separated committed and decommitted entries. (llvm#101409)
  [MIPS] Fix missing ANDI optimization (llvm#97689)
  [Clang] Add env var for nvptx-arch/amdgpu-arch timeout (llvm#102521)
  [asan] Switch allocator to dynamic base address (llvm#98511)
  [AMDGPU] Move `AMDGPUAttributorPass` to full LTO post link stage (llvm#102086)
  [libc][math][c23] Add fadd{l,f128} C23 math functions (llvm#102531)
  [mlir][bazel] revert bazel rule change for DLTITransformOps
  [msan] Support vst{2,3,4}_lane instructions (llvm#101215)
  Revert "[MLIR][DLTI][Transform] Introduce transform.dlti.query (llvm#101561)"
  [X86] pr57673.ll - generate MIR test checks
  [mlir][vector][test] Split tests from vector-transfer-flatten.mlir (llvm#102584)
  [mlir][bazel] add bazel rule for DLTITransformOps
  OpenMPOpt: Remove dead include
  [IR] Add method to GlobalVariable to change type of initializer. (llvm#102553)
  [flang][cuda] Force default allocator in device code (llvm#102238)
  [llvm] Construct SmallVector<SDValue> with ArrayRef (NFC) (llvm#102578)
  [MLIR][DLTI][Transform] Introduce transform.dlti.query (llvm#101561)
  [AMDGPU][AsmParser][NFC] Remove a misleading comment. (llvm#102604)
  [Arm][AArch64][Clang] Respect function's branch protection attributes. (llvm#101978)
  [mlir] Verifier: steal bit to track seen instead of set. (llvm#102626)
  [Clang] Fix Handling of Init Capture with Parameter Packs in LambdaScopeForCallOperatorInstantiationRAII (llvm#100766)
  [X86] Convert truncsat clamping patterns to use SDPatternMatch. NFC.
  [gn] Give two scripts argparse.RawDescriptionHelpFormatter
  [bazel] Add missing dep for the SPIRVToLLVM target
  [Clang] Simplify specifying passes via -Xoffload-linker (llvm#102483)
  [bazel] Port for d45de80
  [SelectionDAG] Use unaligned store/load to move AVX registers onto stack for `insertelement` (llvm#82130)
  [Clang][OMPX] Add the code generation for multi-dim `num_teams` (llvm#101407)
  [ARM] Regenerate big-endian-vmov.ll. NFC
  [AMDGPU][AsmParser][NFCI] All NamedIntOperands to be of the i32 type. (llvm#102616)
  [libc][math][c23] Add totalorderl function. (llvm#102564)
  [mlir][spirv] Support `memref` in `convert-to-spirv` pass (llvm#102534)
  [MLIR][GPU-LLVM] Convert `gpu.func` to `llvm.func` (llvm#101664)
  Fix a unit test input file (llvm#102567)
  [llvm-readobj][COFF] Dump hybrid objects for ARM64X files. (llvm#102245)
  AMDGPU/NewPM: Port SIFixSGPRCopies to new pass manager (llvm#102614)
  [MemoryBuiltins] Simplify getCalledFunction() helper (NFC)
  [AArch64] Add invalid 1 x vscale costs for reductions and reduction-operations. (llvm#102105)
  [MemoryBuiltins] Handle allocator attributes on call-site
  LSV/test/AArch64: add missing lit.local.cfg; fix build (llvm#102607)
  Revert "Enable logf128 constant folding for hosts with 128bit floats (llvm#96287)"
  [RISCV] Add Syntacore SCR5 RV32/64 processors definition (llvm#102285)
  [InstCombine] Remove unnecessary RUN line from test (NFC)
  [flang][OpenMP] Handle multiple ranges in `num_teams` clause (llvm#102535)
  [mlir][vector] Add tests for scalable vectors in one-shot-bufferize.mlir (llvm#102361)
  [mlir][vector] Disable `vector.matrix_multiply` for scalable vectors (llvm#102573)
  [clang] Implement CWG2627 Bit-fields and narrowing conversions (llvm#78112)
  [NFC] Use references to avoid copying (llvm#99863)
  Revert "[mlir][ArmSME] Pattern to swap shape_cast(tranpose) with transpose(shape_cast) (llvm#100731)" (llvm#102457)
  [IRBuilder] Generate nuw GEPs for struct member accesses (llvm#99538)
  [bazel] Port for 9b06e25
  [CodeGen][NewPM] Improve start/stop pass error message CodeGenPassBuilder (llvm#102591)
  [AArch64] Implement TRBMPAM_EL1 system register (llvm#102485)
  [InstCombine] Fixing wrong select folding in vectors with undef elements (llvm#102244)
  [AArch64] Sink operands to fmuladd. (llvm#102297)
  LSV: document hang reported in llvm#37865 (llvm#102479)
  Enable logf128 constant folding for hosts with 128bit floats (llvm#96287)
  [RISCV][clang] Remove bfloat base type in non-zvfbfmin vcreate (llvm#102146)
  [RISCV][clang] Add missing `zvfbfmin` to `vget_v` intrinsic (llvm#102149)
  [mlir][vector] Add mask elimination transform (llvm#99314)
  [Clang][Interp] Fix display of syntactically-invalid note for member function calls (llvm#102170)
  [bazel] Port for 3fffa6d
  [DebugInfo][RemoveDIs] Use iterator-inserters in clang (llvm#102006)
  ...

Signed-off-by: Edwiin Kusuma Jaya <kutemeikito0905@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[llvm-lit] Environment variable not found in compiler-rt subproject with lit internal shell
4 participants