Fix __has_trivial_destructor crash when the type is incomplete with unknown array bounds.

[puneetha]

Summary

Clang crashes at this test case,
struct I;
int a = __has_trivial_destructor(I[]);

With this error,
clang-5.0: /llvm/tools/clang/include/clang/AST/DeclCXX.h:604: clang::CXXRecordDecl::DefinitionData& clang::CXXRecordDecl::data() const: Assertion `DD && "queried property of class with no definition"' failed.

#9 0xffffffffb737c7c7 clang::CXXRecordDecl::data() const /llvm/tools/clang/include/clang/AST/DeclCXX.h:605:0
#10 0xffffffffb737c877 clang::CXXRecordDecl::hasTrivialDestructor() const /llvm/tools/clang/include/clang/AST/DeclCXX.h:1293:0
#11 0x0a9d3ea2 EvaluateUnaryTypeTrait(clang::Sema&, clang::TypeTrait, clang::SourceLocation, clang::QualType) /llvm/tools/clang/lib/Sema/SemaExprCXX.cpp:4416:0
#12 0x0aa5e589 evaluateTypeTrait(clang::Sema&, clang::TypeTrait, clang::SourceLocation, llvm::ArrayRef<clang::TypeSourceInfo*>, clang::SourceLocation) /llvm/tools/clang/lib/Sema/SemaExprCXX.cpp:4560:0

Analysis:
According to the https://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html, __has_trivial_destructor(type) requires type to be a complete type, (possibly cv-qualified) void, or an array of unknown bound.
When we pass an array of unknown bounds of an incomplete type, it crashes because clang is not able to get the DefinitionData of the type which is empty. But an if the type is array of unknown bounds but is complete type, then the DefinitionData is not empty, so there is no crash.

GCC handles this by giving an error if we pass an array of unknown bounds of an incomplete type. This patch is added to give a similar error when compiled with clang, as well.

Thanks,
Puneetha

[puneetha]

Incorrectly rejecting __is_destructible queries on arrays of unknown bound of incomplete types.

[rjmccall]

Test cases?

[puneetha]

Added testcase.

[rjmccall]

lib/Sema/SemaExprCXX.cpp

4128

I don't understand the difference you're creating between traits here. Three specific traits about destructibility allow incomplete array types regardless of whether the base type is incomplete, but the rest do not?

Anyway, I think what you want here is basically just:

if (auto ArrayTy = S.Context.getAsIncompleteArrayType(ArgTy)) {
  ArgTy = ArrayTy->getElementType();
}

[rsmith]

lib/Sema/SemaExprCXX.cpp

4088–4089

Please move the UTT_Has* cases up here, since this is the rule that actually applies to them.

4109

Please update this comment to indicate that the GCC documentation is wrong about the constraints that GCC actually imposes on these traits.

[puneetha]

Updated files to address the review comments.

[puneetha]

lib/Sema/SemaExprCXX.cpp

4128

Of my understanding, these traits are defined by MSVC. There is no mention of them in the GCC type-traits documentation. For these traits, GCC lets us pass an array of incomplete bounds for any base type, complete or incomplete.

Please correct me if I am wrong.

[rjmccall]

lib/Sema/SemaExprCXX.cpp

4128

I see. If we're matching GCC bug-for-bug, it doesn't really matter if the behavior seems inconsistent.

[rsmith]

lib/Sema/SemaExprCXX.cpp

4128

Generally: when the C++ standard has a trait named std::foo and Clang has a __foo builtin, the latter is a complete implementation of the former (and if other compilers have bugs in their implementations, we are not bug-for-bug compatible). That covers the UTT_Is* traits here.

The other traits are typically extensions from other vendors, often used to implement pre-standard names for traits. Many of those are considered deprecated, and we try to be mostly bug-for-bug compatible with GCC or MSVC when implementing them. That covers the UTT_Has* traits here.

Per commentary in PR33232, MSVC's STL has nearly entirely moved off the deprecated traits, so it's mostly GCC compatibility we care about here.

[rsmith]

It'd be good to also add tests for the other traits you're changing.

[puneetha]

Added test cases for all the affecting traits.

[rsmith]

LGTM, thanks. Do you need someone to commit for you?

[puneetha]

This is committed by karthik, at r306519.

Thank you.

lib/Sema/SemaExprCXX.cpp

static bool CheckUnaryTypeTraitTypeCompleteness(Sema &S, TypeTrait UTT,
   //   If T is a class type, T shall be a complete type.
   case UTT_IsFinal:
   case UTT_IsSealed:
     if (ArgTy->getAsCXXRecordDecl())
       return !S.RequireCompleteType(
           Loc, ArgTy, diag::err_incomplete_type_used_in_type_trait_expr);
     return true;
 
   // C++1z [meta.unary.prop]:
   //   remove_all_extents_t<T> shall be a complete type or cv void.
   case UTT_IsAggregate:
   case UTT_IsTrivial:
   case UTT_IsTriviallyCopyable:
   case UTT_IsStandardLayout:
   case UTT_IsPOD:
   case UTT_IsLiteral:
     ArgTy = QualType(ArgTy->getBaseElementTypeUnsafe(), 0);
     LLVM_FALLTHROUGH;
 
   // C++1z [meta.unary.prop]:
   //   T shall be a complete type, cv void, or an array of unknown bound.
   case UTT_IsDestructible:
   case UTT_IsNothrowDestructible:
   case UTT_IsTriviallyDestructible:
+    if (ArgTy->isIncompleteArrayType() || ArgTy->isVoidType())
+      return true;
+
+    return !S.RequireCompleteType(
+        Loc, ArgTy, diag::err_incomplete_type_used_in_type_trait_expr);
   // Per the GCC type traits documentation, the same constraints apply to these.
   case UTT_HasNothrowAssign:
   case UTT_HasNothrowMoveAssign:
   case UTT_HasNothrowConstructor:
   case UTT_HasNothrowCopy:
   case UTT_HasTrivialAssign:
   case UTT_HasTrivialMoveAssign:
   case UTT_HasTrivialDefaultConstructor:
   case UTT_HasTrivialMoveConstructor:
   case UTT_HasTrivialCopy:
   case UTT_HasTrivialDestructor:
   case UTT_HasVirtualDestructor:
-    if (ArgTy->isIncompleteArrayType() || ArgTy->isVoidType())
+    if(ArgTy->isVoidType())
+      return true;
+
+    if (ArgTy->isIncompleteArrayType()) {
+      QualType ElTy = QualType(ArgTy->getBaseElementTypeUnsafe(), 0);
+      if (!ElTy->isIncompleteType())
         return true;
+    }
 
     return !S.RequireCompleteType(
         Loc, ArgTy, diag::err_incomplete_type_used_in_type_trait_expr);
   }
 }
 
 static bool HasNoThrowOperator(const RecordType *RT, OverloadedOperatorKind Op,
                                Sema &Self, SourceLocation KeyLoc, ASTContext &C,

test/SemaCXX/type-traits.cpp

void has_trivial_destructor() {
   { int arr[T(__has_trivial_destructor(HasCopyAssign))]; }
   { int arr[T(__has_trivial_destructor(HasMoveAssign))]; }
   { int arr[T(__has_trivial_destructor(const Int))]; }
   { int arr[T(__has_trivial_destructor(DerivesAr))]; }
   { int arr[T(__has_trivial_destructor(VirtAr))]; }
   { int arr[T(__has_trivial_destructor(AllDefaulted))]; }
   { int arr[T(__has_trivial_destructor(AllDeleted))]; }
   { int arr[T(__has_trivial_destructor(DerivesHasRef))]; }
+  { int arr[T(__has_trivial_destructor(ACompleteType[]))]; }
 
   { int arr[F(__has_trivial_destructor(HasDest))]; }
+  { int arr[F(__has_trivial_destructor(AnIncompleteType[]))]; } // expected-error {{incomplete type}}
   { int arr[F(__has_trivial_destructor(void))]; }
   { int arr[F(__has_trivial_destructor(cvoid))]; }
   { int arr[F(__has_trivial_destructor(AllPrivate))]; }
   { int arr[F(__has_trivial_destructor(ExtDefaulted))]; }
 }
 
 struct A { ~A() {} };
 template<typename> struct B : A { };