Index: clang/lib/Sema/SemaExpr.cpp =================================================================== --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -15750,8 +15750,46 @@ QualType PromoteType; if (TInfo->getType()->isPromotableIntegerType()) { PromoteType = Context.getPromotedIntegerType(TInfo->getType()); - if (Context.typesAreCompatible(PromoteType, 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()) + 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() && + 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; Index: clang/test/SemaCXX/varargs.cpp =================================================================== --- clang/test/SemaCXX/varargs.cpp +++ clang/test/SemaCXX/varargs.cpp @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 -std=c++03 -verify %s -// RUN: %clang_cc1 -std=c++11 -verify %s +// RUN: %clang_cc1 -std=c++03 -Wno-c++11-extensions -triple i386-pc-unknown -verify %s +// RUN: %clang_cc1 -std=c++11 -triple x86_64-apple-darwin9 -verify %s __builtin_va_list ap; @@ -28,6 +28,33 @@ }; } +// Ensure the correct behavior for promotable type UB checking. +void promotable(int a, ...) { + enum Unscoped1 { One = 0x7FFFFFFF }; + (void)__builtin_va_arg(ap, Unscoped1); // ok + + enum Unscoped2 { Two = 0xFFFFFFFF }; + (void)__builtin_va_arg(ap, Unscoped2); // ok + + enum class Scoped { Three }; + (void)__builtin_va_arg(ap, Scoped); // ok + + enum Fixed : int { Four }; + (void)__builtin_va_arg(ap, Fixed); // ok + + enum FixedSmall : char { Five }; + (void)__builtin_va_arg(ap, FixedSmall); // expected-warning {{second argument to 'va_arg' is of promotable type 'FixedSmall'; this va_arg has undefined behavior because arguments will be promoted to 'int'}} + + enum FixedLarge : long long { Six }; + (void)__builtin_va_arg(ap, FixedLarge); // ok + + // Ensure that qualifiers are ignored. + (void)__builtin_va_arg(ap, const volatile int); // ok + + // Ensure that signed vs unsigned doesn't matter either. + (void)__builtin_va_arg(ap, unsigned int); +} + #if __cplusplus >= 201103L // We used to have bugs identifying the correct enclosing function scope in a // lambda.