[clang] [Sema] Fix crash on invalid code with parenthesized aggregate initialization (PR #76232)
https://github.com/ilya-biryukov created https://github.com/llvm/llvm-project/pull/76232 Fixes #76228. Use the same logic as braced init lists, also adds a test that puts incomplete types in various positions to check for regressions in the future. >From 491f3b09a2064c82c1646ca1d0c2987478bb4f51 Mon Sep 17 00:00:00 2001 From: Ilya Biryukov Date: Fri, 22 Dec 2023 12:33:34 +0100 Subject: [PATCH] [Sema] Fix crash on invalid code with parenthesized aggregate initialization Fixes #76228. Use the same logic as braced init lists, also adds a test that puts incomplete types in various positions to check for regressions in the future. --- clang/lib/Sema/SemaInit.cpp | 8 clang/test/SemaCXX/crash-GH76228.cpp | 28 2 files changed, 36 insertions(+) create mode 100644 clang/test/SemaCXX/crash-GH76228.cpp diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index d5ba7fd3413718..f768d2726b0a1c 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -5512,6 +5512,14 @@ static void TryOrBuildParenListInitialization( } else if (auto *RT = Entity.getType()->getAs()) { bool IsUnion = RT->isUnionType(); const CXXRecordDecl *RD = cast(RT->getDecl()); +if (RD->isInvalidDecl()) { + // Exit early to avoid confusion when processing members. + // We do the same for braced list initialization in + // `CheckStructUnionTypes`. + Sequence.SetFailed( + clang::InitializationSequence::FK_ParenthesizedListInitFailed); + return; +} if (!IsUnion) { for (const CXXBaseSpecifier &Base : RD->bases()) { diff --git a/clang/test/SemaCXX/crash-GH76228.cpp b/clang/test/SemaCXX/crash-GH76228.cpp new file mode 100644 index 00..a10b9994c5e532 --- /dev/null +++ b/clang/test/SemaCXX/crash-GH76228.cpp @@ -0,0 +1,28 @@ +// RUN: %clang_cc1 -std=c++20 -verify %s +// Check we don't crash on incomplete members and bases when handling parenthesized initialization. +class incomplete; // expected-note@-0 3 {{forward declaration of 'incomplete'}} +struct foo { + int a; + incomplete b; + // expected-error@-1 {{incomplete type}} +}; +foo a1(0); + +struct one_int { +int a; +}; +struct bar : one_int, incomplete {}; +// expected-error@-1 {{incomplete type}} +bar a2(0); + +incomplete a3[3](1,2,3); +// expected-error@-1 {{incomplete type}} + +struct qux : foo { +}; +qux a4(0); + +struct fred { +foo a[3]; +}; +fred a5(0); \ No newline at end of file ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Fix crash on invalid code with parenthesized aggregate initialization (PR #76232)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Ilya Biryukov (ilya-biryukov) Changes Fixes #76228. Use the same logic as braced init lists, also adds a test that puts incomplete types in various positions to check for regressions in the future. --- Full diff: https://github.com/llvm/llvm-project/pull/76232.diff 2 Files Affected: - (modified) clang/lib/Sema/SemaInit.cpp (+8) - (added) clang/test/SemaCXX/crash-GH76228.cpp (+28) ``diff diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index d5ba7fd3413718..f768d2726b0a1c 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -5512,6 +5512,14 @@ static void TryOrBuildParenListInitialization( } else if (auto *RT = Entity.getType()->getAs()) { bool IsUnion = RT->isUnionType(); const CXXRecordDecl *RD = cast(RT->getDecl()); +if (RD->isInvalidDecl()) { + // Exit early to avoid confusion when processing members. + // We do the same for braced list initialization in + // `CheckStructUnionTypes`. + Sequence.SetFailed( + clang::InitializationSequence::FK_ParenthesizedListInitFailed); + return; +} if (!IsUnion) { for (const CXXBaseSpecifier &Base : RD->bases()) { diff --git a/clang/test/SemaCXX/crash-GH76228.cpp b/clang/test/SemaCXX/crash-GH76228.cpp new file mode 100644 index 00..a10b9994c5e532 --- /dev/null +++ b/clang/test/SemaCXX/crash-GH76228.cpp @@ -0,0 +1,28 @@ +// RUN: %clang_cc1 -std=c++20 -verify %s +// Check we don't crash on incomplete members and bases when handling parenthesized initialization. +class incomplete; // expected-note@-0 3 {{forward declaration of 'incomplete'}} +struct foo { + int a; + incomplete b; + // expected-error@-1 {{incomplete type}} +}; +foo a1(0); + +struct one_int { +int a; +}; +struct bar : one_int, incomplete {}; +// expected-error@-1 {{incomplete type}} +bar a2(0); + +incomplete a3[3](1,2,3); +// expected-error@-1 {{incomplete type}} + +struct qux : foo { +}; +qux a4(0); + +struct fred { +foo a[3]; +}; +fred a5(0); \ No newline at end of file `` https://github.com/llvm/llvm-project/pull/76232 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Fix crash on invalid code with parenthesized aggregate initialization (PR #76232)
https://github.com/ilya-biryukov updated https://github.com/llvm/llvm-project/pull/76232 >From 491f3b09a2064c82c1646ca1d0c2987478bb4f51 Mon Sep 17 00:00:00 2001 From: Ilya Biryukov Date: Fri, 22 Dec 2023 12:33:34 +0100 Subject: [PATCH 1/2] [Sema] Fix crash on invalid code with parenthesized aggregate initialization Fixes #76228. Use the same logic as braced init lists, also adds a test that puts incomplete types in various positions to check for regressions in the future. --- clang/lib/Sema/SemaInit.cpp | 8 clang/test/SemaCXX/crash-GH76228.cpp | 28 2 files changed, 36 insertions(+) create mode 100644 clang/test/SemaCXX/crash-GH76228.cpp diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index d5ba7fd3413718..f768d2726b0a1c 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -5512,6 +5512,14 @@ static void TryOrBuildParenListInitialization( } else if (auto *RT = Entity.getType()->getAs()) { bool IsUnion = RT->isUnionType(); const CXXRecordDecl *RD = cast(RT->getDecl()); +if (RD->isInvalidDecl()) { + // Exit early to avoid confusion when processing members. + // We do the same for braced list initialization in + // `CheckStructUnionTypes`. + Sequence.SetFailed( + clang::InitializationSequence::FK_ParenthesizedListInitFailed); + return; +} if (!IsUnion) { for (const CXXBaseSpecifier &Base : RD->bases()) { diff --git a/clang/test/SemaCXX/crash-GH76228.cpp b/clang/test/SemaCXX/crash-GH76228.cpp new file mode 100644 index 00..a10b9994c5e532 --- /dev/null +++ b/clang/test/SemaCXX/crash-GH76228.cpp @@ -0,0 +1,28 @@ +// RUN: %clang_cc1 -std=c++20 -verify %s +// Check we don't crash on incomplete members and bases when handling parenthesized initialization. +class incomplete; // expected-note@-0 3 {{forward declaration of 'incomplete'}} +struct foo { + int a; + incomplete b; + // expected-error@-1 {{incomplete type}} +}; +foo a1(0); + +struct one_int { +int a; +}; +struct bar : one_int, incomplete {}; +// expected-error@-1 {{incomplete type}} +bar a2(0); + +incomplete a3[3](1,2,3); +// expected-error@-1 {{incomplete type}} + +struct qux : foo { +}; +qux a4(0); + +struct fred { +foo a[3]; +}; +fred a5(0); \ No newline at end of file >From c8b0de00c1836cb6eaf864081139886ead3f20cc Mon Sep 17 00:00:00 2001 From: Ilya Biryukov Date: Fri, 22 Dec 2023 12:55:51 +0100 Subject: [PATCH 2/2] Add a trailing newline to the test file --- clang/test/SemaCXX/crash-GH76228.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/SemaCXX/crash-GH76228.cpp b/clang/test/SemaCXX/crash-GH76228.cpp index a10b9994c5e532..33a9395823127e 100644 --- a/clang/test/SemaCXX/crash-GH76228.cpp +++ b/clang/test/SemaCXX/crash-GH76228.cpp @@ -25,4 +25,4 @@ qux a4(0); struct fred { foo a[3]; }; -fred a5(0); \ No newline at end of file +fred a5(0); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Fix crash on invalid code with parenthesized aggregate initialization (PR #76232)
https://github.com/hokein approved this pull request. Thanks, looks good. https://github.com/llvm/llvm-project/pull/76232 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Fix crash on invalid code with parenthesized aggregate initialization (PR #76232)
https://github.com/ilya-biryukov closed https://github.com/llvm/llvm-project/pull/76232 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Fix crash on invalid code with parenthesized aggregate initialization (PR #76232)
https://github.com/shafik commented: This is missing a release note. https://github.com/llvm/llvm-project/pull/76232 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Fix crash on invalid code with parenthesized aggregate initialization (PR #76232)
vitalybuka wrote: Breaks multiple bots https://lab.llvm.org/buildbot/#/builders/168/builds/17604 https://lab.llvm.org/buildbot/#/builders/74/builds/24426 https://lab.llvm.org/buildbot/#/builders/239/builds/4971 and some others https://github.com/llvm/llvm-project/pull/76232 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Fix crash on invalid code with parenthesized aggregate initialization (PR #76232)
vvereschaka wrote: these changes break the libc++ tests - `llvm-libc++-static.cfg.in::transform_error.mandates.verify.cpp` ``` # .---command stderr # | error: 'expected-error' diagnostics expected but not seen: # | File * Line * (directive at C:\buildbot\as-builder-2\x-aarch64\llvm-project\libcxx\test\libcxx\utilities\expected\expected.void\transform_error.mandates.verify.cpp:49): {{(excess elements in struct initializer|no matching constructor for initialization of)}}{{.*}} # | File * Line * (directive at C:\buildbot\as-builder-2\x-aarch64\llvm-project\libcxx\test\libcxx\utilities\expected\expected.void\transform_error.mandates.verify.cpp:53): {{(excess elements in struct initializer|no matching constructor for initialization of)}}{{.*}} # | File * Line * (directive at C:\buildbot\as-builder-2\x-aarch64\llvm-project\libcxx\test\libcxx\utilities\expected\expected.void\transform_error.mandates.verify.cpp:61): {{(excess elements in struct initializer|no matching constructor for initialization of)}}{{.*}} # | File * Line * (directive at C:\buildbot\as-builder-2\x-aarch64\llvm-project\libcxx\test\libcxx\utilities\expected\expected.void\transform_error.mandates.verify.cpp:63): {{(excess elements in struct initializer|no matching constructor for initialization of)}}{{.*}} # | File * Line * (directive at C:\buildbot\as-builder-2\x-aarch64\llvm-project\libcxx\test\libcxx\utilities\expected\expected.void\transform_error.mandates.verify.cpp:70): {{(excess elements in struct initializer|no matching constructor for initialization of)}}{{.*}} # | File * Line * (directive at C:\buildbot\as-builder-2\x-aarch64\llvm-project\libcxx\test\libcxx\utilities\expected\expected.void\transform_error.mandates.verify.cpp:72): {{(excess elements in struct initializer|no matching constructor for initialization of)}}{{.*}} # | File * Line * (directive at C:\buildbot\as-builder-2\x-aarch64\llvm-project\libcxx\test\libcxx\utilities\expected\expected.void\transform_error.mandates.verify.cpp:79): {{(excess elements in struct initializer|no matching constructor for initialization of)}}{{.*}} # | File * Line * (directive at C:\buildbot\as-builder-2\x-aarch64\llvm-project\libcxx\test\libcxx\utilities\expected\expected.void\transform_error.mandates.verify.cpp:81): {{(excess elements in struct initializer|no matching constructor for initialization of)}}{{.*}} # | 8 errors generated. # `- # error: command failed with exit status: 1 ``` https://lab.llvm.org/buildbot/#/builders/119/builds/16381 https://lab.llvm.org/buildbot/#/builders/60/builds/15226 https://github.com/llvm/llvm-project/pull/76232 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Sema] Fix crash on invalid code with parenthesized aggregate initialization (PR #76232)
ilya-biryukov wrote: Thanks for reverting it and sorry for the trouble, I will make sure to check libc++ tests before a reland. Clang does report less errors now, but that's expected. The errors were spurious, caused by Clang trying to initialize an "invalid" class (more details below in case you're interested). Failed `static_assert` is what made the class invalid in the first place and Clang fails parenthesized initialization of such classes early now. We should not do parenthesized initialization here as it should only be applied to aggregates, this class has constructors and is not an aggregate in the first place. The only reason we attempted aggregate init is because invalid classes are left internally in inconsistent states sometimes (there are many reasons why class is invalid). So less errors is better here, they were only adding noise and confusion in the first place. I will reland the commit with updated libc++ tests. The commit did not change the set of correct programs that we compile, it's just showing less spurious errors. https://github.com/llvm/llvm-project/pull/76232 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits