Skip to content
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

Clang crash on valid code since #87933 #92414

Closed
yronglin opened this issue May 16, 2024 · 3 comments
Closed

Clang crash on valid code since #87933 #92414

yronglin opened this issue May 16, 2024 · 3 comments
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" crash-on-valid

Comments

@yronglin
Copy link
Contributor

namespace std {
template <class T> class initializer_list {};
}

template <typename T, int> class C {
public:
  C(std::initializer_list<T>);
};

template <typename T> using Ptr =__remove_pointer(T) *;
template <typename T>  C(T) ->  C<Ptr<T>, sizeof(T)>;

class A {
public:
  template <typename T1, typename T2>
  T1 *some_func(T2 &&);
};

struct B : A {
  int *ar = some_func<int>(C{some_func<int>(0)});
  B() {}
};

I've debug in local. The crash issue caused by initializer rebuild failed in

SFINAETrap Trap(*this);
runWithSufficientStackSpace(Loc, [&] {
Res = Immediate.TransformInitializer(Field->getInClassInitializer(),
/*CXXDirectInit=*/false);
});
, if remove Line 5717 SFINAETrap Trap(*this);, we can got diagnostics like the following:

./main.cpp:23:28: error: no viable constructor or deduction guide for deduction of template arguments of 'C'
   23 |   int *ar = some_func<int>(C{some_func<int>(0)});
      |                            ^
./main.cpp:6:34: note: candidate template ignored: couldn't infer template argument 'T'
    6 | template <typename T, int> class C {
      |                                  ^
./main.cpp:8:3: note: candidate template ignored: couldn't infer template argument ''
    8 |   C(std::initializer_list<T>);
      |   ^
./main.cpp:12:24: note: candidate template ignored: couldn't infer template argument 'T'
   12 | template <typename T>  C(T) ->  C<Ptr<T>, sizeof(T)>;
      |                        ^
1 error generated.

But why does the check pass during Parse phase but fails when we rebuilding CXXDefaultInitExpr? Through tracing, I found that the root cause was the parameter bool ListInitialization that passed to RebuildCXXTemporaryObjectExpr(fall throuth to Sema::BuildCXXTypeConstructExpr). During parsing, ListInitialization was true, but it became false during rebuilding, it's cause InitializationKind to become DirectInit instead of DirectListInit. Finally, causing Sema::DeduceTemplateSpecializationFromInitializer fail.

// FIXME: We should just pass E->isListInitialization(), but we're not
// prepared to handle list-initialization without a child InitListExpr.
SourceLocation LParenLoc = T->getTypeLoc().getEndLoc();
return getDerived().RebuildCXXTemporaryObjectExpr(
T, LParenLoc, Args, E->getEndLoc(),
/*ListInitialization=*/LParenLoc.isInvalid());

Therefore, I think the key to the problem is to fix TreeTransform.h:14116's FIXME. As the comments in TreeTransform.h:14116 said, we should pass E->isListInitialization(), because E is actually list initialization, I have tried this modification, but it will cause 3 lit failures. We have not try to rebuild the CXXDefaultInitExpr before this PR, so it's works fine before.

@github-actions github-actions bot added the clang Clang issues not falling into any other category label May 16, 2024
@yronglin yronglin added clang:frontend Language frontend issues, e.g. anything involving "Sema" crash-on-valid and removed clang Clang issues not falling into any other category labels May 16, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented May 16, 2024

@llvm/issue-subscribers-clang-frontend

Author: None (yronglin)

```C++ namespace std { template <class T> class initializer_list {}; }

template <typename T, int> class C {
public:
C(std::initializer_list<T>);
};

template <typename T> using Ptr =__remove_pointer(T) *;
template <typename T> C(T) -> C<Ptr<T>, sizeof(T)>;

class A {
public:
template <typename T1, typename T2>
T1 *some_func(T2 &&);
};

struct B : A {
int *ar = some_func<int>(C{some_func<int>(0)});
B() {}
};

I've debug in local. The crash issue caused by initializer rebuild failed in https://github.com/llvm/llvm-project/blob/8e00703be9ceb41d9b80c2bc8f024a9610b9aaa1/clang/lib/Sema/SemaExpr.cpp#L5717-L5721 , if remove Line 5717 `SFINAETrap Trap(*this);`, we can got diagnostics like the following:

./main.cpp:23:28: error: no viable constructor or deduction guide for deduction of template arguments of 'C'
23 | int *ar = some_func<int>(C{some_func<int>(0)});
| ^
./main.cpp:6:34: note: candidate template ignored: couldn't infer template argument 'T'
6 | template <typename T, int> class C {
| ^
./main.cpp:8:3: note: candidate template ignored: couldn't infer template argument ''
8 | C(std::initializer_list<T>);
| ^
./main.cpp:12:24: note: candidate template ignored: couldn't infer template argument 'T'
12 | template <typename T> C(T) -> C<Ptr<T>, sizeof(T)>;
| ^
1 error generated.

But why does the check pass during Parse phase but fails when we rebuilding `CXXDefaultInitExpr`? Through tracing, I found that the root cause was the parameter `bool ListInitialization` that passed to `RebuildCXXTemporaryObjectExpr`(fall throuth to `Sema::BuildCXXTypeConstructExpr`). During parsing, `ListInitialization` was `true`, but it became `false` during rebuilding, it's cause `InitializationKind` to become DirectInit instead of DirectListInit. Finally, causing `Sema::DeduceTemplateSpecializationFromInitializer` fail.

https://github.com/llvm/llvm-project/blob/8e00703be9ceb41d9b80c2bc8f024a9610b9aaa1/clang/lib/Sema/TreeTransform.h#L14116-L14121

Therefore, I think the key to the problem is to fix `TreeTransform.h:14116`'s FIXME. As the comments in `TreeTransform.h:14116` said, we should pass `E-&gt;isListInitialization()`, because `E` is actually list initialization, I have tried this modification, but it will cause 3 lit failures. We have not try to rebuild the `CXXDefaultInitExpr` before this PR, so it's works fine before.
</details>

@shafik
Copy link
Collaborator

shafik commented May 18, 2024

This does not crash for me: https://godbolt.org/z/9Eec5333E

Can you confirm this still crashes and if so provide a godbolt.

@yronglin
Copy link
Contributor Author

Thank you for take a look. This crash caused by #87933, and it has been revert, I've another PR to reapply CWG1815 #92527 . And it' no longer a crash issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" crash-on-valid
Projects
None yet
Development

No branches or pull requests

3 participants