Index: clang/lib/Sema/SemaExpr.cpp =================================================================== --- clang/lib/Sema/SemaExpr.cpp +++ clang/lib/Sema/SemaExpr.cpp @@ -15750,7 +15750,27 @@ 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() to + // test this. However, in C++, that checks for *same type*, which causes + // false positives when passing an enumeration type to va_arg. Instead, + // attempt to merge the types and see how well that works. + QualType Merged = + Context.mergeTypes(PromoteType, TInfo->getType(), + /*OfBlockPointer*/ false, /*Unqualified*/ true); + if (!Merged.isNull()) PromoteType = QualType(); } if (TInfo->getType()->isSpecificBuiltinType(BuiltinType::Float)) Index: clang/test/SemaCXX/varargs.cpp =================================================================== --- clang/test/SemaCXX/varargs.cpp +++ clang/test/SemaCXX/varargs.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -std=c++03 -verify %s +// RUN: %clang_cc1 -std=c++03 -Wno-c++11-extensions -verify %s // RUN: %clang_cc1 -std=c++11 -verify %s __builtin_va_list ap; @@ -28,6 +28,30 @@ }; } +// Ensure the correct behavior for promotable type UB checking. +void promotable(int a, ...) { + enum Unscoped { One }; + (void)__builtin_va_arg(ap, Unscoped); // ok + + enum class Scoped { Two }; + (void)__builtin_va_arg(ap, Scoped); // ok + + enum Fixed : int { Three }; + (void)__builtin_va_arg(ap, Fixed); // ok + + enum FixedSmall : char { Four }; + (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 { Five }; + (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.