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-19 cannot mangle what clang-17 mangles #115990

Open
kelbon opened this issue Nov 13, 2024 · 21 comments · May be fixed by #117845
Open

clang-19 cannot mangle what clang-17 mangles #115990

kelbon opened this issue Nov 13, 2024 · 21 comments · May be fixed by #117845
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" platform:windows regression:19 Regression in 19 release

Comments

@kelbon
Copy link
Contributor

kelbon commented Nov 13, 2024

I updated the clang version from 17 to 19 on windows and now i get error:

error: cannot mangle this template type parameter type yet

(no additional info provided)

@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" and removed new issue labels Nov 13, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 13, 2024

@llvm/issue-subscribers-clang-frontend

Author: None (kelbon)

I updated the clang version from 17 to 19 on windows and now i get error: ``` error: cannot mangle this template type parameter type yet ``` (no additional info provided)

@zyn0217 zyn0217 added the incomplete Issue not complete (e.g. missing a reproducer, build arguments, etc.) label Nov 13, 2024
@zyn0217
Copy link
Contributor

zyn0217 commented Nov 13, 2024

Without a reproducer we're also unable to tell what was broken :(

@kelbon
Copy link
Contributor Author

kelbon commented Nov 13, 2024

Without a reproducer we're also unable to tell what was broken :(

Its really huge private file with codegen and compile time strings, i tried to minimize it, but it still really big.

if the compiler would give at least some diagnostics, for example the line number and the name of the type that is trying to mangle, it would be much easier

P.S. i checked localy, clang 18 compiles successfully. Something in clang-19 mangling breaks compilation

@EugeneZelenko EugeneZelenko added the regression:19 Regression in 19 release label Nov 13, 2024
@efriedma-quic
Copy link
Collaborator

It looks like this is Windows-specific; the Itanium mangler can't generate an error message like this.

https://llvm.org/docs/HowToSubmitABug.html#front-end-bugs has some tools you can use to automatically reduce testcases, if you can't share the original. But maybe @MaxEW707 has some idea what might trigger this.

CC @rnk @zmodem

@MaxEW707
Copy link
Contributor

@kelbon What C++ version are you building against. Notably pre C++20 or C++20?

I have time this weekend to look into this and try to reproduce locally based on the changes made to make the microsoft mangling code more conformant with MSVC in clang 19.

for example the line number

That is odd. The source range is passed into the error diagnostic. We must be passing in an empty source range. I'll dig into this as well.

@kelbon
Copy link
Contributor Author

kelbon commented Nov 14, 2024

@kelbon What C++ version are you building against. Notably pre C++20 or C++20?

I have time this weekend to look into this and try to reproduce locally based on the changes made to make the microsoft mangling code more conformant with MSVC in clang 19.

for example the line number

That is odd. The source range is passed into the error diagnostic. We must be passing in an empty source range. I'll dig into this as well.

C++20.
Empty source range may be because its coroutines and some inner compiler generated code ?

This code also produces gcc-14 internal error on 'end translation unit'...

@MaxEW707
Copy link
Contributor

Sounds good. If you can get a reproducer that will be greatly appreciated :).

@MaxEW707
Copy link
Contributor

@kelbon I haven't been able to find a reproducer yet after going thru the commits that modified the mangling code.

Its really huge private file with codegen and compile time strings, i tried to minimize it, but it still really big.

If you are able to strip away or replace any internal private details with generic stuff such as replacing strings with random characters or renaming class names that won't leak any private details then even a large reproducer will help me track down what is going awry.

@kelbon
Copy link
Contributor Author

kelbon commented Nov 17, 2024

@kelbon I haven't been able to find a reproducer yet after going thru the commits that modified the mangling code.

Its really huge private file with codegen and compile time strings, i tried to minimize it, but it still really big.

If you are able to strip away or replace any internal private details with generic stuff such as replacing strings with random characters or renaming class names that won't leak any private details then even a large reproducer will help me track down what is going awry.

If it possible, may be improve diagnostic or for example replace it with segfault, so i can run it on my code and get preprocessed sources + right place where code fails?
Anyway i will try to reduce example, but its not easy

@MaxEW707
Copy link
Contributor

@kelbon I pushed a local branch here off of 19.1.3, https://github.com/MaxEW707/llvm-project/tree/mew/assert-invalid-mangle-1913, which turns all the diagnostic error paths into an llvm_unreachable.

If you can build that with assertions enabled then we should get a nice stack trace with the current parser tokens and the current decl we are trying to mangle :).

@kelbon
Copy link
Contributor Author

kelbon commented Nov 17, 2024

@kelbon I pushed a local branch here off of 19.1.3, https://github.com/MaxEW707/llvm-project/tree/mew/assert-invalid-mangle-1913, which turns all the diagnostic error paths into an llvm_unreachable.

If you can build that with assertions enabled then we should get a nice stack trace with the current parser tokens and the current decl we are trying to mangle :).

im not sure how to build clang under windows, im using

cmake -S llvm -B build -G "Ninja" -DLLVM_ENABLE_PROJECTS='clang' -DLLVM_ENABLE_ASSERTIONS=ON -DCMAKE_BUILD_TYPE=Debug -DLLVM_HOST_TRIPLE=x86_64 -DCMAKE_EXPORT_COMPILE_COMMANDS=1 -DCMAKE_INSTALL_PREFIX=../install

  • ninja clang

results in (while compiling example code in cmake check compiler)

clang: error: unable to execute command: program not executable
clang: error: linker (via gcc) command failed with exit code 1 (use -v to see invocation)

i cannot set -fsyntax-only in cmake, because it checks compiler

@kelbon
Copy link
Contributor Author

kelbon commented Nov 17, 2024

Okay after some workarounds i rebuild clang and get assertion failure (before type mangling)

Assertion failed: isa<UsingDecl>(Cand.getDecl()) && "illegal Kind of operator = Decl", file D:/llvm_mangle_error/llvm-project/clang/lib/Sema/SemaLookup.cpp, line 3483

@MaxEW707 where i can send all 230k preprocessed lines and full error log for you?

@kelbon
Copy link
Contributor Author

kelbon commented Nov 17, 2024

anyway:
error log for compiler run for several targets:
src/long_poll.cpp.obj
src/bot.cpp.obj
src/telegram.cpp.obj

all failed with assert while parsing Update struct (but with different call stack)

Assertion failed: isa(Cand.getDecl()) && "illegal Kind of operator = Decl", file D:/llvm_mangle_error/llvm-project/clang/lib/Sema/SemaLookup.cpp, line 3483

https://pastebin.com/SY4bQs3S

link for downloading files with preprocessed code:
https://gofile.io/d/MZ79hE

@MaxEW707
Copy link
Contributor

@kelbon Can you attach the preprocessed cpp source code file(s) here on github. In the comment box there should be an icon with a paper clip to attach files. Thanks :).

@kelbon
Copy link
Contributor Author

kelbon commented Nov 17, 2024

@MaxEW707

We don’t support that file type.
Try again with GIF, JPEG, JPG, MOV, MP4, PNG, SVG, WEBM, CPUPROFILE, CSV, DMP, DOCX, FODG, FODP, FODS, FODT, GZ, > JSON, JSONC, LOG, MD, ODF, ODG, ODP, ODS, ODT, PATCH, PDF, PPTX, TGZ, TXT, XLS, XLSX or ZIP.

Thats why i added extension .txt for all files

(--)

@MaxEW707
Copy link
Contributor

@kelbon @efriedma-quic I managed to get a minimal reproducer for the "Assertion failed: isa(Cand.getDecl()) && "illegal Kind of operator = Decl", file F:\llvm-project\clang\lib\Sema\SemaLookup.cpp, line 3483" assertion.

godbolt for the reproducer

template<typename T>
struct Base
{
};

template<typename... Types>
struct Derived : private Base<Types>...
{
public:
    using Base<Types>::operator=...;
    Derived& operator=(const Derived& other)
    {
        return *this;
    }
};

struct Foo {};
struct Update
{
    Derived<Foo> data;
};

It appears the assertion hit was a red herring and is unrelated to the mangling issue here.
That is the assertion is always fired in debug builds of clang going back to 17.x.x.
Since release builds of clang compile the code fine the assertion is no longer valid. I'll make a separate github issue sometime next week for this assertion failure.

Now knowing that the assertion is an unrelated issue I can workaround that and dig into the mangling issue. I'll do that over the next week :).

@MaxEW707
Copy link
Contributor

@kelbon I can't reproduce the invalid mangling on the 3 source files provided with the faulty assertion removed.

I updated my local branch here, https://github.com/MaxEW707/llvm-project/tree/mew/assert-invalid-mangle-1913, to remove said faulty assert. If you can pull that down, rebuild clang, and then build your code so we can hopefully get a stack trace of the invalid mangling that would be greatly appreciated. Thanks for all the help so far in tracking this issue down :).

@kelbon
Copy link
Contributor Author

kelbon commented Nov 18, 2024

...
@MaxEW707

https://pastebin.com/FDKNxGPj

test_oneof-ce9437.cpp.txt
test_oneof-ce9437.sh.txt

@MaxEW707
Copy link
Contributor

Here is a minimal reproducer. https://godbolt.org/z/7nPvac68a

namespace ns0
{

template <typename T>
concept test_api_type = requires(const T& t)
{
    { T::test([](){}) };
};

template<typename T>
struct test {};

template <test_api_type T>
struct test<T> {};

} // namespace ns0

struct Baz
{
    template<typename F>
    static constexpr decltype(auto) test(F&&) {}
};

void test()
{
    ns0::test<Baz> t;
}

Working on a fix. Should be up sometime this week :).

@EugeneZelenko EugeneZelenko removed the incomplete Issue not complete (e.g. missing a reproducer, build arguments, etc.) label Nov 19, 2024
@kelbon
Copy link
Contributor Author

kelbon commented Nov 20, 2024

Here is a minimal reproducer. https://godbolt.org/z/7nPvac68a

namespace ns0
{

template <typename T>
concept test_api_type = requires(const T& t)
{
    { T::test([](){}) };
};

template<typename T>
struct test {};

template <test_api_type T>
struct test<T> {};

} // namespace ns0

struct Baz
{
    template<typename F>
    static constexpr decltype(auto) test(F&&) {}
};

void test()
{
    ns0::test<Baz> t;
}

Working on a fix. Should be up sometime this week :).

Thanks, i workaround [] {} lambda in concepts and runs code. Its interesting to observe how benchmark runs faster with every new clang version (specially coroutines)

@MaxEW707
Copy link
Contributor

For reference I bisecting the PR that breaks the MSVC mangling to this: #83997

https://godbolt.org/z/xYYPM4noY
Notice in the clang 19 godbolt we get an extra template instantation in the AST that we end up mangling.

`-CXXMethodDecl 0xcb71098 <line:18:5, line:21:5> line:18:27 referenced constexpr test2 'void (Widget<type-parameter-0-0>::(lambda at <source>:4:16) &&)' implicit_instantiation static implicit-inline
|     |-TemplateArgument type 'Widget<type-parameter-0-0>::(lambda at <source>:4:16)'

The TemplateTypeParamType of type-parameter-0-0 which we don't implement currently breaks us.

I am still digging if this extra mangling when doing the concept constraint is actually intended or an intended byproduct.

@zyn0217 if you have any insight :).

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" platform:windows regression:19 Regression in 19 release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants