[clang] c6c8d4a - [modules] Fix crash in call to `FunctionDecl::setPure()`

[Xun Li via cfe-commits]

Author: Andrew Gallagher
Date: 2020-11-18T11:55:29-08:00
New Revision: c6c8d4a13ebd5ce1c3c7e8632312ab8c2dc6afa0

URL: https://github.com/llvm/llvm-project/commit/c6c8d4a13ebd5ce1c3c7e8632312ab8c2dc6afa0
DIFF: https://github.com/llvm/llvm-project/commit/c6c8d4a13ebd5ce1c3c7e8632312ab8c2dc6afa0.diff

LOG: [modules] Fix crash in call to `FunctionDecl::setPure()`

In some cases, when deserializing a `CXXMethodDecl` of a `CXXSpecializationTemplateDecl`,
the call to `FunctionDecl::setPure()` happens before the `DefinitionData` member has been
populated (which appears to happen lower down in a `mergeRedeclarable` call), causing a
crash (https://reviews.llvm.org/P8228).

This diff fixes this by deferring the `FunctionDecl::setPure()` till after the `DefinitionData` has
been filled in.

Reviewed By: lxfind

Differential Revision: https://reviews.llvm.org/D86853

Added: 
    clang/test/Modules/Inputs/set-pure-crash/a.h
    clang/test/Modules/Inputs/set-pure-crash/b.h
    clang/test/Modules/Inputs/set-pure-crash/c.h
    clang/test/Modules/Inputs/set-pure-crash/module.modulemap
    clang/test/Modules/set-pure-crash.cpp

Modified: 
    clang/lib/Serialization/ASTReaderDecl.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 797232885687..6bfb9bd783b5 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -868,7 +868,10 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
   FD->setInlineSpecified(Record.readInt());
   FD->setImplicitlyInline(Record.readInt());
   FD->setVirtualAsWritten(Record.readInt());
-  FD->setPure(Record.readInt());
+  // We defer calling `FunctionDecl::setPure()` here as for methods of
+  // `CXXTemplateSpecializationDecl`s, we may not have connected up the
+  // definition (which is required for `setPure`).
+  const bool Pure = Record.readInt();
   FD->setHasInheritedPrototype(Record.readInt());
   FD->setHasWrittenPrototype(Record.readInt());
   FD->setDeletedAsWritten(Record.readInt());
@@ -1015,6 +1018,10 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
   }
   }
 
+  // Defer calling `setPure` until merging above has guaranteed we've set
+  // `DefinitionData` (as this will need to access it).
+  FD->setPure(Pure);
+
   // Read in the parameters.
   unsigned NumParams = Record.readInt();
   SmallVector<ParmVarDecl *, 16> Params;

diff  --git a/clang/test/Modules/Inputs/set-pure-crash/a.h b/clang/test/Modules/Inputs/set-pure-crash/a.h
new file mode 100644
index 000000000000..f52458e0daeb
--- /dev/null
+++ b/clang/test/Modules/Inputs/set-pure-crash/a.h
@@ -0,0 +1,11 @@
+#pragma once
+
+struct Tag {};
+
+template <typename T>
+class Base {
+public:
+  virtual void func() = 0;
+};
+
+Base<Tag> bar();

diff  --git a/clang/test/Modules/Inputs/set-pure-crash/b.h b/clang/test/Modules/Inputs/set-pure-crash/b.h
new file mode 100644
index 000000000000..eef7c2b9faaa
--- /dev/null
+++ b/clang/test/Modules/Inputs/set-pure-crash/b.h
@@ -0,0 +1,14 @@
+#pragma once
+
+#include "a.h"
+#include "c.h"
+
+template <typename Fun, typename = simple<Fun>>
+void foo(Fun) {}
+
+class Child : public Base<Tag> {
+public:
+  void func() {
+    foo([]() {});
+  }
+};

diff  --git a/clang/test/Modules/Inputs/set-pure-crash/c.h b/clang/test/Modules/Inputs/set-pure-crash/c.h
new file mode 100644
index 000000000000..d5b7cd19461f
--- /dev/null
+++ b/clang/test/Modules/Inputs/set-pure-crash/c.h
@@ -0,0 +1,5 @@
+#pragma once
+
+template <typename>
+struct simple {
+};

diff  --git a/clang/test/Modules/Inputs/set-pure-crash/module.modulemap b/clang/test/Modules/Inputs/set-pure-crash/module.modulemap
new file mode 100644
index 000000000000..50bbe84e5d07
--- /dev/null
+++ b/clang/test/Modules/Inputs/set-pure-crash/module.modulemap
@@ -0,0 +1,11 @@
+module a {
+  header "a.h"
+}
+
+module b {
+  header "b.h"
+}
+
+module c {
+  header "c.h"
+}

diff  --git a/clang/test/Modules/set-pure-crash.cpp b/clang/test/Modules/set-pure-crash.cpp
new file mode 100644
index 000000000000..197e0cb00d27
--- /dev/null
+++ b/clang/test/Modules/set-pure-crash.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I %S/Inputs/set-pure-crash -verify %s -o %t
+
+// expected-no-diagnostics
+
+#include "b.h"
+#include "c.h"
+
+auto t = simple<const char *>();