Re: [PATCH] Make any_cast compare typeinfo as well as function pointers
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 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=271565=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=271561=271562 As discussed on IRC, the failure was seen at r271565 and fixed at r271566.
Re: [PATCH] Make any_cast compare typeinfo as well as function pointers
On 24/05/19 10:57 +0200, Christophe Lyon wrote: Hi Jonathan, On Thu, 23 May 2019 at 16:13, Jonathan Wakely 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=271565=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=271561=271562
Re: [PATCH] Make any_cast compare typeinfo as well as function pointers
Hi Jonathan, On Thu, 23 May 2019 at 16:13, Jonathan Wakely 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
[PATCH] Make any_cast compare typeinfo as well as function pointers
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. commit 330b70bbe09569c3d2c753e27186805977b78c8d Author: Jonathan Wakely Date: Thu May 23 14:43:12 2019 +0100 Make any_cast compare typeinfo as well as function pointers 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. 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 != ::_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 == ::_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(a) is well-formed. diff --git a/libstdc++-v3/include/std/any b/libstdc++-v3/include/std/any index 8cbab9b..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 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 == ::_Manager<_Up>::_S_manage) + // First try comparing function addresses, which works without RTTI + else if (__any->_M_manager == ::_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.