Skip to content

Commit acc143d

Browse files
Update checkother.cpp
1 parent 0314f16 commit acc143d

1 file changed

Lines changed: 21 additions & 15 deletions

File tree

‎lib/checkother.cpp‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3513,9 +3513,11 @@ void CheckOtherImpl::redundantCopyError(const Token *tok,const std::string& varn
35133513
// Checking for shift by negative values
35143514
//---------------------------------------------------------------------------
35153515

3516-
static bool isNegative(const Token *tok, const Settings &settings)
3516+
static const ValueFlow::Value* isNegative(const Token *tok, const Settings &settings)
35173517
{
3518-
return tok->valueType() && tok->valueType()->sign == ValueType::SIGNED && tok->getValueLE(-1LL, settings);
3518+
if (!tok->valueType() || tok->valueType()->sign != ValueType::SIGNED)
3519+
return nullptr;
3520+
return tok->getValueLE(-1LL, settings);
35193521
}
35203522

35213523
void CheckOtherImpl::checkNegativeBitwiseShift()
@@ -3551,24 +3553,28 @@ void CheckOtherImpl::checkNegativeBitwiseShift()
35513553
if (ternary)
35523554
continue;
35533555

3554-
// Get negative rhs value. preferably a value which doesn't have 'condition'.
3555-
if (portability && isNegative(tok->astOperand1(), mSettings))
3556-
negativeBitwiseShiftError(tok, 1);
3557-
else if (isNegative(tok->astOperand2(), mSettings))
3558-
negativeBitwiseShiftError(tok, 2);
3556+
const ValueFlow::Value* value = isNegative(tok->astOperand1(), mSettings); // lhs
3557+
if (portability && value)
3558+
negativeBitwiseShiftError(tok, true, value);
3559+
else {
3560+
value = isNegative(tok->astOperand2(), mSettings); // rhs
3561+
if (value)
3562+
negativeBitwiseShiftError(tok, false, value);
3563+
}
35593564
}
35603565
}
35613566

35623567

3563-
void CheckOtherImpl::negativeBitwiseShiftError(const Token *tok, int op)
3568+
void CheckOtherImpl::negativeBitwiseShiftError(const Token *tok, bool isLHS, const ValueFlow::Value* v)
35643569
{
3565-
if (op == 1)
3566-
// LHS - this is used by intention in various software, if it
3567-
// is used often in a project and works as expected then this is
3568-
// a portability issue
3569-
reportError(tok, Severity::portability, "shiftNegativeLHS", "Shifting a negative value is technically undefined behaviour", CWE758, Certainty::normal);
3570-
else // RHS
3571-
reportError(tok, Severity::error, "shiftNegative", "Shifting by a negative value is undefined behaviour", CWE758, Certainty::normal);
3570+
// LHS - this is used by intention in various software, if it
3571+
// is used often in a project and works as expected then this is
3572+
// a portability issue
3573+
const char* id = isLHS ? "shiftNegativeLHS" : "shiftNegative";
3574+
const std::string msg = isLHS ? "Shifting a negative value is technically undefined behaviour" : "Shifting by a negative value is undefined behaviour";
3575+
const Severity severity = isLHS ? Severity::portability : (v && v->errorSeverity() && !v->conditional ? Severity::error : Severity::warning);
3576+
const ErrorPath errorPath = getErrorPath(tok, v, msg);
3577+
reportError(errorPath, severity, id, msg, CWE758, Certainty::normal);
35723578
}
35733579

35743580
//---------------------------------------------------------------------------

0 commit comments

Comments
 (0)