diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeVarargCheck.cpp @@ -10,6 +10,7 @@ #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/TargetInfo.h" using namespace clang::ast_matchers; @@ -60,8 +61,45 @@ AST_MATCHER(QualType, isVAList) { ASTContext &Context = Finder->getASTContext(); QualType Desugar = Node.getDesugaredType(Context); - return Context.getBuiltinVaListType().getDesugaredType(Context) == Desugar || - Context.getBuiltinMSVaListType().getDesugaredType(Context) == Desugar; + QualType NodeTy = Node.getUnqualifiedType(); + + auto CheckVaList = [](QualType NodeTy, QualType Expected, + const ASTContext &Context) { + if (NodeTy == Expected) + return true; + QualType Desugar = NodeTy; + QualType Ty; + do { + Ty = Desugar; + Desugar = Ty.getSingleStepDesugaredType(Context); + if (Desugar == Expected) + return true; + } while (Desugar != Ty); + return false; + }; + + // The internal implementation of __builtin_va_list depends on the target + // type. Some targets implements va_list as 'char *' or 'void *'. + // In these cases we need to remove all typedefs one by one to check this. + using BuiltinVaListKind = TargetInfo::BuiltinVaListKind; + BuiltinVaListKind VaListKind = Context.getTargetInfo().getBuiltinVaListKind(); + if (VaListKind == BuiltinVaListKind::CharPtrBuiltinVaList || + VaListKind == BuiltinVaListKind::VoidPtrBuiltinVaList) { + if (CheckVaList(NodeTy, Context.getBuiltinVaListType(), Context)) + return true; + } else if (Desugar == + Context.getBuiltinVaListType().getDesugaredType(Context)) { + return true; + } + + // We also need to check the implementation of __builtin_ms_va_list in the + // same way, because it may differ from the va_list implementation. + if (Desugar == Context.getBuiltinMSVaListType().getDesugaredType(Context) && + CheckVaList(NodeTy, Context.getBuiltinMSVaListType(), Context)) { + return true; + } + + return false; } AST_MATCHER_P(AdjustedType, hasOriginalType, diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg-ms.cpp @@ -0,0 +1,26 @@ +// Purpose: +// Ensure that the 'cppcoreguidelines-pro-type-vararg' check works with the +// built-in va_list on Windows systems. + +// REQUIRES: system-windows + +// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-vararg %t + +void test_ms_va_list(int a, ...) { + __builtin_ms_va_list ap; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare variables of type va_list; use variadic templates instead + __builtin_ms_va_start(ap, a); + int b = __builtin_va_arg(ap, int); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: do not use va_arg to define c-style vararg functions; use variadic templates instead + __builtin_ms_va_end(ap); +} + +void test_typedefs(int a, ...) { + typedef __builtin_ms_va_list my_va_list1; + my_va_list1 ap1; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare variables of type va_list; use variadic templates instead + + using my_va_list2 = __builtin_ms_va_list; + my_va_list2 ap2; + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not declare variables of type va_list; use variadic templates instead +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg.cpp --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-vararg.cpp @@ -58,3 +58,11 @@ (void)__builtin_isinf_sign(0.f); (void)__builtin_prefetch(nullptr); } + +// Some implementations of __builtin_va_list and __builtin_ms_va_list desugared +// as 'char *' or 'void *'. This test checks whether we are handling this case +// correctly and not generating false positives. +void no_false_positive_desugar_va_list(char *in) { + char *tmp1 = in; + void *tmp2 = in; +}