Clang ignores visibility attribute of a template method of a template class

[igorkudrin]

For the following source clang generates the symbol _ZN1tIcE3barILi1EEEvv with the DEFAULT visibility:

template <class a>
struct t {
    template <int>
    static __attribute__((__visibility__("hidden")))
    void bar() {}
};

void test() {
    t<char>::bar<1>();
}

> clang -c test.cpp -target x86_64 -o test.o
> readelf -sW test.o
...
     5: 0000000000000000     6 FUNC    WEAK   DEFAULT     5 _ZN1tIcE3barILi1EEEvv

gcc for the same case generates a HIDDEN symbol: https://godbolt.org/z/9z7soKfoa

The problem may be similar to #31462, but there are no explicit instantiations with different visibility attributes here. Applying D153835 or D154774 fixes this issue, but these revisions seem to be abandoned. #72092 does not resolve it.

[llvmbot]

@llvm/issue-subscribers-c-1

Author: Igor Kudrin (igorkudrin)

For the following source `clang` generates the symbol `_ZN1tIcE3barILi1EEEvv` with the `DEFAULT` visibility: ```cpp template <class a> struct t { template <int> static __attribute__((__visibility__("hidden"))) void bar() {} };

void test() {
t<char>::bar<1>();
}

> clang -c test.cpp -target x86_64 -o test.o
> readelf -sW test.o
...
5: 0000000000000000 6 FUNC WEAK DEFAULT 5 _ZN1tIcE3barILi1EEEvv

`gcc` for the same case generates a `HIDDEN` symbol: https://godbolt.org/z/9z7soKfoa

The problem may be similar to #<!-- -->31462, but there are no explicit instantiations with different visibility attributes here. Applying [D153835](https://reviews.llvm.org/D153835) or [D154774](https://reviews.llvm.org/D154774) fixes this issue, but these revisions seem to be abandoned. #<!-- -->72092 does not resolve it.
</details>

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Igor Kudrin (igorkudrin)

For the following source `clang` generates the symbol `_ZN1tIcE3barILi1EEEvv` with the `DEFAULT` visibility: ```cpp template <class a> struct t { template <int> static __attribute__((__visibility__("hidden"))) void bar() {} };

void test() {
t<char>::bar<1>();
}

> clang -c test.cpp -target x86_64 -o test.o
> readelf -sW test.o
...
5: 0000000000000000 6 FUNC WEAK DEFAULT 5 _ZN1tIcE3barILi1EEEvv

`gcc` for the same case generates a `HIDDEN` symbol: https://godbolt.org/z/9z7soKfoa

The problem may be similar to #<!-- -->31462, but there are no explicit instantiations with different visibility attributes here. Applying [D153835](https://reviews.llvm.org/D153835) or [D154774](https://reviews.llvm.org/D154774) fixes this issue, but these revisions seem to be abandoned. #<!-- -->72092 does not resolve it.
</details>

[rjmccall]

I agree that this ought to generate a hidden symbol.

[erichkeane]

Unfortunately attributes on templates aren't particularly automated and have to be transformed explicitly in our tree transform. This is almost definitely just a case where we don't properly transform this attribute, so it is lost from the instantiated version in teh AST.

You can see that happens here:
https://godbolt.org/z/Yes17aevq

The uninstantiated version is:

| | `-FunctionTemplateDecl <line:3:5, line:5:17> col:10 bar
| |   |-NonTypeTemplateParmDecl <line:3:15> col:18 'int' depth 1 index 0
| |   `-CXXMethodDecl <line:4:5, line:5:17> col:10 bar 'void ()' static implicit-inline
| |     |-CompoundStmt <col:16, col:17>
| |     `-VisibilityAttr <line:4:27, col:50> Hidden

The instantiated version is:

|   |-CXXRecordDecl <col:1, col:8> col:8 implicit struct t
|   `-FunctionTemplateDecl <line:3:5, line:5:17> col:10 bar
|     |-NonTypeTemplateParmDecl <line:3:15> col:18 'int' depth 0 index 0
|     |-CXXMethodDecl <line:4:5, line:5:17> col:10 bar 'void ()' static implicit-inline
|     `-CXXMethodDecl <line:4:5, line:5:17> col:10 used bar 'void ()' implicit_instantiation static implicit-inline
|       |-TemplateArgument integral '1'
|       `-CompoundStmt <col:16, col:17>

So the attribute disappears, likely because we didn't transform it. I suspect this is a 'pretty easy' bug? But one someone needs to find time for.

[erichkeane]

So the problem generally comes down to Sema::InstantiateAttrs. in SemaTemplateInstantiateDecls.

We currently ignore/don't include any attribute that we aren't explicitly handling. We ended up having to do this because when we first wanted to start instantiating attributes, some implementations depended on it not happening, so it needs to be 'opt in', and the behavior of each is unique enough that opting in is more than just setting a field in tablegen.

However, we're currently in a situation where we manage to get a bunch of attributes that don't survive isntantiation and we do so silently.

I kind of want to add a warning to this function for any unsupported ones, and we then have to explicitly make a decision. If we really want it ignored, we put a no-op in, if we want it to do something, we have to do something.

I believe I am about done for the night on this so I'll have to think it over... but I think I'm once again at the point where i want an improvement on this, rather than just 'fixing' this 1 example. But if someone wants to take over this (and do the warning/etc above + test changes required to make it do something), please let me know, I'd be willing ot help via review.