Audit Runtime build infrastructure #1588
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
... by recording it into logs instead. This eliminates CMake deprecation warnings from the external project using a low minimum version. Warning message: Compatibility with CMake < 3.5 will be removed from a future version of CMake.
|
Hello. You may have forgotten to update the changelog!
|
Migration based on the following guide: https://github.com/catchorg/Catch2/blob/v3.8.0/docs/migrate-v2-to-v3.md The new version has separate headers for faster compile times with a static library. It also updated its minimum CMake version which allows to get rid of this CMake deprecation warning: Compatibility with CMake < 3.10 will be removed from a future version of CMake. Additional cleanup in the test directory.
The problem is a faulty copy from std::string to char*. The followwing
error has been resolved:
AddressSanitizer: heap-buffer-overflow
READ of size 20976
#1 0x1029c71e4 in runCircuit openqasm_python_module.cpp:186
std::vector's reserve does not change the size of a vector, it is merely
a memory optimization. So while the underlying memory buffer is resized,
the container itself is not. Resolved the following error:
AddressSanitizer: container-overflow
WRITE of size 8
#0 0x104d2f7a0 in CATCH2_INTERNAL_TEST_10() Test_NullQubit.cpp:117
QuantumDevices are allocated on the heap and need to be freed. Fixes the
following error:
LeakSanitizer: detected memory leaks
Direct leak of 72 byte(s) in 1 object(s) allocated from:
#0 0x5599a5edf01d in operator new(unsigned long)
#1 0x7f44d215e812 in NullQubitFactory /home/davidi/catalyst/runtime/lib/backend/null_qubit/NullQubit.cpp:4:1
#2 0x5599a5ee172d in loadDevice(...) /home/davidi/catalyst/runtime/tests/TestUtils.hpp:46:18
Enables clickable file+line number links by using the commonly used format in error message printing.
In the libraries we dlopen, the RPATH to the dlopen target has been hardcoded with build-time information, which will not work after distributing the package. Instead, we should rely on known relative paths, such as $ORIGIN.
- remove catalyst_qir_runtime target
- this target existed only for testing, and was not suited as a
general interface target to the runtime since it includes all
internal headers and properties as well
- an explicit testing target is now added in the test directory
- remove object target for the capi as it serves no purpose
- prefer private by default for cmake target properties
- use public to propagate header information correctly for each target
.. and into the backend utility includes.
.. since we load the function dynamically.
dime10
force-pushed
the
warnings-as-errors
branch
from
54e9160 to
46bfc41
Compare
mlxd
reviewed
Mar 27, 2025
mlxd
reviewed
Mar 27, 2025
dime10
force-pushed
the
warnings-as-errors
branch
2 times, most recently
from
ba70c0f to
3a95d05
Compare
ASAN flags are now computed independently in the runtime Makefile, instead of coming from the top-level one. This has two advantages: - one can modify the default value for ENABLE_ASAN in the runtime Makefile, and still run from make from the top-level - environment variables are adjustable for each scope, for instance when running the runtime tests, which are executables, we should not preload the ASAN RT since it is already embedded in the executable ASAN flags for running Python tests have been adjusted to account for certain peculiarities: - dlopen cannot find libraries specified by name only since it loses RPATH information from the calling library when replaced by ASAN - tests that generate executables are skipped since correctly linking them from Catalyst's frontend is difficult
GCC, while supporting ASAN in principle, and while runtime tests can successfully be be run with it, Python frontend tests still fail (both lit and pytest).
Resolves the following error (due to inclusion of a source file
instead of a header file):
ERROR: AddressSanitizer: odr-violation
[1] size=37 'typeinfo name for Catalyst::Runtime::Device::OQDDevice' /home/davidi/catalyst/runtime/tests/Test_OQDDevice.cpp
[2] size=37 'typeinfo name for Catalyst::Runtime::Device::OQDDevice' /home/davidi/catalyst/runtime/lib/backend/oqd/OQDDevice.cpp
dime10
force-pushed
the
warnings-as-errors
branch
from
3a95d05 to
ffc67dc
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1588 +/- ##
==========================================
+ Coverage 96.08% 96.78% +0.70%
==========================================
Files 81 80 -1
Lines 8615 8565 -50
Branches 816 816
==========================================
+ Hits 8278 8290 +12
+ Misses 280 218 -62
Partials 57 57 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
3 tasks
erick-xanadu
added
the
reviewer:require-wheels
dime10
added
the
author:build-wheels
Our LAPACKE dependency throws the following error:
CMake Error at runtime/build/_lapacke-accelerate/src/lapacke-accelerate-build/_deps/reference-lapack-src/INSTALL/CMakeLists.txt:1 (cmake_minimum_required):
Compatibility with CMake < 3.5 has been removed from CMake.
dime10
force-pushed
the
warnings-as-errors
branch
from
bd4d0ba to
1d4f97a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The primary, high-level goal is to enable stricter safety checks in our build systems. In addition to stricter checks, ideally, all our build recipes should produce clean, warning-free outputs, as ignoring one set of warnings can easily lead to overlooking other warnings.
This PR takes a look at the runtime and improves the following:
ENABLE_ASANacross tested platformsThe above was tested on the runtime with the following configurations:
test-runtime,coverage-runtime,lit,pytestThe PR does not enable ASAN by default or set up an ASAN CI build (yet).