Skip to content

Commit 52f4e2e

Browse files
JeewoongKimJeewoongKim
committed
Fix Ticket #8442: detect unselectable switch cases
1 parent e6f7b92 commit 52f4e2e

3 files changed

Lines changed: 122 additions & 0 deletions

File tree

‎lib/checkother.cpp‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -968,6 +968,66 @@ void CheckOtherImpl::suspiciousCaseInSwitchError(const Token* tok, const std::st
968968
"Using an operator like '" + operatorString + "' in a case label is suspicious. Did you intend to use a bitwise operator, multiple case labels or if/else instead?", CWE398, Certainty::inconclusive);
969969
}
970970

971+
void CheckOtherImpl::checkUnreachableSwitchCase()
972+
{
973+
if (!mSettings.severity.isEnabled(Severity::style))
974+
return;
975+
976+
logChecker("CheckOther::checkUnreachableSwitchCase"); // style
977+
978+
const SymbolDatabase* symbolDatabase = mTokenizer->getSymbolDatabase();
979+
980+
for (const Scope& scope : symbolDatabase->scopeList) {
981+
if (scope.type != ScopeType::eSwitch || !scope.bodyStart)
982+
continue;
983+
const Token* rpar = scope.bodyStart->previous();
984+
if (!Token::simpleMatch(rpar, ")"))
985+
continue;
986+
const Token* lpar = rpar->link();
987+
if (!lpar)
988+
continue;
989+
const Token* condition = lpar->astOperand2();
990+
if (!condition)
991+
continue;
992+
const ValueFlow::Value* switchValue =
993+
condition->getKnownValue(ValueFlow::Value::ValueType::INT);
994+
if (!switchValue)
995+
continue;
996+
997+
for (const Token* tok = scope.bodyStart->next();
998+
tok && tok != scope.bodyEnd;
999+
tok = tok->next()) {
1000+
1001+
// Do not inspect cases belonging to a nested switch.
1002+
if (Token::simpleMatch(tok, "{") &&
1003+
tok->scope()->type == ScopeType::eSwitch) {
1004+
tok = tok->link();
1005+
continue;
1006+
}
1007+
if (!Token::simpleMatch(tok, "case"))
1008+
continue;
1009+
const Token* caseExpression = tok->astOperand1();
1010+
if (!caseExpression)
1011+
continue;
1012+
const ValueFlow::Value* caseValue =
1013+
caseExpression->getKnownValue(ValueFlow::Value::ValueType::INT);
1014+
if (!caseValue)
1015+
continue;
1016+
if (switchValue->intvalue == caseValue->intvalue)
1017+
continue;
1018+
unreachableSwitchCaseError(tok, caseExpression->expressionString());
1019+
}
1020+
}
1021+
}
1022+
1023+
void CheckOtherImpl::unreachableSwitchCaseError(const Token* tok, const std::string& caseExpression)
1024+
{
1025+
reportError(tok, Severity::style, "unreachableSwitchCase",
1026+
"Switch case '" + caseExpression +
1027+
"' can never be selected because the switch condition has a known value.",
1028+
CWE561, Certainty::normal);
1029+
}
1030+
9711031
static bool isNestedInSwitch(const Scope* scope)
9721032
{
9731033
while (scope) {
@@ -4820,6 +4880,7 @@ void CheckOther::runChecks(const Tokenizer &tokenizer, ErrorLogger& errorLogger)
48204880
checkOther.checkCharVariable();
48214881
checkOther.redundantBitwiseOperationInSwitchError();
48224882
checkOther.checkSuspiciousCaseInSwitch();
4883+
checkOther.checkUnreachableSwitchCase();
48234884
checkOther.checkDuplicateBranch();
48244885
checkOther.checkDuplicateExpression();
48254886
checkOther.checkRedundantAssignment();
@@ -4907,6 +4968,7 @@ void CheckOther::getErrorMessages(ErrorLogger& errorLogger, const Settings &sett
49074968
c.duplicateExpressionTernaryError(nullptr, ErrorPath{});
49084969
c.duplicateBreakError(nullptr, false);
49094970
c.unreachableCodeError(nullptr, nullptr, false);
4971+
c.unreachableSwitchCaseError(nullptr, "case");
49104972
c.unsignedLessThanZeroError(nullptr, nullptr, "varname");
49114973
c.unsignedPositiveError(nullptr, nullptr, "varname");
49124974
c.pointerLessThanZeroError(nullptr, nullptr);

‎lib/checkother.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,9 @@ class CPPCHECKLIB CheckOtherImpl : public CheckImpl {
186186
/** @brief %Check for code like 'case A||B:'*/
187187
void checkSuspiciousCaseInSwitch();
188188

189+
/** @brief %Check for case labels that cannot be selected */
190+
void checkUnreachableSwitchCase();
191+
189192
/** @brief %Check for objects that are destroyed immediately */
190193
void checkMisusedScopedObject();
191194

@@ -290,6 +293,7 @@ class CPPCHECKLIB CheckOtherImpl : public CheckImpl {
290293
void redundantCopyError(const Token *tok1, const Token* tok2, const std::string& var);
291294
void redundantBitwiseOperationInSwitchError(const Token *tok, const std::string &varname);
292295
void suspiciousCaseInSwitchError(const Token* tok, const std::string& operatorString);
296+
void unreachableSwitchCaseError(const Token* tok, const std::string& caseExpression);
293297
void selfAssignmentError(const Token *tok, const std::string &varname);
294298
void misusedScopeObjectError(const Token *tok, const std::string &varname, bool isAssignment = false);
295299
void duplicateBranchError(const Token *tok1, const Token *tok2, ErrorPath errors);

‎test/testother.cpp‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ class TestOther : public TestFixture {
142142
TEST_CASE(switchRedundantOperationTest);
143143
TEST_CASE(switchRedundantBitwiseOperationTest);
144144
TEST_CASE(unreachableCode);
145+
TEST_CASE(unreachableSwitchCase); // #8442
145146
TEST_CASE(redundantContinue);
146147

147148
TEST_CASE(suspiciousCase);
@@ -6348,6 +6349,61 @@ class TestOther : public TestFixture {
63486349
ASSERT_EQUALS("", errout_str());
63496350
}
63506351

6352+
void unreachableSwitchCase() {
6353+
check("enum T { A, B};\n"
6354+
"void f(const T &t) {\n"
6355+
" if (t == A) {\n"
6356+
" switch (t) {\n"
6357+
" case A:\n"
6358+
" break;\n"
6359+
" case B:\n"
6360+
" break;\n"
6361+
" }\n"
6362+
" }\n"
6363+
"}\n");
6364+
ASSERT_EQUALS("[test.cpp:7:9]: (style) Switch case 'B' can never be selected because the switch condition has a known value. [unreachableSwitchCase]\n", errout_str());
6365+
6366+
check("void f(int t) {\n"
6367+
" if (t == 0) {\n"
6368+
" switch (t) {\n"
6369+
" case 0:\n"
6370+
" break;\n"
6371+
" case 1:\n"
6372+
" break;\n"
6373+
" }\n"
6374+
" }\n"
6375+
"}\n");
6376+
ASSERT_EQUALS("[test.cpp:6:9]: (style) Switch case '1' can never be selected because the switch condition has a known value. [unreachableSwitchCase]\n", errout_str());
6377+
6378+
check("void f(int t) {\n"
6379+
" switch (t) {\n"
6380+
" case 0:\n"
6381+
" break;\n"
6382+
" case 1:\n"
6383+
" break;\n"
6384+
" }\n"
6385+
"}\n");
6386+
ASSERT_EQUALS("", errout_str());
6387+
6388+
check("void f(int x, int y) {\n"
6389+
" if (x == 0) {\n"
6390+
" switch (x) {\n"
6391+
" case 0:\n"
6392+
" switch (y) {\n"
6393+
" case 1:\n"
6394+
" break;\n"
6395+
" case 2:\n"
6396+
" break;\n"
6397+
" }\n"
6398+
" break;\n"
6399+
" case 1:\n"
6400+
" break;\n"
6401+
" }\n"
6402+
" }\n"
6403+
"}\n");
6404+
ASSERT_EQUALS("[test.cpp:12:9]: (style) Switch case '1' can never be selected because the switch condition has a known value. [unreachableSwitchCase]\n", errout_str());
6405+
}
6406+
63516407
void redundantContinue() {
63526408
check("void f() {\n" // #11195
63536409
" for (int i = 0; i < 10; ++i) {\n"

0 commit comments

Comments
 (0)