llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.39k stars 11.73k forks source link

[libcxxabi][ItaniumDemangle] Mangling produced by clang for lambdas with template parameters fail to demangle #73521

Open Michael137 opened 10 months ago

Michael137 commented 10 months ago

Take this example:

int main() {
    auto foo = [](auto) -> int {
        return 50;
    };
    foo(50);
}

With top-of-tree clang this mangles to: _ZZ4mainENK3$_0clIiEEiT_ which llvm-cxxfilt fails to demangle. Stepping through the code, the failure occurs in parseTemplateParam (when parsing T_ at the end).

The problem was introduced in the (de)mangler changes in https://reviews.llvm.org/D147655. Specifically, it added a SaveTemplateParams to parseLocalName, which resets the TemplateParams when we're done parsing the name. This trips up parseTemplateParam.

GCC mangles the above to _ZZ4mainENKUlT_E_clIiEEiS_, which demangles fine. The difference here is that GCC mangles the name as a <lambda-sig> (i.e., Ul), whereas clang mangles the lambda as an unnamed struct with internal linkage. The difference between a local unnamed struct and a lambda is that lambdas can have template parameters. Maybe this difference was overlooked in D147655?

@zygoloid @urnathan

llvmbot commented 10 months ago

@llvm/issue-subscribers-tools-llvm-cxxfilt

Author: Michael Buch (Michael137)

Take this example: ``` int main() { auto foo = [](auto) -> int { return 50; }; foo(50); } ``` With top-of-tree clang this mangles to: `_ZZ4mainENK3$_0clIiEEiT_` which `llvm-cxxfilt` fails to demangle. Stepping through the code, the failure occurs in `parseTemplateParam` (when parsing `T_` at the end). The problem was introduced in the (de)mangler changes in https://reviews.llvm.org/D147655. Specifically, it added a [`SaveTemplateParams` to `parseLocalName`](https://github.com/llvm/llvm-project/blob/79b03306af5c11d354fa90db8bfd7818cd811ef5/libcxxabi/src/demangle/ItaniumDemangle.h#L2951-L2953), which resets the `TemplateParams` when we're done parsing the name. This trips up `parseTemplateParam`. GCC mangles the above to `_ZZ4mainENKUlT_E_clIiEEiS_`, which demangles fine. The difference here is that GCC mangles the name as a `<lambda-sig>` (i.e., `Ul`), whereas clang mangles the lambda as an unnamed struct with internal linkage. The difference between a local unnamed struct and a lambda is that lambdas can have template parameters. Maybe this difference was looked over in `D147655`? @zygoloid @urnathan
Michael137 commented 10 months ago

Or should we be always mangling lambdas as <lambda-sig>, like GCC does? Itanium ABI seems to leave the encoding up to the implementation:

In some contexts, such closure types are unique to the translation unit: This ABI therefore does not specify an encoding for such cases (but an implementation must ensure that any internal encoding does not conflict with this ABI).

zygoloid commented 10 months ago

The problem was introduced in the (de)mangler changes in https://reviews.llvm.org/D147655. Specifically, it added a SaveTemplateParams to parseLocalName, which resets the TemplateParams when we're done parsing the name. This trips up parseTemplateParam.

I think it would be correct to SaveTemplateParams only if State == nullptr (that is, if the local name isn't actually the name of the overall encoding). This would be necessary if we had any local templates other than as members of lambda closure types, because in that case there wouldn't be something for the S_ substitution to refer back to. (There are no such things in C++ currently, but that'll probably change eventually!)

Or should we be always mangling lambdas as <lambda-sig>, like GCC does?

I think that would be a good idea, even if we fix LLVM's demangler. There is a defined mangling for local lambdas, so we should use it and not invent our own thing; that way, we'll get better demanglings that actually say the name is a member of a lambda rather than a member of $_0. (This'll make manglings a little longer, but it's probably worth it to make demangled names more useful.)

Michael137 commented 10 months ago

Mangling internal lambdas as unnamed class seems to have been introduced in 6f88e5e0d71b6d9a16234b2c4ce0125af80a7ed4

hahn-absint commented 6 months ago

Hi, I run into this issue when updating from LLVM 17 to LLVM 18. I wonder whether there are any news on the status or plans to fix the demangling issue? Thanks a lot.

Michael137 commented 6 months ago

Hi, I run into this issue when updating from LLVM 17 to LLVM 18. I wonder whether there are any news on the status or plans to fix the demangling issue? Thanks a lot.

Funnily enough I was going to look into this again soon. Though not sure when exactly that'll be. Hopefully in the coming weeks