[clang] Don't emit debug vtable information for consteval functions

[luken-google]

Summary

Fixes https://github.com/llvm/llvm-project/issues/55065

[luken-google]

Note that there's still an underlying issue where the linker complains of missing vtable information for both Base and Derived, which I'm treating as a separate issue and will either file or de-dup against an existing issue.

[shafik]

Maybe @aprantl you want to take a look at this.

clang/lib/CodeGen/CGDebugInfo.cpp

1762

It is not clear to me if this is the core issue or not. Can you explain this a little better.

clang/test/CodeGenCXX/cxx20-consteval-crash.cpp

95

Please add the newline back.

[luken-google]

clang/lib/CodeGen/CGDebugInfo.cpp

1762

Thanks for the feedback!

Sure, here's what I'm thinking. This code calls ItaniumVTableContext::getMethodVTableIndex(GlobalDecl) on line 1771 below. That method asserts in VTableBuilder.cpp:2281 because it tries to look up a vtable entry for the consteval method and fails to do so. Any consteval methods are prevented from gaining vtable entries by the logic in VTableContextBase::hasVtableSlot(const CXXMethodDecl *MD):

bool VTableContextBase::hasVtableSlot(const CXXMethodDecl *MD) {
  return MD->isVirtual() && !MD->isConsteval();
}

This call is peppered throughout the VTableBuilder code, but the relevant call for our purposes is in ItaniumVTableBuilder::AddMethods() in VTableBuilder.cpp:1492, which causes any consteval method to be skipped when the code is iterating through all virtual member functions and adding them to the vtable.

My fix mirrors the logic in VTableContextBase::hasVtableSlot here, because the code assumes that if the method is virtual it has a vtable index, which consteval functions don't. Writing this, I've convinced myself it's better to just call that method directly, so I've refactored the code to avoid the duplicate logic.

[luken-google]

Change to method call.

[shafik]

LGTM

clang/lib/CodeGen/CGDebugInfo.cpp

1762

Thank you, that makes sense. Using the function call is much better, it avoids logic duplication and assures that if the logic is updated this section will stay in sync.

[dblaikie]

clang/lib/CodeGen/CGDebugInfo.cpp

1763–1766

Seems this change means the virtual/pure virtual flags would be missed for these virtual-but-not-vtable functions? What does GCC do in terms of debug info for these functions? (I guess it's not super important/especially material - since these consteval functions probably can't ever get code generated for them, so a user can never call them & maybe it's more likely to be good for debuggers not to think of them as virtual functions (because they might expect vtable indexes, etc))

clang/docs/ReleaseNotes.rst

 - Correctly defer dependent immediate function invocations until template instantiation.
   This fixes `GH55601 <https://github.com/llvm/llvm-project/issues/55601>`_.
 - Implemented "Conditionally Trivial Special Member Functions" (`P0848 <https://wg21.link/p0848r3>`_).
   Note: The handling of deleted functions is not yet compliant, as Clang
   does not implement `DR1496 <https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1496>`_
   and `DR1734 <https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1734>`_.
 - Class member variables are now in scope when parsing a ``requires`` clause. Fixes
   `GH55216 <https://github.com/llvm/llvm-project/issues/55216>`_.
-
 - Correctly set expression evaluation context as 'immediate function context' in
   consteval functions.
   This fixes `GH51182 <https://github.com/llvm/llvm-project/issues/51182>`
+- Fixes an assert crash caused by looking up missing vtable information on ``consteval``
+  virtual functions. Fixes `GH55065 <https://github.com/llvm/llvm-project/issues/55065>`_.
 
 
 C++2b Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 
 CUDA/HIP Language Changes in Clang
 ----------------------------------
 

clang/lib/CodeGen/CGDebugInfo.cpp

 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclFriend.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/VTableBuilder.h"
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Version.h"
 #include "clang/Frontend/FrontendOptions.h"
 #include "clang/Lex/HeaderSearchOptions.h"
 #include "clang/Lex/ModuleMap.h"
 #include "clang/Lex/PreprocessorOptions.h"
llvm::DISubprogram *CGDebugInfo::CreateCXXMemberFunction(
 
   // Collect virtual method info.
   llvm::DIType *ContainingType = nullptr;
   unsigned VIndex = 0;
   llvm::DINode::DIFlags Flags = llvm::DINode::FlagZero;
   llvm::DISubprogram::DISPFlags SPFlags = llvm::DISubprogram::SPFlagZero;
   int ThisAdjustment = 0;
 
-  if (Method->isVirtual()) {
+  if (VTableContextBase::hasVtableSlot(Method)) {
     if (Method->isPure())
       SPFlags |= llvm::DISubprogram::SPFlagPureVirtual;
     else
       SPFlags |= llvm::DISubprogram::SPFlagVirtual;
 
     if (CGM.getTarget().getCXXABI().isItaniumFamily()) {
       // It doesn't make sense to give a virtual destructor a vtable index,
       // since a single destructor has two entries in the vtable.
       if (!isa<CXXDestructorDecl>(Method))
         VIndex = CGM.getItaniumVTableContext().getMethodVTableIndex(Method);
     } else {
       // Emit MS ABI vftable information.  There is only one entry for the

clang/test/CodeGenCXX/cxx20-consteval-crash.cpp

 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-unknown-linux-gnu -std=c++20 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -emit-obj -debug-info-kind=constructor -std=c++20 %s -o -
 
 namespace PR50787 {
 // This code would previously cause a crash.
 extern int x_;
 consteval auto& X() { return x_; }
 constexpr auto& x1 = X();
 auto x2 = X();
 

 int function(const Item& item1, const Item& item2) {
   return 0;
 }
 
 int foo() {
   return function(Item{'a'}, Item{'a'});
 }
 } // namespace Issue58871
+
+namespace Issue55065 {
+struct Base {
+  consteval virtual int Get() const = 0;
+};
+
+struct Derived : Base {
+  consteval int Get() const override {
+    return 42;
+  }
+};
+
+int foo() {
+  constexpr Derived a;
+
+  auto val = a.Get();
+  return val;
+}
+} // namespace Issue55065
+