Skip to content

[clang][Sema] Add checks for validity of default ctor's class #78898

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

Merged
merged 6 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,10 @@ Bug Fixes to Attribute Support
Bug Fixes to C++ Support
^^^^^^^^^^^^^^^^^^^^^^^^

- Fix crash when calling the constructor of an invalid class.
Fixes (`#10518 <https://github.com/llvm/llvm-project/issues/10518>`_),
(`#67914 <https://github.com/llvm/llvm-project/issues/10518>`_),
and (`#78388 <https://github.com/llvm/llvm-project/issues/78388>`_)
- Fix crash when using lifetimebound attribute in function with trailing return.
Fixes (`#73619 <https://github.com/llvm/llvm-project/issues/73619>`_)
- Addressed an issue where constraints involving injected class types are perceived
Expand Down
7 changes: 7 additions & 0 deletions clang/lib/Sema/SemaDeclCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5998,6 +5998,10 @@ void Sema::ActOnDefaultCtorInitializers(Decl *CDtorDecl) {

if (CXXConstructorDecl *Constructor
= dyn_cast<CXXConstructorDecl>(CDtorDecl)) {
if (CXXRecordDecl *ClassDecl = Constructor->getParent();
!ClassDecl || ClassDecl->isInvalidDecl()) {
return;
}
Comment on lines +6001 to +6004
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (CXXRecordDecl *ClassDecl = Constructor->getParent();
!ClassDecl || ClassDecl->isInvalidDecl()) {
return;
}
if (CXXRecordDecl *ClassDecl = Constructor->getParent();
!ClassDecl || ClassDecl->isInvalidDecl())
return;

SetCtorInitializers(Constructor, /*AnyErrors=*/false);
DiagnoseUninitializedFields(*this, Constructor);
}
Expand Down Expand Up @@ -14038,6 +14042,9 @@ void Sema::DefineImplicitDefaultConstructor(SourceLocation CurrentLocation,

CXXRecordDecl *ClassDecl = Constructor->getParent();
assert(ClassDecl && "DefineImplicitDefaultConstructor - invalid constructor");
if (ClassDecl->isInvalidDecl()) {
return;
}
Comment on lines +14045 to +14047
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (ClassDecl->isInvalidDecl()) {
return;
}
if (ClassDecl->isInvalidDecl())
return;


SynthesizedFunctionScope Scope(*this, Constructor);

Expand Down
22 changes: 22 additions & 0 deletions clang/test/SemaCXX/crash-GH10518.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// RUN: %clang_cc1 -verify -std=c++98 %s
// RUN: %clang_cc1 -verify -std=c++11 %s
// RUN: %clang_cc1 -verify -std=c++14 %s
// RUN: %clang_cc1 -verify -std=c++17 %s
// RUN: %clang_cc1 -verify -std=c++20 %s
// RUN: %clang_cc1 -verify -std=c++23 %s
// RUN: %clang_cc1 -verify -std=c++2c %s
Comment on lines +1 to +7
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do 11 and 2c if you want, i don't think it's useful to test all language modes for these tests (ditto the other ones)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since newer language modes do not necessarily subsume older ones (think dynamic exception specification), and have disabled code paths, I'm not sure we want to be smart about which language modes to test. If we're going to be smart, then yes, 98, 11, and 2c sounds like a good compromise.


// https://github.com/llvm/llvm-project/issues/10518

template <class T>
class A : public T {
};

template <class T>
class B : public A<T> {
};

template <class T>
class B<int> : public A<T> { // expected-error 0-1 {{}}
B(T *t) {}
};
13 changes: 13 additions & 0 deletions clang/test/SemaCXX/crash-GH49103-2.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// RUN: %clang_cc1 -verify -std=c++98 %s
// RUN: %clang_cc1 -verify -std=c++11 %s
// RUN: %clang_cc1 -verify -std=c++14 %s
// RUN: %clang_cc1 -verify -std=c++17 %s
// RUN: %clang_cc1 -verify -std=c++20 %s
// RUN: %clang_cc1 -verify -std=c++23 %s
// RUN: %clang_cc1 -verify -std=c++2c %s

// https://github.com/llvm/llvm-project/issues/49103

template<class> struct A; // expected-note 0+ {{}}
struct S : __make_integer_seq<A, int, 42> { }; // expected-error 0+ {{}}
S s;
78 changes: 78 additions & 0 deletions clang/test/SemaCXX/crash-GH67914.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// RUN: %clang_cc1 -verify -std=c++98 %s
// RUN: %clang_cc1 -verify -std=c++11 %s
// RUN: %clang_cc1 -verify -std=c++14 %s
// RUN: %clang_cc1 -verify -std=c++17 %s
// RUN: %clang_cc1 -verify -std=c++20 %s
// RUN: %clang_cc1 -verify -std=c++23 %s
// RUN: %clang_cc1 -verify -std=c++2c %s

// https://github.com/llvm/llvm-project/issues/67914

template < typename, int >
struct Mask;

template < int, class >
struct conditional {
using type = Mask< int, 16 >; // expected-warning 0+ {{}}
};

template < class _Then >
struct conditional< 0, _Then > {
using type = _Then; // expected-warning 0+ {{}}
};

template < int _Bp, class, class _Then >
using conditional_t = typename conditional< _Bp, _Then >::type; // expected-warning 0+ {{}}

template < typename, int >
struct Array;

template < typename, int, bool, typename >
struct StaticArrayImpl;

template < typename Value_, int Size_ >
struct Mask : StaticArrayImpl< Value_, Size_, 1, Mask< Value_, Size_ > > { // expected-note 0+ {{}}
template < typename T1 >
Mask(T1) {} // expected-note 0+ {{}}
};

template < typename T >
void load(typename T::MaskType mask) {
T::load_(mask); // expected-note 0+ {{}}
}

template < typename Value_, int IsMask_, typename Derived_ >
struct StaticArrayImpl< Value_, 32, IsMask_, Derived_ > {
using Array1 = conditional_t< IsMask_, void, Array< Value_, 16 > >; // expected-warning 0+ {{}}

template < typename Mask >
static Derived_ load_(Mask mask) {
return Derived_{load< Array1 >(mask.a1), Mask{}}; // expected-error 0+ {{}}
}

Array1 a1;
};

template < typename Derived_ >
struct KMaskBase;

template < typename Derived_ >
struct StaticArrayImpl< float, 16, 0, Derived_ > {
template < typename Mask >
static Derived_ load_(Mask mask);
};

template < typename Derived_ >
struct StaticArrayImpl< float, 16, 1, Mask< float, 16 > > : KMaskBase< Derived_ > {}; // expected-error 0+ {{}}

template < typename Derived_ >
struct StaticArrayImpl< int, 16, 1, Derived_ > {};

template < typename Value_, int Size_ >
struct Array : StaticArrayImpl< Value_, Size_, 0, Array< Value_, Size_ > > {
using MaskType = Mask< Value_, Size_ >; // expected-warning 0+ {{}}
};

void test11_load_masked() {
load< Array< float, 32 > >{} == 0; // expected-error 0+ {{}} expected-warning 0+ {{}} expected-note 0+ {{}}
}
17 changes: 17 additions & 0 deletions clang/test/SemaCXX/crash-GH78388.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// RUN: %clang_cc1 -verify -std=c++98 %s
// RUN: %clang_cc1 -verify -std=c++11 %s
// RUN: %clang_cc1 -verify -std=c++14 %s
// RUN: %clang_cc1 -verify -std=c++17 %s
// RUN: %clang_cc1 -verify -std=c++20 %s
// RUN: %clang_cc1 -verify -std=c++23 %s
// RUN: %clang_cc1 -verify -std=c++2c %s

// https://github.com/llvm/llvm-project/issues/78388

typedef mbstate_t; // expected-error 0+ {{}} expected-note 0+ {{}}
template < typename , typename , typename >
class a // expected-error 0+ {{}}
class b { // expected-error 0+ {{}}
namespace { // expected-note 0+ {{}} expected-note 0+ {{}}
template < typename c > b::operator=() { // expected-error 0+ {{}} expected-note 0+ {{}}
struct :a< c, char, stdmbstate_t > d // expected-error 0+ {{}} expected-warning 0+ {{}}
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ void test() {
e.transform_error(return_unexpected<int&>); // expected-error-re@*:* {{static assertion failed {{.*}}The result of {{.*}} must be a valid template argument for unexpected}}
// expected-error-re@*:* 0-1 {{{{(excess elements in struct initializer|no matching constructor for initialization of)}}{{.*}}}}
// expected-error-re@*:* {{static assertion failed {{.*}}A program that instantiates expected<T, E> with a E that is not a valid argument for unexpected<E> is ill-formed}}
// expected-error-re@*:* {{call to deleted constructor of {{.*}}}}
// expected-error-re@*:* 0-1 {{call to deleted constructor of {{.*}}}}
// expected-error-re@*:* {{union member {{.*}} has reference type {{.*}}}}

e.transform_error(return_no_object<int&>); // expected-error-re@*:* {{static assertion failed {{.*}}The result of {{.*}} must be a valid template argument for unexpected}}
Expand Down