vsk created this revision.

The coverage implementation marks functions which won't be emitted as
'deferred', so that it can emit empty coverage regions for them later
(once their linkages are known).

Functions in dependent contexts are an exception: if there isn't a full
instantiation of a function, it shouldn't be marked 'deferred'. We've
been breaking that rule without much consequence because we just ended
up with useless, extra, empty coverage mappings. With PR32679, this
behavior finally caused a crash, because clang marked a partial template
specialization as 'deferred', causing the MS mangler to choke in its
delayed-template-parsing mode:

  error: cannot mangle this template type parameter type yet
  (http://bugs.llvm.org/show_bug.cgi?id=32679)

Fix this by checking if a decl's context is a dependent context before
marking it 'deferred'.

Based on a patch by Adam Folwarczny!


https://reviews.llvm.org/D32144

Files:
  lib/CodeGen/ModuleBuilder.cpp
  test/CoverageMapping/pr32679.cpp


Index: test/CoverageMapping/pr32679.cpp
===================================================================
--- /dev/null
+++ test/CoverageMapping/pr32679.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -cc1 -triple i686-pc-windows-msvc19.0.0 -emit-obj 
-fprofile-instrument=clang -std=c++14 -fdelayed-template-parsing 
-fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -main-file-name 
pr32679.cpp -o - %s | FileCheck %s -check-prefix=MSABI -implicit-check-not=f2
+// RUN: %clang_cc1 -cc1 -triple %itanium_abi_triple -emit-obj 
-fprofile-instrument=clang -std=c++14 -fcoverage-mapping -dump-coverage-mapping 
-emit-llvm-only -main-file-name pr32679.cpp -o - %s | FileCheck %s 
-check-prefix=ITANIUM -implicit-check-not=f2
+
+template <typename T, int S1>
+struct CreateSpecialization;
+
+template <typename T>
+struct CreateSpecialization<T, 0> {
+  static constexpr T f1() { return 0; }
+  static constexpr T f2() { return 0; }
+};
+
+int main() {
+  CreateSpecialization<int, 0>::f1();
+
+  // Don't emit coverage mapping info for functions in dependent contexts.
+  //
+  // E.g we never fully instantiate CreateSpecialization<T, 0>::f2(), so there
+  // should be no mapping for it.
+
+  return 0;
+}
+
+// MSABI: main:
+// MSABI-NEXT:   File 0, [[@LINE-12]]:12 -> [[@LINE-3]]:2 = #0
+// MSABI-NEXT: ?f1@?$CreateSpecialization@H$0A@@@SAHXZ:
+// MSABI-NEXT:   File 0, [[@LINE-18]]:27 -> [[@LINE-18]]:40 = #0
+
+// ITANIUM: main:
+// ITANIUM-NEXT:   File 0, [[@LINE-17]]:12 -> [[@LINE-8]]:2 = #0
+// ITANIUM-NEXT: _ZN20CreateSpecializationIiLi0EE2f1Ev:
+// ITANIUM-NEXT:   File 0, [[@LINE-23]]:27 -> [[@LINE-23]]:40 = #0
Index: lib/CodeGen/ModuleBuilder.cpp
===================================================================
--- lib/CodeGen/ModuleBuilder.cpp
+++ lib/CodeGen/ModuleBuilder.cpp
@@ -197,7 +197,8 @@
       // Provide some coverage mapping even for methods that aren't emitted.
       // Don't do this for templated classes though, as they may not be
       // instantiable.
-      if (!MD->getParent()->getDescribedClassTemplate())
+      if (!MD->getParent()->isDependentContext() &&
+          !MD->getParent()->getDescribedClassTemplate())
         Builder->AddDeferredUnusedCoverageMapping(MD);
     }
 


Index: test/CoverageMapping/pr32679.cpp
===================================================================
--- /dev/null
+++ test/CoverageMapping/pr32679.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -cc1 -triple i686-pc-windows-msvc19.0.0 -emit-obj -fprofile-instrument=clang -std=c++14 -fdelayed-template-parsing -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -main-file-name pr32679.cpp -o - %s | FileCheck %s -check-prefix=MSABI -implicit-check-not=f2
+// RUN: %clang_cc1 -cc1 -triple %itanium_abi_triple -emit-obj -fprofile-instrument=clang -std=c++14 -fcoverage-mapping -dump-coverage-mapping -emit-llvm-only -main-file-name pr32679.cpp -o - %s | FileCheck %s -check-prefix=ITANIUM -implicit-check-not=f2
+
+template <typename T, int S1>
+struct CreateSpecialization;
+
+template <typename T>
+struct CreateSpecialization<T, 0> {
+  static constexpr T f1() { return 0; }
+  static constexpr T f2() { return 0; }
+};
+
+int main() {
+  CreateSpecialization<int, 0>::f1();
+
+  // Don't emit coverage mapping info for functions in dependent contexts.
+  //
+  // E.g we never fully instantiate CreateSpecialization<T, 0>::f2(), so there
+  // should be no mapping for it.
+
+  return 0;
+}
+
+// MSABI: main:
+// MSABI-NEXT:   File 0, [[@LINE-12]]:12 -> [[@LINE-3]]:2 = #0
+// MSABI-NEXT: ?f1@?$CreateSpecialization@H$0A@@@SAHXZ:
+// MSABI-NEXT:   File 0, [[@LINE-18]]:27 -> [[@LINE-18]]:40 = #0
+
+// ITANIUM: main:
+// ITANIUM-NEXT:   File 0, [[@LINE-17]]:12 -> [[@LINE-8]]:2 = #0
+// ITANIUM-NEXT: _ZN20CreateSpecializationIiLi0EE2f1Ev:
+// ITANIUM-NEXT:   File 0, [[@LINE-23]]:27 -> [[@LINE-23]]:40 = #0
Index: lib/CodeGen/ModuleBuilder.cpp
===================================================================
--- lib/CodeGen/ModuleBuilder.cpp
+++ lib/CodeGen/ModuleBuilder.cpp
@@ -197,7 +197,8 @@
       // Provide some coverage mapping even for methods that aren't emitted.
       // Don't do this for templated classes though, as they may not be
       // instantiable.
-      if (!MD->getParent()->getDescribedClassTemplate())
+      if (!MD->getParent()->isDependentContext() &&
+          !MD->getParent()->getDescribedClassTemplate())
         Builder->AddDeferredUnusedCoverageMapping(MD);
     }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to