aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, dblaikie, rtrieu, friss.
aaron.ballman added a subscriber: cfe-commits.
r267338 improved the diagnostic checking for undefined behavior with
va_start(), but it had some false positives regarding enumerations. The
underlying type for an enumeration in C is either char, signed int, or unsigned
int. In the case the underlying type is chosen to be char (such as when passing
-fshort-enums or using __attribute__((packed)) on the enum declaration), the
enumeration can result in undefined behavior. However, when the underlying type
is signed int or unsigned int (or long long as an extension), there is no
undefined behavior because the types are compatible. This patch silences
diagnostics for the latter while retaining the diagnostics for the former.
This patch addresses PR29140.
https://reviews.llvm.org/D23921
Files:
lib/Sema/SemaChecking.cpp
test/Sema/varargs.c
Index: test/Sema/varargs.c
===================================================================
--- test/Sema/varargs.c
+++ test/Sema/varargs.c
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s -triple i386-pc-unknown
// RUN: %clang_cc1 -fsyntax-only -verify %s -triple x86_64-apple-darwin9
+// RUN: %clang_cc1 -fsyntax-only -fms-compatibility -DMS -verify %s
void f1(int a)
{
@@ -94,3 +95,20 @@
__builtin_va_start(ap, i); // expected-warning {{passing a parameter
declared with the 'register' keyword to 'va_start' has undefined behavior}}
__builtin_va_end(ap);
}
+
+enum __attribute__((packed)) E1 {
+ one1
+};
+
+void f13(enum E1 e, ...) {
+ __builtin_va_list va;
+ __builtin_va_start(va, e);
+#ifndef MS
+ // In Microsoft compatibility mode, all enum types are int, but in
+ // non-ms-compatibility mode, this enumeration type will undergo default
+ // argument promotions.
+ // expected-note@-7 {{parameter of type 'enum E1' is declared here}}
+ // expected-warning@-6 {{passing an object that undergoes default argument
promotion to 'va_start' has undefined behavior}}
+#endif
+ __builtin_va_end(va);
+}
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -3238,8 +3238,17 @@
Diag(TheCall->getArg(1)->getLocStart(),
diag::warn_second_arg_of_va_start_not_last_named_param);
else if (IsCRegister || Type->isReferenceType() ||
- Type->isPromotableIntegerType() ||
- Type->isSpecificBuiltinType(BuiltinType::Float)) {
+ Type->isSpecificBuiltinType(BuiltinType::Float) || [=] {
+ // Promotable integers are UB, but enumerations need a bit of
+ // extra checking to see what their promotable type actually is.
+ if (!Type->isPromotableIntegerType())
+ return false;
+ if (!Type->isEnumeralType())
+ return true;
+ const EnumDecl *ED = Type->getAs<EnumType>()->getDecl();
+ return !(ED &&
+ Context.typesAreCompatible(ED->getPromotionType(),
Type));
+ }()) {
unsigned Reason = 0;
if (Type->isReferenceType()) Reason = 1;
else if (IsCRegister) Reason = 2;
Index: test/Sema/varargs.c
===================================================================
--- test/Sema/varargs.c
+++ test/Sema/varargs.c
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s -triple i386-pc-unknown
// RUN: %clang_cc1 -fsyntax-only -verify %s -triple x86_64-apple-darwin9
+// RUN: %clang_cc1 -fsyntax-only -fms-compatibility -DMS -verify %s
void f1(int a)
{
@@ -94,3 +95,20 @@
__builtin_va_start(ap, i); // expected-warning {{passing a parameter declared with the 'register' keyword to 'va_start' has undefined behavior}}
__builtin_va_end(ap);
}
+
+enum __attribute__((packed)) E1 {
+ one1
+};
+
+void f13(enum E1 e, ...) {
+ __builtin_va_list va;
+ __builtin_va_start(va, e);
+#ifndef MS
+ // In Microsoft compatibility mode, all enum types are int, but in
+ // non-ms-compatibility mode, this enumeration type will undergo default
+ // argument promotions.
+ // expected-note@-7 {{parameter of type 'enum E1' is declared here}}
+ // expected-warning@-6 {{passing an object that undergoes default argument promotion to 'va_start' has undefined behavior}}
+#endif
+ __builtin_va_end(va);
+}
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -3238,8 +3238,17 @@
Diag(TheCall->getArg(1)->getLocStart(),
diag::warn_second_arg_of_va_start_not_last_named_param);
else if (IsCRegister || Type->isReferenceType() ||
- Type->isPromotableIntegerType() ||
- Type->isSpecificBuiltinType(BuiltinType::Float)) {
+ Type->isSpecificBuiltinType(BuiltinType::Float) || [=] {
+ // Promotable integers are UB, but enumerations need a bit of
+ // extra checking to see what their promotable type actually is.
+ if (!Type->isPromotableIntegerType())
+ return false;
+ if (!Type->isEnumeralType())
+ return true;
+ const EnumDecl *ED = Type->getAs<EnumType>()->getDecl();
+ return !(ED &&
+ Context.typesAreCompatible(ED->getPromotionType(), Type));
+ }()) {
unsigned Reason = 0;
if (Type->isReferenceType()) Reason = 1;
else if (IsCRegister) Reason = 2;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
