Author: aaronballman Date: Wed Sep 30 09:09:38 2015 New Revision: 248907 URL: http://llvm.org/viewvc/llvm-project?rev=248907&view=rev Log: Adding a checker (misc-non-copyable-objects) that detects situations where a non-copyable C type is being dereferenced, such as FILE or pthread_mutex_t. Corresponds to the CERT C++ secure coding rule: https://www.securecoding.cert.org/confluence/display/c/FIO38-C.+Do+not+copy+a+FILE+object
Added: clang-tools-extra/trunk/clang-tidy/misc/NonCopyableObjects.cpp clang-tools-extra/trunk/clang-tidy/misc/NonCopyableObjects.h clang-tools-extra/trunk/docs/clang-tidy/checks/misc-non-copyable-objects.rst clang-tools-extra/trunk/test/clang-tidy/misc-non-copyable-objects.c clang-tools-extra/trunk/test/clang-tidy/misc-non-copyable-objects.cpp Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=248907&r1=248906&r2=248907&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original) +++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Wed Sep 30 09:09:38 2015 @@ -13,6 +13,7 @@ add_clang_library(clangTidyMiscModule MoveConstructorInitCheck.cpp NewDeleteOverloadsCheck.cpp NoexceptMoveConstructorCheck.cpp + NonCopyableObjects.cpp SizeofContainerCheck.cpp StaticAssertCheck.cpp SwappedArgumentsCheck.cpp Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=248907&r1=248906&r2=248907&view=diff ============================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original) +++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Wed Sep 30 09:09:38 2015 @@ -21,6 +21,7 @@ #include "MoveConstructorInitCheck.h" #include "NewDeleteOverloadsCheck.h" #include "NoexceptMoveConstructorCheck.h" +#include "NonCopyableObjects.h" #include "SizeofContainerCheck.h" #include "StaticAssertCheck.h" #include "SwappedArgumentsCheck.h" @@ -58,8 +59,9 @@ public: "misc-new-delete-overloads"); CheckFactories.registerCheck<NoexceptMoveConstructorCheck>( "misc-noexcept-move-constructor"); - CheckFactories.registerCheck<SizeofContainerCheck>( - "misc-sizeof-container"); + CheckFactories.registerCheck<NonCopyableObjectsCheck>( + "misc-non-copyable-objects"); + CheckFactories.registerCheck<SizeofContainerCheck>("misc-sizeof-container"); CheckFactories.registerCheck<StaticAssertCheck>( "misc-static-assert"); CheckFactories.registerCheck<SwappedArgumentsCheck>( Added: clang-tools-extra/trunk/clang-tidy/misc/NonCopyableObjects.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/NonCopyableObjects.cpp?rev=248907&view=auto ============================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/NonCopyableObjects.cpp (added) +++ clang-tools-extra/trunk/clang-tidy/misc/NonCopyableObjects.cpp Wed Sep 30 09:09:38 2015 @@ -0,0 +1,96 @@ +//===--- NonCopyableObjects.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 "NonCopyableObjects.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include <algorithm> + +using namespace clang::ast_matchers; + +namespace clang { +namespace { +// FIXME: it would be good to make a list that is also user-configurable so that +// users can add their own elements to the list. However, it may require some +// extra thought since POSIX types and FILE types are usable in different ways. +bool isPOSIXTypeName(StringRef ClassName) { + static const char *TypeNames[] = { + "::pthread_cond_t", + "::pthread_mutex_t", + "pthread_cond_t", + "pthread_mutex_t" + }; + return std::binary_search(std::begin(TypeNames), std::end(TypeNames), + ClassName); +} + +bool isFILETypeName(StringRef ClassName) { + static const char *TypeNames[] = { + "::FILE", + "FILE", + "std::FILE" + }; + return std::binary_search(std::begin(TypeNames), std::end(TypeNames), + ClassName); +} + +AST_MATCHER(NamedDecl, isFILEType) { + return isFILETypeName(Node.getQualifiedNameAsString()); +} + +AST_MATCHER(NamedDecl, isPOSIXType) { + return isPOSIXTypeName(Node.getQualifiedNameAsString()); +} +} // namespace + +namespace tidy { +void NonCopyableObjectsCheck::registerMatchers(MatchFinder *Finder) { + // There are two ways to get into trouble with objects like FILE *: + // dereferencing the pointer type to be a non-pointer type, and declaring + // the type as a non-pointer type in the first place. While the declaration + // itself could technically be well-formed in the case where the type is not + // an opaque type, it's highly suspicious behavior. + // + // POSIX types are a bit different in that it's reasonable to declare a + // non-pointer variable or data member of the type, but it is not reasonable + // to dereference a pointer to the type, or declare a parameter of non-pointer + // type. + auto BadFILEType = hasType(namedDecl(isFILEType()).bind("type_decl")); + auto BadPOSIXType = hasType(namedDecl(isPOSIXType()).bind("type_decl")); + auto BadEitherType = anyOf(BadFILEType, BadPOSIXType); + + Finder->addMatcher( + namedDecl(anyOf(varDecl(BadFILEType), fieldDecl(BadFILEType))) + .bind("decl"), + this); + Finder->addMatcher(parmVarDecl(BadPOSIXType).bind("decl"), this); + Finder->addMatcher( + expr(unaryOperator(hasOperatorName("*"), BadEitherType)).bind("expr"), + this); +} + +void NonCopyableObjectsCheck::check(const MatchFinder::MatchResult &Result) { + const auto *D = Result.Nodes.getNodeAs<NamedDecl>("decl"); + const auto *BD = Result.Nodes.getNodeAs<NamedDecl>("type_decl"); + const auto *E = Result.Nodes.getNodeAs<Expr>("expr"); + + if (D && BD) + diag(D->getLocation(), "'%0' declared as type '%1', which is unsafe to copy" + "; did you mean '%1 *'?") + << D->getName() << BD->getName(); + else if (E) + diag(E->getExprLoc(), + "expression has opaque data structure type '%0'; type should only be " + "used as a pointer and not dereferenced") + << BD->getName(); +} + +} // namespace tidy +} // namespace clang + Added: clang-tools-extra/trunk/clang-tidy/misc/NonCopyableObjects.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/NonCopyableObjects.h?rev=248907&view=auto ============================================================================== --- clang-tools-extra/trunk/clang-tidy/misc/NonCopyableObjects.h (added) +++ clang-tools-extra/trunk/clang-tidy/misc/NonCopyableObjects.h Wed Sep 30 09:09:38 2015 @@ -0,0 +1,31 @@ +//===--- NonCopyableObjects.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_MISC_NONCOPYABLEOBJECTS_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NONCOPYABLEOBJECTS_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { + +/// The check flags dereferences and non-pointer declarations of objects that +/// are not meant to be passed by value, such as C FILE objects. +class NonCopyableObjectsCheck : public ClangTidyCheck { +public: + NonCopyableObjectsCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_NONCOPYABLEOBJECTS_H Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=248907&r1=248906&r2=248907&view=diff ============================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Wed Sep 30 09:09:38 2015 @@ -32,6 +32,7 @@ List of clang-tidy Checks misc-move-constructor-init misc-new-delete-overloads misc-noexcept-move-constructor + misc-non-copyable-objects misc-sizeof-container misc-static-assert misc-swapped-arguments Added: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-non-copyable-objects.rst URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/misc-non-copyable-objects.rst?rev=248907&view=auto ============================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-non-copyable-objects.rst (added) +++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-non-copyable-objects.rst Wed Sep 30 09:09:38 2015 @@ -0,0 +1,9 @@ +misc-non-copyable-objects +========================= + +The check flags dereferences and non-pointer declarations of objects that are +not meant to be passed by value, such as C FILE objects or POSIX +pthread_mutex_t objects. + +This check corresponds to CERT C++ Coding Standard rule `FIO38-C. Do not copy a FILE object +<https://www.securecoding.cert.org/confluence/display/c/FIO38-C.+Do+not+copy+a+FILE+object>`_. Added: clang-tools-extra/trunk/test/clang-tidy/misc-non-copyable-objects.c URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-non-copyable-objects.c?rev=248907&view=auto ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/misc-non-copyable-objects.c (added) +++ clang-tools-extra/trunk/test/clang-tidy/misc-non-copyable-objects.c Wed Sep 30 09:09:38 2015 @@ -0,0 +1,43 @@ +// RUN: %python %S/check_clang_tidy.py %s misc-non-copyable-objects %t + +typedef struct FILE {} FILE; +typedef struct pthread_cond_t {} pthread_cond_t; +typedef int pthread_mutex_t; + +// CHECK-MESSAGES: :[[@LINE+1]]:13: warning: 'f' declared as type 'FILE', which is unsafe to copy; did you mean 'FILE *'? [misc-non-copyable-objects] +void g(FILE f); +// CHECK-MESSAGES: :[[@LINE+1]]:24: warning: 'm' declared as type 'pthread_mutex_t', which is unsafe to copy; did you mean 'pthread_mutex_t *'? +void h(pthread_mutex_t m); +// CHECK-MESSAGES: :[[@LINE+1]]:23: warning: 'c' declared as type 'pthread_cond_t', which is unsafe to copy; did you mean 'pthread_cond_t *'? +void i(pthread_cond_t c); + +struct S { + pthread_cond_t c; // ok + // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: 'f' declared as type 'FILE', which is unsafe to copy; did you mean 'FILE *'? + FILE f; +}; + +void func(FILE *f) { + // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: 'f1' declared as type 'FILE', which is unsafe to copy; did you mean 'FILE *'? + FILE f1; // match + // CHECK-MESSAGES: :[[@LINE+2]]:8: warning: 'f2' declared as type 'FILE', which is unsafe to copy; did you mean 'FILE *'? + // CHECK-MESSAGES: :[[@LINE+1]]:13: warning: expression has opaque data structure type 'FILE'; type should only be used as a pointer and not dereferenced + FILE f2 = *f; + // CHECK-MESSAGES: :[[@LINE+1]]:15: warning: 'f3' declared as type 'FILE', which is unsafe to copy; did you mean 'FILE *'? + struct FILE f3; + // CHECK-MESSAGES: :[[@LINE+1]]:16: warning: expression has opaque data structure type 'FILE'; type should only be used as a pointer and not dereferenced + (void)sizeof(*f); + (void)sizeof(FILE); + // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: expression has opaque data structure type 'FILE'; type should only be used as a pointer and not dereferenced + g(*f); + + pthread_mutex_t m; // ok + h(m); // ok + + pthread_cond_t c; // ok + i(c); // ok + + pthread_mutex_t *m1 = &m; // ok + // CHECK-MESSAGES: :[[@LINE+1]]:5: warning: expression has opaque data structure type 'pthread_mutex_t'; type should only be used as a pointer and not dereferenced + h(*m1); +} \ No newline at end of file Added: clang-tools-extra/trunk/test/clang-tidy/misc-non-copyable-objects.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-non-copyable-objects.cpp?rev=248907&view=auto ============================================================================== --- clang-tools-extra/trunk/test/clang-tidy/misc-non-copyable-objects.cpp (added) +++ clang-tools-extra/trunk/test/clang-tidy/misc-non-copyable-objects.cpp Wed Sep 30 09:09:38 2015 @@ -0,0 +1,26 @@ +// RUN: %python %S/check_clang_tidy.py %s misc-non-copyable-objects %t + +namespace std { +typedef struct FILE {} FILE; +} +using namespace std; + +// CHECK-MESSAGES: :[[@LINE+1]]:18: warning: 'f' declared as type 'FILE', which is unsafe to copy; did you mean 'FILE *'? [misc-non-copyable-objects] +void g(std::FILE f); + +struct S { + // CHECK-MESSAGES: :[[@LINE+1]]:10: warning: 'f' declared as type 'FILE', which is unsafe to copy; did you mean 'FILE *'? + ::FILE f; +}; + +void func(FILE *f) { + // CHECK-MESSAGES: :[[@LINE+1]]:13: warning: 'f1' declared as type 'FILE', which is unsafe to copy; did you mean 'FILE *'? + std::FILE f1; // match + // CHECK-MESSAGES: :[[@LINE+2]]:10: warning: 'f2' declared as type 'FILE', which is unsafe to copy; did you mean 'FILE *'? + // CHECK-MESSAGES: :[[@LINE+1]]:15: warning: expression has opaque data structure type 'FILE'; type should only be used as a pointer and not dereferenced + ::FILE f2 = *f; // match, match + // CHECK-MESSAGES: :[[@LINE+1]]:15: warning: 'f3' declared as type 'FILE', which is unsafe to copy; did you mean 'FILE *'? + struct FILE f3; // match + // CHECK-MESSAGES: :[[@LINE+1]]:16: warning: expression has opaque data structure type 'FILE'; type should only be used as a pointer and not dereferenced + (void)sizeof(*f); // match +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits