Index: clang-tidy/bugprone/BugproneTidyModule.cpp =================================================================== --- clang-tidy/bugprone/BugproneTidyModule.cpp +++ clang-tidy/bugprone/BugproneTidyModule.cpp @@ -23,6 +23,7 @@ #include "InaccurateEraseCheck.h" #include "IncorrectRoundingsCheck.h" #include "IntegerDivisionCheck.h" +#include "IoFunctionsCheck.h" #include "LambdaFunctionNameCheck.h" #include "MacroParenthesesCheck.h" #include "MacroRepeatedSideEffectsCheck.h" @@ -82,6 +83,8 @@ "bugprone-incorrect-roundings"); CheckFactories.registerCheck( "bugprone-integer-division"); + CheckFactories.registerCheck( + "bugprone-io-functions"); CheckFactories.registerCheck( "bugprone-lambda-function-name"); CheckFactories.registerCheck( Index: clang-tidy/bugprone/CMakeLists.txt =================================================================== --- clang-tidy/bugprone/CMakeLists.txt +++ clang-tidy/bugprone/CMakeLists.txt @@ -14,6 +14,7 @@ InaccurateEraseCheck.cpp IncorrectRoundingsCheck.cpp IntegerDivisionCheck.cpp + IoFunctionsCheck.cpp LambdaFunctionNameCheck.cpp MacroParenthesesCheck.cpp MacroRepeatedSideEffectsCheck.cpp Index: clang-tidy/bugprone/IoFunctionsCheck.h =================================================================== --- /dev/null +++ clang-tidy/bugprone/IoFunctionsCheck.h @@ -0,0 +1,38 @@ +//===--- IoFunctionsCheck.h - clang-tidy-------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_IO_FUNCTIONS_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_IO_FUNCTIONS_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace bugprone { + +/// This check warns for cases when the return value of certain standard +/// iostream C functions (fgetc(), getc(), getchar(), fgetwc(), getwc(), +/// getwchar(), istream::get()) return int value which is then incorrectly +/// stored in char typed values which can cause overflow at type conversion. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-io-functions.html +class IoFunctionsCheck : public ClangTidyCheck { +public: + IoFunctionsCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace bugprone +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_IO_FUNCTIONS_H Index: clang-tidy/bugprone/IoFunctionsCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/bugprone/IoFunctionsCheck.cpp @@ -0,0 +1,59 @@ +//===--- IoFunctionsCheck.cpp - clang-tidy---------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "IoFunctionsCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace bugprone { + +void IoFunctionsCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + implicitCastExpr( + hasImplicitDestinationType(isAnyCharacter()), + anyOf(has(callExpr(callee( + functionDecl(hasAnyName("::getc", "::getchar", "::fgetc", + "::getwchar", "::getwc", "::fgetwc", + "::std::getc", "::std::getchar", + "::std::fgetc", "::std::getwchar", + "::std::getwc", "::std::fgetwc")) + .bind("FuncDecl")))), + has(cxxMemberCallExpr( + on(hasType(namedDecl(hasAnyName("std::istream")))), + callee(cxxMethodDecl(hasName("get")).bind("DeclOfGet"))))), + hasCastKind(CK_IntegralCast)) + .bind("ImplicitCast"), + this); +} + +void IoFunctionsCheck::check(const MatchFinder::MatchResult &Result) { + const auto *MatchedImplicitCastToChar = + Result.Nodes.getNodeAs("ImplicitCast"); + const auto *FuncDecl = Result.Nodes.getNodeAs("FuncDecl"); + const auto *DeclGet = Result.Nodes.getNodeAs("DeclOfGet"); + + auto Diag = diag( + MatchedImplicitCastToChar->getLocStart(), + "consider to cast the return value of %0 from type integer to type char, " + "possible loss of precision if an error has occurred or the end of file " + "has been reached, EOF and WEOF can be lost"); + + if (FuncDecl) + Diag << FuncDecl; + else + Diag << DeclGet; +} + +} // namespace bugprone +} // namespace tidy +} // namespace clang Index: clang-tidy/cert/CERTTidyModule.cpp =================================================================== --- clang-tidy/cert/CERTTidyModule.cpp +++ clang-tidy/cert/CERTTidyModule.cpp @@ -10,6 +10,7 @@ #include "../ClangTidy.h" #include "../ClangTidyModule.h" #include "../ClangTidyModuleRegistry.h" +#include "../bugprone/IoFunctionsCheck.h" #include "../google/UnnamedNamespaceInHeaderCheck.h" #include "../misc/NewDeleteOverloadsCheck.h" #include "../misc/NonCopyableObjects.h" @@ -70,6 +71,7 @@ // FLP CheckFactories.registerCheck("cert-flp30-c"); // FIO + CheckFactories.registerCheck("cert-fio34-c"); CheckFactories.registerCheck("cert-fio38-c"); // ERR CheckFactories.registerCheck("cert-err34-c"); Index: clang-tidy/cert/CMakeLists.txt =================================================================== --- clang-tidy/cert/CMakeLists.txt +++ clang-tidy/cert/CMakeLists.txt @@ -21,6 +21,7 @@ clangBasic clangLex clangTidy + clangTidyBugproneModule clangTidyGoogleModule clangTidyMiscModule clangTidyPerformanceModule Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -60,6 +60,13 @@ Finds functions which may throw an exception directly or indirectly, but they should not. +- New `bugprone-io-functions + `_ check + + Check if the ``fgetc()``, ``getc()``, ``getchat()``, ``fgetwc()``, + ``getwc()``, ``getwchar()``, ``istream::get()`` standard iostream C functions + return values incorrectly stored in char type value. + - New :doc:`bugprone-parent-virtual-call ` check. Index: docs/clang-tidy/checks/bugprone-io-functions.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/bugprone-io-functions.rst @@ -0,0 +1,42 @@ +.. title:: clang-tidy - bugprone-io-functions + +bugprone-io-functions +===================== + +``cert-fio34-c`` redirects here as an alias for this check. + +This check warns for cases when the return value of certain standard iostream C +functions (``fgetc()``, ``getc()``, ``getchar()``, ``fgetwc()``, ``getwc()``, +``getwchar()``, ``istream::get()``) return int value which is then incorrectly +stored in char typed values which can cause overflow at type conversion. +This check is valid in C and C++. + +Checking for implicit cast from integer to char. + +.. code-block:: c++ + + char c; + c = std::getchar(); // Implicit cast from integer to char + +Check will warns for ``fgetc()``, ``getc()``, ``getchar()``, ``fgetwc()``, +``getwc()``, ``getwchar()`` functions from global or 'std' namespace and will +warn for ``get()`` function from 'istream' namespace. + +.. code-block:: c++ + + namespace ownnamespace { ... } + std::istream is = ... + char c; + c = is.get(); // Warning about incorrectly stored int value + c = ownnamespace::get(); // Won't warn + +Check will also warns for incorrectly used functions in parameter passing. + +.. code-block:: c++ + + void f(char c){ ... } + f(std::fgetwc( ... )); // Warning about passed int value instead of char + +This check corresponds to CERT C Coding Standard rule `FIO34-C. Distinguish +between characters read from a file and EOF or WEOF +`_. Index: docs/clang-tidy/checks/cert-fio34-c.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/cert-fio34-c.rst @@ -0,0 +1,10 @@ +.. title:: clang-tidy - cert-fio34-c +.. meta:: + :http-equiv=refresh: 5;URL=bugprone-io-functions.html + +cert-fio34-c +============ + +The cert-fio34-c check is an alias, please see +`bugprone-io-functions `_ for more +information. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -31,6 +31,7 @@ bugprone-inaccurate-erase bugprone-incorrect-roundings bugprone-integer-division + bugprone-io-functions bugprone-lambda-function-name bugprone-macro-parentheses bugprone-macro-repeated-side-effects @@ -71,6 +72,7 @@ cert-err58-cpp cert-err60-cpp cert-err61-cpp (redirects to misc-throw-by-value-catch-by-reference) + cert-fio34-c (redirects to bugprone-io-functions) cert-fio38-c (redirects to misc-non-copyable-objects) cert-flp30-c cert-msc30-c (redirects to cert-msc50-cpp) Index: test/clang-tidy/bugprone-io-functions.cpp =================================================================== --- /dev/null +++ test/clang-tidy/bugprone-io-functions.cpp @@ -0,0 +1,121 @@ +// RUN: %check_clang_tidy %s bugprone-io-functions %t + +typedef int wint_t; +typedef void *FILE; + +namespace std { +int getchar(); +int getc(FILE); +int fgetc(FILE); +wint_t getwchar(); +wint_t getwc(FILE); +wint_t fgetwc(FILE); +class istream { +public: + wint_t get(); +}; +} // namespace std + +int char_io_getchar() { + char c; + c = std::getchar(); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'getchar' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached, EOF and WEOF can be lost [bugprone-io-functions] + return c; +} + +int char_io_getc(FILE *fp) { + char c; + c = std::getc(fp); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'getc' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached, EOF and WEOF can be lost [bugprone-io-functions] + return c; +} + +int char_io_fgetc(FILE *fp) { + char c; + c = std::fgetc(fp); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'fgetc' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached, EOF and WEOF can be lost [bugprone-io-functions] + return c; +} + +int char_io_getwchar() { + char c; + c = std::getwchar(); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'getwchar' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached, EOF and WEOF can be lost [bugprone-io-functions] + return c; +} + +int char_io_getwc(FILE *fp) { + char c; + c = std::getwc(fp); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'getwc' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached, EOF and WEOF can be lost [bugprone-io-functions] + return c; +} + +int char_io_fgetwc(FILE *fp) { + char c; + c = std::fgetwc(fp); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'fgetwc' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached, EOF and WEOF can be lost [bugprone-io-functions] + return c; +} + +int char_io_get(std::istream &is) { + char c; + c = is.get(); + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: consider to cast the return value of 'get' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached, EOF and WEOF can be lost [bugprone-io-functions] + return c; +} + +void char_type_in_argument(char c) {} + +void call_char_type_in_argument_with_getchar(FILE *fp) { + char_type_in_argument(std::fgetc(fp)); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: consider to cast the return value of 'fgetc' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached, EOF and WEOF can be lost [bugprone-io-functions] +} + +void call_char_type_in_argument_with_getwchar() { + char_type_in_argument(std::getwchar()); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: consider to cast the return value of 'getwchar' from type integer to type char, possible loss of precision if an error has occurred or the end of file has been reached, EOF and WEOF can be lost [bugprone-io-functions] +} + +// Negatives +int int_io_getchar() { + int x; + x = std::getchar(); + return x; +} + +int int_io_getc(FILE *fp) { + int x; + x = std::getc(fp); + return x; +} + +int int_io_fgetc(FILE *fp) { + int x; + x = std::fgetc(fp); + return x; +} + +int int_io_getwchar() { + int x; + x = std::getwchar(); + return x; +} + +int int_io_getwc(FILE *fp) { + int x; + x = std::getwc(fp); + return x; +} + +int int_io_fgetwc(FILE *fp) { + int x; + x = std::fgetwc(fp); + return x; +} + +int int_io_get(std::istream &is) { + int x; + x = is.get(); + return x; +}