Skip to content

Fixes for sanitizer errors from SPEC CPU testing #7697

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+29 −7
Open

Fixes for sanitizer errors from SPEC CPU testing #7697

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
05bac66
Sanitizer fix: shift exponent too large
heshpdx Jul 23, 2025
3410738
Sanitizer fix: overflow in std::abs
heshpdx Jul 23, 2025
6f70d59
Sanitizer fix: signed integer overflow
heshpdx Jul 23, 2025
6c02e8d
Change long long to MathLib::bigint
heshpdx Jul 23, 2025
2f95d90
Change remaining long long to MathLib::bigint
heshpdx Jul 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions lib/infer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,12 @@ namespace {
Interval result;
const ValueFlow::Value* minValue = getCompareValue(values, predicate, std::less<MathLib::bigint>{});
if (minValue) {
if (minValue->isImpossible() && minValue->bound == ValueFlow::Value::Bound::Upper)
result.setMinValue(minValue->intvalue + 1, minValue);
if (minValue->isImpossible() && minValue->bound == ValueFlow::Value::Bound::Upper) {
if (std::numeric_limits<MathLib::bigint>::max() == minValue->intvalue)
result.setMinValue(minValue->intvalue, minValue);
else
result.setMinValue(minValue->intvalue + 1, minValue);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if not doing the addition still leads to a useful value, or if we should bail out somehow for LLONG_MAX.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@pfultz2 Maybe you have some insights?

}
if (minValue->isPossible() && minValue->bound == ValueFlow::Value::Bound::Lower)
result.setMinValue(minValue->intvalue, minValue);
if (!minValue->isImpossible() && (minValue->bound == ValueFlow::Value::Bound::Point || minValue->isKnown()) &&
Expand All @@ -137,8 +141,12 @@ namespace {
}
const ValueFlow::Value* maxValue = getCompareValue(values, predicate, std::greater<MathLib::bigint>{});
if (maxValue) {
if (maxValue->isImpossible() && maxValue->bound == ValueFlow::Value::Bound::Lower)
result.setMaxValue(maxValue->intvalue - 1, maxValue);
if (maxValue->isImpossible() && maxValue->bound == ValueFlow::Value::Bound::Lower) {
if (std::numeric_limits<MathLib::bigint>::min() == maxValue->intvalue)
result.setMaxValue(minValue->intvalue, maxValue);
else
result.setMaxValue(maxValue->intvalue - 1, maxValue);
}
if (maxValue->isPossible() && maxValue->bound == ValueFlow::Value::Bound::Upper)
result.setMaxValue(maxValue->intvalue, maxValue);
assert(!maxValue->isKnown());
Expand Down Expand Up @@ -312,14 +320,20 @@ std::vector<ValueFlow::Value> infer(const ValuePtr<InferModel>& model,
result.push_back(std::move(value));
} else {
if (!diff.minvalue.empty()) {
ValueFlow::Value value(diff.minvalue.front() - 1);
int adder(0);
if (std::numeric_limits<MathLib::bigint>::min() < diff.minvalue.front())
adder = -1;
ValueFlow::Value value(diff.minvalue.front() + adder);
value.setImpossible();
value.bound = ValueFlow::Value::Bound::Upper;
addToErrorPath(value, diff.minRef);
result.push_back(std::move(value));
}
if (!diff.maxvalue.empty()) {
ValueFlow::Value value(diff.maxvalue.front() + 1);
int adder(0);
if (std::numeric_limits<MathLib::bigint>::max() > diff.maxvalue.front())
adder = 1;
ValueFlow::Value value(diff.maxvalue.front() + adder);
value.setImpossible();
value.bound = ValueFlow::Value::Bound::Lower;
addToErrorPath(value, diff.maxRef);
Expand Down
6 changes: 5 additions & 1 deletion lib/token.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2015,7 +2015,11 @@ static bool isAdjacent(const ValueFlow::Value& x, const ValueFlow::Value& y)
return true;
if (x.valueType == ValueFlow::Value::ValueType::FLOAT)
return false;
return std::abs(x.intvalue - y.intvalue) == 1;

// original abs() is not safe against overflows:
// return std::abs(x.intvalue - y.intvalue) == 1;
return (y.intvalue != std::numeric_limits<MathLib::bigint>::max() && x.intvalue == y.intvalue + 1) ||
(y.intvalue != std::numeric_limits<MathLib::bigint>::min() && x.intvalue == y.intvalue - 1);
}

static bool removePointValue(std::list<ValueFlow::Value>& values, std::list<ValueFlow::Value>::iterator& x)
Expand Down
4 changes: 4 additions & 0 deletions lib/vf_common.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include <limits>
#include <utility>
#include <vector>
#include <algorithm>

namespace ValueFlow
{
Expand Down Expand Up @@ -101,6 +102,9 @@ namespace ValueFlow
if (value_size == 0)
return value;

// sizeof(long long) = 8
value_size = std::min(sizeof(MathLib::bigint), value_size);
Copy link
Owner

Choose a reason for hiding this comment

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

I have the feeling we want to be able to truncate according to sizeof(int) here. I.e. the result of ~0U + 2U should be 1.


const MathLib::biguint unsignedMaxValue = std::numeric_limits<MathLib::biguint>::max() >> ((sizeof(unsignedMaxValue) - value_size) * 8);
const MathLib::biguint signBit = 1ULL << (value_size * 8 - 1);
value &= unsignedMaxValue;
Expand Down