Portability warning when 'char' type is used as array index

lib/astutils.cpp

@@ -24,9 +24,24 @@
 #include "tokenize.h"
 #include <set>
 
+static bool astIsCharWithSign(const Token *tok, ValueType::Sign sign)
+{
+    if (!tok)
+        return false;
+    const ValueType *valueType = tok->valueType();
+    if (!valueType)
+        return false;
+    return valueType->type == ValueType::Type::CHAR && valueType->pointer == 0U && valueType->sign == sign;
+}
+
 bool astIsSignedChar(const Token *tok)
 {
-    return tok && tok->valueType() && tok->valueType()->sign == ValueType::Sign::SIGNED && tok->valueType()->type == ValueType::Type::CHAR && tok->valueType()->pointer == 0U;
+    return astIsCharWithSign(tok, ValueType::Sign::SIGNED);
+}
+
+bool astIsUnknownSignChar(const Token *tok)
+{
+    return astIsCharWithSign(tok, ValueType::Sign::UNKNOWN_SIGN);
 }
 
 bool astIsIntegral(const Token *tok, bool unknown)

lib/astutils.h

@@ -29,6 +29,8 @@ class Token;
 
 /** Is expression a 'signed char' if no promotion is used */
 bool astIsSignedChar(const Token *tok);
+/** Is expression a 'char' if no promotion is used? */
+bool astIsUnknownSignChar(const Token *tok);
 /** Is expression of integral type? */
 bool astIsIntegral(const Token *tok, bool unknown);
 /** Is expression of floating point type? */

lib/checkother.cpp

@@ -1459,7 +1459,9 @@ void CheckOther::passedByValueError(const Token *tok, const std::string &parname
 
 void CheckOther::checkCharVariable()
 {
-    if (!_settings->isEnabled("warning"))
+    const bool warning = _settings->isEnabled("warning");
+    const bool portability = _settings->isEnabled("portability");
+    if (!warning && !portability)
         return;
 
     const SymbolDatabase *symbolDatabase = _tokenizer->getSymbolDatabase();
@@ -1468,12 +1470,16 @@ void CheckOther::checkCharVariable()
         const Scope * scope = symbolDatabase->functionScopes[i];
         for (const Token* tok = scope->classStart; tok != scope->classEnd; tok = tok->next()) {
             if (Token::Match(tok, "%var% [")) {
-                if (!tok->variable() || !tok->variable()->isArray())
+                if (!tok->variable())
+                    continue;
+                if (!tok->variable()->isArray() && !tok->variable()->isPointer())
                     continue;
                 const Token *index = tok->next()->astOperand2();
-                if (astIsSignedChar(index) && index->getValueGE(0x80, _settings))
+                if (warning && tok->variable()->isArray() && astIsSignedChar(index) && index->getValueGE(0x80, _settings))
-                    charArrayIndexError(tok);
+                    signedCharArrayIndexError(tok);
-            } else if (Token::Match(tok, "[&|^]") && tok->astOperand2() && tok->astOperand1()) {
+                if (portability && astIsUnknownSignChar(index) && index->getValueGE(0x80, _settings))
+                    unknownSignCharArrayIndexError(tok);
+            } else if (warning && Token::Match(tok, "[&|^]") && tok->astOperand2() && tok->astOperand1()) {
                 bool warn = false;
                 if (astIsSignedChar(tok->astOperand1())) {
                     const ValueFlow::Value *v1 = tok->astOperand1()->getValueLE(-1, _settings);
@@ -1502,17 +1508,27 @@ void CheckOther::checkCharVariable()
     }
 }
 
-void CheckOther::charArrayIndexError(const Token *tok)
+void CheckOther::signedCharArrayIndexError(const Token *tok)
 {
     reportError(tok,
                 Severity::warning,
-                "charArrayIndex",
+                "signedCharArrayIndex",
                 "Signed 'char' type used as array index.\n"
                 "Signed 'char' type used as array index. If the value "
                 "can be greater than 127 there will be a buffer underflow "
                 "because of sign extension.");
 }
 
+void CheckOther::unknownSignCharArrayIndexError(const Token *tok)
+{
+    reportError(tok,
+                Severity::portability,
+                "unknownSignCharArrayIndex",
+                "'char' type used as array index.\n"
+                "'char' type used as array index. Values greater that 127 will be "
+                "treated depending on whether 'char' is signed or unsigned on target platform.");
+}
+
 void CheckOther::charBitOpError(const Token *tok)
 {
     reportError(tok,

lib/checkother.h

@@ -218,7 +218,8 @@ private:
     void invalidPointerCastError(const Token* tok, const std::string& from, const std::string& to, bool inconclusive);
     void passedByValueError(const Token *tok, const std::string &parname);
     void constStatementError(const Token *tok, const std::string &type);
-    void charArrayIndexError(const Token *tok);
+    void signedCharArrayIndexError(const Token *tok);
+    void unknownSignCharArrayIndexError(const Token *tok);
     void charBitOpError(const Token *tok);
     void variableScopeError(const Token *tok, const std::string &varname);
     void zerodivError(const Token *tok, bool inconclusive);
@@ -280,7 +281,8 @@ private:
         c.cstyleCastError(0);
         c.passedByValueError(0, "parametername");
         c.constStatementError(0, "type");
-        c.charArrayIndexError(0);
+        c.signedCharArrayIndexError(0);
+        c.unknownSignCharArrayIndexError(0);
         c.charBitOpError(0);
         c.variableScopeError(0, "varname");
         c.redundantAssignmentInSwitchError(0, 0, "var");

test/testcharvar.cpp

@@ -32,6 +32,7 @@ private:
     void run() {
         settings.platform(Settings::Unspecified);
         settings.addEnabled("warning");
+        settings.addEnabled("portability");
 
         TEST_CASE(array_index_1);
         TEST_CASE(array_index_2);
@@ -67,6 +68,14 @@ private:
               "    char ch = 0x80;\n"
               "    buf[ch] = 0;\n"
               "}");
+        ASSERT_EQUALS("[test.cpp:5]: (portability) 'char' type used as array index.\n", errout.str());
+
+        check("int buf[256];\n"
+              "void foo()\n"
+              "{\n"
+              "    char ch = 0;\n"
+              "    buf[ch] = 0;\n"
+              "}");
         ASSERT_EQUALS("", errout.str());
 
         check("int buf[256];\n"
@@ -85,6 +94,14 @@ private:
               "}");
         ASSERT_EQUALS("", errout.str());
 
+        check("int buf[256];\n"
+              "void foo()\n"
+              "{\n"
+              "    char ch = 0x80;\n"
+              "    buf[ch] = 0;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:5]: (portability) 'char' type used as array index.\n", errout.str());
+
         check("int buf[256];\n"
               "void foo(signed char ch)\n"
               "{\n"
@@ -92,6 +109,53 @@ private:
               "}");
         ASSERT_EQUALS("", errout.str());
 
+        check("int buf[256];\n"
+              "void foo(char ch)\n"
+              "{\n"
+              "    buf[ch] = 0;\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void foo(char* buf)\n"
+              "{\n"
+              "    char ch = 0x80;"
+              "    buf[ch] = 0;\n"
+              "}");
+        ASSERT_EQUALS("[test.cpp:3]: (portability) 'char' type used as array index.\n", errout.str());
+
+        check("void foo(char* buf)\n"
+              "{\n"
+              "    char ch = 0;"
+              "    buf[ch] = 0;\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void foo(char* buf)\n"
+              "{\n"
+              "    buf['A'] = 0;\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("void foo(char* buf, char ch)\n"
+              "{\n"
+              "    buf[ch] = 0;\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("int flags[256];\n"
+              "void foo(const char* str)\n"
+              "{\n"
+              "    flags[*str] = 0;\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
+        check("int flags[256];\n"
+              "void foo(const char* str)\n"
+              "{\n"
+              "    flags[(unsigned char)*str] = 0;\n"
+              "}");
+        ASSERT_EQUALS("", errout.str());
+
         check("void foo(const char str[])\n"
               "{\n"
               "    map[str] = 0;\n"