Bug/527 #530
Merged
Bug/527 #530
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
Within TPIE's data structures, e.g. tpie::array<T>, there is the memory_usage(size) function to derive the amount of memory to use for a certain number of elements. Yet if one calls this function with size derived from memory_fits(64UINT_MAX) then you end up with undefined behaviour. Specifically, Clang 15 has (an intermediate?) result cast to a 32 bit unsigned integer (unsigned long) rather than a 64 bit one (unsigned long long). The proper solution is to change the specific piece of code, such that there is no unexpected downcasting. The quickfix is to just change Adiar's code to just assert whether we are outside TPIE's safe territory of 4 PiB of RAM (which is very realistic anyway).
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #530 +/- ##
=============================================
+ Coverage 97.094% 97.124% +0.029%
=============================================
Files 82 82
Lines 5644 5632 -12
=============================================
- Hits 5480 5470 -10
+ Misses 164 162 -2
☔ View full report in Codecov by Sentry. |
This sanity check probably has unintended consequences. Let us just trust TPIE instead and know that we never are going to call these functions with so large values that we will see the undefined behaviour of Clang.
Benchmark Report 🟢
Minimum running time (s) for 9-Queens:
|
Benchmark Report 🟢
Minimum running time (s) for 12-Queens:
|
Benchmark Report 🟡
Minimum running time (s) for 14-Queens:
|
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.
Hotfix to close #527 .
Essentially we remove the faulty computation that was trying to safe-guard against TPIE overflowing. This call to TPIE's
memory_usagecalled it with such a large value that we ran into undefined behaviour (see the discussion in #527).While I was thinking to add a safe-guard for TPIE's max memory, it just seems best to trust this piece of code and know, that we never call it with so large values that Clang introduces undefined behaviour.