diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index ae5537ab7c51..55109d089bef 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -919,6 +919,8 @@ def err_attribute_requires_objc_interface : Error< "attribute may only be applied to an Objective-C interface">; def warn_nonnull_pointers_only : Warning< "nonnull attribute only applies to pointer arguments">; +def err_attribute_invalid_implicit_this_argument : Error< + "'%0' attribute is invalid for the implicit this argument">; def err_ownership_type : Error< "%0 attribute only applies to %1 arguments">; def err_format_strftime_third_parameter : Error< @@ -927,6 +929,9 @@ def err_format_attribute_requires_variadic : Error< "format attribute requires variadic function">; def err_format_attribute_not : Error<"format argument not %0">; def err_format_attribute_result_not : Error<"function does not return %0">; +def err_format_attribute_implicit_this_format_string : Error< + "format attribute cannot specify the implicit this argument as the format " + "string">; def err_attribute_invalid_size : Error< "vector size not an integral multiple of component size">; def err_attribute_zero_size : Error<"zero vector size">; diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 3c3e3ae37726..439ce531e64a 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -127,6 +127,12 @@ static bool isFunctionOrMethodVariadic(const Decl *d) { } } +static bool isInstanceMethod(const Decl *d) { + if (const CXXMethodDecl *MethodDecl = dyn_cast(d)) + return MethodDecl->isInstance(); + return false; +} + static inline bool isNSStringType(QualType T, ASTContext &Ctx) { const ObjCObjectPointerType *PT = T->getAs(); if (!PT) @@ -323,7 +329,10 @@ static void HandleNonNullAttr(Decl *d, const AttributeList &Attr, Sema &S) { return; } - unsigned NumArgs = getFunctionOrMethodNumArgs(d); + // In C++ the implicit 'this' function parameter also counts, and they are + // counted from one. + bool HasImplicitThisParam = isInstanceMethod(d); + unsigned NumArgs = getFunctionOrMethodNumArgs(d) + HasImplicitThisParam; // The nonnull attribute only applies to pointers. llvm::SmallVector NonNullArgs; @@ -351,6 +360,15 @@ static void HandleNonNullAttr(Decl *d, const AttributeList &Attr, Sema &S) { } --x; + if (HasImplicitThisParam) { + if (x == 0) { + S.Diag(Attr.getLoc(), + diag::err_attribute_invalid_implicit_this_argument) + << "nonnull" << Ex->getSourceRange(); + return; + } + --x; + } // Is the function argument a pointer type? QualType T = getFunctionOrMethodArgType(d, x); @@ -454,7 +472,10 @@ static void HandleOwnershipAttr(Decl *d, const AttributeList &AL, Sema &S) { return; } - unsigned NumArgs = getFunctionOrMethodNumArgs(d); + // In C++ the implicit 'this' function parameter also counts, and they are + // counted from one. + bool HasImplicitThisParam = isInstanceMethod(d); + unsigned NumArgs = getFunctionOrMethodNumArgs(d) + HasImplicitThisParam; llvm::StringRef Module = AL.getParameterName()->getName(); @@ -484,6 +505,15 @@ static void HandleOwnershipAttr(Decl *d, const AttributeList &AL, Sema &S) { continue; } --x; + if (HasImplicitThisParam) { + if (x == 0) { + S.Diag(AL.getLoc(), diag::err_attribute_invalid_implicit_this_argument) + << "ownership" << IdxExpr->getSourceRange(); + return; + } + --x; + } + switch (K) { case OwnershipAttr::Takes: case OwnershipAttr::Holds: { @@ -1397,10 +1427,13 @@ static void HandleFormatArgAttr(Decl *d, const AttributeList &Attr, Sema &S) { << Attr.getName() << 0 /*function*/; return; } - // FIXME: in C++ the implicit 'this' function parameter also counts. this is - // needed in order to be compatible with GCC the index must start with 1. - unsigned NumArgs = getFunctionOrMethodNumArgs(d); + + // In C++ the implicit 'this' function parameter also counts, and they are + // counted from one. + bool HasImplicitThisParam = isInstanceMethod(d); + unsigned NumArgs = getFunctionOrMethodNumArgs(d) + HasImplicitThisParam; unsigned FirstIdx = 1; + // checks for the 2nd argument Expr *IdxExpr = static_cast(Attr.getArg(0)); llvm::APSInt Idx(32); @@ -1419,6 +1452,15 @@ static void HandleFormatArgAttr(Decl *d, const AttributeList &Attr, Sema &S) { unsigned ArgIdx = Idx.getZExtValue() - 1; + if (HasImplicitThisParam) { + if (ArgIdx == 0) { + S.Diag(Attr.getLoc(), diag::err_attribute_invalid_implicit_this_argument) + << "format_arg" << IdxExpr->getSourceRange(); + return; + } + ArgIdx--; + } + // make sure the format string is really a string QualType Ty = getFunctionOrMethodArgType(d, ArgIdx); @@ -1445,7 +1487,8 @@ static void HandleFormatArgAttr(Decl *d, const AttributeList &Attr, Sema &S) { return; } - d->addAttr(::new (S.Context) FormatArgAttr(Attr.getLoc(), S.Context, Idx.getZExtValue())); + d->addAttr(::new (S.Context) FormatArgAttr(Attr.getLoc(), S.Context, + Idx.getZExtValue())); } enum FormatAttrKind { @@ -1551,7 +1594,10 @@ static void HandleFormatAttr(Decl *d, const AttributeList &Attr, Sema &S) { return; } - unsigned NumArgs = getFunctionOrMethodNumArgs(d); + // In C++ the implicit 'this' function parameter also counts, and they are + // counted from one. + bool HasImplicitThisParam = isInstanceMethod(d); + unsigned NumArgs = getFunctionOrMethodNumArgs(d) + HasImplicitThisParam; unsigned FirstIdx = 1; llvm::StringRef Format = Attr.getParameterName()->getName(); @@ -1582,16 +1628,6 @@ static void HandleFormatAttr(Decl *d, const AttributeList &Attr, Sema &S) { return; } - // FIXME: We should handle the implicit 'this' parameter in a more generic - // way that can be used for other arguments. - bool HasImplicitThisParam = false; - if (CXXMethodDecl *MD = dyn_cast(d)) { - if (MD->isInstance()) { - HasImplicitThisParam = true; - NumArgs++; - } - } - if (Idx.getZExtValue() < FirstIdx || Idx.getZExtValue() > NumArgs) { S.Diag(Attr.getLoc(), diag::err_attribute_argument_out_of_bounds) << "format" << 2 << IdxExpr->getSourceRange(); @@ -1603,8 +1639,9 @@ static void HandleFormatAttr(Decl *d, const AttributeList &Attr, Sema &S) { if (HasImplicitThisParam) { if (ArgIdx == 0) { - S.Diag(Attr.getLoc(), diag::err_format_attribute_not) - << "a string type" << IdxExpr->getSourceRange(); + S.Diag(Attr.getLoc(), + diag::err_format_attribute_implicit_this_format_string) + << IdxExpr->getSourceRange(); return; } ArgIdx--; diff --git a/clang/test/SemaCXX/attr-format.cpp b/clang/test/SemaCXX/attr-format.cpp index 0c1eb53ea07f..bf131496007b 100644 --- a/clang/test/SemaCXX/attr-format.cpp +++ b/clang/test/SemaCXX/attr-format.cpp @@ -1,8 +1,23 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s struct S { static void f(const char*, ...) __attribute__((format(printf, 1, 2))); + static const char* f2(const char*) __attribute__((format_arg(1))); // GCC has a hidden 'this' argument in member functions which is why // the format argument is argument 2 here. void g(const char*, ...) __attribute__((format(printf, 2, 3))); + const char* g2(const char*) __attribute__((format_arg(2))); + + void h(const char*, ...) __attribute__((format(printf, 1, 4))); // \ + expected-error{{implicit this argument as the format string}} + void h2(const char*, ...) __attribute__((format(printf, 2, 1))); // \ + expected-error{{out of bounds}} + const char* h3(const char*) __attribute__((format_arg(1))); // \ + expected-error{{invalid for the implicit this argument}} }; + +// PR5521 +struct A { void a(const char*,...) __attribute((format(printf,2,3))); }; +void b(A x) { + x.a("%d", 3); +} diff --git a/clang/test/SemaCXX/attr-nonnull.cpp b/clang/test/SemaCXX/attr-nonnull.cpp new file mode 100644 index 000000000000..e5b5329ffca9 --- /dev/null +++ b/clang/test/SemaCXX/attr-nonnull.cpp @@ -0,0 +1,18 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +struct S { + static void f(const char*, const char*) __attribute__((nonnull(1))); + + // GCC has a hidden 'this' argument in member functions, so the middle + // argument is the one that must not be null. + void g(const char*, const char*, const char*) __attribute__((nonnull(3))); + + void h(const char*) __attribute__((nonnull(1))); // \ + expected-error{{invalid for the implicit this argument}} +}; + +void test(S s) { + s.f(0, ""); // expected-warning{{null passed}} + s.f("", 0); + s.g("", 0, ""); // expected-warning{{null passed}} + s.g(0, "", 0); +} diff --git a/clang/test/SemaCXX/format-attribute.cpp b/clang/test/SemaCXX/format-attribute.cpp deleted file mode 100644 index 92b7cf517eff..000000000000 --- a/clang/test/SemaCXX/format-attribute.cpp +++ /dev/null @@ -1,8 +0,0 @@ -// RUN: %clang_cc1 -fsyntax-only -verify %s - -// PR5521 -struct A { void a(const char*,...) __attribute((format(printf,2,3))); }; -void b(A x) { - x.a("%d", 3); -} -struct X { void a(const char*,...) __attribute((format(printf,1,3))); }; // expected-error {{format argument not a string type}}