intel / safe-arithmetic

Safe arithmetic library for C++20 and above. Safe arithmetic ensures correctness of arithmetic operations at compile-time. It protects against overflow, underflow, divide by zero, and out-of-bounds index access. This provides both functional correctness as well as greater protection against related security threats.
https://intel.github.io/safe-arithmetic/
Boost Software License 1.0
83 stars 10 forks source link

Update CI infrastructure #17

Closed elbeno closed 1 year ago

elbeno commented 1 year ago

This is mostly formulaic, but the UB sanitizer turned up some issues in 64-bit tests.

elbeno commented 1 year ago

There is also an issue with tests that don't have any "real" google tests, but only rapidcheck tests. They don't play well with gtest_discover_tests (I assume reporting no tests) - the result is that they will rebuild every build... adding at least one (possibly degenerate) "plain test" would fix this.

lukevalenty commented 1 year ago

This is mostly formulaic, but the UB sanitizer turned up some issues in 64-bit tests.

Can you share which tests had this issue? Was it an issue in the test itself using 64-but primitives, or in safe arithmetic?

If a test hit 64-bit overflow for negation, that should fail the test because the big_integer should have the correct value while the 64-bit primitive will overflow or have other UB. Right?

elbeno commented 1 year ago

It is a problem with the tests, not the library. And the test doesn't fail; it triggers UBSan. For example https://github.com/intel/safe-arithmetic/blob/main/test/safe/big_integer/detail/plus.cpp#L37 when the generated values are such that a + b (in regular C++ artihmetic) overflows. This is true for the plus, minus, and multiply tests, and the negate test. And it makes sense that rapidcheck should generate boundary conditions...

lukevalenty commented 1 year ago

Looks like I need to update the branch protection rules on the repo. 😅

elbeno commented 1 year ago

Should be building now... clang-format reordering headers produced a failure I guess.

elbeno commented 1 year ago

Tests are "passing" (see ASan/TSan tests), but failing because of CTest interaction with rapidcheck & the compile-fail tests. And of course the UBSan tests have a couple of issues. I'm assuming this isn't holding up the actual merge though and we can fix this all up in future PRs?