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++/g++ disagree on how template arguments in substitutions are resolved #68

Open
rocallahan opened this issue Nov 2, 2018 · 5 comments

Comments

@rocallahan
Copy link

rocallahan commented Nov 2, 2018

Basically the question is: when a template parameter reference like T_ occurs in a substitution, is the reference looked up in the template instance where the substitution is defined, or where it is used?

It appears that llvm-cxxfilt assumes the former, but c++filt assumes the latter. Consider the (hand-written) mangled symbol _Z5helloIXadL_Z6ignoreI9RangitotoEvT_EEEvS2_:

[roc@localhost cpp_demangle]$ c++filt --version
GNU c++filt (GNU Binutils) 2.29.51
Copyright (C) 2018 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) any later version.
This program has absolutely no warranty.
[roc@localhost cpp_demangle]$ c++filt _Z5helloIXadL_Z6ignoreI9RangitotoEvT_EEEvS2_
void hello<&(void ignore<Rangitoto>(Rangitoto))>(&(void ignore<Rangitoto>(Rangitoto)))
[roc@localhost cpp_demangle]$ llvm-cxxfilt --version
LLVM (http://llvm.org/):
 LLVM version 7.0.0svn
 Optimized build.
 Default target: x86_64-unknown-linux-gnu
 Host CPU: skylake
[roc@localhost cpp_demangle]$ llvm-cxxfilt _Z5helloIXadL_Z6ignoreI9RangitotoEvT_EEEvS2_
void hello<&(void ignore<Rangitoto>(Rangitoto))>(Rangitoto)

In this case both tools agree that the S2_ substitution refers to the T_, but they disagree on what that expands to.

To my reading, the spec isn't clear on this issue. The most relevant text I can find is

Note that substitutable components are the represented symbolic constructs, not their associated mangling character strings.

which suggests the definition instance is preferred, which also seems logical to me.

@rocallahan
Copy link
Author

I suspect the same problem could occur with function parameter references, though an example would have to be very complicated.

@zygoloid
Copy link
Contributor

The original mangled name is invalid. The right way to refer to Rangitoto in the substitution would be S1_, not S2_, which is what compilers produce for examples that mangle that way, and the modified mangling is demangled the same way by libstdc++'s demangler and libc++abi's demangler.

We can reach cases that demonstrate the problem in question with an example like this:

template<typename T> void ignore(T*);
struct Rangitoto {};
struct Foobar {};
template<typename T, void(*)(Foobar*)> void hello(T*) {}
template void hello<Rangitoto, ignore<Foobar> >(Rangitoto*);

Everyone mangles this as _Z5helloI9RangitotoXadL_Z6ignoreI6FoobarEvPT_EEEvS4_. However, libstdc++ demangles that to the source declaration, libc++abi demangles it to a function taking Foobar* instead.

In summary, Clang, GCC, ICC, and the libstdc++ demangler all agree that when a S<n>_ refers to a T<n>_, the T<n>_ is interpreted in the context where the S<n>_ appears, not in its original context; only the libc++abi demangler disagrees. Presumably that is the rule we should use, then. However, see the problem under discussion in #106, which points out that this kind of situation is hard to demangle in general.

@rocallahan
Copy link
Author

rocallahan commented Apr 20, 2021

In summary, Clang, GCC, ICC, and the libstdc++ demangler all agree that when a S<n>_ refers to a T<n>_, the T<n>_ is interpreted in the context where the S_ appears, not in its original context; .. Presumably that is the rule we should use

That's great news. I hope you're able to update the spec text to clarify that.

@rocallahan
Copy link
Author

For the record, the real-life issue that triggered this bug report was:
gimli-rs/cpp_demangle#165 (comment)
In particular this symbol was generated by (some circa-2018 version of) clang++:

_ZN7mozilla6detail12ListenerImplINS_14AbstractThreadEZNS_20MediaEventSourceImplILNS_14ListenerPolicyE0EJNS_13TimedMetadataEEE15ConnectInternalIS2_NS_12MediaDecoderEMS8_FvOS5_EEENS_8EnableIfIXsr8TakeArgsIT1_EE5valueENS_18MediaEventListenerEE4TypeEPT_PT0_SD_EUlS9_E_JS5_EE17ApplyWithArgsImplISL_EENSC_IXsr8TakeArgsISH_EE5valueEvE4TypeERKSH_S9_

llvm-cxxfilt 11.0 demangles it to

mozilla::EnableIf<TakeArgs<mozilla::AbstractThread>::value, void>::Type mozilla::detail::ListenerImpl<mozilla::AbstractThread, mozilla::EnableIf<TakeArgs<void (mozilla::MediaDecoder::*)(mozilla::TimedMetadata&&)>::value, mozilla::MediaEventListener>::Type mozilla::MediaEventSourceImpl<(mozilla::ListenerPolicy)0, mozilla::TimedMetadata>::ConnectInternal<mozilla::AbstractThread, mozilla::MediaDecoder, void (mozilla::MediaDecoder::*)(mozilla::TimedMetadata&&)>(mozilla::AbstractThread*, mozilla::MediaDecoder*, void (mozilla::MediaDecoder::*)(mozilla::TimedMetadata&&))::'lambda'(mozilla::TimedMetadata&&), mozilla::TimedMetadata>::ApplyWithArgsImpl<mozilla::EnableIf<TakeArgs<void (mozilla::MediaDecoder::*)(mozilla::TimedMetadata&&)>::value, mozilla::MediaEventListener>::Type mozilla::MediaEventSourceImpl<(mozilla::ListenerPolicy)0, mozilla::TimedMetadata>::ConnectInternal<mozilla::AbstractThread, mozilla::MediaDecoder, void (mozilla::MediaDecoder::*)(mozilla::TimedMetadata&&)>(mozilla::AbstractThread*, mozilla::MediaDecoder*, void (mozilla::MediaDecoder::*)(mozilla::TimedMetadata&&))::'lambda'(mozilla::TimedMetadata&&)>(mozilla::AbstractThread const&, mozilla::TimedMetadata&&)

while c++filt 2.35 fails to demangle it.

Perhaps that version of clang++ mangled the symbol incorrectly. I would have tried to come up with a minimal C++ testcase but I'm not adept at taking a symbol like that and reverse-engineering some minimal C++ code that produces it...

@rocallahan
Copy link
Author

libc++abi demangles it to a function taking Foobar* instead.

I hope someone fixes this. Once that's fixed, it'll be interesting to see if that changes llvm-cxxfilt's demangling of the Mozilla symbol above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants