Commit 9b4faa11 authored by Alan Zhao's avatar Alan Zhao
Browse files

[clang] Fix overly aggressive lifetime checks for parenthesized aggregate initialization 

Before this patch, initialized class members would have the LifetimeKind
LK_MemInitializer, which does not allow for binding a temporary to a
reference. Binding to a temporary however is allowed in parenthesized
aggregate initialization, even if it leads to a dangling reference. To
fix this, we create a new EntityKind, EK_ParenAggInitMember, which has
LifetimeKind LK_FullExpression.

This patch does *not* attempt to diagnose dangling references as a
result of using this feature.

This patch also refactors TryOrBuildParenListInitialization(...) to
accomodate creating different InitializedEntity objects.

Fixes #61567

[0]: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p0960r3.html

Reviewed By: shafik

Differential Revision: https://reviews.llvm.org/D148274
parent 266d65c9

clang/docs/ReleaseNotes.rst

+6 −0
Original line number Diff line number Diff line
@@ -373,9 +373,15 @@ Bug Fixes to C++ Support 
- Fix bug in the computation of the ``__has_unique_object_representations``

  builtin for types with unnamed bitfields.

  (`#61336 <https://github.com/llvm/llvm-project/issues/61336>`_)

<<<<<<< HEAD

- Fix default member initializers sometimes being ignored when performing

  parenthesized aggregate initialization of templated types.

  (`#62266 <https://github.com/llvm/llvm-project/issues/62266>`_)

=======

- Fix overly aggressive lifetime checks for parenthesized aggregate

  initialization.

  (`#61567 <https://github.com/llvm/llvm-project/issues/61567>`_)

>>>>>>> c7422b289522 ([clang] Fix overly aggressive lifetime checks for parenthesized aggregate initialization)



Bug Fixes to AST Handling

^^^^^^^^^^^^^^^^^^^^^^^^^

clang/include/clang/Basic/DiagnosticSemaKinds.td

+2 −1
Original line number Diff line number Diff line
@@ -2126,7 +2126,8 @@ def err_init_conversion_failed : Error< 
  "exception object|a member subobject|an array element|a new value|a value|a "

  "base class|a constructor delegation|a vector element|a block element|a "

  "block element|a complex element|a lambda capture|a compound literal "

  "initializer|a related result|a parameter of CF audited function}0 "

  "initializer|a related result|a parameter of CF audited function|a "

  "structured binding|a member subobject}0 "

  "%diff{of type $ with an %select{rvalue|lvalue}2 of type $|"

  "with an %select{rvalue|lvalue}2 of incompatible type}1,3"

  "%select{|: different classes%diff{ ($ vs $)|}5,6"

clang/include/clang/Sema/Initialization.h

+16 −2
Original line number Diff line number Diff line
@@ -123,6 +123,10 @@ public: 
    /// decomposition declaration.

    EK_Binding,



    /// The entity being initialized is a non-static data member subobject of an

    /// object initialized via parenthesized aggregate initialization.

    EK_ParenAggInitMember,



    // Note: err_init_conversion_failed in DiagnosticSemaKinds.td uses this

    // enum as an index for its first %select.  When modifying this list,

    // that diagnostic text needs to be updated as well.
@@ -227,8 +231,10 @@ private: 


  /// Create the initialization entity for a member subobject.

  InitializedEntity(FieldDecl *Member, const InitializedEntity *Parent,

                    bool Implicit, bool DefaultMemberInit)

      : Kind(EK_Member), Parent(Parent), Type(Member->getType()),

                    bool Implicit, bool DefaultMemberInit,

                    bool IsParenAggInit = false)

      : Kind(IsParenAggInit ? EK_ParenAggInitMember : EK_Member),

        Parent(Parent), Type(Member->getType()),

        Variable{Member, Implicit, DefaultMemberInit} {}



  /// Create the initialization entity for an array element.
@@ -388,6 +394,14 @@ public: 
    return InitializedEntity(Member->getAnonField(), Parent, Implicit, false);

  }



  /// Create the initialization entity for a member subobject initialized via

  /// parenthesized aggregate init.

  static InitializedEntity InitializeMemberFromParenAggInit(FieldDecl *Member) {

    return InitializedEntity(Member, /*Parent=*/nullptr, /*Implicit=*/false,

                             /*DefaultMemberInit=*/false,

                             /*IsParenAggInit=*/true);

  }



  /// Create the initialization entity for a default member initializer.

  static InitializedEntity

  InitializeMemberFromDefaultMemberInitializer(FieldDecl *Member) {

clang/lib/Sema/SemaAccess.cpp

+2 −1
Original line number Diff line number Diff line
@@ -1651,7 +1651,8 @@ Sema::AccessResult Sema::CheckConstructorAccess(SourceLocation UseLoc, 
       << Entity.getBaseSpecifier()->getType() << getSpecialMember(Constructor);

    break;



  case InitializedEntity::EK_Member: {

  case InitializedEntity::EK_Member:

  case InitializedEntity::EK_ParenAggInitMember: {

    const FieldDecl *Field = cast<FieldDecl>(Entity.getDecl());

    PD = PDiag(diag::err_access_field_ctor);

    PD << Field->getType() << getSpecialMember(Constructor);

clang/lib/Sema/SemaInit.cpp

+200 −133
Original line number Diff line number Diff line
@@ -1186,6 +1186,7 @@ static void warnBracedScalarInit(Sema &S, const InitializedEntity &Entity, 
  case InitializedEntity::EK_LambdaToBlockConversionBlockElement:

  case InitializedEntity::EK_Binding:

  case InitializedEntity::EK_StmtExprResult:

  case InitializedEntity::EK_ParenAggInitMember:

    llvm_unreachable("unexpected braced scalar init");

  }
@@ -3359,6 +3360,7 @@ DeclarationName InitializedEntity::getName() const { 


  case EK_Variable:

  case EK_Member:

  case EK_ParenAggInitMember:

  case EK_Binding:

  case EK_TemplateParameter:

    return Variable.VariableOrMember->getDeclName();
@@ -3390,6 +3392,7 @@ ValueDecl *InitializedEntity::getDecl() const { 
  switch (getKind()) {

  case EK_Variable:

  case EK_Member:

  case EK_ParenAggInitMember:

  case EK_Binding:

  case EK_TemplateParameter:

    return Variable.VariableOrMember;
@@ -3431,6 +3434,7 @@ bool InitializedEntity::allowsNRVO() const { 
  case EK_Parameter_CF_Audited:

  case EK_TemplateParameter:

  case EK_Member:

  case EK_ParenAggInitMember:

  case EK_Binding:

  case EK_New:

  case EK_Temporary:
@@ -3465,7 +3469,10 @@ unsigned InitializedEntity::dumpImpl(raw_ostream &OS) const { 
  case EK_Result: OS << "Result"; break;

  case EK_StmtExprResult: OS << "StmtExprResult"; break;

  case EK_Exception: OS << "Exception"; break;

  case EK_Member: OS << "Member"; break;

  case EK_Member:

  case EK_ParenAggInitMember:

    OS << "Member";

    break;

  case EK_Binding: OS << "Binding"; break;

  case EK_New: OS << "New"; break;

  case EK_Temporary: OS << "Temporary"; break;
@@ -5303,41 +5310,152 @@ static void TryOrBuildParenListInitialization( 
    Sema &S, const InitializedEntity &Entity, const InitializationKind &Kind,

    ArrayRef<Expr *> Args, InitializationSequence &Sequence, bool VerifyOnly,

    ExprResult *Result = nullptr) {

  unsigned ArgIndexToProcess = 0;

  unsigned EntityIndexToProcess = 0;

  SmallVector<Expr *, 4> InitExprs;

  QualType ResultType;

  Expr *ArrayFiller = nullptr;

  FieldDecl *InitializedFieldInUnion = nullptr;



  // Process entities (i.e. array members, base classes, or class fields) by

  // adding an initialization expression to InitExprs for each entity to

  // initialize.

  auto ProcessEntities = [&](auto Range) -> bool {

    bool IsUnionType = Entity.getType()->isUnionType();

    for (InitializedEntity SubEntity : Range) {

      // Unions should only have one initializer expression.

      // If there are more initializers than it will be caught when we check

      // whether Index equals Args.size().

      if (ArgIndexToProcess == 1 && IsUnionType)

  auto HandleInitializedEntity = [&](const InitializedEntity &SubEntity,

                                     const InitializationKind &SubKind,

                                     Expr *Arg, Expr **InitExpr = nullptr) {

    InitializationSequence IS = [&]() {

      if (Arg)

        return InitializationSequence(S, SubEntity, SubKind, Arg);

      return InitializationSequence(S, SubEntity, SubKind, std::nullopt);

    }();



    if (IS.Failed()) {

      if (!VerifyOnly) {

        if (Arg)

          IS.Diagnose(S, SubEntity, SubKind, Arg);

        else

          IS.Diagnose(S, SubEntity, SubKind, std::nullopt);

      } else {

        Sequence.SetFailed(

            InitializationSequence::FK_ParenthesizedListInitFailed);

      }



      return false;

    }

    if (!VerifyOnly) {

      ExprResult ER;

      if (Arg)

        ER = IS.Perform(S, SubEntity, SubKind, Arg);

      else

        ER = IS.Perform(S, SubEntity, SubKind, std::nullopt);

      if (InitExpr)

        *InitExpr = ER.get();

      else

        InitExprs.push_back(ER.get());

    }

    return true;

  };



      bool IsMember = SubEntity.getKind() == InitializedEntity::EK_Member;

  if (const ArrayType *AT =

          S.getASTContext().getAsArrayType(Entity.getType())) {

    SmallVector<InitializedEntity, 4> ElementEntities;

    uint64_t ArrayLength;

    // C++ [dcl.init]p16.5

    //   if the destination type is an array, the object is initialized as

    //   follows. Let x1, . . . , xk be the elements of the expression-list. If

    //   the destination type is an array of unknown bound, it is defined as

    //   having k elements.

    if (const ConstantArrayType *CAT =

            S.getASTContext().getAsConstantArrayType(Entity.getType())) {

      ArrayLength = CAT->getSize().getZExtValue();

      ResultType = Entity.getType();

    } else if (const VariableArrayType *VAT =

                   S.getASTContext().getAsVariableArrayType(Entity.getType())) {

      // Braced-initialization of variable array types is not allowed, even if

      // the size is greater than or equal to the number of args, so we don't

      // allow them to be initialized via parenthesized aggregate initialization

      // either.

      const Expr *SE = VAT->getSizeExpr();

      S.Diag(SE->getBeginLoc(), diag::err_variable_object_no_init)

          << SE->getSourceRange();

      return;

    } else {

      assert(isa<IncompleteArrayType>(Entity.getType()));

      ArrayLength = Args.size();

    }

    EntityIndexToProcess = ArrayLength;



    //   ...the ith array element is copy-initialized with xi for each

    //   1 <= i <= k

    for (Expr *E : Args) {

      InitializedEntity SubEntity = InitializedEntity::InitializeElement(

          S.getASTContext(), EntityIndexToProcess, Entity);

      InitializationKind SubKind = InitializationKind::CreateForInit(

          E->getExprLoc(), /*isDirectInit=*/false, E);

      if (!HandleInitializedEntity(SubEntity, SubKind, E))

        return;

    }

    //   ...and value-initialized for each k < i <= n;

    if (ArrayLength > Args.size()) {

      InitializedEntity SubEntity = InitializedEntity::InitializeElement(

          S.getASTContext(), Args.size(), Entity);

      InitializationKind SubKind = InitializationKind::CreateValue(

          Kind.getLocation(), Kind.getLocation(), Kind.getLocation(), true);

      if (!HandleInitializedEntity(SubEntity, SubKind, nullptr, &ArrayFiller))

        return;

    }



    if (ResultType.isNull()) {

      ResultType = S.Context.getConstantArrayType(

          AT->getElementType(), llvm::APInt(/*numBits=*/32, ArrayLength),

          /*SizeExpr=*/nullptr, ArrayType::Normal, 0);

    }

  } else if (auto *RT = Entity.getType()->getAs<RecordType>()) {

    bool IsUnion = RT->isUnionType();

    const CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());



    if (!IsUnion) {

      for (const CXXBaseSpecifier &Base : RD->bases()) {

        InitializedEntity SubEntity = InitializedEntity::InitializeBase(

            S.getASTContext(), &Base, false, &Entity);

        if (EntityIndexToProcess < Args.size()) {

          // C++ [dcl.init]p16.6.2.2.

          //   ...the object is initialized is follows. Let e1, ..., en be the

          //   elements of the aggregate([dcl.init.aggr]). Let x1, ..., xk be

          //   the elements of the expression-list...The element ei is

          //   copy-initialized with xi for 1 <= i <= k.

          Expr *E = Args[EntityIndexToProcess];

          InitializationKind SubKind = InitializationKind::CreateForInit(

              E->getExprLoc(), /*isDirectInit=*/false, E);

          if (!HandleInitializedEntity(SubEntity, SubKind, E))

            return;

        } else {

          // We've processed all of the args, but there are still base classes

          // that have to be initialized.

          // C++ [dcl.init]p17.6.2.2

          //   The remaining elements...otherwise are value initialzed

          InitializationKind SubKind = InitializationKind::CreateValue(

              Kind.getLocation(), Kind.getLocation(), Kind.getLocation(),

              /*IsImplicit=*/true);

          if (!HandleInitializedEntity(SubEntity, SubKind, nullptr))

            return;

        }

        EntityIndexToProcess++;

      }

    }



    for (FieldDecl *FD : RD->fields()) {

      // Unnamed bitfields should not be initialized at all, either with an arg

      // or by default.

      if (IsMember && cast<FieldDecl>(SubEntity.getDecl())->isUnnamedBitfield())

      if (FD->isUnnamedBitfield())

        continue;



      if (ArgIndexToProcess < Args.size()) {

        // There are still expressions in Args that haven't been processed.

        // Let's match them to the current entity to initialize.

        Expr *E = Args[ArgIndexToProcess++];

      InitializedEntity SubEntity =

          InitializedEntity::InitializeMemberFromParenAggInit(FD);



      if (EntityIndexToProcess < Args.size()) {

        //   ...The element ei is copy-initialized with xi for 1 <= i <= k.

        Expr *E = Args[EntityIndexToProcess];



        // Incomplete array types indicate flexible array members. Do not allow

        // paren list initializations of structs with these members, as GCC

        // doesn't either.

        if (IsMember) {

          auto *FD = cast<FieldDecl>(SubEntity.getDecl());

        if (FD->getType()->isIncompleteArrayType()) {

          if (!VerifyOnly) {

            S.Diag(E->getBeginLoc(), diag::err_flexible_array_init)
@@ -5346,136 +5464,70 @@ static void TryOrBuildParenListInitialization( 
          }

          Sequence.SetFailed(

              InitializationSequence::FK_ParenthesizedListInitFailed);

            return false;

          }

          return;

        }



        InitializationKind SubKind = InitializationKind::CreateForInit(

            E->getExprLoc(), /*isDirectInit=*/false, E);

        InitializationSequence SubSeq(S, SubEntity, SubKind, E);



        if (SubSeq.Failed()) {

          if (!VerifyOnly)

            SubSeq.Diagnose(S, SubEntity, SubKind, E);

          else

            Sequence.SetFailed(

                InitializationSequence::FK_ParenthesizedListInitFailed);

        if (!HandleInitializedEntity(SubEntity, SubKind, E))

          return;



          return false;

        }

        if (!VerifyOnly) {

          ExprResult ER = SubSeq.Perform(S, SubEntity, SubKind, E);

          InitExprs.push_back(ER.get());

          if (IsMember && IsUnionType)

            InitializedFieldInUnion = cast<FieldDecl>(SubEntity.getDecl());

        // Unions should have only one initializer expression, so we bail out

        // after processing the first field. If there are more initializers then

        // it will be caught when we later check whether EntityIndexToProcess is

        // less than Args.size();

        if (IsUnion) {

          InitializedFieldInUnion = FD;

          EntityIndexToProcess = 1;

          break;

        }

      } else {

        // We've processed all of the args, but there are still entities that

        // We've processed all of the args, but there are still members that

        // have to be initialized.

        if (IsMember) {

          // C++ [dcl.init]p17.6.2.2

          //   The remaining elements are initialized with their default member

          //   initializers, if any

          auto *FD = cast<FieldDecl>(SubEntity.getDecl());

        if (FD->hasInClassInitializer()) {

          if (!VerifyOnly) {

            // C++ [dcl.init]p16.6.2.2

            //   The remaining elements are initialized with their default

            //   member initializers, if any

            ExprResult DIE = S.BuildCXXDefaultInitExpr(FD->getLocation(), FD);

            if (DIE.isInvalid())

                return false;

              return;

            S.checkInitializerLifetime(SubEntity, DIE.get());

            InitExprs.push_back(DIE.get());

          }

            continue;

          }

        }

        // Remaining class elements without default member initializers and

        // array elements are value initialized:

        //

        } else {

          // C++ [dcl.init]p17.6.2.2

          //   The remaining elements...otherwise are value initialzed

        //

        // C++ [dcl.init]p17.5

        //   if the destination type is an array, the object is initialized as

        // . follows. Let x1, . . . , xk be the elements of the expression-list

        //   ...Let n denote the array size...the ith array element is...value-

        //   initialized for each k < i <= n.

        InitializationKind SubKind = InitializationKind::CreateValue(

            Kind.getLocation(), Kind.getLocation(), Kind.getLocation(), true);

        InitializationSequence SubSeq(S, SubEntity, SubKind, std::nullopt);

        if (SubSeq.Failed()) {

          if (!VerifyOnly)

            SubSeq.Diagnose(S, SubEntity, SubKind, std::nullopt);

          return false;

        }

          if (FD->getType()->isReferenceType()) {

            Sequence.SetFailed(

                InitializationSequence::FK_ParenthesizedListInitFailed);

            if (!VerifyOnly) {

          ExprResult ER = SubSeq.Perform(S, SubEntity, SubKind, std::nullopt);

          if (SubEntity.getKind() == InitializedEntity::EK_ArrayElement) {

            ArrayFiller = ER.get();

            return true;

              SourceRange SR = Kind.getParenOrBraceRange();

              S.Diag(SR.getEnd(), diag::err_init_reference_member_uninitialized)

                  << FD->getType() << SR;

              S.Diag(FD->getLocation(), diag::note_uninit_reference_member);

            }

          InitExprs.push_back(ER.get());

            return;

          }

          InitializationKind SubKind = InitializationKind::CreateValue(

              Kind.getLocation(), Kind.getLocation(), Kind.getLocation(), true);

          if (!HandleInitializedEntity(SubEntity, SubKind, nullptr))

            return;

        }

      }

    return true;

  };



  if (const ArrayType *AT =

          S.getASTContext().getAsArrayType(Entity.getType())) {



    SmallVector<InitializedEntity, 4> ElementEntities;

    uint64_t ArrayLength;

    // C++ [dcl.init]p17.5

    //   if the destination type is an array, the object is initialized as

    //   follows. Let x1, . . . , xk be the elements of the expression-list. If

    //   the destination type is an array of unknown bound, it is define as

    //   having k elements.

    if (const ConstantArrayType *CAT =

            S.getASTContext().getAsConstantArrayType(Entity.getType()))

      ArrayLength = CAT->getSize().getZExtValue();

    else

      ArrayLength = Args.size();



    if (ArrayLength >= Args.size()) {

      for (uint64_t I = 0; I < ArrayLength; ++I)

        ElementEntities.push_back(

            InitializedEntity::InitializeElement(S.getASTContext(), I, Entity));



      if (!ProcessEntities(ElementEntities))

        return;



      ResultType = S.Context.getConstantArrayType(

          AT->getElementType(), llvm::APInt(/*numBits=*/32, ArrayLength),

          nullptr, ArrayType::Normal, 0);

      EntityIndexToProcess++;

    }

  } else if (auto *RT = Entity.getType()->getAs<RecordType>()) {

    const CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());



    auto BaseRange = map_range(RD->bases(), [&](auto &base) {

      return InitializedEntity::InitializeBase(S.getASTContext(), &base, false,

                                               &Entity);

    });

    auto FieldRange = map_range(RD->fields(), [](auto *field) {

      return InitializedEntity::InitializeMember(field);

    });



    if (!ProcessEntities(BaseRange))

      return;



    if (!ProcessEntities(FieldRange))

      return;



    ResultType = Entity.getType();

  }



  // Not all of the args have been processed, so there must've been more args

  // than were required to initialize the element.

  if (ArgIndexToProcess < Args.size()) {

  if (EntityIndexToProcess < Args.size()) {

    Sequence.SetFailed(InitializationSequence::FK_ParenthesizedListInitFailed);

    if (!VerifyOnly) {

      QualType T = Entity.getType();

      int InitKind = T->isArrayType() ? 0 : T->isUnionType() ? 3 : 4;

      SourceRange ExcessInitSR(Args[ArgIndexToProcess]->getBeginLoc(),

      SourceRange ExcessInitSR(Args[EntityIndexToProcess]->getBeginLoc(),

                               Args.back()->getEndLoc());

      S.Diag(Kind.getLocation(), diag::err_excess_initializers)

          << InitKind << ExcessInitSR;
@@ -6441,6 +6493,7 @@ getAssignmentAction(const InitializedEntity &Entity, bool Diagnose = false) { 
    return Sema::AA_Converting;



  case InitializedEntity::EK_Member:

  case InitializedEntity::EK_ParenAggInitMember:

  case InitializedEntity::EK_Binding:

  case InitializedEntity::EK_ArrayElement:

  case InitializedEntity::EK_VectorElement:
@@ -6461,6 +6514,7 @@ static bool shouldBindAsTemporary(const InitializedEntity &Entity) { 
  switch (Entity.getKind()) {

  case InitializedEntity::EK_ArrayElement:

  case InitializedEntity::EK_Member:

  case InitializedEntity::EK_ParenAggInitMember:

  case InitializedEntity::EK_Result:

  case InitializedEntity::EK_StmtExprResult:

  case InitializedEntity::EK_New:
@@ -6505,6 +6559,7 @@ static bool shouldDestroyEntity(const InitializedEntity &Entity) { 
      return false;



    case InitializedEntity::EK_Member:

    case InitializedEntity::EK_ParenAggInitMember:

    case InitializedEntity::EK_Binding:

    case InitializedEntity::EK_Variable:

    case InitializedEntity::EK_Parameter:
@@ -6541,6 +6596,7 @@ static SourceLocation getInitializationLoc(const InitializedEntity &Entity, 


  case InitializedEntity::EK_ArrayElement:

  case InitializedEntity::EK_Member:

  case InitializedEntity::EK_ParenAggInitMember:

  case InitializedEntity::EK_Parameter:

  case InitializedEntity::EK_Parameter_CF_Audited:

  case InitializedEntity::EK_TemplateParameter:
@@ -7107,7 +7163,15 @@ static LifetimeResult getEntityLifetime( 
  case InitializedEntity::EK_Exception:

    // FIXME: Can we diagnose lifetime problems with exceptions?

    return {nullptr, LK_FullExpression};



  case InitializedEntity::EK_ParenAggInitMember:

    //   -- A temporary object bound to a reference element of an aggregate of

    //      class type initialized from a parenthesized expression-list

    //      [dcl.init, 9.3] persists until the completion of the full-expression

    //      containing the expression-list.

    return {nullptr, LK_FullExpression};

  }



  llvm_unreachable("unknown entity kind");

}
@@ -9223,7 +9287,9 @@ ExprResult InitializationSequence::Perform(Sema &S, 
    S.checkInitializerLifetime(Entity, Init);



  // Diagnose non-fatal problems with the completed initialization.

  if (Entity.getKind() == InitializedEntity::EK_Member &&

  if (InitializedEntity::EntityKind EK = Entity.getKind();

      (EK == InitializedEntity::EK_Member ||

       EK == InitializedEntity::EK_ParenAggInitMember) &&

      cast<FieldDecl>(Entity.getDecl())->isBitField())

    S.CheckBitFieldInitialization(Kind.getLocation(),

                                  cast<FieldDecl>(Entity.getDecl()),
@@ -9677,7 +9743,8 @@ bool InitializationSequence::Diagnose(Sema &S, 
      case OR_No_Viable_Function:

        if (Kind.getKind() == InitializationKind::IK_Default &&

            (Entity.getKind() == InitializedEntity::EK_Base ||

             Entity.getKind() == InitializedEntity::EK_Member) &&

             Entity.getKind() == InitializedEntity::EK_Member ||

             Entity.getKind() == InitializedEntity::EK_ParenAggInitMember) &&

            isa<CXXConstructorDecl>(S.CurContext)) {

          // This is implicit default initialization of a member or

          // base within a constructor. If no viable function was