[abi] method parameter mangled as a substitution does not demangle correctly

[VitaNuo]

While fiddling with the LLVM demangler, I noticed that in the following example

template <typename T>
struct Bar{};

template <typename K>
struct Foo {
  template <typename T, template<typename U> typename S>
  void method(S<T> t, S<K> k);
};

void s() {
   Foo<int> k;
   Bar<int> i;
   Bar<bool> b;
   k.method(b, i);
}

the call k.method(b, i) is mangled as _ZN3FooIiE6methodIb3BarEEvT0_IT_ES3_IiE, which appears right (Foo<int>::method<bool, Bar>(Bar<bool>, Bar<int>)).
However, running the demangler

llvm-cxxfilt -n _ZN3FooIiE6methodIb3BarEEvT0_IT_ES3_IiE

produces void Foo<int>::method<bool, Bar>(Bar<bool>, bool<int>).

bool<int> is obviously wrong and seems to stem from the S3_ substitution.

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Viktoriia Bakalova (VitaNuo)

While fiddling with the LLVM demangler, I noticed that in the following example

template <typename T>
struct Bar{};

template <typename K>
struct Foo {
  template <typename T, template<typename U> typename S>
  void method(S<T> t, S<K> k);
};

void s() {
   Foo<int> k;
   Bar<int> i;
   Bar<bool> b;
   k.method(b, i);
}

the call k.method(b, i) is mangled as _ZN3FooIiE6methodIb3BarEEvT0_IT_ES3_IiE, which appears right (Foo<int>::method<bool, Bar>(Bar<bool>, Bar<int>)).
However, running the demangler

llvm-cxxfilt -n _ZN3FooIiE6methodIb3BarEEvT0_IT_ES3_IiE

produces void Foo<int>::method<bool, Bar>(Bar<bool>, bool<int>).

bool<int> is obviously wrong and seems to stem from the S3_ substitution.

[Michael137]

I do wonder why we don't mangle the second function parameter as T0_IiE. Also, arent builtin types supposed to be excluded from being substitution candidates?

[VitaNuo]

After debugging this a bit:

• S3_ is a substitution for

TemplateTypeParmType 0x55556a218950 'type-parameter-0-0' dependent depth 0 index 0`

which stands for bool indeed.

I guess it's added to the substitution table due to being a template parameter although it's a builtin type.

So it's probably a bug in the mangling then.

Overall, it doesn't seem possible to substitute Bar<int> as a whole, as Bar<int> is never encountered until the second method parameter is mangled. It's possible to substitute Bar, however. In this example, the ClassTemplateDecl that corresponds to Bar is added to the substitution table as S2_.

So I think the correct mangling would be _ZN3FooIiE6methodIb3BarEEvT0_IT_ES2_IiE.
It's possible to demangle the latter to void Foo<int>::method<bool, Bar>(Bar<bool>, Bar<int>) with llvm-cxxfilt.

[ilya-biryukov]

Maybe there are problems with the mangler too, but given that GCC also mangles as S3, I suspect it's the demangler after all.
See https://gcc.godbolt.org/z/e9j4Mh79r

Note that GCC's demangler from compiler explorer actually demangles correctly too while Clang's demangler produces this error. I am not sure which ones they use (likely c++filt?)

[Michael137]

Yea what I meant was i find it surprising that the demangler considered bool a valid substitution candidate (i.e., something is adding it into the Subs table when it shouldn't). There seems to be some discrepancy between what Clang/GCC and the demangler consider a substitution. First instinct leans towards this being an issue with the demangler.

EDIT:
Although just looking at the mangled string, I can't see why S3_ should be Bar tbh. I would've thought the substitutions by the time we got to S3_ would look something like:

0. Foo
1. Foo<int>
2. Foo<int>::method
3. Bar
4. Bar<bool>
5. bool

(also, I suspect the reason why bool ends up in the substitutions is that it's being added when we parsed T_, which the spec says are valid substitution candidates. Though this still doesn't explain why we seem to have put it at index 4).

[VitaNuo]

The order I observe during mangling (with some parts a bit confusing to me):

0. (S_) ClassTemplateDecl (Foo)
1. (S0_) ClassTemplateSpecializationDecl (Foo<int>)
2. (S1_) FunctionTemplateDecl (Foo<int>::method)
3. (S2_) ClassTemplateDecl (Bar definition?)
4. (S3_) TemplateTemplateParmDecl (Bar in the method template arguments?)
5. (S4_) TemplateTypeParmType (bool in the method template arguments?)
6. (S5_) TemplateSpecializationType (Bar<bool)
7. (S6_) TemplateSpecializationType (Bar<int>)

Give the above order and assumptions, S3_ in mangling might be right (it substitutes the Bar in Bar<int> parameter with the Bar from the method argument list that it's seen before).

Demangling might be building substitutions for method template arguments in the reverse order (S3_ for bool and S4_ for Bar), which might be causing this bug.

FWIW the order in the demangler seems to make more sense to me then, since bool is the first argument to method template, but in any case, they should at least agree.

EDIT:

S2_ is added to the substitution table when mangling method template arguments (bool is ignored as a primitive type).
S3_ and S4_ are added when mangling Bar<bool>, so the ordering makes sense.

I think I agree by now that the bug is in the demangler.

[ilya-biryukov]

The S4_ is actually a substitution for T_, which is coming from the template arguments of method.
(And happens to be bool in our case).
And S3_ is not Bar, but a substitution for T0_ (which happens to be Bar in our case).

I've checked the S4 specifically by using bool and T in further parameters and observing the mangling
https://gcc.godbolt.org/z/Eq6o3h6oK (note that bool stays b, but T substituted for bool gets replaced with S4_)

My theory is that the mangling is correct, but demanling seems to be getting confused either when adding or retrieving substitutions that involve Ts or when resolving Ts into the actual types.

[Michael137]

Demangling might be building substitutions for method template arguments in the reverse order (S3_ for bool and S4_ for Bar), which might be causing this bug.

Yea i agree, (i think) that's absolutely what happens. When parsing T0_IT_E, we would've first parsed T_ and put it into the subs table (which here is bool). Then we put Bar<bool> into the table. So we ended up switching the order of S3_ and S4_.

The demangler should probably add the substitutions in following order: Bar (for T0_), bool (for T_), Bar<bool> (for the entire parameter)? Currently we parse the template argument before substituting Bar (actually I don't know if we ever add the substitution for T0_. Based on my reading of parseType looks like we only ever add it for T_ and the entire Bar<bool>. So maybe the real issue is that we never add the basename as a subsitution).

[VitaNuo]

So maybe the real issue is that we never add the basename as a subsitution.

I think this is correct. The code seems to add a substitution when parsing the type of the template argument (bool) and then the type of the method argument (Bar<bool>). It never adds a substitution after seeing a template template parameter name (Bar).