-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[clang] Fix crash on invalid std::initializer_list<T> template-id
#132284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
In `Sema::BuildStdInitializerList`, check that the synthesized template-id `std::initializer_list<T>` is valid (which might not be the case if the template has associated constraints or subsequent parameters with default arguments) before forming the type.
|
@llvm/pr-subscribers-clang Author: None (offsetof) ChangesIn Fixes #132256 Full diff: https://github.com/llvm/llvm-project/pull/132284.diff 2 Files Affected:
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 7b2e0df8cb55d..54f7bc9a3b021 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -12182,10 +12182,14 @@ QualType Sema::BuildStdInitializerList(QualType Element, SourceLocation Loc) {
Args.addArgument(TemplateArgumentLoc(TemplateArgument(Element),
Context.getTrivialTypeSourceInfo(Element,
Loc)));
+
+ QualType T = CheckTemplateIdType(TemplateName(StdInitializerList), Loc, Args);
+ if (T.isNull())
+ return QualType();
+
return Context.getElaboratedType(
ElaboratedTypeKeyword::None,
- NestedNameSpecifier::Create(Context, nullptr, getStdNamespace()),
- CheckTemplateIdType(TemplateName(StdInitializerList), Loc, Args));
+ NestedNameSpecifier::Create(Context, nullptr, getStdNamespace()), T);
}
bool Sema::isInitListConstructor(const FunctionDecl *Ctor) {
diff --git a/clang/test/SemaCXX/invalid-std-initializer-list.cpp b/clang/test/SemaCXX/invalid-std-initializer-list.cpp
new file mode 100644
index 0000000000000..080a712759c45
--- /dev/null
+++ b/clang/test/SemaCXX/invalid-std-initializer-list.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 %s -verify -std=c++20
+
+namespace std {
+
+template<class T, class = T::x> // expected-error 2 {{type 'int' cannot be used prior to '::' because it has no members}}
+class initializer_list;
+
+}
+
+auto x = {1}; // expected-note {{in instantiation of default argument for 'initializer_list<int>' required here}}
+
+void f() {
+ for(int x : {1, 2}); // expected-note {{in instantiation of default argument for 'initializer_list<int>' required here}}
+}
|
zyn0217
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a release note, otherwise looks good.
|
|
||
| auto x = {1}; // expected-note {{in instantiation of default argument for 'initializer_list<int>' required here}} | ||
|
|
||
| void f() { | ||
| for(int x : {1, 2}); // expected-note {{in instantiation of default argument for 'initializer_list<int>' required here}} | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We usually wrap the test with a namespace of its corresponding GH number.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a namespace and a release note.
|
@ojhunt as a fyi :) |
cor3ntin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Will you need me to merge that for you?
| Context.getTrivialTypeSourceInfo(Element, | ||
| Loc))); | ||
|
|
||
| QualType T = CheckTemplateIdType(TemplateName(StdInitializerList), Loc, Args); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably keep this as an additional sanity check, however I think in this particular case the error is in the validity check in LookupStdInitializerList, where it is checking the minimum required template parameters, rather than ensuring real correctness.
It should probably be doing Params->size() != 1, and ensuring that there isn't a parameter pack or any default template arguments.
Changing those validity checks will also result in a much more direct error saying std::initializer_list is defined incorrectly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened a separate PR for this change: #133822
Yes please, thank you @cor3ntin |
In
Sema::BuildStdInitializerList, check that the synthesized template-idstd::initializer_list<T>is valid (which might not be the case if the template has associated constraints or subsequent parameters with default arguments) before forming the type.Fixes #132256