[clang][bytecode] Devirtualize calls during con- and destruction

[tbaederr]

When compiliung compiling a ctor or dtor, we need to devirtualize the virtual function calls so we always call the implementation of the current class.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

When compiliung compiling a ctor or dtor, we need to devirtualize the virtual function calls so we always call the implementation of the current class.

---

Full diff: https://github.com/llvm/llvm-project/pull/147685.diff

4 Files Affected:

• (modified) clang/lib/AST/ByteCode/Compiler.cpp (+36-3)
• (modified) clang/lib/AST/ByteCode/Compiler.h (+2)
• (modified) clang/test/AST/ByteCode/cxx20.cpp (+71-2)
• (modified) clang/test/AST/ByteCode/records.cpp (+2-4)

diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index ee9c3890794af..e7c085750b7ad 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -4912,6 +4912,18 @@ bool Compiler<Emitter>::VisitBuiltinCallExpr(const CallExpr *E,
   return true;
 }
 
+static const Expr *stripDerivedToBaseCasts(const Expr *E) {
+  if (const auto *PE = dyn_cast<ParenExpr>(E))
+    return stripDerivedToBaseCasts(PE->getSubExpr());
+
+  if (const auto *CE = dyn_cast<CastExpr>(E);
+      CE &&
+      (CE->getCastKind() == CK_DerivedToBase || CE->getCastKind() == CK_NoOp))
+    return stripDerivedToBaseCasts(CE->getSubExpr());
+
+  return E;
+}
+
 template <class Emitter>
 bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
   const FunctionDecl *FuncDecl = E->getDirectCallee();
@@ -4995,6 +5007,7 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
     }
   }
 
+  bool Devirtualized = false;
   std::optional<unsigned> CalleeOffset;
   // Add the (optional, implicit) This pointer.
   if (const auto *MC = dyn_cast<CXXMemberCallExpr>(E)) {
@@ -5013,8 +5026,26 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
         return false;
       if (!this->emitGetMemberPtrBase(E))
         return false;
-    } else if (!this->visit(MC->getImplicitObjectArgument())) {
-      return false;
+    } else {
+      const auto *InstancePtr = MC->getImplicitObjectArgument();
+      if (isa_and_nonnull<CXXDestructorDecl>(CompilingFunction) ||
+          isa_and_nonnull<CXXConstructorDecl>(CompilingFunction)) {
+        const auto *Stripped = stripDerivedToBaseCasts(InstancePtr);
+        if (isa<CXXThisExpr>(Stripped)) {
+          FuncDecl =
+              cast<CXXMethodDecl>(FuncDecl)->getCorrespondingMethodInClass(
+                  Stripped->getType()->getPointeeType()->getAsCXXRecordDecl());
+          Devirtualized = true;
+          if (!this->visit(Stripped))
+            return false;
+        } else {
+          if (!this->visit(InstancePtr))
+            return false;
+        }
+      } else {
+        if (!this->visit(InstancePtr))
+          return false;
+      }
     }
   } else if (const auto *PD =
                  dyn_cast<CXXPseudoDestructorExpr>(E->getCallee())) {
@@ -5060,7 +5091,7 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
 
     bool IsVirtual = false;
     if (const auto *MD = dyn_cast<CXXMethodDecl>(FuncDecl))
-      IsVirtual = MD->isVirtual();
+      IsVirtual = !Devirtualized && MD->isVirtual();
 
     // In any case call the function. The return value will end up on the stack
     // and if the function has RVO, we already have the pointer on the stack to
@@ -6027,6 +6058,8 @@ bool Compiler<Emitter>::visitFunc(const FunctionDecl *F) {
   // Classify the return type.
   ReturnType = this->classify(F->getReturnType());
 
+  this->CompilingFunction = F;
+
   if (const auto *Ctor = dyn_cast<CXXConstructorDecl>(F))
     return this->compileConstructor(Ctor);
   if (const auto *Dtor = dyn_cast<CXXDestructorDecl>(F))
diff --git a/clang/lib/AST/ByteCode/Compiler.h b/clang/lib/AST/ByteCode/Compiler.h
index 1a5fd86785872..05ba7460b343b 100644
--- a/clang/lib/AST/ByteCode/Compiler.h
+++ b/clang/lib/AST/ByteCode/Compiler.h
@@ -448,6 +448,8 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>,
   OptLabelTy ContinueLabel;
   /// Default case label.
   OptLabelTy DefaultLabel;
+
+  const FunctionDecl *CompilingFunction = nullptr;
 };
 
 extern template class Compiler<ByteCodeEmitter>;
diff --git a/clang/test/AST/ByteCode/cxx20.cpp b/clang/test/AST/ByteCode/cxx20.cpp
index e0fb38e106102..cc315d36456d5 100644
--- a/clang/test/AST/ByteCode/cxx20.cpp
+++ b/clang/test/AST/ByteCode/cxx20.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fexperimental-new-constant-interpreter -std=c++20 -verify=both,expected -fcxx-exceptions %s -DNEW_INTERP
-// RUN: %clang_cc1 -fcxx-exceptions -std=c++20 -verify=both,ref -fcxx-exceptions %s
+// RUN: %clang_cc1 -fcxx-exceptions -std=c++20 -verify=both,expected -fcxx-exceptions %s -DNEW_INTERP -fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -fcxx-exceptions -std=c++20 -verify=both,ref      -fcxx-exceptions %s
 
 void test_alignas_operand() {
   alignas(8) char dummy;
@@ -1015,3 +1015,72 @@ namespace OnePastEndDtor {
     (&a+1)->~A(); // both-note {{destruction of dereferenced one-past-the-end pointer}}
   }
 }
+
+namespace Virtual {
+  struct NonZeroOffset { int padding = 123; };
+
+  constexpr void assert(bool b) { if (!b) throw 0; }
+
+  // Ensure that we pick the right final overrider during construction.
+  struct A {
+    virtual constexpr char f() const { return 'A'; }
+    char a = f();
+    constexpr ~A() { assert(f() == 'A'); }
+  };
+  struct NoOverrideA : A {};
+  struct B : NonZeroOffset, NoOverrideA {
+    virtual constexpr char f() const { return 'B'; }
+    char b = f();
+    constexpr ~B() { assert(f() == 'B'); }
+  };
+  struct NoOverrideB : B {};
+  struct C : NonZeroOffset, A {
+    virtual constexpr char f() const { return 'C'; }
+    A *pba;
+    char c = ((A*)this)->f();
+    char ba = pba->f();
+    constexpr C(A *pba) : pba(pba) {}
+    constexpr ~C() { assert(f() == 'C'); }
+  };
+  struct D : NonZeroOffset, NoOverrideB, C { // both-warning {{inaccessible}}
+    virtual constexpr char f() const { return 'D'; }
+    char d = f();
+    constexpr D() : C((B*)this) {}
+    constexpr ~D() { assert(f() == 'D'); }
+  };
+  constexpr int n = (D(), 0);
+
+  constexpr D d;
+  static_assert(((B&)d).a == 'A');
+  static_assert(((C&)d).a == 'A');
+  static_assert(d.b == 'B');
+  static_assert(d.c == 'C');
+  // During the construction of C, the dynamic type of B's A is B.
+  static_assert(d.ba == 'B'); // expected-error {{failed}} \
+                              // expected-note {{expression evaluates to}}
+  static_assert(d.d == 'D');
+  static_assert(d.f() == 'D');
+  constexpr const A &a = (B&)d;
+  constexpr const B &b = d;
+  static_assert(a.f() == 'D');
+  static_assert(b.f() == 'D');
+
+
+  class K {
+  public:
+    int a = f();
+
+    virtual constexpr int f() { return 10; }
+  };
+
+  class L : public K {
+  public:
+    int b = f();
+    int c =((L*)this)->f();
+  };
+
+  constexpr L l;
+  static_assert(l.a == 10);
+  static_assert(l.b == 10);
+  static_assert(l.c == 10);
+}
diff --git a/clang/test/AST/ByteCode/records.cpp b/clang/test/AST/ByteCode/records.cpp
index 9361d6ddeda70..d369c64bc3904 100644
--- a/clang/test/AST/ByteCode/records.cpp
+++ b/clang/test/AST/ByteCode/records.cpp
@@ -759,10 +759,8 @@ namespace CtorDtor {
   static_assert(B.i == 1 && B.j == 1, "");
 
   constexpr Derived D;
-  static_assert(D.i == 1, ""); // expected-error {{static assertion failed}} \
-                               // expected-note {{2 == 1}}
-  static_assert(D.j == 1, ""); // expected-error {{static assertion failed}} \
-                               // expected-note {{2 == 1}}
+  static_assert(D.i == 1, "");
+  static_assert(D.j == 1, "");
 
   constexpr Derived2 D2; // ref-error {{must be initialized by a constant expression}} \
                          // ref-note {{in call to 'Derived2()'}} \