Skip to content

Conversation

@petrhosek
Copy link
Member

@petrhosek petrhosek commented Sep 9, 2024

We shouldn't assume that we're using system zlib installation.

We shouldn't assume that we're using system zlib installation
@llvmbot
Copy link
Member

llvmbot commented Sep 9, 2024

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

Author: Petr Hosek (petrhosek)

Changes

We shouldn't assume that we're using system zlib installation.


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

3 Files Affected:

  • (modified) compiler-rt/test/fuzzer/compressed.test (+2-2)
  • (modified) compiler-rt/test/lit.common.cfg.py (+2)
  • (modified) compiler-rt/test/lit.common.configured.in (+2)
diff --git a/compiler-rt/test/fuzzer/compressed.test b/compiler-rt/test/fuzzer/compressed.test
index 284f7ed96ca851..59a43cc807337f 100644
--- a/compiler-rt/test/fuzzer/compressed.test
+++ b/compiler-rt/test/fuzzer/compressed.test
@@ -4,7 +4,7 @@ REQUIRES: zlib
 # unsupported by this test.
 UNSUPPORTED: i386, target=arm{{.*}}
 # Custom mutator should find this bug, w/o custom -- no chance.
-RUN: %cpp_compiler %S/CompressedTest.cpp -o %t-CompressedTestCustom -DCUSTOM_MUTATOR -lz
-RUN: %cpp_compiler %S/CompressedTest.cpp -o %t-CompressedTestPlain -lz
+RUN: %cpp_compiler %S/CompressedTest.cpp -o %t-CompressedTestCustom -DCUSTOM_MUTATOR -I%zlib_include_dir %zlib_library
+RUN: %cpp_compiler %S/CompressedTest.cpp -o %t-CompressedTestPlain -I%zlib_include_dir %zlib_library
 RUN: not %run %t-CompressedTestCustom -seed=1 -runs=1000000
 RUN:     %run %t-CompressedTestPlain  -seed=1 -runs=1000000
diff --git a/compiler-rt/test/lit.common.cfg.py b/compiler-rt/test/lit.common.cfg.py
index 0690c3a18efdbc..1c6fbc80cd4137 100644
--- a/compiler-rt/test/lit.common.cfg.py
+++ b/compiler-rt/test/lit.common.cfg.py
@@ -312,6 +312,8 @@ def push_dynamic_library_lookup_path(config, new_path):
 
 if config.have_zlib:
     config.available_features.add("zlib")
+    config.substitutions.append(("%zlib_include_dir", config.zlib_include_dir))
+    config.substitutions.append(("%zlib_library", config.zlib_library))
 
 if config.have_internal_symbolizer:
     config.available_features.add("internal_symbolizer")
diff --git a/compiler-rt/test/lit.common.configured.in b/compiler-rt/test/lit.common.configured.in
index 8889b816b149fc..049133dc965c31 100644
--- a/compiler-rt/test/lit.common.configured.in
+++ b/compiler-rt/test/lit.common.configured.in
@@ -71,6 +71,8 @@ else:
 
 set_default("have_internal_symbolizer", @COMPILER_RT_ENABLE_INTERNAL_SYMBOLIZER_PYBOOL@)
 set_default("have_zlib", @ZLIB_FOUND_PYBOOL@)
+set_default("zlib_include_dir", "@ZLIB_INCLUDE_DIR@")
+set_default("zlib_library", "@ZLIB_LIBRARY@")
 set_default("libcxx_used", "@LLVM_LIBCXX_USED@")
 
 # LLVM tools dir can be passed in lit parameters, so try to

@petrhosek
Copy link
Member Author

This was uncovered by #107611 since this test wasn't being built previously in the runtimes build.

@petrhosek petrhosek requested a review from vitalybuka September 9, 2024 22:12
Copy link
Collaborator

@smeenai smeenai left a comment

Choose a reason for hiding this comment

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

I'm not familiar with the test, but the fix seems pretty straightforward, and hopefully bots have good coverage for this.

@petrhosek
Copy link
Member Author

Looks like the presubmit bots are happy so I'm going to merge the change.

@petrhosek petrhosek merged commit eb0e4b1 into llvm:main Sep 10, 2024
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.

4 participants