[clang] [clang][Sema]fix crash of invalid friend declaration with storage-class (PR #190597)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 21 02:08:18 PDT 2026
https://github.com/firstmoonlight updated https://github.com/llvm/llvm-project/pull/190597
>From a64173fcc8f4bd095bb14530cee6a9f6b9deea4c Mon Sep 17 00:00:00 2001
From: victorl <liuvicsen at gmail.com>
Date: Wed, 8 Apr 2026 21:41:05 +0800
Subject: [PATCH 1/3] [clang][Sema]fix crash of invalid friend declaration with
storage-class specifier
---
clang/include/clang/Sema/DeclSpec.h | 2 +
clang/lib/Sema/DeclSpec.cpp | 157 ++++++++++++-----------
clang/test/CXX/class/class.friend/p6.cpp | 1 +
3 files changed, 83 insertions(+), 77 deletions(-)
diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h
index 6e5421c7072c7..d9e41679ba6ea 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -887,6 +887,8 @@ class DeclSpec {
/// DeclSpec is guaranteed self-consistent, even if an error occurred.
void Finish(Sema &S, const PrintingPolicy &Policy);
+ void CheckTypeSpec(Sema &S, const PrintingPolicy &Policy);
+
const WrittenBuiltinSpecs& getWrittenBuiltinSpecs() const {
return writtenBS;
}
diff --git a/clang/lib/Sema/DeclSpec.cpp b/clang/lib/Sema/DeclSpec.cpp
index 479a959e0aadc..68197dd872c21 100644
--- a/clang/lib/Sema/DeclSpec.cpp
+++ b/clang/lib/Sema/DeclSpec.cpp
@@ -1162,6 +1162,76 @@ void DeclSpec::Finish(Sema &S, const PrintingPolicy &Policy) {
// Check the type specifier components first. No checking for an invalid
// type.
+ CheckTypeSpec(S, Policy);
+
+ // C++ [class.friend]p6:
+ // No storage-class-specifier shall appear in the decl-specifier-seq
+ // of a friend declaration.
+ if (isFriendSpecified() &&
+ (getStorageClassSpec() || getThreadStorageClassSpec())) {
+ SmallString<32> SpecName;
+ SourceLocation SCLoc;
+ FixItHint StorageHint, ThreadHint;
+
+ if (DeclSpec::SCS SC = getStorageClassSpec()) {
+ SpecName = getSpecifierName(SC);
+ SCLoc = getStorageClassSpecLoc();
+ StorageHint = FixItHint::CreateRemoval(SCLoc);
+ }
+
+ if (DeclSpec::TSCS TSC = getThreadStorageClassSpec()) {
+ if (!SpecName.empty())
+ SpecName += " ";
+ SpecName += getSpecifierName(TSC);
+ SCLoc = getThreadStorageClassSpecLoc();
+ ThreadHint = FixItHint::CreateRemoval(SCLoc);
+ }
+
+ S.Diag(SCLoc, diag::err_friend_decl_spec)
+ << SpecName << StorageHint << ThreadHint;
+
+ ClearStorageClassSpecs();
+ }
+
+ // C++11 [dcl.fct.spec]p5:
+ // The virtual specifier shall be used only in the initial
+ // declaration of a non-static class member function;
+ // C++11 [dcl.fct.spec]p6:
+ // The explicit specifier shall be used only in the declaration of
+ // a constructor or conversion function within its class
+ // definition;
+ if (isFriendSpecified() && (isVirtualSpecified() || hasExplicitSpecifier())) {
+ StringRef Keyword;
+ FixItHint Hint;
+ SourceLocation SCLoc;
+
+ if (isVirtualSpecified()) {
+ Keyword = "virtual";
+ SCLoc = getVirtualSpecLoc();
+ Hint = FixItHint::CreateRemoval(SCLoc);
+ } else {
+ Keyword = "explicit";
+ SCLoc = getExplicitSpecLoc();
+ Hint = FixItHint::CreateRemoval(getExplicitSpecRange());
+ }
+
+ S.Diag(SCLoc, diag::err_friend_decl_spec) << Keyword << Hint;
+
+ FS_virtual_specified = false;
+ FS_explicit_specifier = ExplicitSpecifier();
+ FS_virtualLoc = FS_explicitLoc = SourceLocation();
+ }
+
+ assert(!TypeSpecOwned || isDeclRep((TST)TypeSpecType));
+
+ // Okay, now we can infer the real type.
+
+ // TODO: return "auto function" and other bad things based on the real type.
+
+ // 'data definition has no type or storage class'?
+}
+
+void DeclSpec::CheckTypeSpec(Sema &S, const PrintingPolicy &Policy) {
if (TypeSpecType == TST_error)
return;
@@ -1228,8 +1298,8 @@ void DeclSpec::Finish(Sema &S, const PrintingPolicy &Policy) {
(TypeSpecType != TST_int) && (TypeSpecType != TST_int128)) ||
TypeAltiVecPixel) {
S.Diag(TSTLoc, diag::err_invalid_vector_bool_decl_spec)
- << (TypeAltiVecPixel ? "__pixel" :
- getSpecifierName((TST)TypeSpecType, Policy));
+ << (TypeAltiVecPixel ? "__pixel"
+ : getSpecifierName((TST)TypeSpecType, Policy));
}
// vector bool __int128 requires Power10 (or ZVector).
if ((TypeSpecType == TST_int128) &&
@@ -1345,10 +1415,8 @@ void DeclSpec::Finish(Sema &S, const PrintingPolicy &Policy) {
// use. Need information about the backend.
if (TypeSpecComplex != TSC_unspecified) {
if (TypeSpecType == TST_unspecified) {
- S.Diag(TSCLoc, diag::ext_plain_complex)
- << FixItHint::CreateInsertion(
- S.getLocForEndOfToken(getTypeSpecComplexLoc()),
- " double");
+ S.Diag(TSCLoc, diag::ext_plain_complex) << FixItHint::CreateInsertion(
+ S.getLocForEndOfToken(getTypeSpecComplexLoc()), " double");
TypeSpecType = TST_double; // _Complex -> _Complex double.
} else if (TypeSpecType == TST_int || TypeSpecType == TST_char) {
// Note that this intentionally doesn't include _Complex _Bool.
@@ -1378,14 +1446,14 @@ void DeclSpec::Finish(Sema &S, const PrintingPolicy &Policy) {
if (S.getSourceManager().isBeforeInTranslationUnit(
getThreadStorageClassSpecLoc(), getStorageClassSpecLoc()))
S.Diag(getStorageClassSpecLoc(),
- diag::err_invalid_decl_spec_combination)
- << DeclSpec::getSpecifierName(getThreadStorageClassSpec())
- << SourceRange(getThreadStorageClassSpecLoc());
+ diag::err_invalid_decl_spec_combination)
+ << DeclSpec::getSpecifierName(getThreadStorageClassSpec())
+ << SourceRange(getThreadStorageClassSpecLoc());
else
S.Diag(getThreadStorageClassSpecLoc(),
- diag::err_invalid_decl_spec_combination)
- << DeclSpec::getSpecifierName(getStorageClassSpec())
- << SourceRange(getStorageClassSpecLoc());
+ diag::err_invalid_decl_spec_combination)
+ << DeclSpec::getSpecifierName(getStorageClassSpec())
+ << SourceRange(getStorageClassSpecLoc());
// Discard the thread storage class specifier to recover.
ThreadStorageClassSpec = TSCS_unspecified;
ThreadStorageClassSpecLoc = SourceLocation();
@@ -1442,71 +1510,6 @@ void DeclSpec::Finish(Sema &S, const PrintingPolicy &Policy) {
S.Diag(ConstexprLoc, diag::warn_cxx20_compat_consteval);
else if (getConstexprSpecifier() == ConstexprSpecKind::Constinit)
S.Diag(ConstexprLoc, diag::warn_cxx20_compat_constinit);
- // C++ [class.friend]p6:
- // No storage-class-specifier shall appear in the decl-specifier-seq
- // of a friend declaration.
- if (isFriendSpecified() &&
- (getStorageClassSpec() || getThreadStorageClassSpec())) {
- SmallString<32> SpecName;
- SourceLocation SCLoc;
- FixItHint StorageHint, ThreadHint;
-
- if (DeclSpec::SCS SC = getStorageClassSpec()) {
- SpecName = getSpecifierName(SC);
- SCLoc = getStorageClassSpecLoc();
- StorageHint = FixItHint::CreateRemoval(SCLoc);
- }
-
- if (DeclSpec::TSCS TSC = getThreadStorageClassSpec()) {
- if (!SpecName.empty()) SpecName += " ";
- SpecName += getSpecifierName(TSC);
- SCLoc = getThreadStorageClassSpecLoc();
- ThreadHint = FixItHint::CreateRemoval(SCLoc);
- }
-
- S.Diag(SCLoc, diag::err_friend_decl_spec)
- << SpecName << StorageHint << ThreadHint;
-
- ClearStorageClassSpecs();
- }
-
- // C++11 [dcl.fct.spec]p5:
- // The virtual specifier shall be used only in the initial
- // declaration of a non-static class member function;
- // C++11 [dcl.fct.spec]p6:
- // The explicit specifier shall be used only in the declaration of
- // a constructor or conversion function within its class
- // definition;
- if (isFriendSpecified() && (isVirtualSpecified() || hasExplicitSpecifier())) {
- StringRef Keyword;
- FixItHint Hint;
- SourceLocation SCLoc;
-
- if (isVirtualSpecified()) {
- Keyword = "virtual";
- SCLoc = getVirtualSpecLoc();
- Hint = FixItHint::CreateRemoval(SCLoc);
- } else {
- Keyword = "explicit";
- SCLoc = getExplicitSpecLoc();
- Hint = FixItHint::CreateRemoval(getExplicitSpecRange());
- }
-
- S.Diag(SCLoc, diag::err_friend_decl_spec)
- << Keyword << Hint;
-
- FS_virtual_specified = false;
- FS_explicit_specifier = ExplicitSpecifier();
- FS_virtualLoc = FS_explicitLoc = SourceLocation();
- }
-
- assert(!TypeSpecOwned || isDeclRep((TST) TypeSpecType));
-
- // Okay, now we can infer the real type.
-
- // TODO: return "auto function" and other bad things based on the real type.
-
- // 'data definition has no type or storage class'?
}
bool DeclSpec::isMissingDeclaratorOk() {
diff --git a/clang/test/CXX/class/class.friend/p6.cpp b/clang/test/CXX/class/class.friend/p6.cpp
index e4c59f781e3de..a96dd8a3d4e4a 100644
--- a/clang/test/CXX/class/class.friend/p6.cpp
+++ b/clang/test/CXX/class/class.friend/p6.cpp
@@ -19,4 +19,5 @@ class A {
#else
friend thread_local class G; // expected-error {{'thread_local' is invalid in friend declarations}}
#endif
+ friend register enum; // expected-error {{expected identifier or '{'}} expected-error {{'register' is invalid in friend declarations}}
};
>From 976342ec548b0ccb534d4700cc412e6097623c25 Mon Sep 17 00:00:00 2001
From: victorl <liuvicsen at gmail.com>
Date: Sun, 12 Apr 2026 22:58:31 +0800
Subject: [PATCH 2/3] add function CheckFriendSpec
---
clang/include/clang/Sema/DeclSpec.h | 2 +
clang/lib/Sema/DeclSpec.cpp | 118 ++++++++++++++--------------
2 files changed, 63 insertions(+), 57 deletions(-)
diff --git a/clang/include/clang/Sema/DeclSpec.h b/clang/include/clang/Sema/DeclSpec.h
index d9e41679ba6ea..35119d8b29688 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -889,6 +889,8 @@ class DeclSpec {
void CheckTypeSpec(Sema &S, const PrintingPolicy &Policy);
+ void CheckFriendSpec(Sema &S, const PrintingPolicy &Policy);
+
const WrittenBuiltinSpecs& getWrittenBuiltinSpecs() const {
return writtenBS;
}
diff --git a/clang/lib/Sema/DeclSpec.cpp b/clang/lib/Sema/DeclSpec.cpp
index 68197dd872c21..5ee5ab809a59e 100644
--- a/clang/lib/Sema/DeclSpec.cpp
+++ b/clang/lib/Sema/DeclSpec.cpp
@@ -1164,63 +1164,7 @@ void DeclSpec::Finish(Sema &S, const PrintingPolicy &Policy) {
// type.
CheckTypeSpec(S, Policy);
- // C++ [class.friend]p6:
- // No storage-class-specifier shall appear in the decl-specifier-seq
- // of a friend declaration.
- if (isFriendSpecified() &&
- (getStorageClassSpec() || getThreadStorageClassSpec())) {
- SmallString<32> SpecName;
- SourceLocation SCLoc;
- FixItHint StorageHint, ThreadHint;
-
- if (DeclSpec::SCS SC = getStorageClassSpec()) {
- SpecName = getSpecifierName(SC);
- SCLoc = getStorageClassSpecLoc();
- StorageHint = FixItHint::CreateRemoval(SCLoc);
- }
-
- if (DeclSpec::TSCS TSC = getThreadStorageClassSpec()) {
- if (!SpecName.empty())
- SpecName += " ";
- SpecName += getSpecifierName(TSC);
- SCLoc = getThreadStorageClassSpecLoc();
- ThreadHint = FixItHint::CreateRemoval(SCLoc);
- }
-
- S.Diag(SCLoc, diag::err_friend_decl_spec)
- << SpecName << StorageHint << ThreadHint;
-
- ClearStorageClassSpecs();
- }
-
- // C++11 [dcl.fct.spec]p5:
- // The virtual specifier shall be used only in the initial
- // declaration of a non-static class member function;
- // C++11 [dcl.fct.spec]p6:
- // The explicit specifier shall be used only in the declaration of
- // a constructor or conversion function within its class
- // definition;
- if (isFriendSpecified() && (isVirtualSpecified() || hasExplicitSpecifier())) {
- StringRef Keyword;
- FixItHint Hint;
- SourceLocation SCLoc;
-
- if (isVirtualSpecified()) {
- Keyword = "virtual";
- SCLoc = getVirtualSpecLoc();
- Hint = FixItHint::CreateRemoval(SCLoc);
- } else {
- Keyword = "explicit";
- SCLoc = getExplicitSpecLoc();
- Hint = FixItHint::CreateRemoval(getExplicitSpecRange());
- }
-
- S.Diag(SCLoc, diag::err_friend_decl_spec) << Keyword << Hint;
-
- FS_virtual_specified = false;
- FS_explicit_specifier = ExplicitSpecifier();
- FS_virtualLoc = FS_explicitLoc = SourceLocation();
- }
+ CheckFriendSpec(S, Policy);
assert(!TypeSpecOwned || isDeclRep((TST)TypeSpecType));
@@ -1512,6 +1456,66 @@ void DeclSpec::CheckTypeSpec(Sema &S, const PrintingPolicy &Policy) {
S.Diag(ConstexprLoc, diag::warn_cxx20_compat_constinit);
}
+void DeclSpec::CheckFriendSpec(Sema &S, const PrintingPolicy &Policy) {
+ // C++ [class.friend]p6:
+ // No storage-class-specifier shall appear in the decl-specifier-seq
+ // of a friend declaration.
+ if (isFriendSpecified() &&
+ (getStorageClassSpec() || getThreadStorageClassSpec())) {
+ SmallString<32> SpecName;
+ SourceLocation SCLoc;
+ FixItHint StorageHint, ThreadHint;
+
+ if (DeclSpec::SCS SC = getStorageClassSpec()) {
+ SpecName = getSpecifierName(SC);
+ SCLoc = getStorageClassSpecLoc();
+ StorageHint = FixItHint::CreateRemoval(SCLoc);
+ }
+
+ if (DeclSpec::TSCS TSC = getThreadStorageClassSpec()) {
+ if (!SpecName.empty())
+ SpecName += " ";
+ SpecName += getSpecifierName(TSC);
+ SCLoc = getThreadStorageClassSpecLoc();
+ ThreadHint = FixItHint::CreateRemoval(SCLoc);
+ }
+
+ S.Diag(SCLoc, diag::err_friend_decl_spec)
+ << SpecName << StorageHint << ThreadHint;
+
+ ClearStorageClassSpecs();
+ }
+
+ // C++11 [dcl.fct.spec]p5:
+ // The virtual specifier shall be used only in the initial
+ // declaration of a non-static class member function;
+ // C++11 [dcl.fct.spec]p6:
+ // The explicit specifier shall be used only in the declaration of
+ // a constructor or conversion function within its class
+ // definition;
+ if (isFriendSpecified() && (isVirtualSpecified() || hasExplicitSpecifier())) {
+ StringRef Keyword;
+ FixItHint Hint;
+ SourceLocation SCLoc;
+
+ if (isVirtualSpecified()) {
+ Keyword = "virtual";
+ SCLoc = getVirtualSpecLoc();
+ Hint = FixItHint::CreateRemoval(SCLoc);
+ } else {
+ Keyword = "explicit";
+ SCLoc = getExplicitSpecLoc();
+ Hint = FixItHint::CreateRemoval(getExplicitSpecRange());
+ }
+
+ S.Diag(SCLoc, diag::err_friend_decl_spec) << Keyword << Hint;
+
+ FS_virtual_specified = false;
+ FS_explicit_specifier = ExplicitSpecifier();
+ FS_virtualLoc = FS_explicitLoc = SourceLocation();
+ }
+}
+
bool DeclSpec::isMissingDeclaratorOk() {
TST tst = getTypeSpecType();
return isDeclRep(tst) && getRepAsDecl() != nullptr &&
>From 3fa5b0266465f86521a1ef0e00a229d1fc340f39 Mon Sep 17 00:00:00 2001
From: victorl <liuvicsen at gmail.com>
Date: Tue, 21 Apr 2026 17:07:17 +0800
Subject: [PATCH 3/3] revert some unnecessary formart
---
clang/lib/Sema/DeclSpec.cpp | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)
diff --git a/clang/lib/Sema/DeclSpec.cpp b/clang/lib/Sema/DeclSpec.cpp
index 5ee5ab809a59e..bae08c213e9ec 100644
--- a/clang/lib/Sema/DeclSpec.cpp
+++ b/clang/lib/Sema/DeclSpec.cpp
@@ -1242,8 +1242,8 @@ void DeclSpec::CheckTypeSpec(Sema &S, const PrintingPolicy &Policy) {
(TypeSpecType != TST_int) && (TypeSpecType != TST_int128)) ||
TypeAltiVecPixel) {
S.Diag(TSTLoc, diag::err_invalid_vector_bool_decl_spec)
- << (TypeAltiVecPixel ? "__pixel"
- : getSpecifierName((TST)TypeSpecType, Policy));
+ << (TypeAltiVecPixel ? "__pixel" :
+ getSpecifierName((TST)TypeSpecType, Policy));
}
// vector bool __int128 requires Power10 (or ZVector).
if ((TypeSpecType == TST_int128) &&
@@ -1359,8 +1359,10 @@ void DeclSpec::CheckTypeSpec(Sema &S, const PrintingPolicy &Policy) {
// use. Need information about the backend.
if (TypeSpecComplex != TSC_unspecified) {
if (TypeSpecType == TST_unspecified) {
- S.Diag(TSCLoc, diag::ext_plain_complex) << FixItHint::CreateInsertion(
- S.getLocForEndOfToken(getTypeSpecComplexLoc()), " double");
+ S.Diag(TSCLoc, diag::ext_plain_complex)
+ << FixItHint::CreateInsertion(
+ S.getLocForEndOfToken(getTypeSpecComplexLoc()),
+ " double");
TypeSpecType = TST_double; // _Complex -> _Complex double.
} else if (TypeSpecType == TST_int || TypeSpecType == TST_char) {
// Note that this intentionally doesn't include _Complex _Bool.
@@ -1390,14 +1392,14 @@ void DeclSpec::CheckTypeSpec(Sema &S, const PrintingPolicy &Policy) {
if (S.getSourceManager().isBeforeInTranslationUnit(
getThreadStorageClassSpecLoc(), getStorageClassSpecLoc()))
S.Diag(getStorageClassSpecLoc(),
- diag::err_invalid_decl_spec_combination)
- << DeclSpec::getSpecifierName(getThreadStorageClassSpec())
- << SourceRange(getThreadStorageClassSpecLoc());
+ diag::err_invalid_decl_spec_combination)
+ << DeclSpec::getSpecifierName(getThreadStorageClassSpec())
+ << SourceRange(getThreadStorageClassSpecLoc());
else
S.Diag(getThreadStorageClassSpecLoc(),
- diag::err_invalid_decl_spec_combination)
- << DeclSpec::getSpecifierName(getStorageClassSpec())
- << SourceRange(getStorageClassSpecLoc());
+ diag::err_invalid_decl_spec_combination)
+ << DeclSpec::getSpecifierName(getStorageClassSpec())
+ << SourceRange(getStorageClassSpecLoc());
// Discard the thread storage class specifier to recover.
ThreadStorageClassSpec = TSCS_unspecified;
ThreadStorageClassSpecLoc = SourceLocation();
@@ -1473,15 +1475,14 @@ void DeclSpec::CheckFriendSpec(Sema &S, const PrintingPolicy &Policy) {
}
if (DeclSpec::TSCS TSC = getThreadStorageClassSpec()) {
- if (!SpecName.empty())
- SpecName += " ";
+ if (!SpecName.empty()) SpecName += " ";
SpecName += getSpecifierName(TSC);
SCLoc = getThreadStorageClassSpecLoc();
ThreadHint = FixItHint::CreateRemoval(SCLoc);
}
S.Diag(SCLoc, diag::err_friend_decl_spec)
- << SpecName << StorageHint << ThreadHint;
+ << SpecName << StorageHint << ThreadHint;
ClearStorageClassSpecs();
}
@@ -1508,7 +1509,8 @@ void DeclSpec::CheckFriendSpec(Sema &S, const PrintingPolicy &Policy) {
Hint = FixItHint::CreateRemoval(getExplicitSpecRange());
}
- S.Diag(SCLoc, diag::err_friend_decl_spec) << Keyword << Hint;
+ S.Diag(SCLoc, diag::err_friend_decl_spec)
+ << Keyword << Hint;
FS_virtual_specified = false;
FS_explicit_specifier = ExplicitSpecifier();
More information about the cfe-commits
mailing list