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-17 regression] clang from trunk crashes inside clang::TemplateArgument::pack_size() #63782

Closed
loskutov opened this issue Jul 10, 2023 · 13 comments
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" confirmed Verified by a second party crash Prefer [crash-on-valid] or [crash-on-invalid] regression

Comments

@loskutov
Copy link
Contributor

loskutov commented Jul 10, 2023

The following piece of code leads to ICE when compiled with clang from trunk:

template<bool... Vals>
constexpr bool All = (Vals && ...);

template<bool... Bs>
class Class {
    template<typename>
    requires All<Bs...>
    void Foo();
};

template<bool... Bs>
template<typename>
requires All<Bs...>
void Class<Bs...>::Foo() {
};

Compiler Explorer with full stacktrace: https://godbolt.org/z/n6bro9Tcd

@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" crash Prefer [crash-on-valid] or [crash-on-invalid] and removed new issue labels Jul 10, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 10, 2023

@llvm/issue-subscribers-clang-frontend

@shafik
Copy link
Collaborator

shafik commented Jul 10, 2023

@Fznamznon any chance your recent fix could be involved here?

@Fznamznon
Copy link
Contributor

@Fznamznon any chance your recent fix could be involved here?

I don't think so. I'm not seeing any MemberExprs created (the patch tackles only them), also reverting the patch locally and seeing the same assertion failure seems pretty assuring.

@cor3ntin cor3ntin added confirmed Verified by a second party regression labels Jul 12, 2023
@loskutov
Copy link
Contributor Author

loskutov commented Jul 12, 2023

According to git-bisect, 6db007a authored by @alexander-shaposhnikov is the first bad commit.

The properly symbolized stacktrace: https://pastebin.ubuntu.com/p/q3s55j3tNC/

@michael-jabbour-sonarsource
Copy link

michael-jabbour-sonarsource commented Aug 29, 2023

Another crashing example that results in a similar stacktrace:

using size_t = decltype(sizeof(int));

template<typename... Ts> struct S {
  template<size_t N>
  requires(N <= sizeof...(Ts))
  void f(int);
};

template<typename... Ts>
template<size_t N>
requires(N <= sizeof...(Ts))
void S<Ts...>::f(int) {}

This also bisects to 6db007a. Compiler Explorer link: https://godbolt.org/z/s9YvEG5rn.

@shafik
Copy link
Collaborator

shafik commented Aug 29, 2023

@erichkeane
Copy link
Collaborator

I really hope @alexander-shaposhnikov can take a look at this, I've been away from this for long enough, and I likely wont have a chance to look at it 'soon'.

@alexander-shaposhnikov
Copy link
Collaborator

Hi, I'll try to look into this in the coming weeks, but I'm really busy with completely unrelated things at the moment / can't promise.

@erichkeane
Copy link
Collaborator

I spent a little time that I had to spare in the debugger with this one, but I'm a touch lost. The function in question is: https://clang.llvm.org/doxygen/classclang_1_1Sema.html#a0b0f6c317ec8c9f08dcb406114c43644

The issue is at line 732: https://clang.llvm.org/doxygen/SemaTemplateVariadic_8cpp_source.html#l00732

In @michael-jabbour-sonarsource 's example, we're assuming the argument (the only one, at 0,0) is a pack, it is NOT. It is a PackExpansionType (Ts...), which SEEMS right, but I have very little idea what is going on in this function, or why a PackExpansionType is not sufficiently 'pack like' here to work? I'm hopeful @alexander-shaposhnikov has some luck on this, but my time is pretty limited unfortunately right now as well.

@erichkeane
Copy link
Collaborator

Further digging shows that just handling the PackExpansionType isn't sufficient. I suspect Alexander's patch ended up putting the pack into the MLTAL in a way that it shouldn't have, but I don't see how/why.

@erichkeane
Copy link
Collaborator

@alejandro-alvarez-sonarsource
Copy link
Contributor

Hello.
I believe this was fixed by #80594. clang trunk with assertions does not crash anymore with the given example: https://godbolt.org/z/1TnqxbE1E

@shafik
Copy link
Collaborator

shafik commented Jun 12, 2024

Looks fixed in trunk

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" confirmed Verified by a second party crash Prefer [crash-on-valid] or [crash-on-invalid] regression
Projects
None yet
Development

No branches or pull requests

10 participants