MitalAshok updated this revision to Diff 549176.
MitalAshok added a comment.
Address feedback; Reuse DefaultArgumentPromotion instead of duplicating its
functionality when building VaArgExpr
There is the added bonus for it working with dependent types somewhat
(`template<typename T> __builtin_va_arg(ap, T[2]);` will warn in the template
instead of waiting for it to be instantiated).
I can add the check for dependent types back if this is unwanted.
I don't think there are any edge cases where this will give a false warning.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156054/new/
https://reviews.llvm.org/D156054
Files:
clang/docs/ReleaseNotes.rst
clang/lib/AST/FormatString.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/CXX/drs/dr7xx.cpp
clang/test/CodeGen/xcore-abi.c
clang/test/Sema/format-pointer.c
clang/test/Sema/format-strings-pedantic.c
clang/test/SemaCXX/format-strings-0x-nopedantic.cpp
clang/test/SemaCXX/varargs.cpp
clang/www/cxx_dr_status.html
Index: clang/www/cxx_dr_status.html
===================================================================
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -4373,7 +4373,7 @@
<td><a href="https://cplusplus.github.io/CWG/issues/722.html";>722</a></td>
<td>CD2</td>
<td>Can <TT>nullptr</TT> be passed to an ellipsis?</td>
- <td class="none" align="center">Unknown</td>
+ <td class="unreleased" align="center">Clang 18</td>
</tr>
<tr id="726">
<td><a href="https://cplusplus.github.io/CWG/issues/726.html";>726</a></td>
Index: clang/test/SemaCXX/varargs.cpp
===================================================================
--- clang/test/SemaCXX/varargs.cpp
+++ clang/test/SemaCXX/varargs.cpp
@@ -55,6 +55,16 @@
(void)__builtin_va_arg(ap, unsigned int);
(void)__builtin_va_arg(ap, bool); // expected-warning {{second argument to 'va_arg' is of promotable type 'bool'; this va_arg has undefined behavior because arguments will be promoted to 'int'}}
+
+ (void)__builtin_va_arg(ap, float); // expected-warning {{second argument to 'va_arg' is of promotable type 'float'; this va_arg has undefined behavior because arguments will be promoted to 'double'}}
+ (void)__builtin_va_arg(ap, __fp16); // expected-warning {{second argument to 'va_arg' is of promotable type '__fp16'; this va_arg has undefined behavior because arguments will be promoted to 'double'}}
+
+#if __cplusplus >= 201103L
+ (void)__builtin_va_arg(ap, decltype(nullptr)); // expected-warning {{second argument to 'va_arg' is of promotable type 'decltype(nullptr)' (aka 'std::nullptr_t'); this va_arg has undefined behavior because arguments will be promoted to 'void *'}}
+#endif
+
+ (void)__builtin_va_arg(ap, int[3]); // expected-warning {{second argument to 'va_arg' is of promotable type 'int[3]'; this va_arg has undefined behavior because arguments will be promoted to 'int *'}}
+ (void)__builtin_va_arg(ap, const int[3]); // expected-warning {{second argument to 'va_arg' is of promotable type 'const int[3]'; this va_arg has undefined behavior because arguments will be promoted to 'const int *'}}
}
#if __cplusplus >= 201103L
Index: clang/test/SemaCXX/format-strings-0x-nopedantic.cpp
===================================================================
--- clang/test/SemaCXX/format-strings-0x-nopedantic.cpp
+++ clang/test/SemaCXX/format-strings-0x-nopedantic.cpp
@@ -5,6 +5,7 @@
extern int printf(const char *restrict, ...);
}
-void f(char *c) {
+void f(char *c, int *q) {
printf("%p", c);
+ printf("%p", q);
}
Index: clang/test/Sema/format-strings-pedantic.c
===================================================================
--- clang/test/Sema/format-strings-pedantic.c
+++ clang/test/Sema/format-strings-pedantic.c
@@ -1,6 +1,7 @@
// RUN: %clang_cc1 -fsyntax-only -verify -Wno-format -Wformat-pedantic %s
// RUN: %clang_cc1 -xobjective-c -fblocks -fsyntax-only -verify -Wno-format -Wformat-pedantic %s
// RUN: %clang_cc1 -xc++ -fsyntax-only -verify -Wno-format -Wformat-pedantic %s
+// RUN: %clang_cc1 -std=c2x -fsyntax-only -verify -Wno-format -Wformat-pedantic %s
__attribute__((format(printf, 1, 2)))
int printf(const char *restrict, ...);
@@ -14,7 +15,7 @@
printf("%p", (id)0); // expected-warning {{format specifies type 'void *' but the argument has type 'id'}}
#endif
-#ifdef __cplusplus
- printf("%p", nullptr); // expected-warning {{format specifies type 'void *' but the argument has type 'std::nullptr_t'}}
+#if !__is_identifier(nullptr)
+ printf("%p", nullptr);
#endif
}
Index: clang/test/Sema/format-pointer.c
===================================================================
--- /dev/null
+++ clang/test/Sema/format-pointer.c
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -xc -Wformat %s -verify
+// RUN: %clang_cc1 -xc -Wformat -std=c2x %s -verify
+// RUN: %clang_cc1 -xc++ -Wformat %s -verify
+// RUN: %clang_cc1 -xobjective-c -Wformat -fblocks %s -verify
+// RUN: %clang_cc1 -xobjective-c++ -Wformat -fblocks %s -verify
+// RUN: %clang_cc1 -xc -std=c2x -Wformat %s -pedantic -verify=expected,pedantic
+// RUN: %clang_cc1 -xc++ -Wformat %s -pedantic -verify=expected,pedantic
+// RUN: %clang_cc1 -xobjective-c -Wformat -fblocks -pedantic %s -verify=expected,pedantic
+
+__attribute__((__format__(__printf__, 1, 2)))
+int printf(const char *, ...);
+__attribute__((__format__(__scanf__, 1, 2)))
+int scanf(const char *, ...);
+
+void f(void *vp, const void *cvp, char *cp, signed char *scp, int *ip) {
+ int arr[2];
+
+ printf("%p", cp);
+ printf("%p", cvp);
+ printf("%p", vp);
+ printf("%p", scp);
+ printf("%p", ip); // pedantic-warning {{format specifies type 'void *' but the argument has type 'int *'}}
+ printf("%p", arr); // pedantic-warning {{format specifies type 'void *' but the argument has type 'int *'}}
+
+ scanf("%p", &vp);
+ scanf("%p", &cvp);
+ scanf("%p", (void *volatile*)&vp);
+ scanf("%p", (const void *volatile*)&cvp);
+ scanf("%p", &cp); // pedantic-warning {{format specifies type 'void **' but the argument has type 'char **'}}
+ scanf("%p", &ip); // pedantic-warning {{format specifies type 'void **' but the argument has type 'int **'}}
+ scanf("%p", &arr); // expected-warning {{format specifies type 'void **' but the argument has type 'int (*)[2]'}}
+
+#if !__is_identifier(nullptr)
+ typedef __typeof__(nullptr) nullptr_t;
+ nullptr_t np = nullptr;
+ nullptr_t *npp = &np;
+
+ printf("%p", np);
+ scanf("%p", &np); // expected-warning {{format specifies type 'void **' but the argument has type 'nullptr_t *'}}
+ scanf("%p", &npp); // pedantic-warning {{format specifies type 'void **' but the argument has type 'nullptr_t **'}}
+#endif
+
+#ifdef __OBJC__
+ id i = 0;
+ void (^b)(void) = ^{};
+
+ printf("%p", i); // pedantic-warning {{format specifies type 'void *' but the argument has type 'id'}}
+ printf("%p", b); // pedantic-warning {{format specifies type 'void *' but the argument has type 'void (^)(void)'}}
+ scanf("%p", &i); // pedantic-warning {{format specifies type 'void **' but the argument has type 'id *'}}
+ scanf("%p", &b); // pedantic-warning {{format specifies type 'void **' but the argument has type 'void (^*)(void)'}}
+#endif
+
+}
Index: clang/test/CodeGen/xcore-abi.c
===================================================================
--- clang/test/CodeGen/xcore-abi.c
+++ clang/test/CodeGen/xcore-abi.c
@@ -77,6 +77,7 @@
// CHECK: call void @f(ptr noundef [[V5]])
int* v6 = va_arg (ap, int[4]); // an unusual aggregate type
+ // expected-warning@-1{{second argument to 'va_arg' is of promotable type 'int[4]'}}
f(v6);
// CHECK: [[I:%[a-z0-9]+]] = load ptr, ptr [[AP]]
// CHECK: [[P:%[a-z0-9]+]] = load ptr, ptr [[I]]
Index: clang/test/CXX/drs/dr7xx.cpp
===================================================================
--- clang/test/CXX/drs/dr7xx.cpp
+++ clang/test/CXX/drs/dr7xx.cpp
@@ -53,6 +53,15 @@
#endif
}
+namespace dr722 { // dr722: 18
+#if __cplusplus >= 201103L
+ int x = __builtin_printf("%p", nullptr);
+ void f(__builtin_va_list args) {
+ __builtin_va_arg(args, decltype(nullptr)); // expected-warning {{second argument to 'va_arg' is of promotable type 'decltype(nullptr)' (aka 'std::nullptr_t'); this va_arg has undefined behavior because arguments will be promoted to 'void *'}}
+ }
+#endif
+}
+
namespace dr727 { // dr727: partial
struct A {
template<typename T> struct C; // expected-note 6{{here}}
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -923,6 +923,13 @@
E = Temp.get();
}
+ // C++ [expr.call]p7, per DR722:
+ // An argument that has (possibly cv-qualified) type std::nullptr_t is
+ // converted to void* ([conv.ptr]).
+ // (This does not apply to C2x nullptr)
+ if (getLangOpts().CPlusPlus && E->getType()->isNullPtrType())
+ E = ImpCastExprToType(E, Context.VoidPtrTy, CK_NullToPointer).get();
+
return E;
}
@@ -936,9 +943,9 @@
// enumeration, pointer, pointer to member, or class type, the program
// is ill-formed.
//
- // Since we've already performed array-to-pointer and function-to-pointer
- // decay, the only such type in C++ is cv void. This also handles
- // initializer lists as variadic arguments.
+ // Since we've already performed null pointer conversion, array-to-pointer
+ // decay and function-to-pointer decay, the only such type in C++ is cv
+ // void. This also handles initializer lists as variadic arguments.
if (Ty->isVoidType())
return VAK_Invalid;
@@ -17277,65 +17284,25 @@
<< TInfo->getType()
<< TInfo->getTypeLoc().getSourceRange();
}
-
- // Check for va_arg where arguments of the given type will be promoted
- // (i.e. this va_arg is guaranteed to have undefined behavior).
- QualType PromoteType;
- if (Context.isPromotableIntegerType(TInfo->getType())) {
- PromoteType = Context.getPromotedIntegerType(TInfo->getType());
- // [cstdarg.syn]p1 defers the C++ behavior to what the C standard says,
- // and C2x 7.16.1.1p2 says, in part:
- // If type is not compatible with the type of the actual next argument
- // (as promoted according to the default argument promotions), the
- // behavior is undefined, except for the following cases:
- // - both types are pointers to qualified or unqualified versions of
- // compatible types;
- // - one type is a signed integer type, the other type is the
- // corresponding unsigned integer type, and the value is
- // representable in both types;
- // - one type is pointer to qualified or unqualified void and the
- // other is a pointer to a qualified or unqualified character type.
- // Given that type compatibility is the primary requirement (ignoring
- // qualifications), you would think we could call typesAreCompatible()
- // directly to test this. However, in C++, that checks for *same type*,
- // which causes false positives when passing an enumeration type to
- // va_arg. Instead, get the underlying type of the enumeration and pass
- // that.
- QualType UnderlyingType = TInfo->getType();
- if (const auto *ET = UnderlyingType->getAs<EnumType>())
- UnderlyingType = ET->getDecl()->getIntegerType();
- if (Context.typesAreCompatible(PromoteType, UnderlyingType,
- /*CompareUnqualified*/ true))
- PromoteType = QualType();
-
- // If the types are still not compatible, we need to test whether the
- // promoted type and the underlying type are the same except for
- // signedness. Ask the AST for the correctly corresponding type and see
- // if that's compatible.
- if (!PromoteType.isNull() && !UnderlyingType->isBooleanType() &&
- PromoteType->isUnsignedIntegerType() !=
- UnderlyingType->isUnsignedIntegerType()) {
- UnderlyingType =
- UnderlyingType->isUnsignedIntegerType()
- ? Context.getCorrespondingSignedType(UnderlyingType)
- : Context.getCorrespondingUnsignedType(UnderlyingType);
- if (Context.typesAreCompatible(PromoteType, UnderlyingType,
- /*CompareUnqualified*/ true))
- PromoteType = QualType();
- }
- }
- if (TInfo->getType()->isSpecificBuiltinType(BuiltinType::Float))
- PromoteType = Context.DoubleTy;
- if (!PromoteType.isNull())
- DiagRuntimeBehavior(TInfo->getTypeLoc().getBeginLoc(), E,
- PDiag(diag::warn_second_parameter_to_va_arg_never_compatible)
- << TInfo->getType()
- << PromoteType
- << TInfo->getTypeLoc().getSourceRange());
}
QualType T = TInfo->getType().getNonLValueExprType(Context);
- return new (Context) VAArgExpr(BuiltinLoc, E, TInfo, RPLoc, T, IsMS);
+ auto *ResultExpr = new (Context) VAArgExpr(BuiltinLoc, E, TInfo, RPLoc, T, IsMS);
+
+ // Check for va_arg where arguments of the given type will be promoted
+ // (i.e. this va_arg is guaranteed to have undefined behavior).
+ ExprResult PromotedExpr = DefaultArgumentPromotion(ResultExpr);
+ QualType PromoteType = PromotedExpr.get()->getType();
+
+ if (!Context.typesAreCompatible(PromoteType, ResultExpr->getType(), true)) {
+ DiagRuntimeBehavior(TInfo->getTypeLoc().getBeginLoc(), E,
+ PDiag(diag::warn_second_parameter_to_va_arg_never_compatible)
+ << TInfo->getType()
+ << PromoteType
+ << TInfo->getTypeLoc().getSourceRange());
+ }
+
+ return ResultExpr;
}
ExprResult Sema::ActOnGNUNullExpr(SourceLocation TokenLoc) {
Index: clang/lib/AST/FormatString.cpp
===================================================================
--- clang/lib/AST/FormatString.cpp
+++ clang/lib/AST/FormatString.cpp
@@ -481,20 +481,8 @@
}
case CStrTy: {
- const PointerType *PT = argTy->getAs<PointerType>();
- if (!PT)
- return NoMatch;
- QualType pointeeTy = PT->getPointeeType();
- if (const BuiltinType *BT = pointeeTy->getAs<BuiltinType>())
- switch (BT->getKind()) {
- case BuiltinType::Char_U:
- case BuiltinType::UChar:
- case BuiltinType::Char_S:
- case BuiltinType::SChar:
- return Match;
- default:
- break;
- }
+ if (const auto *PT = argTy->getAs<PointerType>(); PT && PT->getPointeeType()->isCharType())
+ return Match;
return NoMatch;
}
@@ -529,15 +517,21 @@
}
case CPointerTy:
- if (argTy->isVoidPointerType()) {
- return Match;
- } if (argTy->isPointerType() || argTy->isObjCObjectPointerType() ||
- argTy->isBlockPointerType() || argTy->isNullPtrType()) {
+ if (const auto *PT = argTy->getAs<PointerType>()) {
+ QualType PointeeTy = PT->getPointeeType();
+ if (PointeeTy->isVoidType() || (!Ptr && PointeeTy->isCharType()))
+ return Match;
return NoMatchPedantic;
- } else {
- return NoMatch;
}
+ if (argTy->isNullPtrType())
+ return MatchPromotion;
+
+ if (argTy->isObjCObjectPointerType() || argTy->isBlockPointerType())
+ return NoMatchPedantic;
+
+ return NoMatch;
+
case ObjCPointerTy: {
if (argTy->getAs<ObjCObjectPointerType>() ||
argTy->getAs<BlockPointerType>())
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -89,6 +89,8 @@
Resolutions to C++ Defect Reports
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Implemented `DR722 <https://wg21.link/CWG722>`_ which promotes ``nullptr`` to ``void*``
+ when passed to a C-style variadic function.
C Language Changes
------------------
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
