[clang] [Clang][Sema] Fix crash when 'this' is used in a dependent class scope function template specialization that instantiates to a static member function (PR #87541)

Krystian Stasiowski via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 9 05:31:18 PDT 2024


https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/87541

>From 6ad6b5e698c3ae6fd8e881582fcfa4c8bb231da4 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Wed, 3 Apr 2024 14:33:27 -0400
Subject: [PATCH 1/4] [Clang][Sema] Fix crash when 'this' is used in a
 dependent class scope function template specialization that instantiates to a
 static member function

---
 clang/include/clang/Sema/Sema.h               |  8 +++-
 clang/lib/Sema/SemaExpr.cpp                   |  5 ++-
 clang/lib/Sema/SemaExprCXX.cpp                | 44 +++++++++++++------
 clang/lib/Sema/SemaExprMember.cpp             | 44 ++++++++++++++-----
 .../lib/Sema/SemaTemplateInstantiateDecl.cpp  |  8 ++++
 clang/lib/Sema/TreeTransform.h                |  7 +--
 ...ms-function-specialization-class-scope.cpp | 38 ++++++++++++++--
 7 files changed, 118 insertions(+), 36 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 56d66a4486e0e74..57117963d84f1d2 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -5428,7 +5428,8 @@ class Sema final : public SemaBase {
 
   ExprResult BuildDeclarationNameExpr(const CXXScopeSpec &SS, LookupResult &R,
                                       bool NeedsADL,
-                                      bool AcceptInvalidDecl = false);
+                                      bool AcceptInvalidDecl = false,
+                                      bool NeedUnresolved = false);
   ExprResult BuildDeclarationNameExpr(
       const CXXScopeSpec &SS, const DeclarationNameInfo &NameInfo, NamedDecl *D,
       NamedDecl *FoundD = nullptr,
@@ -6580,7 +6581,10 @@ class Sema final : public SemaBase {
                             SourceLocation RParenLoc);
 
   //// ActOnCXXThis -  Parse 'this' pointer.
-  ExprResult ActOnCXXThis(SourceLocation loc);
+  ExprResult ActOnCXXThis(SourceLocation Loc);
+
+  /// Check whether the type of 'this' is valid in the current context.
+  bool CheckCXXThisType(SourceLocation Loc, QualType Type);
 
   /// Build a CXXThisExpr and mark it referenced in the current context.
   Expr *BuildCXXThisExpr(SourceLocation Loc, QualType Type, bool IsImplicit);
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 8db4fffeecfe35f..7b91bbe0b2054d3 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -3442,10 +3442,11 @@ static bool ShouldLookupResultBeMultiVersionOverload(const LookupResult &R) {
 
 ExprResult Sema::BuildDeclarationNameExpr(const CXXScopeSpec &SS,
                                           LookupResult &R, bool NeedsADL,
-                                          bool AcceptInvalidDecl) {
+                                          bool AcceptInvalidDecl,
+                                          bool NeedUnresolved) {
   // If this is a single, fully-resolved result and we don't need ADL,
   // just build an ordinary singleton decl ref.
-  if (!NeedsADL && R.isSingleResult() &&
+  if (!NeedUnresolved && !NeedsADL && R.isSingleResult() &&
       !R.getAsSingle<FunctionTemplateDecl>() &&
       !ShouldLookupResultBeMultiVersionOverload(R))
     return BuildDeclarationNameExpr(SS, R.getLookupNameInfo(), R.getFoundDecl(),
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index cfb5c6b6f283373..d84c66e5969f74a 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1415,26 +1415,42 @@ bool Sema::CheckCXXThisCapture(SourceLocation Loc, const bool Explicit,
 }
 
 ExprResult Sema::ActOnCXXThis(SourceLocation Loc) {
-  /// C++ 9.3.2: In the body of a non-static member function, the keyword this
-  /// is a non-lvalue expression whose value is the address of the object for
-  /// which the function is called.
+  // C++20 [expr.prim.this]p1:
+  //   The keyword this names a pointer to the object for which an
+  //   implicit object member function is invoked or a non-static
+  //   data member's initializer is evaluated.
   QualType ThisTy = getCurrentThisType();
 
-  if (ThisTy.isNull()) {
-    DeclContext *DC = getFunctionLevelDeclContext();
+  if (CheckCXXThisType(Loc, ThisTy))
+    return ExprError();
 
-    if (const auto *Method = dyn_cast<CXXMethodDecl>(DC);
-        Method && Method->isExplicitObjectMemberFunction()) {
-      return Diag(Loc, diag::err_invalid_this_use) << 1;
-    }
+  return BuildCXXThisExpr(Loc, ThisTy, /*IsImplicit=*/false);
+}
 
-    if (isLambdaCallWithExplicitObjectParameter(CurContext))
-      return Diag(Loc, diag::err_invalid_this_use) << 1;
+bool Sema::CheckCXXThisType(SourceLocation Loc, QualType Type) {
+  if (!Type.isNull())
+    return false;
 
-    return Diag(Loc, diag::err_invalid_this_use) << 0;
+  // C++20 [expr.prim.this]p3:
+  //   If a declaration declares a member function or member function template
+  //   of a class X, the expression this is a prvalue of type
+  //   "pointer to cv-qualifier-seq X" wherever X is the current class between
+  //   the optional cv-qualifier-seq and the end of the function-definition,
+  //   member-declarator, or declarator. It shall not appear within the
+  //   declaration of either a static member function or an explicit object
+  //   member function of the current class (although its type and value
+  //   category are defined within such member functions as they are within
+  //   an implicit object member function).
+  DeclContext *DC = getFunctionLevelDeclContext();
+  if (const auto *Method = dyn_cast<CXXMethodDecl>(DC);
+      Method && Method->isExplicitObjectMemberFunction()) {
+    Diag(Loc, diag::err_invalid_this_use) << 1;
+  } else if (isLambdaCallWithExplicitObjectParameter(CurContext)) {
+    Diag(Loc, diag::err_invalid_this_use) << 1;
+  } else {
+    Diag(Loc, diag::err_invalid_this_use) << 0;
   }
-
-  return BuildCXXThisExpr(Loc, ThisTy, /*IsImplicit=*/false);
+  return true;
 }
 
 Expr *Sema::BuildCXXThisExpr(SourceLocation Loc, QualType Type,
diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp
index 32998ae60eafe2b..e21adc95bf60c10 100644
--- a/clang/lib/Sema/SemaExprMember.cpp
+++ b/clang/lib/Sema/SemaExprMember.cpp
@@ -61,6 +61,10 @@ enum IMAKind {
   /// The reference is a contextually-permitted abstract member reference.
   IMA_Abstract,
 
+  /// Whether the context is static is dependent on the enclosing template (i.e.
+  /// in a dependent class scope explicit specialization).
+  IMA_Dependent,
+
   /// The reference may be to an unresolved using declaration and the
   /// context is not an instance method.
   IMA_Unresolved_StaticOrExplicitContext,
@@ -91,10 +95,20 @@ static IMAKind ClassifyImplicitMemberAccess(Sema &SemaRef,
 
   DeclContext *DC = SemaRef.getFunctionLevelDeclContext();
 
-  bool isStaticOrExplicitContext =
-      SemaRef.CXXThisTypeOverride.isNull() &&
-      (!isa<CXXMethodDecl>(DC) || cast<CXXMethodDecl>(DC)->isStatic() ||
-       cast<CXXMethodDecl>(DC)->isExplicitObjectMemberFunction());
+  bool couldInstantiateToStatic = false;
+  bool isStaticOrExplicitContext = SemaRef.CXXThisTypeOverride.isNull();
+
+  if (auto *MD = dyn_cast<CXXMethodDecl>(DC)) {
+    if (!MD->isStatic() && !MD->isExplicitObjectMemberFunction())
+      isStaticOrExplicitContext = false;
+
+    // A dependent class scope function template explicit specialization
+    // which has no explicit object parameter and is declared without 'static'
+    // could instantiate to either a static or non-static member function.
+    if (MD->getDependentSpecializationInfo() && !MD->isStatic() &&
+        !MD->isExplicitObjectMemberFunction())
+      couldInstantiateToStatic = true;
+  }
 
   if (R.isUnresolvableResult())
     return isStaticOrExplicitContext ? IMA_Unresolved_StaticOrExplicitContext
@@ -123,6 +137,9 @@ static IMAKind ClassifyImplicitMemberAccess(Sema &SemaRef,
   if (Classes.empty())
     return IMA_Static;
 
+  if (couldInstantiateToStatic)
+    return IMA_Dependent;
+
   // C++11 [expr.prim.general]p12:
   //   An id-expression that denotes a non-static data member or non-static
   //   member function of a class can only be used:
@@ -268,27 +285,30 @@ ExprResult Sema::BuildPossibleImplicitMemberExpr(
     const CXXScopeSpec &SS, SourceLocation TemplateKWLoc, LookupResult &R,
     const TemplateArgumentListInfo *TemplateArgs, const Scope *S,
     UnresolvedLookupExpr *AsULE) {
-  switch (ClassifyImplicitMemberAccess(*this, R)) {
+  switch (IMAKind Classification = ClassifyImplicitMemberAccess(*this, R)) {
   case IMA_Instance:
-    return BuildImplicitMemberExpr(SS, TemplateKWLoc, R, TemplateArgs, true, S);
-
   case IMA_Mixed:
   case IMA_Mixed_Unrelated:
   case IMA_Unresolved:
-    return BuildImplicitMemberExpr(SS, TemplateKWLoc, R, TemplateArgs, false,
-                                   S);
-
+    return BuildImplicitMemberExpr(
+        SS, TemplateKWLoc, R, TemplateArgs,
+        /*IsKnownInstance=*/Classification == IMA_Instance, S);
   case IMA_Field_Uneval_Context:
     Diag(R.getNameLoc(), diag::warn_cxx98_compat_non_static_member_use)
       << R.getLookupNameInfo().getName();
     [[fallthrough]];
   case IMA_Static:
   case IMA_Abstract:
+  case IMA_Dependent:
   case IMA_Mixed_StaticOrExplicitContext:
   case IMA_Unresolved_StaticOrExplicitContext:
     if (TemplateArgs || TemplateKWLoc.isValid())
-      return BuildTemplateIdExpr(SS, TemplateKWLoc, R, false, TemplateArgs);
-    return AsULE ? AsULE : BuildDeclarationNameExpr(SS, R, false);
+      return BuildTemplateIdExpr(SS, TemplateKWLoc, R, /*RequiresADL=*/false,
+                                 TemplateArgs);
+    return AsULE ? AsULE
+                 : BuildDeclarationNameExpr(
+                       SS, R, /*NeedsADL=*/false, /*AcceptInvalidDecl=*/false,
+                       /*NeedUnresolved=*/Classification == IMA_Dependent);
 
   case IMA_Error_StaticOrExplicitContext:
   case IMA_Error_Unrelated:
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 1cb071e4eb7d1cb..2a036feb438cac4 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5091,6 +5091,14 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation,
   EnterExpressionEvaluationContext EvalContext(
       *this, Sema::ExpressionEvaluationContext::PotentiallyEvaluated);
 
+  Qualifiers ThisTypeQuals;
+  CXXRecordDecl *ThisContext = nullptr;
+  if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(Function)) {
+    ThisContext = Method->getParent();
+    ThisTypeQuals = Method->getMethodQualifiers();
+  }
+  CXXThisScopeRAII ThisScope(*this, ThisContext, ThisTypeQuals);
+
   // Introduce a new scope where local variable instantiations will be
   // recorded, unless we're actually a member function within a local
   // class, in which case we need to merge our results with the parent
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 66c2c7dd6be8f9b..cf8fe36bfa53642 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -3307,12 +3307,13 @@ class TreeTransform {
 
   /// Build a new C++ "this" expression.
   ///
-  /// By default, builds a new "this" expression without performing any
-  /// semantic analysis. Subclasses may override this routine to provide
-  /// different behavior.
+  /// By default, performs semantic analysis to build a new "this" expression.
+  /// Subclasses may override this routine to provide different behavior.
   ExprResult RebuildCXXThisExpr(SourceLocation ThisLoc,
                                 QualType ThisType,
                                 bool isImplicit) {
+    if (getSema().CheckCXXThisType(ThisLoc, ThisType))
+      return ExprError();
     return getSema().BuildCXXThisExpr(ThisLoc, ThisType, isImplicit);
   }
 
diff --git a/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp b/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp
index dcab9bfaeabcb08..e72a98b637da5bb 100644
--- a/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp
+++ b/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp
@@ -1,7 +1,6 @@
-// RUN: %clang_cc1 -fms-extensions -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fms-extensions -fdelayed-template-parsing -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fms-extensions -fsyntax-only -Wno-unused-value -verify %s
+// RUN: %clang_cc1 -fms-extensions -fdelayed-template-parsing -fsyntax-only -Wno-unused-value -verify %s
 
-// expected-no-diagnostics
 class A {
 public:
   template<class U> A(U p) {}
@@ -76,3 +75,36 @@ struct S {
   int f<0>(int);
 };
 }
+
+namespace UsesThis {
+  template<typename T>
+  struct A {
+    int x;
+
+    template<typename U>
+    static void f();
+
+    template<>
+    void f<int>() {
+      this->x; // expected-error {{invalid use of 'this' outside of a non-static member function}}
+      x; // expected-error {{invalid use of member 'x' in static member function}}
+      A::x; // expected-error {{invalid use of member 'x' in static member function}}
+      +x; // expected-error {{invalid use of member 'x' in static member function}}
+      +A::x; // expected-error {{invalid use of member 'x' in static member function}}
+    }
+
+    template<typename U>
+    void g();
+
+    template<>
+    void g<int>() {
+      this->x;
+      x;
+      A::x;
+      +x;
+      +A::x;
+    }
+  };
+
+  template struct A<int>; // expected-note {{in instantiation of function template specialization}}
+}

>From 16a132b1a7fdd006efee4b2ab66e62e7ca8e8083 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Thu, 4 Apr 2024 09:28:31 -0400
Subject: [PATCH 2/4] [FOLD] simplify ClassifyImplicitMemberAccess

---
 clang/lib/Sema/SemaExprMember.cpp | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp
index e21adc95bf60c10..8cd2288d279cc71 100644
--- a/clang/lib/Sema/SemaExprMember.cpp
+++ b/clang/lib/Sema/SemaExprMember.cpp
@@ -99,15 +99,13 @@ static IMAKind ClassifyImplicitMemberAccess(Sema &SemaRef,
   bool isStaticOrExplicitContext = SemaRef.CXXThisTypeOverride.isNull();
 
   if (auto *MD = dyn_cast<CXXMethodDecl>(DC)) {
-    if (!MD->isStatic() && !MD->isExplicitObjectMemberFunction())
+    if (MD->isImplicitObjectMemberFunction()) {
       isStaticOrExplicitContext = false;
-
-    // A dependent class scope function template explicit specialization
-    // which has no explicit object parameter and is declared without 'static'
-    // could instantiate to either a static or non-static member function.
-    if (MD->getDependentSpecializationInfo() && !MD->isStatic() &&
-        !MD->isExplicitObjectMemberFunction())
-      couldInstantiateToStatic = true;
+      // A dependent class scope function template explicit specialization
+      // that is neither declared 'static' nor with an explicit object
+      // parameter could instantiate to a static or non-static member function.
+      couldInstantiateToStatic = MD->getDependentSpecializationInfo();
+    }
   }
 
   if (R.isUnresolvableResult())

>From 52d18a6a885f275dc2fd5ba726e74d3446dd790f Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Thu, 4 Apr 2024 09:36:12 -0400
Subject: [PATCH 3/4] [FOLD] add release note

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

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 28e8ddb3c41c3e9..d8853f0156aaed6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -500,6 +500,8 @@ Bug Fixes to C++ Support
 - Fix an issue caused by not handling invalid cases when substituting into the parameter mapping of a constraint. Fixes (#GH86757).
 - Fixed a bug that prevented member function templates of class templates declared with a deduced return type
   from being explicitly specialized for a given implicit instantiation of the class template.
+- Fixed a crash when ``this`` is used in a dependent class scope function template specialization
+  that instantiates to a static member function.
 
 - Fix crash when inheriting from a cv-qualified type. Fixes:
   (`#35603 <https://github.com/llvm/llvm-project/issues/35603>`_)

>From 2b01663ab5f77573c7bf8f88333a07abd00e6119 Mon Sep 17 00:00:00 2001
From: Krystian Stasiowski <sdkrystian at gmail.com>
Date: Tue, 9 Apr 2024 08:31:03 -0400
Subject: [PATCH 4/4] [FOLD] additional test

---
 .../ms-function-specialization-class-scope.cpp            | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp b/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp
index e72a98b637da5bb..6977623a0816eda 100644
--- a/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp
+++ b/clang/test/SemaTemplate/ms-function-specialization-class-scope.cpp
@@ -104,7 +104,13 @@ namespace UsesThis {
       +x;
       +A::x;
     }
+
+    template<typename U>
+    static auto h() -> A*;
+
+    template<>
+    auto h<int>() -> decltype(this); // expected-error {{'this' cannot be used in a static member function declaration}}
   };
 
-  template struct A<int>; // expected-note {{in instantiation of function template specialization}}
+  template struct A<int>; // expected-note 2{{in instantiation of}}
 }



More information about the cfe-commits mailing list