llvm dev - Jul 2018 - Is it really valid to discard externally instantiated functions from a TU when marked inline?

Hi,

While investigating the situation of visibility annotations and linkage in
libc++ with the goal of removing uses of `__always_inline__`, Eric Fiselier and
I stumbled upon the attached test case, which I don't think Clang compiles
properly. Here's the gist of the test case, reduced to the important parts
(see the attachment if you want to repro):
    
    // RUN: %cxx -shared -o %T/libtest.so %flags %compile_flags -fPIC %s
    // RUN: %cxx -c -o %T/main.o %flags %compile_flags %s -O2 -DBUILDING_MAIN
    // RUN: %cxx -o %T/test.exe -L%T/ -Wl,-rpath,%T/ -ltest %T/main.o
    // RUN: %T/test.exe

    template <class T>
    struct Foo {
        inline __attribute__((visibility("hidden")))
        int __init(int x) { /* LOTS OF CODE */ }

        inline __attribute__((visibility("default")))
        int bar(int x) { return __init(x); }
    };

    extern template struct Foo<int>;

    #ifdef BUILDING_MAIN
        int main() {
            Foo<int> f;
            f.bar(101);
        }
    #else
        template struct Foo<int>;
    #endif

When running the attached file in `lit`, we get:

    Undefined symbols for architecture x86_64:
      "Foo<int>::__init(int)", referenced from:
          _main in main.o
    ld: symbol(s) not found for architecture x86_64

The idea here is that `__init` is a pretty big function, and we're promising
that an external definition of it is available through the use of the extern
template declaration. With the appropriate optimization level (O2 and above),
LLVM decides not to include the definition of `__init` in the executable and to
use the one available externally. Unfortunately, `__init` has hidden visibility,
and so the definition in the .so is not visible to the executable, and the link
fails.

Where I think Clang/LLVM goes wrong is when it decides to remove the
instantiation in the executable in favor of the one that is hypothetically
provided externally. Indeed, the Standard says in [temp.explicit]/12
(http://eel.is/c++draft/temp.explicit#12):

    Except for inline functions and variables, declarations with types deduced
from their initializer or return value ([dcl.spec.auto]), const variables of
literal types, variables of reference types, and class template specializations,
explicit instantiation declarations have the effect of suppressing the implicit
instantiation of the definition of the entity to which they refer. [ Note: The
intent is that an inline function that is the subject of an explicit
instantiation declaration will still be implicitly instantiated when odr-used so
that the body can be considered for inlining, but that no out-of-line copy of
the inline function would be generated in the translation unit. — end note ]

Only reading the normative wording, it seems like LLVM should leave the
instantiation there because it can't actually assume that there will be a
definition provided elsewhere (yes, despite the extern template declaration,
because the function is inline). Then, the non-normative note seems to be
approving of what LLVM is doing, but I'm wondering whether that's really
the intended behavior.

Questions:
1. Is what LLVM's doing there legal?
2. If it is legal, then libc++ needs a way to express that a function should
either be inlined in the caller, or emitted in the TU and de-duplicated later (I
think that’s linkonce_odr), despite there being an extern template declaration
promising a definition elsewhere. I think the current situation is that the
function gets available_externally linkage instead. Is there a way to express
this at the C++ source code level?

Thank you,
Louis Dionne

-------------- next part --------------
A non-text attachment was scrubbed...
Name: linkage.sh.cpp
Type: application/octet-stream
Size: 1051 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180710/a9b08edc/attachment.obj>

On Tue, Jul 10, 2018 at 7:20 PM, Louis Dionne via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Hi,
>
> While investigating the situation of visibility annotations and linkage in
> libc++ with the goal of removing uses of `__always_inline__`, Eric Fiselier
> and I stumbled upon the attached test case, which I don't think Clang
> compiles properly. Here's the gist of the test case, reduced to the
> important parts (see the attachment if you want to repro):
>
>     // RUN: %cxx -shared -o %T/libtest.so %flags %compile_flags -fPIC %s
>     // RUN: %cxx -c -o %T/main.o %flags %compile_flags %s -O2
> -DBUILDING_MAIN
>     // RUN: %cxx -o %T/test.exe -L%T/ -Wl,-rpath,%T/ -ltest %T/main.o
>     // RUN: %T/test.exe
>
>     template <class T>
>     struct Foo {
>         inline __attribute__((visibility("hidden")))
>         int __init(int x) { /* LOTS OF CODE */ }
>
>         inline __attribute__((visibility("default")))
>         int bar(int x) { return __init(x); }
>     };
>
>     extern template struct Foo<int>;
>
>     #ifdef BUILDING_MAIN
>         int main() {
>             Foo<int> f;
>             f.bar(101);
>         }
>     #else
>         template struct Foo<int>;
>     #endif
>
> When running the attached file in `lit`, we get:
>
>     Undefined symbols for architecture x86_64:
>       "Foo<int>::__init(int)", referenced from:
>           _main in main.o
>     ld: symbol(s) not found for architecture x86_64
>
> The idea here is that `__init` is a pretty big function, and we're
> promising that an external definition of it is available through the use of
> the extern template declaration. With the appropriate optimization level
> (O2 and above), LLVM decides not to include the definition of `__init` in
> the executable and to use the one available externally. Unfortunately,
> `__init` has hidden visibility, and so the definition in the .so is not
> visible to the executable, and the link fails.
>
> Where I think Clang/LLVM goes wrong is when it decides to remove the
> instantiation in the executable in favor of the one that is hypothetically
> provided externally. Indeed, the Standard says in [temp.explicit]/12 (
> http://eel.is/c++draft/temp.explicit#12):
>
>     Except for inline functions and variables, declarations with types
> deduced from their initializer or return value ([dcl.spec.auto]), const
> variables of literal types, variables of reference types, and class
> template specializations, explicit instantiation declarations have the
> effect of suppressing the implicit instantiation of the definition of the
> entity to which they refer. [ Note: The intent is that an inline function
> that is the subject of an explicit instantiation declaration will still be
> implicitly instantiated when odr-used so that the body can be considered
> for inlining, but that no out-of-line copy of the inline function would be
> generated in the translation unit. — end note ]
>
> Only reading the normative wording, it seems like LLVM should leave the
> instantiation there because it can't actually assume that there will be
a
> definition provided elsewhere (yes, despite the extern template
> declaration, because the function is inline). Then, the non-normative note
> seems to be approving of what LLVM is doing, but I'm wondering whether
> that's really the intended behavior.
>
> Questions:
> 1. Is what LLVM's doing there legal?
>
The Standard does not say anything about the instantiation producing a
definition associated with object file that results from translating the
current translation unit. The program is only valid if there is indeed an
explicit instantiation provided elsewhere. That said, the as-if rule that
allows the suppression of the definition assumes that a provided explicit
instantiation can be linked against. It is up the the designers of the
extension (the visibility attribute) to say whether or not the rule with
templates having hidden visibility is that the explicit instantiation needs
to be provided in the same "module".

> 2. If it is legal, then libc++ needs a way to express that a function
> should either be inlined in the caller, or emitted in the TU and
> de-duplicated later (I think that’s linkonce_odr), despite there being an
> extern template declaration promising a definition elsewhere. I think the
> current situation is that the function gets available_externally linkage
> instead. Is there a way to express this at the C++ source code level?
>
> Thank you,
> Louis Dionne
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180710/1a04094e/attachment.html>

This is probably much more of a question for the Clang list....

On Tue, Jul 10, 2018 at 6:21 PM Hubert Tong via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> On Tue, Jul 10, 2018 at 7:20 PM, Louis Dionne via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> Hi,
>>
>> While investigating the situation of visibility annotations and linkage
>> in libc++ with the goal of removing uses of `__always_inline__`, Eric
>> Fiselier and I stumbled upon the attached test case, which I don't
think
>> Clang compiles properly. Here's the gist of the test case, reduced
to the
>> important parts (see the attachment if you want to repro):
>>
>>     // RUN: %cxx -shared -o %T/libtest.so %flags %compile_flags -fPIC
%s
>>     // RUN: %cxx -c -o %T/main.o %flags %compile_flags %s -O2
>> -DBUILDING_MAIN
>>     // RUN: %cxx -o %T/test.exe -L%T/ -Wl,-rpath,%T/ -ltest %T/main.o
>>     // RUN: %T/test.exe
>>
>>     template <class T>
>>     struct Foo {
>>         inline __attribute__((visibility("hidden")))
>>         int __init(int x) { /* LOTS OF CODE */ }
>>
>>         inline __attribute__((visibility("default")))
>>         int bar(int x) { return __init(x); }
>>     };
>>
>>     extern template struct Foo<int>;
>>
>>     #ifdef BUILDING_MAIN
>>         int main() {
>>             Foo<int> f;
>>             f.bar(101);
>>         }
>>     #else
>>         template struct Foo<int>;
>>     #endif
>>
>> When running the attached file in `lit`, we get:
>>
>>     Undefined symbols for architecture x86_64:
>>       "Foo<int>::__init(int)", referenced from:
>>           _main in main.o
>>     ld: symbol(s) not found for architecture x86_64
>>
>> The idea here is that `__init` is a pretty big function, and we're
>> promising that an external definition of it is available through the
use of
>> the extern template declaration. With the appropriate optimization
level
>> (O2 and above), LLVM decides not to include the definition of `__init`
in
>> the executable and to use the one available externally. Unfortunately,
>> `__init` has hidden visibility, and so the definition in the .so is not
>> visible to the executable, and the link fails.
>>
>> Where I think Clang/LLVM goes wrong is when it decides to remove the
>> instantiation in the executable in favor of the one that is
hypothetically
>> provided externally. Indeed, the Standard says in [temp.explicit]/12 (
>> http://eel.is/c++draft/temp.explicit#12):
>>
>>     Except for inline functions and variables, declarations with types
>> deduced from their initializer or return value ([dcl.spec.auto]), const
>> variables of literal types, variables of reference types, and class
>> template specializations, explicit instantiation declarations have the
>> effect of suppressing the implicit instantiation of the definition of
the
>> entity to which they refer. [ Note: The intent is that an inline
function
>> that is the subject of an explicit instantiation declaration will still
be
>> implicitly instantiated when odr-used so that the body can be
considered
>> for inlining, but that no out-of-line copy of the inline function would
be
>> generated in the translation unit. — end note ]
>>
>> Only reading the normative wording, it seems like LLVM should leave the
>> instantiation there because it can't actually assume that there
will be a
>> definition provided elsewhere (yes, despite the extern template
>> declaration, because the function is inline). Then, the non-normative
note
>> seems to be approving of what LLVM is doing, but I'm wondering
whether
>> that's really the intended behavior.
>>
>> Questions:
>> 1. Is what LLVM's doing there legal?
>>
> The Standard does not say anything about the instantiation producing a
> definition associated with object file that results from translating the
> current translation unit. The program is only valid if there is indeed an
> explicit instantiation provided elsewhere. That said, the as-if rule that
> allows the suppression of the definition assumes that a provided explicit
> instantiation can be linked against. It is up the the designers of the
> extension (the visibility attribute) to say whether or not the rule with
> templates having hidden visibility is that the explicit instantiation needs
> to be provided in the same "module".
>
>
>> 2. If it is legal, then libc++ needs a way to express that a function
>> should either be inlined in the caller, or emitted in the TU and
>> de-duplicated later (I think that’s linkonce_odr), despite there being
an
>> extern template declaration promising a definition elsewhere. I think
the
>> current situation is that the function gets available_externally
linkage
>> instead. Is there a way to express this at the C++ source code level?
>>
>> Thank you,
>> Louis Dionne
>>
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20180711/7b409f16/attachment.html>