Fix PR 10758: Infinite recursion when dealing with copy-initialization

[arphaman]

Summary

This patch fixes a bug that's tracked by PR 10758 and duplicates like PR 30343.

The bug causes clang to crash with a stack overflow while recursing infinitely trying to perform copy-initialization on a type without a copy constructor but with a constructor that accepts another type that can be constructed using the original type. This example shows the structures and code that causes this behavior:

struct Bar;
struct Foo {
  Foo(Bar);
};
struct Bar {
  Bar(const Foo&);
  Bar(Bar&);
};
Foo foo(const Bar &b) {
  return b;
}

The patch fixes this bug by detecting the recursive behavior and failing correctly with an appropriate error message. It also tries to provide a meaningful diagnostic note about the constructor which leads to this behavior, like in the example shown below:

foo.cpp:6:3: note: candidate constructor not viable: no known conversion from 'const Bar' to 'const Foo &' for 1st argument
  Bar(const Foo&);
  ^

[arphaman]

Ping

[arphaman]

Rebased the patch.

[v.g.vassilev]

I am not very familiar with this code here. This seems a reasonable fix to me. Unless @rsmith and @ahatanak have objections, please go ahead and land it some time next week.

[cynecx]

Would it be possible to land this patch? I would really like to see this fixed.

[cynecx]

Gentle ping.

[arphaman]

I will commit it today.

cfe/trunk/include/clang/Sema/Sema.h

public:
   llvm::SmallSet<SpecialMemberDecl, 4> SpecialMembersBeingDeclared;
 
   /// The function definitions which were renamed as part of typo-correction
   /// to match their respective declarations. We want to keep track of them
   /// to ensure that we don't emit a "redefinition" error if we encounter a
   /// correctly named definition after the renamed definition.
   llvm::SmallPtrSet<const NamedDecl *, 4> TypoCorrectedFunctionDefinitions;
 
+  /// Stack of types that correspond to the parameter entities that are
+  /// currently being copy-initialized. Can be empty.
+  llvm::SmallVector<QualType, 4> CurrentParameterCopyTypes;
+
   void ReadMethodPool(Selector Sel);
   void updateOutOfDateSelector(Selector Sel);
 
   /// Private Helper predicate to check for 'self'.
   bool isSelfExpr(Expr *RExpr);
   bool isSelfExpr(Expr *RExpr, const ObjCMethodDecl *Method);
 
   /// \brief Cause the active diagnostic on the DiagosticsEngine to be

cfe/trunk/lib/Sema/SemaInit.cpp

Sema::PerformCopyInitialization(const InitializedEntity &Entity,
   if (EqualLoc.isInvalid())
     EqualLoc = InitE->getLocStart();
 
   InitializationKind Kind = InitializationKind::CreateCopy(InitE->getLocStart(),
                                                            EqualLoc,
                                                            AllowExplicit);
   InitializationSequence Seq(*this, Entity, Kind, InitE, TopLevelOfInitList);
 
+  // Prevent infinite recursion when performing parameter copy-initialization.
+  const bool ShouldTrackCopy =
+      Entity.isParameterKind() && Seq.isConstructorInitialization();
+  if (ShouldTrackCopy) {
+    if (llvm::find(CurrentParameterCopyTypes, Entity.getType()) !=
+        CurrentParameterCopyTypes.end()) {
+      Seq.SetOverloadFailure(
+          InitializationSequence::FK_ConstructorOverloadFailed,
+          OR_No_Viable_Function);
+
+      // Try to give a meaningful diagnostic note for the problematic
+      // constructor.
+      const auto LastStep = Seq.step_end() - 1;
+      assert(LastStep->Kind ==
+             InitializationSequence::SK_ConstructorInitialization);
+      const FunctionDecl *Function = LastStep->Function.Function;
+      auto Candidate =
+          llvm::find_if(Seq.getFailedCandidateSet(),
+                        [Function](const OverloadCandidate &Candidate) -> bool {
+                          return Candidate.Viable &&
+                                 Candidate.Function == Function &&
+                                 Candidate.Conversions.size() > 0;
+                        });
+      if (Candidate != Seq.getFailedCandidateSet().end() &&
+          Function->getNumParams() > 0) {
+        Candidate->Viable = false;
+        Candidate->FailureKind = ovl_fail_bad_conversion;
+        Candidate->Conversions[0].setBad(BadConversionSequence::no_conversion,
+                                         InitE,
+                                         Function->getParamDecl(0)->getType());
+      }
+    }
+    CurrentParameterCopyTypes.push_back(Entity.getType());
+  }
+
   ExprResult Result = Seq.Perform(*this, Entity, Kind, InitE);
 
+  if (ShouldTrackCopy)
+    CurrentParameterCopyTypes.pop_back();
+
   return Result;
 }
 
 QualType Sema::DeduceTemplateSpecializationFromInitializer(
     TypeSourceInfo *TSInfo, const InitializedEntity &Entity,
     const InitializationKind &Kind, MultiExprArg Inits) {
   auto *DeducedTST = dyn_cast<DeducedTemplateSpecializationType>(
       TSInfo->getType()->getContainedDeducedType());

cfe/trunk/test/SemaCXX/constructor-initializer.cpp

namespace PR12049 {
   };
 }
 
 namespace PR14073 {
   struct S1 { union { int n; }; S1() : n(n) {} };  // expected-warning {{field 'n' is uninitialized when used here}}
   struct S2 { union { union { int n; }; char c; }; S2() : n(n) {} };  // expected-warning {{field 'n' is uninitialized when used here}}
   struct S3 { struct { int n; }; S3() : n(n) {} };  // expected-warning {{field 'n' is uninitialized when used here}}
 }
+
+namespace PR10758 {
+struct A;
+struct B {
+  B (A const &); // expected-note 2 {{candidate constructor not viable: no known conversion from 'const PR10758::B' to 'const PR10758::A &' for 1st argument}}
+  B (B &); // expected-note 2 {{candidate constructor not viable: 1st argument ('const PR10758::B') would lose const qualifier}}
+};
+struct A {
+  A (B); // expected-note 2 {{passing argument to parameter here}}
+};
+
+B f(B const &b) {
+  return b; // expected-error {{no matching constructor for initialization of 'PR10758::B'}}
+}
+
+A f2(const B &b) {
+  return b; // expected-error {{no matching constructor for initialization of 'PR10758::B'}}
+}
+}