Fix #14743 (New check: ftell() result is unspecified when file is opened in mode "t")

.gitignore

@@ -2,6 +2,7 @@
 *.gcno
 *.gch
 *.o
+*.a
 *.pyc
 /cppcheck
 /cppcheck.exe

lib/checkio.cpp

@@ -53,6 +53,7 @@ namespace {
 // CVE ID used:
 static const CWE CWE119(119U);  // Improper Restriction of Operations within the Bounds of a Memory Buffer
 static const CWE CWE398(398U);  // Indicator of Poor Code Quality
+static const CWE CWE474(474U);  // Use of Function with Inconsistent Implementations
 static const CWE CWE664(664U);  // Improper Control of a Resource Through its Lifetime
 static const CWE CWE685(685U);  // Function Call With Incorrect Number of Arguments
 static const CWE CWE686(686U);  // Function Call With Incorrect Argument Type
@@ -116,6 +117,8 @@ namespace {
         nonneg int op_indent{};
         enum class AppendMode : std::uint8_t { UNKNOWN_AM, APPEND, APPEND_EX };
         AppendMode append_mode = AppendMode::UNKNOWN_AM;
+        enum class ReadMode : std::uint8_t { READ_TEXT, READ_BIN };
+        ReadMode read_mode = ReadMode::READ_BIN;
         std::string filename;
         explicit Filepointer(OpenMode mode_ = OpenMode::UNKNOWN_OM)
             : mode(mode_) {}
@@ -188,6 +191,7 @@ void CheckIO::checkFileUsage()
                 }
             } else if (Token::Match(tok, "%name% (") && tok->previous() && (!tok->previous()->isName() || Token::Match(tok->previous(), "return|throw"))) {
                 std::string mode;
+                bool isftell = false;
                 const Token* fileTok = nullptr;
                 const Token* fileNameTok = nullptr;
                 Filepointer::Operation operation = Filepointer::Operation::NONE;
@@ -249,6 +253,9 @@ void CheckIO::checkFileUsage()
                     fileTok = tok->tokAt(2);
                     if ((tok->str() == "ungetc" || tok->str() == "ungetwc") && fileTok)
                         fileTok = fileTok->nextArgument();
+                    else if (tok->str() == "ftell") {
+                        isftell = true;
+                    }
                     operation = Filepointer::Operation::UNIMPORTANT;
                 } else if (!Token::Match(tok, "if|for|while|catch|switch") && !mSettings->library.isFunctionConst(tok->str(), true)) {
                     const Token* const end2 = tok->linkAt(1);
@@ -304,10 +311,15 @@ void CheckIO::checkFileUsage()
                             f.append_mode = Filepointer::AppendMode::APPEND_EX;
                         else
                             f.append_mode = Filepointer::AppendMode::APPEND;
+                    }
+                    else if (mode.find('r') != std::string::npos &&
+                             mode.find('t') != std::string::npos) {
+                        f.read_mode = Filepointer::ReadMode::READ_TEXT;
                     } else
                         f.append_mode = Filepointer::AppendMode::UNKNOWN_AM;
                     f.mode_indent = indent;
                     break;
+
                 case Filepointer::Operation::POSITIONING:
                     if (f.mode == OpenMode::CLOSED)
                         useClosedFileError(tok);
@@ -340,6 +352,8 @@ void CheckIO::checkFileUsage()
                 case Filepointer::Operation::UNIMPORTANT:
                     if (f.mode == OpenMode::CLOSED)
                         useClosedFileError(tok);
+                    if (isftell && f.read_mode == Filepointer::ReadMode::READ_TEXT && printPortability)
+                        ftellFileError(tok);
                     break;
                 case Filepointer::Operation::UNKNOWN_OP:
                     f.mode = OpenMode::UNKNOWN_OM;
@@ -398,6 +412,18 @@ void CheckIO::seekOnAppendedFileError(const Token *tok)
                 "seekOnAppendedFile", "Repositioning operation performed on a file opened in append mode has no effect.", CWE398, Certainty::normal);
 }
 
+void CheckIO::ftellFileError(const Token *tok)
+{
+    reportError(tok, Severity::portability,
+                "ftellTextModeFile", "The ftell function obtains the current value of the file position indicator"
+                " for the stream pointed to by stream. For a binary stream, the value is the number of characters"
+                " from the beginning of the file. For a text stream, its file position indicator contains unspecified"
+                " information, usable by the fseek function for returning the file position indicator for the stream"
+                " to its position at the time of the ftell call; the difference between two such return values is"
+                " not necessarily a meaningful measure of the number of characters written or read. See 7.21.9.4"
+                " in C11 standard.", CWE474, Certainty::normal);
+}
+
 void CheckIO::incompatibleFileOpenError(const Token *tok, const std::string &filename)
 {
     reportError(tok, Severity::warning,

lib/checkio.h

@@ -106,6 +106,7 @@ class CPPCHECKLIB CheckIO : public Check {
     void writeReadOnlyFileError(const Token *tok);
     void useClosedFileError(const Token *tok);
     void seekOnAppendedFileError(const Token *tok);
+    void ftellFileError(const Token *tok);
     void incompatibleFileOpenError(const Token *tok, const std::string &filename);
     void invalidScanfError(const Token *tok);
     void wrongPrintfScanfArgumentsError(const Token* tok,
@@ -140,6 +141,7 @@ class CPPCHECKLIB CheckIO : public Check {
                "- Missing or wrong width specifiers in 'scanf' format string\n"
                "- Use a file that has been closed\n"
                "- File input/output without positioning results in undefined behaviour\n"
+               "- Using 'ftell' on a file opened in text mode\n"
                "- Read to a file that has only been opened for writing (or vice versa)\n"
                "- Repositioning operation on a file opened in append mode\n"
                "- The same file can't be open for read and write at the same time on different streams\n"

man/checkers/ftellTextModeFile.md

@@ -0,0 +1,57 @@
+# ftellModeTextFile 
+
+**Message**: The ftell function obtains the current value of the file position indicator for the stream pointed to by stream. For a binary stream, the value is the number of characters from the beginning of the file. For a text stream, its file position indicator contains unspecified information, usable by the fseek function for returning the file position indicator for the stream to its position at the time of the ftell call; the difference between two such return values is not necessarily a meaningful measure of the number of characters written or read. See a7.21.9.4 in C11 standard.<br/>
+**Category**: Portability<br/>
+**Severity**: Style<br/>
+**Language**: C/C++
+
+## Description
+
+This checker detects the use of ftell() on a file open in text (or translate) mode. The text mode is not consistent
+ in between Linux and Windows system and may cause ftell() to return the wrong offset inside a text file.
+
+This warning helps improve code quality by:
+- Making the intent clear that the use of ftell() in "t" mode may cause portability problem.
+
+## Motivation
+
+This checker improves portability accross system.
+
+## How to fix
+
+According to C11, the file must be opened in binary mode 'b' to prevent this problem.
+
+Before:
+```cpp
+     FILE *f = fopen("Example.txt", "rt");
+     if (f)
+     {
+        int position;
+        struct stat st;
+
+        position = fseek(f, 0, SEEK_END);
+        fstat(f, &st);
+        printf( "Position  %d\n", ftell(f);
+        printf( "File size %d\n, st.st_size);
+        fclose(f);
+     }
+
+```
+
+After:
+```cpp
+
+     FILE *f = fopen("Example.txt", "rb");
+     if (f)
+     {
+        fseek(f, 0, SEEK_END);
+        printf( "Offset %d\n", ftell(f);
+        fclose(f);
+     }
+
+```
+
+## Notes
+
+See https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/fopen-wfopen?view=msvc-170
+

test/testio.cpp

@@ -44,6 +44,7 @@ class TestIO : public TestFixture {
         TEST_CASE(fileIOwithoutPositioning);
         TEST_CASE(seekOnAppendedFile);
         TEST_CASE(fflushOnInputStream);
+        TEST_CASE(ftellCompatibility);
         TEST_CASE(incompatibleFileOpen);
 
         TEST_CASE(testScanf1); // Scanf without field limiters
@@ -704,6 +705,22 @@ class TestIO : public TestFixture {
         ASSERT_EQUALS("", errout_str()); // #6566
     }
 
+    void ftellCompatibility() {
+
+        check("void foo() {\n"
+              "     FILE *f = fopen(\"\", \"rt\");\n"
+              "     if (f)\n"
+              "     {\n"
+              "         extern long position;\n"
+              "         fseek(f, 0, SEEK_END);\n"
+              "         position = ftell(f);\n"
+              "         fclose(f);\n"
+              "     }\n"
+              "}\n", dinit(CheckOptions, $.portability = true));
+        ASSERT_EQUALS("[test.cpp:7:21]: (portability) The ftell function obtains the current value of the file position indicator for the stream pointed to by stream. For a binary stream, the value is the number of characters from the beginning of the file. For a text stream, its file position indicator contains unspecified information, usable by the fseek function for returning the file position indicator for the stream to its position at the time of the ftell call; the difference between two such return values is not necessarily a meaningful measure of the number of characters written or read. See 7.21.9.4 in C11 standard. [ftellTextModeFile]\n", errout_str());
+    }
+
+
     void fflushOnInputStream() {
         check("void foo()\n"
               "{\n"