Skip to content

[clang] fix sema init crashing on initialization sequences #98102

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.

Sign up for GitHub

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 2 commits into from
Jul 10, 2024
Merged

[clang] fix sema init crashing on initialization sequences #98102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
097881d
[clang] fix sema init crash for not checking a ExprResult
yuxuanchen1997 Jul 9, 2024
4c2b881
Add a crash-on-valid unit test
yuxuanchen1997 Jul 10, 2024
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
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -813,6 +813,8 @@ Bug Fixes in This Version

- Fixed `static_cast` to array of unknown bound. Fixes (#GH62863).

- Fixed Clang crashing when failing to perform some C++ Initialization Sequences. (#GH98102)

Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Expand Down
4 changes: 4 additions & 0 deletions clang/lib/Sema/SemaInit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5576,6 +5576,10 @@ static void TryOrBuildParenListInitialization(
ExprResult ER;
ER = IS.Perform(S, SubEntity, SubKind,
Arg ? MultiExprArg(Arg) : std::nullopt);

if (ER.isInvalid())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Under what circumstances does IS.Failed() return false above but when we perform the initialization we get a null expression back? That seems strange to me -- I would have naively expected Failed() to return true in such a case. WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

If you look at InitializationSequence::Perform (this function is 1000 lines long), it first checks Failed() then proceed to build a whole bunch of things. There are a couple of places it returned ExprError. I think it's possible to still be invalid after !Failed().

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah you are correct, sorry! I see now that the common code pattern is to check isInvalid() on the returned result.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For ExprResult, I tend to find isUsable to be more clear, and I thought is slightly more strict, as it also catches cases where the ER is unset. That said, we use isInvalid all over the place, assuming that it would have been set to SOMETHING.

return false;

if (InitExpr)
*InitExpr = ER.get();
Copy link
Member Author

Choose a reason for hiding this comment

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

ER.get() is null if we don't apply this fix.

else
Expand Down
33 changes: 33 additions & 0 deletions clang/test/SemaCXX/pr98102.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
// expected-no-diagnostics

template <bool v>
struct BC {
static constexpr bool value = v;
};

template <typename T, typename Arg>
struct Constructible : BC<__is_constructible(T, Arg)> {};

template <typename T>
using Requires = T::value;

template <typename T>
struct optional {
template <typename U, Requires<Constructible<T, U>> = true>
optional(U) {}
};

struct MO {};
struct S : MO {};
struct TB {
TB(optional<S>) {}
};

class TD : TB, MO {
using TB::TB;
};

void foo() {
static_assert(Constructible<TD, TD>::value);
}
Loading