[PATCH] D25051: Fix PR 10758: Infinite recursion when dealing with copy-initialization

[Alex Lorenz via cfe-commits]

arphaman created this revision.
arphaman added a reviewer: rsmith.
arphaman added a subscriber: cfe-commits.
arphaman set the repository for this revision to rL LLVM.

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&);
  ^
```

Repository:
  rL LLVM

https://reviews.llvm.org/D25051

Files:
  include/clang/Sema/Sema.h
  lib/Sema/SemaInit.cpp
  test/SemaCXX/constructor-initializer.cpp

Index: test/SemaCXX/constructor-initializer.cpp
===================================================================
--- test/SemaCXX/constructor-initializer.cpp
+++ test/SemaCXX/constructor-initializer.cpp
@@ -302,3 +302,22 @@
   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'}}
+}
+}
Index: lib/Sema/SemaInit.cpp
===================================================================
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -7932,7 +7932,47 @@
                                                            AllowExplicit);
   InitializationSequence Seq(*this, Entity, Kind, InitE, TopLevelOfInitList);
 
+  // Fix for PR 10758: 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.NumConversions > 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;
 }
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -1023,6 +1023,10 @@
   /// same special member, we should act as if it is not yet declared.
   llvm::SmallSet<SpecialMemberDecl, 4> SpecialMembersBeingDeclared;
 
+  /// 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);
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D25051.72913.patch
Type: text/x-patch
Size: 3902 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160928/28e96192/attachment.bin>