Message ID | 20190523141326.GA21005@redhat.com |
---|---|
State | New |
Headers | show |
Series | Make any_cast compare typeinfo as well as function pointers | expand |
Hi Jonathan, On Thu, 23 May 2019 at 16:13, Jonathan Wakely <jwakely@redhat.com> wrote: > > It's possible for the function pointer comparison to fail even though > the type is correct, because the function could be defined multiple > times with different addresses when shared libraries are in use. > > Retain the function pointer check for the common case where the check > succeeds, but compare typeinfo (if RTTI is enabled) if the first check > fails. > > * include/experimental/any (__any_caster): Use RTTI if comparing > addresses fails, to support non-unique addresses in shared libraries. > * include/std/any (__any_caster): Likewise. > > Tested powerpc64le-linux, committed to trunk, backports to follow. > > It seems OK on trunk, but on gcc-7/gcc-8 branches I see: /libstdc++-v3/testsuite/experimental/any/misc/any_cast_neg.cc:20: /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/aarch64-none-linux-gnu/libstdc++-v3/include/experimental/any: In instantiation of '_ValueType std::experimental::fundamentals_v1::any_cast(const std::experimental::fundamentals_v1::any&) [with _ValueType = int&]': /libstdc++-v3/testsuite/experimental/any/misc/any_cast_neg.cc:28: required from here /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/aarch64-none-linux-gnu/libstdc++-v3/include/experimental/any:358: error: binding reference of type 'int&' to 'const int' discards qualifiers compiler exited with status 1 FAIL: experimental/any/misc/any_cast_neg.cc (test for errors, line 357) FAIL: experimental/any/misc/any_cast_neg.cc (test for excess errors) on arm/aarch64 Christophe
On 24/05/19 10:57 +0200, Christophe Lyon wrote: >Hi Jonathan, > >On Thu, 23 May 2019 at 16:13, Jonathan Wakely <jwakely@redhat.com> wrote: >> >> It's possible for the function pointer comparison to fail even though >> the type is correct, because the function could be defined multiple >> times with different addresses when shared libraries are in use. >> >> Retain the function pointer check for the common case where the check >> succeeds, but compare typeinfo (if RTTI is enabled) if the first check >> fails. >> >> * include/experimental/any (__any_caster): Use RTTI if comparing >> addresses fails, to support non-unique addresses in shared libraries. >> * include/std/any (__any_caster): Likewise. >> >> Tested powerpc64le-linux, committed to trunk, backports to follow. >> >> > >It seems OK on trunk, but on gcc-7/gcc-8 branches I see: >/libstdc++-v3/testsuite/experimental/any/misc/any_cast_neg.cc:20: >/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/aarch64-none-linux-gnu/libstdc++-v3/include/experimental/any: >In instantiation of '_ValueType >std::experimental::fundamentals_v1::any_cast(const >std::experimental::fundamentals_v1::any&) [with _ValueType = int&]': >/libstdc++-v3/testsuite/experimental/any/misc/any_cast_neg.cc:28: >required from here >/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/aarch64-none-linux-gnu/libstdc++-v3/include/experimental/any:358: >error: binding reference of type 'int&' to 'const int' discards >qualifiers >compiler exited with status 1 >FAIL: experimental/any/misc/any_cast_neg.cc (test for errors, line 357) >FAIL: experimental/any/misc/any_cast_neg.cc (test for excess errors) > >on arm/aarch64 I don't see how you can have that error when the test was changed to match on any line: https://gcc.gnu.org/viewcvs/gcc/branches/gcc-7-branch/libstdc%2B%2B-v3/testsuite/experimental/any/misc/any_cast_neg.cc?r1=271566&r2=271565&pathrev=271566 https://gcc.gnu.org/viewcvs/gcc/branches/gcc-8-branch/libstdc%2B%2B-v3/testsuite/experimental/any/misc/any_cast_neg.cc?r1=271562&r2=271561&pathrev=271562
On 24/05/19 11:26 +0100, Jonathan Wakely wrote: >On 24/05/19 10:57 +0200, Christophe Lyon wrote: >>Hi Jonathan, >> >>On Thu, 23 May 2019 at 16:13, Jonathan Wakely <jwakely@redhat.com> wrote: >>> >>>It's possible for the function pointer comparison to fail even though >>>the type is correct, because the function could be defined multiple >>>times with different addresses when shared libraries are in use. >>> >>>Retain the function pointer check for the common case where the check >>>succeeds, but compare typeinfo (if RTTI is enabled) if the first check >>>fails. >>> >>> * include/experimental/any (__any_caster): Use RTTI if comparing >>> addresses fails, to support non-unique addresses in shared libraries. >>> * include/std/any (__any_caster): Likewise. >>> >>>Tested powerpc64le-linux, committed to trunk, backports to follow. >>> >>> >> >>It seems OK on trunk, but on gcc-7/gcc-8 branches I see: >>/libstdc++-v3/testsuite/experimental/any/misc/any_cast_neg.cc:20: >>/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/aarch64-none-linux-gnu/libstdc++-v3/include/experimental/any: >>In instantiation of '_ValueType >>std::experimental::fundamentals_v1::any_cast(const >>std::experimental::fundamentals_v1::any&) [with _ValueType = int&]': >>/libstdc++-v3/testsuite/experimental/any/misc/any_cast_neg.cc:28: >>required from here >>/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-aarch64-none-linux-gnu/gcc3/aarch64-none-linux-gnu/libstdc++-v3/include/experimental/any:358: >>error: binding reference of type 'int&' to 'const int' discards >>qualifiers >>compiler exited with status 1 >>FAIL: experimental/any/misc/any_cast_neg.cc (test for errors, line 357) >>FAIL: experimental/any/misc/any_cast_neg.cc (test for excess errors) >> >>on arm/aarch64 > >I don't see how you can have that error when the test was changed to >match on any line: > >https://gcc.gnu.org/viewcvs/gcc/branches/gcc-7-branch/libstdc%2B%2B-v3/testsuite/experimental/any/misc/any_cast_neg.cc?r1=271566&r2=271565&pathrev=271566 >https://gcc.gnu.org/viewcvs/gcc/branches/gcc-8-branch/libstdc%2B%2B-v3/testsuite/experimental/any/misc/any_cast_neg.cc?r1=271562&r2=271561&pathrev=271562 As discussed on IRC, the failure was seen at r271565 and fixed at r271566.
diff --git a/libstdc++-v3/include/experimental/any b/libstdc++-v3/include/experimental/any index f1d4bbf788c..f7a4c8fa394 100644 --- a/libstdc++-v3/include/experimental/any +++ b/libstdc++-v3/include/experimental/any @@ -432,11 +432,18 @@ inline namespace fundamentals_v1 // is explicitly specialized and has a no-op _S_manage function. using _Vp = conditional_t<__and_<__does_not_decay, __is_copyable>::value, _Up, any::_Op>; - if (__any->_M_manager != &any::_Manager<_Vp>::_S_manage) - return nullptr; - any::_Arg __arg; - __any->_M_manager(any::_Op_access, __any, &__arg); - return __arg._M_obj; + // First try comparing function addresses, which works without RTTI + if (__any->_M_manager == &any::_Manager<_Vp>::_S_manage +#if __cpp_rtti + || __any->type() == typeid(_Tp) +#endif + ) + { + any::_Arg __arg; + __any->_M_manager(any::_Op_access, __any, &__arg); + return __arg._M_obj; + } + return nullptr; } // This overload exists so that std::any_cast<void(*)()>(a) is well-formed. diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any index 8cbabdddd9b..229c7c6b65e 100644 --- a/libstdc++-v3/include/std/any +++ b/libstdc++-v3/include/std/any @@ -503,6 +503,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } // @} + /// @cond undocumented template<typename _Tp> void* __any_caster(const any* __any) { @@ -516,8 +517,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Only copy constructible types can be used for contained values: else if constexpr (!is_copy_constructible_v<_Up>) return nullptr; - // This check is equivalent to __any->type() == typeid(_Tp) - else if (__any->_M_manager == &any::_Manager<_Up>::_S_manage) + // First try comparing function addresses, which works without RTTI + else if (__any->_M_manager == &any::_Manager<_Up>::_S_manage +#if __cpp_rtti + || __any->type() == typeid(_Tp) +#endif + ) { any::_Arg __arg; __any->_M_manager(any::_Op_access, __any, &__arg); @@ -525,6 +530,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION } return nullptr; } + /// @endcond /** * @brief Access the contained object.