Skip to content

Fix #12109 Crash in calculate.h#5587

Merged
chrchr-github merged 7 commits intocppcheck-opensource:mainfrom
chrchr-github:chr_Fix12109
Oct 24, 2023
Merged

Fix #12109 Crash in calculate.h#5587
chrchr-github merged 7 commits intocppcheck-opensource:mainfrom
chrchr-github:chr_Fix12109

Conversation

@chrchr-github
Copy link
Copy Markdown
Collaborator

No description provided.

@chrchr-github
Copy link
Copy Markdown
Collaborator Author

I can't reproduce the FP locally for some reason, although calculate.h is only included in infer.cpp, programmemory.cpp, valueflow.cpp. We should probably also report the source location for header templates.

@firewave
Copy link
Copy Markdown
Collaborator

firewave commented Oct 23, 2023

We should probably also report the source location for header templates.

See also https://trac.cppcheck.net/ticket/11913.

@firewave
Copy link
Copy Markdown
Collaborator

I can't reproduce the FP locally for some reason, although calculate.h is only included in infer.cpp, programmemory.cpp, valueflow.cpp.

I also cannot reproduce it. I really need to make it easier to run the selfcheck. I am considering making it a build target after another round of cleanups so you can easily run it locally and to get rid of the duplicated code in the CI.

Comment thread lib/calculate.h Outdated
return wrap(x * y);
case '/':
if (isZero(y)) {
if (isZero(y) || (std::is_integral<T>::value && std::is_signed<T>::value && isEqual(y, T(-1)) && isEqual(x, std::numeric_limits<T>::min()))) { // cppcheck-suppress knownConditionTrueFalse
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Write std::is_integral<T>{} and std::is_signed<T>{} instead of ::value.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I would have used std::is_integral_v<T> if it was available, but yours works in C++11 and also "fixes" the FP 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants