[clang] [Clang] [Sema] Do not attempt to dump the layout of dependent types when `-fdump-record-layouts-complete` is passed (PR #83688)

via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 3 02:17:05 PST 2024


https://github.com/Sirraide updated https://github.com/llvm/llvm-project/pull/83688

>From 4abc148104769bd756042c73edeb7ee54397a05f Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Sat, 2 Mar 2024 20:41:22 +0100
Subject: [PATCH 1/5] [Clang] [Sema] Do not attempt to dump the layout of
 dependent types

---
 clang/lib/AST/Decl.cpp              |  2 +-
 clang/test/Layout/dump-complete.cpp | 29 ++++++++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 5d6bb72a208a1a..d069cd65732310 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -5042,7 +5042,7 @@ void RecordDecl::completeDefinition() {
 
   // Layouts are dumped when computed, so if we are dumping for all complete
   // types, we need to force usage to get types that wouldn't be used elsewhere.
-  if (Ctx.getLangOpts().DumpRecordLayoutsComplete)
+  if (Ctx.getLangOpts().DumpRecordLayoutsComplete && !isDependentType())
     (void)Ctx.getASTRecordLayout(this);
 }
 
diff --git a/clang/test/Layout/dump-complete.cpp b/clang/test/Layout/dump-complete.cpp
index 9ccbf477c7052e..0a91d3329e6921 100644
--- a/clang/test/Layout/dump-complete.cpp
+++ b/clang/test/Layout/dump-complete.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -emit-llvm-only -fdump-record-layouts-complete %s | FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only -fdump-record-layouts-complete %s | FileCheck %s
 
 struct a {
   int x;
@@ -12,7 +12,34 @@ class c {};
 
 class d;
 
+template <typename>
+struct s {
+  int x;
+};
+
+template <typename T>
+struct ts {
+  T x;
+};
+
+void f() {
+  ts<int> a;
+  ts<double> b;
+}
+
+namespace gh83684 {
+template <class Pointer>
+struct AllocationResult {
+  Pointer ptr = nullptr;
+  int count = 0;
+};
+}
+
 // CHECK:          0 | struct a
 // CHECK:          0 | struct b
 // CHECK:          0 | class c
+// CHECK:          0 | struct ts<int>
+// CHECK:          0 | struct ts<double>
 // CHECK-NOT:      0 | class d
+// CHECK-NOT:      0 | struct s
+// CHECK-NOT:      0 | struct AllocationResult

>From 3056a8414cae4648a3cc8244cacef29e6dc00564 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Sat, 2 Mar 2024 20:45:24 +0100
Subject: [PATCH 2/5] [Clang] Add release note

---
 clang/docs/ReleaseNotes.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index f44fef28b9f17f..69cf0cb643e6aa 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -308,6 +308,10 @@ Miscellaneous Bug Fixes
 Miscellaneous Clang Crashes Fixed
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
+- Do not attempt to dump the layout of dependent types when ``-fdump-record-layouts-complete``
+  is passed.
+  Fixes (`#83684 <https://github.com/llvm/llvm-project/issues/83684>`_).
+
 OpenACC Specific Changes
 ------------------------
 

>From cdf14b7cef9e08d932b649dc2724362aeea56a40 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Sun, 3 Mar 2024 10:44:38 +0100
Subject: [PATCH 3/5] [Clang] Add comment and test case for explicit
 specialisation

---
 clang/lib/AST/Decl.cpp              | 3 +++
 clang/test/Layout/dump-complete.cpp | 8 ++++++++
 2 files changed, 11 insertions(+)

diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index d069cd65732310..a3e4e13ffdc74d 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -5042,6 +5042,9 @@ void RecordDecl::completeDefinition() {
 
   // Layouts are dumped when computed, so if we are dumping for all complete
   // types, we need to force usage to get types that wouldn't be used elsewhere.
+  //
+  // If the type is dependent, then we can't compute its layout because there
+  // is no way for us to know the size or alignment of a dependent type.
   if (Ctx.getLangOpts().DumpRecordLayoutsComplete && !isDependentType())
     (void)Ctx.getASTRecordLayout(this);
 }
diff --git a/clang/test/Layout/dump-complete.cpp b/clang/test/Layout/dump-complete.cpp
index 0a91d3329e6921..7fed145084dccb 100644
--- a/clang/test/Layout/dump-complete.cpp
+++ b/clang/test/Layout/dump-complete.cpp
@@ -22,9 +22,15 @@ struct ts {
   T x;
 };
 
+template <>
+struct ts<void> {
+  float f;
+};
+
 void f() {
   ts<int> a;
   ts<double> b;
+  ts<void> c;
 }
 
 namespace gh83684 {
@@ -38,6 +44,8 @@ struct AllocationResult {
 // CHECK:          0 | struct a
 // CHECK:          0 | struct b
 // CHECK:          0 | class c
+// CHECK:          0 | struct ts<void>
+// CHECK-NEXT:     0 |   float
 // CHECK:          0 | struct ts<int>
 // CHECK:          0 | struct ts<double>
 // CHECK-NOT:      0 | class d

>From ac9b16aa4a26a9926e221937fc48150640acd09e Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Sun, 3 Mar 2024 11:16:04 +0100
Subject: [PATCH 4/5] [Clang] Don't try to print the layout of an invalid decl

---
 clang/lib/AST/Decl.cpp                      | 7 +++++--
 clang/test/Layout/dump-complete-invalid.cpp | 6 ++++++
 2 files changed, 11 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/Layout/dump-complete-invalid.cpp

diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index a3e4e13ffdc74d..57a92357f6e5ba 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -5044,8 +5044,11 @@ void RecordDecl::completeDefinition() {
   // types, we need to force usage to get types that wouldn't be used elsewhere.
   //
   // If the type is dependent, then we can't compute its layout because there
-  // is no way for us to know the size or alignment of a dependent type.
-  if (Ctx.getLangOpts().DumpRecordLayoutsComplete && !isDependentType())
+  // is no way for us to know the size or alignment of a dependent type. Also
+  // ignore declarations marked as invalid since 'getASTRecordLayout()' asserts
+  // on that.
+  if (Ctx.getLangOpts().DumpRecordLayoutsComplete && !isDependentType() &&
+      !isInvalidDecl())
     (void)Ctx.getASTRecordLayout(this);
 }
 
diff --git a/clang/test/Layout/dump-complete-invalid.cpp b/clang/test/Layout/dump-complete-invalid.cpp
new file mode 100644
index 00000000000000..d3ec1b382ce7c6
--- /dev/null
+++ b/clang/test/Layout/dump-complete-invalid.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -verify -fsyntax-only -fdump-record-layouts-complete %s
+
+struct Incomplete; // expected-note {{forward declaration}}
+
+// Check we don't crash on trying to print out an invalid declaration.
+struct Invalid : Incomplete {}; // expected-error {{base class has incomplete type}}

>From 120c52be35c666046a17caa3cfa8a359c6eb9dd5 Mon Sep 17 00:00:00 2001
From: Sirraide <aeternalmail at gmail.com>
Date: Sun, 3 Mar 2024 11:16:55 +0100
Subject: [PATCH 5/5] [Clang] Update release notes

---
 clang/docs/ReleaseNotes.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 69cf0cb643e6aa..89f96ee3819994 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -308,8 +308,8 @@ Miscellaneous Bug Fixes
 Miscellaneous Clang Crashes Fixed
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-- Do not attempt to dump the layout of dependent types when ``-fdump-record-layouts-complete``
-  is passed.
+- Do not attempt to dump the layout of dependent types or invalid declarations
+  when ``-fdump-record-layouts-complete`` is passed.
   Fixes (`#83684 <https://github.com/llvm/llvm-project/issues/83684>`_).
 
 OpenACC Specific Changes



More information about the cfe-commits mailing list