[clang] c138f3c - [clang] Fix ICE with typeid & polymorphic class (pr50497)

[Nathan Sidwell via cfe-commits]

Author: Nathan Sidwell
Date: 2021-06-01T12:55:29-07:00
New Revision: c138f3ce5c70ff87a35d77e11f77a77d9d539b49

URL: https://github.com/llvm/llvm-project/commit/c138f3ce5c70ff87a35d77e11f77a77d9d539b49
DIFF: https://github.com/llvm/llvm-project/commit/c138f3ce5c70ff87a35d77e11f77a77d9d539b49.diff

LOG: [clang] Fix ICE with typeid & polymorphic class (pr50497)

This addresses pr50497. The argument of a typeid expression is
unevaluated, *except* when it's a polymorphic type. We handle this by
parsing as unevaluated and then transforming to evaluated if we
discover it should have been an evaluated context.

We do the same in TreeTransform<Derived>::TransformCXXTypeidExpr,
entering unevaluated context before transforming and rebuilding the
typeid. But that's incorrect and can lead us to converting to
evaluated context twice -- and hitting an assert.

During normal template instantiation we're always cloning the
expression, but during generic lambda processing we do not necessarily
AlwaysRebuild, and end up with TransformDeclRefExpr unconditionally
calling MarkDeclRefReferenced around line 10226. That triggers the
assert.

// Mark it referenced in the new context regardless.
// FIXME: this is a bit instantiation-specific.
SemaRef.MarkDeclRefReferenced(E);

This patch makes 2 changes.

a) TreeTransform<Derived>::TransformCXXTypeidExpr only enters
unevaluated context if the typeid's operand is not a polymorphic
glvalue. If it is, it keeps the same evaluation context.

b) Sema::BuildCXXTypeId is altered to only transform to evaluated, if
the current context is unevaluated.

Differential Revision: https://reviews.llvm.org/D103258

Added: 
    clang/test/SemaCXX/pr50497-crash-typeid.cpp

Modified: 
    clang/lib/Sema/SemaExprCXX.cpp
    clang/lib/Sema/TreeTransform.h

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 75ebeaea1072c..c81f512b4fbfa 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -567,11 +567,14 @@ ExprResult Sema::BuildCXXTypeId(QualType TypeInfoType,
       //   polymorphic class type [...] [the] expression is an unevaluated
       //   operand. [...]
       if (RecordD->isPolymorphic() && E->isGLValue()) {
-        // The subexpression is potentially evaluated; switch the context
-        // and recheck the subexpression.
-        ExprResult Result = TransformToPotentiallyEvaluated(E);
-        if (Result.isInvalid()) return ExprError();
-        E = Result.get();
+        if (isUnevaluatedContext()) {
+          // The operand was processed in unevaluated context, switch the
+          // context and recheck the subexpression.
+          ExprResult Result = TransformToPotentiallyEvaluated(E);
+          if (Result.isInvalid())
+            return ExprError();
+          E = Result.get();
+        }
 
         // We require a vtable to query the type at run time.
         MarkVTableUsed(TypeidLoc, RecordD);

diff  --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 80b798702fdd3..2da8618ef44b9 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -11592,15 +11592,20 @@ TreeTransform<Derived>::TransformCXXTypeidExpr(CXXTypeidExpr *E) {
                                              TInfo, E->getEndLoc());
   }
 
-  // We don't know whether the subexpression is potentially evaluated until
-  // after we perform semantic analysis.  We speculatively assume it is
-  // unevaluated; it will get fixed later if the subexpression is in fact
-  // potentially evaluated.
-  EnterExpressionEvaluationContext Unevaluated(
-      SemaRef, Sema::ExpressionEvaluationContext::Unevaluated,
-      Sema::ReuseLambdaContextDecl);
-
-  ExprResult SubExpr = getDerived().TransformExpr(E->getExprOperand());
+  // Typeid's operand is an unevaluated context, unless it's a polymorphic
+  // type.  We must not unilaterally enter unevaluated context here, as then
+  // semantic processing can re-transform an already transformed operand.
+  Expr *Op = E->getExprOperand();
+  auto EvalCtx = Sema::ExpressionEvaluationContext::Unevaluated;
+  if (E->isGLValue())
+    if (auto *RecordT = Op->getType()->getAs<RecordType>())
+      if (cast<CXXRecordDecl>(RecordT->getDecl())->isPolymorphic())
+        EvalCtx = SemaRef.ExprEvalContexts.back().Context;
+
+  EnterExpressionEvaluationContext Unevaluated(SemaRef, EvalCtx,
+                                               Sema::ReuseLambdaContextDecl);
+
+  ExprResult SubExpr = getDerived().TransformExpr(Op);
   if (SubExpr.isInvalid())
     return ExprError();
 

diff  --git a/clang/test/SemaCXX/pr50497-crash-typeid.cpp b/clang/test/SemaCXX/pr50497-crash-typeid.cpp
new file mode 100644
index 0000000000000..3ae61e8279eb3
--- /dev/null
+++ b/clang/test/SemaCXX/pr50497-crash-typeid.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -verify %s -Wno-unevaluated-expression
+// Don't crash (PR50497).
+
+// expected-no-diagnostics
+namespace std {
+class type_info;
+}
+
+class Ex {
+  // polymorphic
+  virtual ~Ex();
+};
+void Frob(const std::type_info &type);
+
+void Foo(Ex *ex) {
+  // generic lambda
+  [=](auto rate) {
+    // typeid
+    Frob(typeid(*ex));
+  }(1);
+
+  [=](auto rate) {
+    // unevaluated nested typeid
+    Frob(typeid((typeid(*ex), ex)));
+  }(1);
+}