[Clang] Make -fcomplete-member-pointers the same as using the Microsoft C++ ABI

clang-tools-extra/clangd/IncludeFixer.cpp

@@ -122,7 +122,8 @@ std::vector<Fix> IncludeFixer::fix(DiagnosticsEngine::Level DiagLevel,
   case diag::err_lambda_incomplete_result:
   //case diag::err_matrix_incomplete_index:
   //case diag::err_matrix_separate_incomplete_index:
-  case diag::err_memptr_incomplete:
+  case diag::warn_memptr_incomplete:
+  case diag::warn_memptr_incomplete_ms:
   case diag::err_new_incomplete_or_sizeless_type:
   case diag::err_objc_incomplete_boxed_expression_type:
   case diag::err_objc_index_incomplete_class_type:

clang/docs/ControlFlowIntegrity.rst

@@ -351,9 +351,9 @@ The compiler will only emit a full CFI check if the member function pointer's
 base type is complete. This is because the complete definition of the base
 type contains information that is necessary to correctly compile the CFI
 check. To ensure that the compiler always emits a full CFI check, it is
-recommended to also pass the flag ``-fcomplete-member-pointers``, which
-enables a non-conforming language extension that requires member pointer
-base types to be complete if they may be used for a call.
+recommended to enable the ``-Wincomplete-member-pointer`` warning, which
+warns if a member pointer does not have a complete base types when the
+member pointer type is specified.
 
 For this scheme to work, all translation units containing the definition
 of a virtual member function (whether inline or not), other than members

clang/include/clang/Basic/DiagnosticGroups.td

@@ -1040,6 +1040,8 @@ def VoidPointerDeref : DiagGroup<"void-ptr-dereference">;
 
 def FUseLdPath : DiagGroup<"fuse-ld-path">;
 
+def MicrosoftIncompleteMemberPointer : DiagGroup<"microsoft-incomplete-member-pointer">;
+
 def Move : DiagGroup<"move", [
     PessimizingMove,
     RedundantMove,
@@ -1102,7 +1104,8 @@ def Most : DiagGroup<"most", [
     PrivateExtern,
     SelTypeCast,
     ExternCCompat,
-    UserDefinedWarnings
+    UserDefinedWarnings,
+    MicrosoftIncompleteMemberPointer
  ]>;
 
 // Thread Safety warnings

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -8082,8 +8082,13 @@ def err_bad_memptr_rhs : Error<
 def err_bad_memptr_lhs : Error<
   "left hand operand to %0 must be a %select{|pointer to }1class "
   "compatible with the right hand operand, but is %2">;
-def err_memptr_incomplete : Error<
-  "member pointer has incomplete base type %0">;
+def warn_memptr_incomplete : Warning<
+  "member pointer has incomplete base type %0">, InGroup<DiagGroup<"incomplete-member-pointer">>, DefaultIgnore;
+def warn_memptr_incomplete_ms : Warning<
+  "this usage of a member pointer with an incomplete base type %0 may cause ODR violations">, InGroup<MicrosoftIncompleteMemberPointer>, DefaultIgnore;
+def note_memptr_incomplete_specify_inheritance : Note<"consider specifying the inheritance model">;
+def note_memptr_incomplete_until_bases : Note<
+  "this will affect the ABI of the member pointer until the bases have been specified">;
 def warn_exception_caught_by_earlier_handler : Warning<
   "exception of type %0 will be caught by earlier handler">,
   InGroup<Exceptions>;

clang/include/clang/Basic/TargetCXXABI.h

@@ -68,6 +68,11 @@ class TargetCXXABI {
     return T.isOSFuchsia();
   }
 
+  // Return true if this target uses the Microsoft C++ ABI by default.
+  static bool defaultABIIsMicrosoft(const llvm::Triple &T) {
+    return T.isKnownWindowsMSVCEnvironment();
+  }
+
   /// A bogus initialization of the platform ABI.
   TargetCXXABI() : TheKind(GenericItanium) {}
 

clang/include/clang/Sema/Sema.h

@@ -14931,6 +14931,23 @@ class Sema final : public SemaBase {
     return RequireCompleteType(Loc, T, Diagnoser);
   }
 
+private:
+  void AssignInheritanceModelToBase(SourceLocation Loc,
+                                    const MemberPointerType *T);
+
+public:
+  /// Called when the Microsoft ABI would require the base type of a member
+  /// pointer to have its inheritance model calculated.
+  void microsoftCompleteMemberPointer(SourceLocation Loc,
+                                      const MemberPointerType *T) {
+    if (getLangOpts().CompleteMemberPointers)
+      AssignInheritanceModelToBase(Loc, T);
+  }
+  void microsoftCompleteMemberPointer(SourceLocation Loc, QualType T) {
+    if (getLangOpts().CompleteMemberPointers)
+      AssignInheritanceModelToBase(Loc, T->getAs<MemberPointerType>());
+  }
+
   /// Determine whether a declaration is visible to name lookup.
   bool isVisible(const NamedDecl *D) {
     return D->isUnconditionallyVisible() ||

clang/lib/AST/Type.cpp

@@ -2413,7 +2413,7 @@ bool Type::isIncompleteType(NamedDecl **Def) const {
     const CXXRecordDecl *RD = ClassTy->getAsCXXRecordDecl();
     ASTContext &Context = RD->getASTContext();
     // Member pointers not in the MS ABI don't get special treatment.
-    if (!Context.getTargetInfo().getCXXABI().isMicrosoft())
+    if (!Context.getLangOpts().CompleteMemberPointers)
       return false;
     // The inheritance attribute might only be present on the most recent
     // CXXRecordDecl, use that one.

clang/lib/Driver/ToolChains/Clang.cpp

@@ -7146,6 +7146,12 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
     CmdArgs.push_back("-fdelayed-template-parsing");
   }
 
+  if (Args.hasFlag(options::OPT_fcomplete_member_pointers,
+                   options::OPT_fno_complete_member_pointers, false)) {
+    CmdArgs.push_back("-fcomplete-member-pointers");
+    CmdArgs.push_back("-Werror=microsoft-incomplete-member-pointer");
+  }
+
   if (Args.hasFlag(options::OPT_fpch_validate_input_files_content,
                    options::OPT_fno_pch_validate_input_files_content, false))
     CmdArgs.push_back("-fvalidate-ast-input-files-content");
@@ -7801,8 +7807,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
                     options::OPT_fno_keep_static_consts);
   Args.addOptInFlag(CmdArgs, options::OPT_fkeep_persistent_storage_variables,
                     options::OPT_fno_keep_persistent_storage_variables);
-  Args.addOptInFlag(CmdArgs, options::OPT_fcomplete_member_pointers,
-                    options::OPT_fno_complete_member_pointers);
   Args.addOptOutFlag(CmdArgs, options::OPT_fcxx_static_destructors,
                      options::OPT_fno_cxx_static_destructors);
 

clang/lib/Frontend/CompilerInvocation.cpp

@@ -4306,9 +4306,16 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args,
       auto Kind = TargetCXXABI::getKind(CXXABI);
       if (!TargetCXXABI::isSupportedCXXABI(T, Kind))
         Diags.Report(diag::err_unsupported_cxx_abi) << CXXABI << T.str();
-      else
+      else {
         Opts.CXXABI = Kind;
+        if (Kind == TargetCXXABI::Microsoft)
+          Opts.CompleteMemberPointers = true;
+      }
     }
+  } else {
+    // The default C++ ABI for this platform is going to be used
+    if (TargetCXXABI::defaultABIIsMicrosoft(T))
+      Opts.CompleteMemberPointers = true;
   }
 
   Opts.RelativeCXXABIVTables =

clang/lib/Sema/SemaCast.cpp

@@ -1793,10 +1793,8 @@ TryStaticMemberPointerUpcast(Sema &Self, ExprResult &SrcExpr, QualType SrcType,
 
   // Lock down the inheritance model right now in MS ABI, whether or not the
   // pointee types are the same.
-  if (Self.Context.getTargetInfo().getCXXABI().isMicrosoft()) {
-    (void)Self.isCompleteType(OpRange.getBegin(), SrcType);
-    (void)Self.isCompleteType(OpRange.getBegin(), DestType);
-  }
+  Self.microsoftCompleteMemberPointer(OpRange.getBegin(), SrcMemPtr);
+  Self.microsoftCompleteMemberPointer(OpRange.getBegin(), DestMemPtr);
 
   // T == T, modulo cv
   if (!Self.Context.hasSameUnqualifiedType(SrcMemPtr->getPointeeType(),
@@ -2335,12 +2333,8 @@ static TryCastResult TryReinterpretCast(Sema &Self, ExprResult &SrcExpr,
         SrcMemPtr->isMemberFunctionPointer())
       return TC_NotApplicable;
 
-    if (Self.Context.getTargetInfo().getCXXABI().isMicrosoft()) {
-      // We need to determine the inheritance model that the class will use if
-      // haven't yet.
-      (void)Self.isCompleteType(OpRange.getBegin(), SrcType);
-      (void)Self.isCompleteType(OpRange.getBegin(), DestType);
-    }
+    Self.microsoftCompleteMemberPointer(OpRange.getBegin(), SrcMemPtr);
+    Self.microsoftCompleteMemberPointer(OpRange.getBegin(), DestMemPtr);
 
     // Don't allow casting between member pointers of different sizes.
     if (Self.Context.getTypeSize(DestMemPtr) !=

clang/lib/Sema/SemaExpr.cpp

@@ -709,9 +709,7 @@ ExprResult Sema::DefaultLvalueConversion(Expr *E) {
     T = T.getUnqualifiedType();
 
   // Under the MS ABI, lock down the inheritance model now.
-  if (T->isMemberPointerType() &&
-      Context.getTargetInfo().getCXXABI().isMicrosoft())
-    (void)isCompleteType(E->getExprLoc(), T);
+  microsoftCompleteMemberPointer(E->getExprLoc(), T);
 
   ExprResult Res = CheckLValueToRValueConversionOperand(E);
   if (Res.isInvalid())
@@ -14065,8 +14063,7 @@ QualType Sema::CheckAddressOfOperand(ExprResult &OrigOp, SourceLocation OpLoc) {
     }
 
     // Under the MS ABI, lock down the inheritance model now.
-    if (Context.getTargetInfo().getCXXABI().isMicrosoft())
-      (void)isCompleteType(OpLoc, MPTy);
+    microsoftCompleteMemberPointer(OpLoc, MPTy);
     return MPTy;
   } else if (lval != Expr::LV_Valid && lval != Expr::LV_IncompleteVoidType) {
     // C99 6.5.3.2p1
@@ -14145,8 +14142,7 @@ QualType Sema::CheckAddressOfOperand(ExprResult &OrigOp, SourceLocation OpLoc) {
               op->getType(),
               Context.getTypeDeclType(cast<RecordDecl>(Ctx)).getTypePtr());
           // Under the MS ABI, lock down the inheritance model now.
-          if (Context.getTargetInfo().getCXXABI().isMicrosoft())
-            (void)isCompleteType(OpLoc, MPTy);
+          microsoftCompleteMemberPointer(OpLoc, MPTy);
           return MPTy;
         }
       }

clang/lib/Sema/SemaExprCXX.cpp

@@ -4607,10 +4607,8 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
 
     // We may not have been able to figure out what this member pointer resolved
     // to up until this exact point.  Attempt to lock-in it's inheritance model.
-    if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
-      (void)isCompleteType(From->getExprLoc(), From->getType());
-      (void)isCompleteType(From->getExprLoc(), ToType);
-    }
+    microsoftCompleteMemberPointer(From->getExprLoc(), From->getType());
+    microsoftCompleteMemberPointer(From->getExprLoc(), ToType);
 
     From =
         ImpCastExprToType(From, ToType, Kind, VK_PRValue, &BasePath, CCK).get();

clang/lib/Sema/SemaOverload.cpp

@@ -16193,8 +16193,7 @@ ExprResult Sema::FixOverloadedFunctionReference(Expr *E, DeclAccessPair Found,
         QualType MemPtrType
           = Context.getMemberPointerType(Fn->getType(), ClassType.getTypePtr());
         // Under the MS ABI, lock down the inheritance model now.
-        if (Context.getTargetInfo().getCXXABI().isMicrosoft())
-          (void)isCompleteType(UnOp->getOperatorLoc(), MemPtrType);
+        microsoftCompleteMemberPointer(UnOp->getOperatorLoc(), MemPtrType);
 
         return UnaryOperator::Create(Context, SubExpr.get(), UO_AddrOf,
                                      MemPtrType, VK_PRValue, OK_Ordinary,

clang/lib/Sema/SemaType.cpp

@@ -2056,10 +2056,8 @@ QualType Sema::BuildArrayType(QualType T, ArraySizeModifier ASM,
 
     // Mentioning a member pointer type for an array type causes us to lock in
     // an inheritance model, even if it's inside an unused typedef.
-    if (Context.getTargetInfo().getCXXABI().isMicrosoft())
-      if (const MemberPointerType *MPTy = T->getAs<MemberPointerType>())
-        if (!MPTy->getClass()->isDependentType())
-          (void)isCompleteType(Loc, T);
+    // FIXME: Verify this, it doesn't appear to always be true
+    microsoftCompleteMemberPointer(Loc, T);
 
   } else {
     // C99 6.7.5.2p1: If the element type is an incomplete or function type,
@@ -8971,35 +8969,61 @@ bool Sema::hasReachableDefinition(NamedDecl *D, NamedDecl **Suggested,
                                  OnlyNeedComplete);
 }
 
-/// Locks in the inheritance model for the given class and all of its bases.
-static void assignInheritanceModel(Sema &S, CXXRecordDecl *RD) {
-  RD = RD->getMostRecentNonInjectedDecl();
-  if (!RD->hasAttr<MSInheritanceAttr>()) {
-    MSInheritanceModel IM;
-    bool BestCase = false;
-    switch (S.MSPointerToMemberRepresentationMethod) {
-    case LangOptions::PPTMK_BestCase:
-      BestCase = true;
-      IM = RD->calculateInheritanceModel();
-      break;
-    case LangOptions::PPTMK_FullGeneralitySingleInheritance:
-      IM = MSInheritanceModel::Single;
-      break;
-    case LangOptions::PPTMK_FullGeneralityMultipleInheritance:
-      IM = MSInheritanceModel::Multiple;
-      break;
-    case LangOptions::PPTMK_FullGeneralityVirtualInheritance:
-      IM = MSInheritanceModel::Unspecified;
-      break;
-    }
+void Sema::AssignInheritanceModelToBase(SourceLocation Loc,
+                                        const MemberPointerType *T) {
+  if (!T || T->getClass()->isDependentType())
+    return;
+
+  assert(getLangOpts().CompleteMemberPointers &&
+         "Should not need assign the inheritance model in this case");
 
-    SourceRange Loc = S.ImplicitMSInheritanceAttrLoc.isValid()
-                          ? S.ImplicitMSInheritanceAttrLoc
-                          : RD->getSourceRange();
-    RD->addAttr(MSInheritanceAttr::CreateImplicit(
-        S.getASTContext(), BestCase, Loc, MSInheritanceAttr::Spelling(IM)));
-    S.Consumer.AssignInheritanceModel(RD);
+  QualType Base = QualType(T->getClass(), 0);
+  (void)isCompleteType(Loc, Base);
+  CXXRecordDecl *RD =
+      T->getClass()->getAsCXXRecordDecl()->getMostRecentNonInjectedDecl();
+  if (RD->hasAttr<MSInheritanceAttr>())
+    return;
+
+  MSInheritanceModel IM;
+  bool BestCase = false;
+  switch (MSPointerToMemberRepresentationMethod) {
+  case LangOptions::PPTMK_BestCase:
+    BestCase = true;
+    IM = RD->calculateInheritanceModel();
+    break;
+  case LangOptions::PPTMK_FullGeneralitySingleInheritance:
+    IM = MSInheritanceModel::Single;
+    break;
+  case LangOptions::PPTMK_FullGeneralityMultipleInheritance:
+    IM = MSInheritanceModel::Multiple;
+    break;
+  case LangOptions::PPTMK_FullGeneralityVirtualInheritance:
+    IM = MSInheritanceModel::Unspecified;
+    break;
   }
+
+  SourceRange AttrLoc = ImplicitMSInheritanceAttrLoc.isValid()
+                            ? ImplicitMSInheritanceAttrLoc
+                            : RD->getSourceRange();
+  RD->addAttr(MSInheritanceAttr::CreateImplicit(
+      Context, BestCase, AttrLoc, MSInheritanceAttr::Spelling(IM)));
+  Consumer.AssignInheritanceModel(RD);
+
+  // Don't warn if the inheritance model is explicitly unspecified because of a
+  // pragma
+  if (IM != MSInheritanceModel::Unspecified || !BestCase)
+    return;
+
+  Diag(Loc, diag::warn_memptr_incomplete_ms) << Base;
+  if (!RD->hasDefinition())
+    Diag(RD->getLocation(), diag::note_forward_declaration) << Base;
+  else if (RD->isParsingBaseSpecifiers())
+    Diag(RD->getDefinition()->getLocation(),
+         diag::note_memptr_incomplete_until_bases);
+  Diag(RD->getLocation(), diag::note_memptr_incomplete_specify_inheritance)
+      << FixItHint::CreateInsertion(RD->getLocation(),
+                                    "__single_inheritance|__multiple_"
+                                    "inheritance|__virtual_inheritance");
 }
 
 bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T,
@@ -9014,19 +9038,15 @@ bool Sema::RequireCompleteTypeImpl(SourceLocation Loc, QualType T,
   //         "Can't ask whether a dependent type is complete");
 
   if (const MemberPointerType *MPTy = T->getAs<MemberPointerType>()) {
-    if (!MPTy->getClass()->isDependentType()) {
-      if (getLangOpts().CompleteMemberPointers &&
-          !MPTy->getClass()->getAsCXXRecordDecl()->isBeingDefined() &&
-          RequireCompleteType(Loc, QualType(MPTy->getClass(), 0), Kind,
-                              diag::err_memptr_incomplete))
-        return true;
-
-      // We lock in the inheritance model once somebody has asked us to ensure
-      // that a pointer-to-member type is complete.
-      if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
-        (void)isCompleteType(Loc, QualType(MPTy->getClass(), 0));
-        assignInheritanceModel(*this, MPTy->getMostRecentCXXRecordDecl());
-      }
+    const Type *Class = MPTy->getClass();
+    if (!Class->isDependentType()) {
+      // Warn if base type of member pointer is incomplete.
+      // Do this before microsoftCompleteMemberPointer can complete it.
+      const CXXRecordDecl *RD = Class->getAsCXXRecordDecl();
+      if (!RD->hasDefinition())
+        Diag(Loc, diag::warn_memptr_incomplete) << QualType(Class, 0);
+
+      microsoftCompleteMemberPointer(Loc, MPTy);
     }
   }
 

clang/test/Misc/warning-wall.c

@@ -93,6 +93,7 @@ CHECK-NEXT:    -Wprivate-extern
 CHECK-NEXT:    -Wcast-of-sel-type
 CHECK-NEXT:    -Wextern-c-compat
 CHECK-NEXT:    -Wuser-defined-warnings
+CHECK-NEXT:  -Wmicrosoft-incomplete-member-pointer
 CHECK-NEXT:  -Wparentheses
 CHECK-NEXT:    -Wlogical-op-parentheses
 CHECK-NEXT:    -Wlogical-not-parentheses