[PATCH] D65577: [ASTImporter] Import default expression of param before creating the param.

[Balázs Kéri via Phabricator via cfe-commits]

balazske created this revision.
Herald added subscribers: cfe-commits, gamesh411, Szelethus, dkrupp.
Herald added a reviewer: martong.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: clang.

The default expression of a parameter variable should be imported before
the parameter variable object is created. Otherwise the function is created
with an incomplete parameter variable (default argument is nullptr) and in
this intermediary state the expression is imported. This import can have
a reference to the incomplete parameter variable that causes crash.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65577

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/test/Analysis/Inputs/ctu-other.cpp
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/ctu-main.cpp


Index: clang/test/Analysis/ctu-main.cpp
===================================================================
--- clang/test/Analysis/ctu-main.cpp
+++ clang/test/Analysis/ctu-main.cpp
@@ -112,6 +112,8 @@
   clang_analyzer_eval(obj->fvcl(1) == 8);      // expected-warning{{FALSE}} expected-warning{{TRUE}}
 }
 
+extern int testDefParmIncompleteImport(int);
+
 int main() {
   clang_analyzer_eval(f(3) == 2); // expected-warning{{TRUE}}
   clang_analyzer_eval(f(4) == 3); // expected-warning{{TRUE}}
@@ -144,4 +146,6 @@
   clang_analyzer_eval(extSubSCN.a == 1); // expected-warning{{TRUE}}
   // clang_analyzer_eval(extSCC.a == 7); // TODO
   clang_analyzer_eval(extU.a == 4); // expected-warning{{TRUE}}
+
+  clang_analyzer_eval(testDefParmIncompleteImport(9) == 9); // expected-warning{{TRUE}}
 }
Index: clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
===================================================================
--- clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
+++ clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
@@ -25,3 +25,4 @@
 c:@extSubSCN ctu-other.cpp.ast
 c:@extSCC ctu-other.cpp.ast
 c:@extU ctu-other.cpp.ast
+c:@F at testDefParmIncompleteImport#I# ctu-other.cpp.ast
Index: clang/test/Analysis/Inputs/ctu-other.cpp
===================================================================
--- clang/test/Analysis/Inputs/ctu-other.cpp
+++ clang/test/Analysis/Inputs/ctu-other.cpp
@@ -131,3 +131,22 @@
   const unsigned int b;
 };
 U extU = {.a = 4};
+
+struct DefParmContext {
+  static const int I;
+  int f();
+};
+
+int fDefParm(int I = DefParmContext::I) {
+  return I;
+}
+
+int testDefParmIncompleteImport(int I) {
+  return fDefParm(I);
+}
+
+const int DefParmContext::I = 0;
+
+int DefParmContext::f() {
+  return fDefParm();
+}
Index: clang/lib/AST/ASTImporter.cpp
===================================================================
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -3842,6 +3842,15 @@
   else
     return Imp.takeError();
 
+  Expr *DefaultArg = nullptr;
+  if (D->hasUninstantiatedDefaultArg()) {
+    if (Error Err = importInto(DefaultArg, D->getUninstantiatedDefaultArg()))
+      return std::move(Err);
+  } else if (D->hasDefaultArg()) {
+    if (Error Err = importInto(DefaultArg, D->getDefaultArg()))
+      return std::move(Err);
+  }
+
   ParmVarDecl *ToParm;
   if (GetImportedOrCreateDecl(ToParm, D, Importer.getToContext(), DC,
                               ToInnerLocStart, ToLocation,
@@ -3853,19 +3862,12 @@
   // Set the default argument.
   ToParm->setHasInheritedDefaultArg(D->hasInheritedDefaultArg());
   ToParm->setKNRPromoted(D->isKNRPromoted());
-
   if (D->hasUninstantiatedDefaultArg()) {
-    if (auto ToDefArgOrErr = import(D->getUninstantiatedDefaultArg()))
-      ToParm->setUninstantiatedDefaultArg(*ToDefArgOrErr);
-    else
-      return ToDefArgOrErr.takeError();
+    ToParm->setUninstantiatedDefaultArg(DefaultArg);
   } else if (D->hasUnparsedDefaultArg()) {
     ToParm->setUnparsedDefaultArg();
   } else if (D->hasDefaultArg()) {
-    if (auto ToDefArgOrErr = import(D->getDefaultArg()))
-      ToParm->setDefaultArg(*ToDefArgOrErr);
-    else
-      return ToDefArgOrErr.takeError();
+    ToParm->setDefaultArg(DefaultArg);
   }
 
   if (D->isObjCMethodParameter()) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D65577.212809.patch
Type: text/x-patch
Size: 3303 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190801/b9d49de8/attachment.bin>