Fixed in r250986. On Thu, Oct 22, 2015 at 1:01 PM Manman Ren via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> It seems that this commit breaks the following bot > http://lab.llvm.org:8080/green/job/clang-stage1-configure-RA_check/12461/ > > Manman > > On Oct 21, 2015, at 1:09 PM, Matthias Gehre via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > > Author: mgehre > Date: Wed Oct 21 15:09:02 2015 > New Revision: 250939 > > URL: http://llvm.org/viewvc/llvm-project?rev=250939&view=rev > Log: > [clang-tidy] add check cppcoreguidelines-pro-type-vararg > > Summary: > This check flags all calls to c-style vararg functions and all use > of va_list, va_start and va_arg. > > Passing to varargs assumes the correct type will be read. This is > fragile because it cannot generally be enforced to be safe in the > language and so relies on programmer discipline to get it right. > > This rule is part of the "Type safety" profile of the C++ Core > Guidelines, see > > https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-type8-avoid-reading-from-varargs-or-passing-vararg-arguments-prefer-variadic-template-parameters-instead > > This commits also reverts > "[clang-tidy] add cert's VariadicFunctionDefCheck as > cppcoreguidelines-pro-type-vararg-def" > because that check makes the SFINAE use of vararg functions impossible. > > Reviewers: alexfh, sbenza, bkramer, aaron.ballman > > Subscribers: cfe-commits > > Differential Revision: http://reviews.llvm.org/D13787 > > Added: > > clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp > > clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.h > > > clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-vararg.rst > > > clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-vararg.cpp > Modified: > clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt > > > clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp > clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst > > Modified: > clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt?rev=250939&r1=250938&r2=250939&view=diff > > ============================================================================== > --- clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt > (original) > +++ clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CMakeLists.txt > Wed Oct 21 15:09:02 2015 > @@ -7,6 +7,7 @@ add_clang_library(clangTidyCppCoreGuidel > ProTypeReinterpretCastCheck.cpp > ProTypeStaticCastDowncastCheck.cpp > ProTypeUnionAccessCheck.cpp > + ProTypeVarargCheck.cpp > > LINK_LIBS > clangAST > @@ -14,7 +15,6 @@ add_clang_library(clangTidyCppCoreGuidel > clangBasic > clangLex > clangTidy > - clangTidyCERTModule > clangTidyMiscModule > clangTidyUtils > clangTooling > > Modified: > clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp?rev=250939&r1=250938&r2=250939&view=diff > > ============================================================================== > --- > clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp > (original) > +++ > clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp > Wed Oct 21 15:09:02 2015 > @@ -10,13 +10,13 @@ > #include "../ClangTidy.h" > #include "../ClangTidyModule.h" > #include "../ClangTidyModuleRegistry.h" > -#include "../cert/VariadicFunctionDefCheck.h" > #include "../misc/AssignOperatorSignatureCheck.h" > #include "ProBoundsPointerArithmeticCheck.h" > #include "ProTypeConstCastCheck.h" > #include "ProTypeReinterpretCastCheck.h" > #include "ProTypeStaticCastDowncastCheck.h" > #include "ProTypeUnionAccessCheck.h" > +#include "ProTypeVarargCheck.h" > > namespace clang { > namespace tidy { > @@ -30,14 +30,14 @@ public: > "cppcoreguidelines-pro-bounds-pointer-arithmetic"); > CheckFactories.registerCheck<ProTypeConstCastCheck>( > "cppcoreguidelines-pro-type-const-cast"); > - CheckFactories.registerCheck<VariadicFunctionDefCheck>( > - "cppcoreguidelines-pro-type-vararg-def"); > CheckFactories.registerCheck<ProTypeReinterpretCastCheck>( > "cppcoreguidelines-pro-type-reinterpret-cast"); > CheckFactories.registerCheck<ProTypeStaticCastDowncastCheck>( > "cppcoreguidelines-pro-type-static-cast-downcast"); > CheckFactories.registerCheck<ProTypeUnionAccessCheck>( > "cppcoreguidelines-pro-type-union-access"); > + CheckFactories.registerCheck<ProTypeVarargCheck>( > + "cppcoreguidelines-pro-type-vararg"); > CheckFactories.registerCheck<misc::AssignOperatorSignatureCheck>( > "cppcoreguidelines-c-copy-assignment-signature"); > } > > Added: > clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp?rev=250939&view=auto > > ============================================================================== > --- > clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp > (added) > +++ > clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp > Wed Oct 21 15:09:02 2015 > @@ -0,0 +1,76 @@ > +//===--- ProTypeVarargCheck.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 "ProTypeVarargCheck.h" > +#include "clang/AST/ASTContext.h" > +#include "clang/ASTMatchers/ASTMatchFinder.h" > + > +using namespace clang::ast_matchers; > + > +namespace clang { > +namespace tidy { > + > +const internal::VariadicDynCastAllOfMatcher<Stmt, VAArgExpr> vAArgExpr; > + > +void ProTypeVarargCheck::registerMatchers(MatchFinder *Finder) { > + if (!getLangOpts().CPlusPlus) > + return; > + > + Finder->addMatcher(vAArgExpr().bind("va_use"), this); > + > + Finder->addMatcher( > + callExpr(callee(functionDecl(isVariadic()))) > + .bind("callvararg"), > + this); > +} > + > +static bool hasSingleVariadicArgumentWithValue(const CallExpr *C, > uint64_t I) { > + const auto *FDecl = dyn_cast<FunctionDecl>(C->getCalleeDecl()); > + if (!FDecl) > + return false; > + > + auto N = FDecl->getNumParams(); // Number of parameters without '...' > + if (C->getNumArgs() != N + 1) > + return false; // more/less than one argument passed to '...' > + > + const auto *IntLit = > + dyn_cast<IntegerLiteral>(C->getArg(N)->IgnoreParenImpCasts()); > + if (!IntLit) > + return false; > + > + if (IntLit->getValue() != I) > + return false; > + > + return true; > +} > + > +void ProTypeVarargCheck::check(const MatchFinder::MatchResult &Result) { > + if (const auto *Matched = > Result.Nodes.getNodeAs<CallExpr>("callvararg")) { > + if (hasSingleVariadicArgumentWithValue(Matched, 0)) > + return; > + diag(Matched->getExprLoc(), "do not call c-style vararg functions"); > + } > + > + if (const auto *Matched = Result.Nodes.getNodeAs<Expr>("va_use")) { > + diag(Matched->getExprLoc(), > + "do not use va_start/va_arg to define c-style vararg functions; " > + "use variadic templates instead"); > + } > + > + if (const auto *Matched = Result.Nodes.getNodeAs<VarDecl>("va_list")) { > + auto SR = Matched->getSourceRange(); > + if (SR.isInvalid()) > + return; // some implicitly generated builtins take va_list > + diag(SR.getBegin(), "do not declare variables of type va_list; " > + "use variadic templates instead"); > + } > +} > + > +} // namespace tidy > +} // namespace clang > > Added: > clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.h > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.h?rev=250939&view=auto > > ============================================================================== > --- > clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.h > (added) > +++ > clang-tools-extra/trunk/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.h > Wed Oct 21 15:09:02 2015 > @@ -0,0 +1,34 @@ > +//===--- ProTypeVarargCheck.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_CPPCOREGUIDELINES_PRO_TYPE_VARARG_H > +#define > LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_PRO_TYPE_VARARG_H > + > +#include "../ClangTidy.h" > + > +namespace clang { > +namespace tidy { > + > +/// This check flags all calls to c-style variadic functions and all use > +/// of va_arg. > +/// > +/// For the user-facing documentation see: > +/// > http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-pro-type-vararg.html > +class ProTypeVarargCheck : public ClangTidyCheck { > +public: > + ProTypeVarargCheck(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_CPPCOREGUIDELINES_PRO_TYPE_VARARG_H > > Added: > clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-vararg.rst > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-vararg.rst?rev=250939&view=auto > > ============================================================================== > --- > clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-vararg.rst > (added) > +++ > clang-tools-extra/trunk/docs/clang-tidy/checks/cppcoreguidelines-pro-type-vararg.rst > Wed Oct 21 15:09:02 2015 > @@ -0,0 +1,12 @@ > +cppcoreguidelines-pro-type-vararg > +===================================== > + > +This check flags all calls to c-style vararg functions and all use > +of va_arg. > +To allow for SFINAE use of vararg functions, a call is not flagged if > +a literal 0 is passed as the only vararg argument. > + > +Passing to varargs assumes the correct type will be read. This is fragile > because it cannot generally be enforced to be safe in the language and so > relies on programmer discipline to get it right. > + > +This rule is part of the "Type safety" profile of the C++ Core > Guidelines, see > + > https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#-type8-avoid-reading-from-varargs-or-passing-vararg-arguments-prefer-variadic-template-parameters-instead > > 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=250939&r1=250938&r2=250939&view=diff > > ============================================================================== > --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original) > +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Wed Oct 21 > 15:09:02 2015 > @@ -9,6 +9,7 @@ List of clang-tidy Checks > cppcoreguidelines-pro-type-reinterpret-cast > cppcoreguidelines-pro-type-static-cast-downcast > cppcoreguidelines-pro-type-union-access > + cppcoreguidelines-pro-type-vararg > google-build-explicit-make-pair > google-build-namespaces > google-build-using-namespace > > Added: > clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-vararg.cpp > URL: > http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-vararg.cpp?rev=250939&view=auto > > ============================================================================== > --- > clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-vararg.cpp > (added) > +++ > clang-tools-extra/trunk/test/clang-tidy/cppcoreguidelines-pro-type-vararg.cpp > Wed Oct 21 15:09:02 2015 > @@ -0,0 +1,51 @@ > +// RUN: %python %S/check_clang_tidy.py %s > cppcoreguidelines-pro-type-vararg %t > + > +void f(int i); > +void f_vararg(int i, ...); > + > +struct C { > + void g_vararg(...); > + void g(const char*); > +} c; > + > +template<typename... P> > +void cpp_vararg(P... p); > + > +void check() { > + f_vararg(1, 7, 9); > + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not call c-style vararg > functions [cppcoreguidelines-pro-type-vararg] > + c.g_vararg("foo"); > + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not call c-style vararg > functions > + > + f(3); // OK > + c.g("foo"); // OK > + cpp_vararg(1, 7, 9); // OK > +} > + > +// ... as a parameter is allowed (e.g. for SFINAE) > +template <typename T> > +void CallFooIfAvailableImpl(T& t, ...) { > + // nothing > +} > +template <typename T> > +void CallFooIfAvailableImpl(T& t, decltype(t.foo())*) { > + t.foo(); > +} > +template <typename T> > +void CallFooIfAvailable(T& t) { > + CallFooIfAvailableImpl(t, 0); // OK to call variadic function when the > argument is a literal 0 > +} > + > +#include <cstdarg> > +void my_printf(const char* format, ...) { > + va_list ap; > + va_start(ap, format); > + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not call c-style vararg > functions > + va_list n; > + va_copy(n, ap); // Don't warn, va_copy is anyway useless without > va_start > + int i = va_arg(ap, int); > + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use va_start/va_arg > to define c-style vararg functions; use variadic templates instead > + va_end(ap); // Don't warn, va_end is anyway useless without va_start > +} > + > +int my_vprintf(const char* format, va_list arg ); // OK to declare > function taking va_list > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits