[Lldb-commits] [lldb] [lldb] Infer MSInheritanceAttr for CXXRecordDecl with DWARF on Windows (PR #115177)

Stefan Gränitz via lldb-commits lldb-commits at lists.llvm.org
Fri Nov 8 05:16:11 PST 2024


https://github.com/weliveindetail updated https://github.com/llvm/llvm-project/pull/115177

>From 32d7b7d5261e651851e5a36c5fd5ae9d927a9fb3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Wed, 6 Nov 2024 15:45:03 +0100
Subject: [PATCH 1/6] [lldb] Infer MSInheritanceAttr for CXXRecordDecl with
 DWARF on Windows

---
 .../Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp      | 5 +++++
 lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp  | 8 +++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index a30d898a93cc4d..319540389c05ee 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -2138,6 +2138,11 @@ bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
   if (record_decl)
     GetClangASTImporter().SetRecordLayout(record_decl, layout_info);
 
+  // TODO: Only necessary if we target the Microsoft C++ ABI
+  auto IM = record_decl->calculateInheritanceModel();
+  record_decl->addAttr(clang::MSInheritanceAttr::CreateImplicit(
+      m_ast.getASTContext(), true, {}, clang::MSInheritanceAttr::Spelling(IM)));
+
   // Now parse all contained types inside of the class. We make forward
   // declarations to all classes, but we need the CXXRecordDecl to have decls
   // for all contained types because we don't get asked for them via the
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index f5063175d6e070..a1e224a395e69f 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2771,8 +2771,14 @@ static bool GetCompleteQualType(clang::ASTContext *ast,
         ast, llvm::cast<clang::AttributedType>(qual_type)->getModifiedType(),
         allow_completion);
 
-  case clang::Type::MemberPointer:
+  case clang::Type::MemberPointer: {
+    auto *MPT = qual_type.getTypePtr()->castAs<clang::MemberPointerType>();
+    if (MPT->getClass()->isRecordType())
+      GetCompleteRecordType(ast, clang::QualType(MPT->getClass(), 0),
+                            allow_completion);
+
     return !qual_type.getTypePtr()->isIncompleteType();
+  }
 
   default:
     break;

>From 651ec094a624c76ba2087b803890e74acd66e6a3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Wed, 6 Nov 2024 17:22:27 +0100
Subject: [PATCH 2/6] Check for MS C++ ABI in DWARF parser

---
 .../SymbolFile/DWARF/DWARFASTParserClang.cpp       | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index 319540389c05ee..9676ba27c555fb 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -43,6 +43,7 @@
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Type.h"
+#include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Demangle/Demangle.h"
 
@@ -2138,10 +2139,15 @@ bool DWARFASTParserClang::CompleteRecordType(const DWARFDIE &die,
   if (record_decl)
     GetClangASTImporter().SetRecordLayout(record_decl, layout_info);
 
-  // TODO: Only necessary if we target the Microsoft C++ ABI
-  auto IM = record_decl->calculateInheritanceModel();
-  record_decl->addAttr(clang::MSInheritanceAttr::CreateImplicit(
-      m_ast.getASTContext(), true, {}, clang::MSInheritanceAttr::Spelling(IM)));
+  // DWARF doesn't have the attribute, but we can infer the value the same way
+  // as Clang Sema does. It's required to calculate the size of pointers to
+  // member functions of this type.
+  if (m_ast.getASTContext().getTargetInfo().getCXXABI().isMicrosoft()) {
+    auto IM = record_decl->calculateInheritanceModel();
+    record_decl->addAttr(clang::MSInheritanceAttr::CreateImplicit(
+        m_ast.getASTContext(), true, {},
+        clang::MSInheritanceAttr::Spelling(IM)));
+  }
 
   // Now parse all contained types inside of the class. We make forward
   // declarations to all classes, but we need the CXXRecordDecl to have decls

>From a43ba8d756edc5cee6fbc1beb586e38d12ea589f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Wed, 6 Nov 2024 18:24:12 +0100
Subject: [PATCH 3/6] Extend test coverage

---
 .../SymbolFile/DWARF/x86/member-pointers.cpp  | 45 +++++++++++++++----
 1 file changed, 36 insertions(+), 9 deletions(-)

diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/member-pointers.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/member-pointers.cpp
index c7bd95774d33f7..463b19c5a866ff 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/member-pointers.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/member-pointers.cpp
@@ -2,23 +2,50 @@
 
 // Itanium ABI:
 // RUN: %clang --target=x86_64-pc-linux -gdwarf -c -o %t_linux.o %s
-// RUN: %lldb -f %t_linux.o -b -o "target variable mp" | FileCheck %s
+// RUN: %lldb -f %t_linux.o -b -o "target variable s1 s2 m1 m2 v1 v2 v3 v4" | FileCheck --check-prefix=CHECK-GNU %s
 //
-// CHECK: (char SI::*) mp = 0x0000000000000000
+// CHECK-GNU: (void (Single1::*)()) s1 = 0x00000000000000000000000000000000
+// CHECK-GNU: (void (Single2::*)()) s2 = 0x00000000000000000000000000000000
+// CHECK-GNU: (void (Multiple1::*)()) m1 = 0x00000000000000000000000000000000
+// CHECK-GNU: (void (Multiple2::*)()) m2 = 0x00000000000000000000000000000000
+// CHECK-GNU: (void (Virtual1::*)()) v1 = 0x00000000000000000000000000000000
+// CHECK-GNU: (void (Virtual2::*)()) v2 = 0x00000000000000000000000000000000
+// CHECK-GNU: (void (Virtual3::*)()) v3 = 0x00000000000000000000000000000000
+// CHECK-GNU: (void (Virtual4::*)()) v4 = 0x00000000000000000000000000000000
 
 // Microsoft ABI:
 // RUN: %clang_cl --target=x86_64-windows-msvc -c -gdwarf -o %t_win.obj -- %s
 // RUN: lld-link /out:%t_win.exe %t_win.obj /nodefaultlib /entry:main /debug
-// RUN: %lldb -f %t_win.exe -b -o "target variable mp" | FileCheck --check-prefix=CHECK-MSVC %s
+// RUN: %lldb -f %t_win.exe -b -o "target variable s1 s2 m1 m2 v1 v2 v3 v4" | FileCheck --check-prefix=CHECK-MSVC %s
 //
-// DWARF has no representation of MSInheritanceAttr, so we cannot determine the size
-// of member-pointers yet. For the moment, make sure we don't crash on such variables.
+// CHECK-MSVC: (void (Single1::*)()) s1 = 0x0000000000000000
 // CHECK-MSVC: error: Unable to determine byte size.
+// CHECK-MSVC: (void (Multiple1::*)()) m1 = 0x00000000000000000000000000000000
+// CHECK-MSVC: error: Unable to determine byte size.
+// CHECK-MSVC: error: Unable to determine byte size.
+// CHECK-MSVC: error: Unable to determine byte size.
+// CHECK-MSVC: error: Unable to determine byte size.
+// CHECK-MSVC: error: Unable to determine byte size.
+
+struct Single1 { void s1() {} };
+struct Single2 : Single1 { void s2() {} };
+
+struct Helper {};
+struct Multiple1 : Single1, Helper { void m1() {} };
+struct Multiple2 : Multiple1 { void m2() {} };
 
-struct SI {
-  char si;
-};
+struct Virtual1 : virtual Single1 { void v1() {} };
+struct Virtual2 : Virtual1 { void v2() {} };
+struct Virtual3 : virtual Multiple1 { void v3() {} };
+struct Virtual4 : Virtual1, Virtual3 { void v4() {} };
 
-char SI::*mp = &SI::si;
+void (Single1::*s1)() = nullptr;
+void (Single2::*s2)() = nullptr;
+void (Multiple1::*m1)() = nullptr;
+void (Multiple2::*m2)() = nullptr;
+void (Virtual1::*v1)() = nullptr;
+void (Virtual2::*v2)() = nullptr;
+void (Virtual3::*v3)() = nullptr;
+void (Virtual4::*v4)() = nullptr;
 
 int main() { return 0; }

>From 0c37c5ef2ee57e76f98de2ffa2f01305c1c1f2d9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Wed, 6 Nov 2024 19:34:33 +0100
Subject: [PATCH 4/6] Fix test for s2 and m2 (doesn't work for v1-4)

---
 .../Shell/SymbolFile/DWARF/x86/member-pointers.cpp | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/member-pointers.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/member-pointers.cpp
index 463b19c5a866ff..6e4cf391eba7c7 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/member-pointers.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/member-pointers.cpp
@@ -15,18 +15,19 @@
 
 // Microsoft ABI:
 // RUN: %clang_cl --target=x86_64-windows-msvc -c -gdwarf -o %t_win.obj -- %s
-// RUN: lld-link /out:%t_win.exe %t_win.obj /nodefaultlib /entry:main /debug
+// RUN: lld-link /out:%t_win.exe %t_win.obj /entry:main /debug /nodefaultlib
 // RUN: %lldb -f %t_win.exe -b -o "target variable s1 s2 m1 m2 v1 v2 v3 v4" | FileCheck --check-prefix=CHECK-MSVC %s
 //
 // CHECK-MSVC: (void (Single1::*)()) s1 = 0x0000000000000000
-// CHECK-MSVC: error: Unable to determine byte size.
+// CHECK-MSVC: (void (Single2::*)()) s2 = 0x0000000000000000
 // CHECK-MSVC: (void (Multiple1::*)()) m1 = 0x00000000000000000000000000000000
-// CHECK-MSVC: error: Unable to determine byte size.
+// CHECK-MSVC: (void (Multiple2::*)()) m2 = 0x00000000000000000000000000000000
 // CHECK-MSVC: error: Unable to determine byte size.
 // CHECK-MSVC: error: Unable to determine byte size.
 // CHECK-MSVC: error: Unable to determine byte size.
 // CHECK-MSVC: error: Unable to determine byte size.
 
+// clang-format off
 struct Single1 { void s1() {} };
 struct Single2 : Single1 { void s2() {} };
 
@@ -37,7 +38,7 @@ struct Multiple2 : Multiple1 { void m2() {} };
 struct Virtual1 : virtual Single1 { void v1() {} };
 struct Virtual2 : Virtual1 { void v2() {} };
 struct Virtual3 : virtual Multiple1 { void v3() {} };
-struct Virtual4 : Virtual1, Virtual3 { void v4() {} };
+struct Virtual4 : Virtual1, Helper { void v4() {} };
 
 void (Single1::*s1)() = nullptr;
 void (Single2::*s2)() = nullptr;
@@ -48,4 +49,7 @@ void (Virtual2::*v2)() = nullptr;
 void (Virtual3::*v3)() = nullptr;
 void (Virtual4::*v4)() = nullptr;
 
-int main() { return 0; }
+int main(int argc, char *argv[]) {
+  int sum = sizeof(Single2) + sizeof(Multiple2) + sizeof(Virtual1) + sizeof(Virtual2) + sizeof(Virtual3) + sizeof(Virtual4);
+  return argc < sum ? 0 : 1;
+}

>From a73b41f77511bc7e36aedc7be8a80c842f10bb5c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Wed, 6 Nov 2024 19:40:57 +0100
Subject: [PATCH 5/6] Patch test for v1-4 (wip)

---
 .../SymbolFile/DWARF/x86/member-pointers.cpp     | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/member-pointers.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/member-pointers.cpp
index 6e4cf391eba7c7..ec42c41216cda8 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/member-pointers.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/member-pointers.cpp
@@ -14,7 +14,7 @@
 // CHECK-GNU: (void (Virtual4::*)()) v4 = 0x00000000000000000000000000000000
 
 // Microsoft ABI:
-// RUN: %clang_cl --target=x86_64-windows-msvc -c -gdwarf -o %t_win.obj -- %s
+// RUN: %clang_cl --target=x86_64-windows-msvc -c -gdwarf -o %t_win.obj /GS- -- %s
 // RUN: lld-link /out:%t_win.exe %t_win.obj /entry:main /debug /nodefaultlib
 // RUN: %lldb -f %t_win.exe -b -o "target variable s1 s2 m1 m2 v1 v2 v3 v4" | FileCheck --check-prefix=CHECK-MSVC %s
 //
@@ -22,10 +22,10 @@
 // CHECK-MSVC: (void (Single2::*)()) s2 = 0x0000000000000000
 // CHECK-MSVC: (void (Multiple1::*)()) m1 = 0x00000000000000000000000000000000
 // CHECK-MSVC: (void (Multiple2::*)()) m2 = 0x00000000000000000000000000000000
-// CHECK-MSVC: error: Unable to determine byte size.
-// CHECK-MSVC: error: Unable to determine byte size.
-// CHECK-MSVC: error: Unable to determine byte size.
-// CHECK-MSVC: error: Unable to determine byte size.
+// CHECK-MSVC: (void (Virtual1::*)()) v1 = 0xffffffff000000000000000000000000
+// CHECK-MSVC: (void (Virtual2::*)()) v2 = 0xffffffff000000000000000000000000
+// CHECK-MSVC: (void (Virtual3::*)()) v3 = 0xffffffff000000000000000000000000
+// CHECK-MSVC: (void (Virtual4::*)()) v4 = 0xffffffff000000000000000000000000
 
 // clang-format off
 struct Single1 { void s1() {} };
@@ -50,6 +50,10 @@ void (Virtual3::*v3)() = nullptr;
 void (Virtual4::*v4)() = nullptr;
 
 int main(int argc, char *argv[]) {
-  int sum = sizeof(Single2) + sizeof(Multiple2) + sizeof(Virtual1) + sizeof(Virtual2) + sizeof(Virtual3) + sizeof(Virtual4);
+  Virtual1 vi1;
+  Virtual2 vi2;
+  Virtual3 vi3;
+  Virtual4 vi4;
+  int sum = sizeof(Single2) + sizeof(Multiple2);
   return argc < sum ? 0 : 1;
 }

>From 1caca47e6292ce569f9ad783853bd6b841ac7b02 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Fri, 8 Nov 2024 14:15:05 +0100
Subject: [PATCH 6/6] Make record type completion conditional in
 TypeSystemClang

---
 .../Plugins/TypeSystem/Clang/TypeSystemClang.cpp | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index a1e224a395e69f..1fc3a156d78ef1 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2772,13 +2772,17 @@ static bool GetCompleteQualType(clang::ASTContext *ast,
         allow_completion);
 
   case clang::Type::MemberPointer: {
-    auto *MPT = qual_type.getTypePtr()->castAs<clang::MemberPointerType>();
-    if (MPT->getClass()->isRecordType())
-      GetCompleteRecordType(ast, clang::QualType(MPT->getClass(), 0),
-                            allow_completion);
+    // MS C++ ABI requires type of the class to be complete of which the pointee
+    // is a member.
+    if (ast->getTargetInfo().getCXXABI().isMicrosoft()) {
+      auto *MPT = qual_type.getTypePtr()->castAs<clang::MemberPointerType>();
+      if (MPT->getClass()->isRecordType())
+        GetCompleteRecordType(ast, clang::QualType(MPT->getClass(), 0),
+                              allow_completion);
 
-    return !qual_type.getTypePtr()->isIncompleteType();
-  }
+      return !qual_type.getTypePtr()->isIncompleteType();
+    }
+    break;
 
   default:
     break;



More information about the lldb-commits mailing list