[committed] libstdc++: Fix constexpr functions in <experimental/internet>
Commit Message
Tested powerpc64le-linux. Pushed to trunk.
-- >8 --
Change ip::basic_endpoint to work in constant expressions, but only for
C++20 and later (due to the use of a union, which cannot change active
member in constexpr evaluation until C++20).
During constant evaluation we cannot inspect the common initial sequence
of basic_endpoint's union members to check whether sin_family == AF_INET
or AF_INET6. This means we need to store an additional boolean member
that remembers whether we have a v4 or v6 address. The address type can
change behind our backs if a user copies an address to the data()
pointer and then calls resize(n), so we need to inspect the sa_family_t
member in the union after a resize and update the boolean. POSIX only
guarantees that the sa_family_t member of each protocol-specific address
structure is at the same offset and of the same type, not that there is
a common initial sequence. The check in resize is done using memcmp, so
that we avoid accessing an inactive member of the union if the
sockaddr_in and sockaddr_in6 structures do not have a common initial
sequence that includes the sa_family_t member.
libstdc++-v3/ChangeLog:
* include/experimental/internet (ip::make_address): Implement
missing overload.
(ip::address_v4::broadcast()): Avoid undefined shift.
(ip::basic_endpoint): Fix member functions for constexpr.
(ip::basic_endpoint::_M_is_v6): Replace member function with
data member, adjust member functions using it.
(ip::basic_endpoint::resize): Update _M_is_v6 based on sockaddr
content.
* testsuite/experimental/net/internet/address/v4/cons.cc: Fix
constexpr checks to work in C++14.
* testsuite/experimental/net/internet/address/v4/creation.cc:
Likewise.
* testsuite/experimental/net/internet/endpoint/cons.cc:
Likewise.
* testsuite/experimental/net/internet/network/v4/cons.cc:
Likewise.
* testsuite/experimental/net/internet/network/v4/members.cc:
Likewise.
* testsuite/experimental/net/internet/endpoint/extensible.cc: New test.
---
libstdc++-v3/include/experimental/internet | 103 +++++++++++++-----
.../net/internet/address/v4/cons.cc | 16 ++-
.../net/internet/address/v4/creation.cc | 14 ++-
.../net/internet/endpoint/cons.cc | 29 +++--
.../net/internet/endpoint/extensible.cc | 47 ++++++++
.../net/internet/network/v4/cons.cc | 17 ++-
.../net/internet/network/v4/members.cc | 20 ++--
7 files changed, 185 insertions(+), 61 deletions(-)
create mode 100644 libstdc++-v3/testsuite/experimental/net/internet/endpoint/extensible.cc
Comments
On Thu, 30 Mar 2023 at 00:40, Jonathan Wakely via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> Tested powerpc64le-linux. Pushed to trunk.
>
> -- >8 --
>
> Change ip::basic_endpoint to work in constant expressions, but only for
> C++20 and later (due to the use of a union, which cannot change active
> member in constexpr evaluation until C++20).
>
> During constant evaluation we cannot inspect the common initial sequence
> of basic_endpoint's union members to check whether sin_family == AF_INET
> or AF_INET6. This means we need to store an additional boolean member
> that remembers whether we have a v4 or v6 address. The address type can
> change behind our backs if a user copies an address to the data()
> pointer and then calls resize(n), so we need to inspect the sa_family_t
> member in the union after a resize and update the boolean. POSIX only
> guarantees that the sa_family_t member of each protocol-specific address
> structure is at the same offset and of the same type, not that there is
> a common initial sequence. The check in resize is done using memcmp, so
> that we avoid accessing an inactive member of the union if the
> sockaddr_in and sockaddr_in6 structures do not have a common initial
> sequence that includes the sa_family_t member.
If we had https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/p2641r2.html
then we wouldn't need the extra boolean. During constant evaluation we
could use the new magic function to check which union member is
active, and during runtime we could inspect the sa_family_t member
(still using memcmp, because POSIX doesn't guarantee a common initial
sequence, even though I think everybody does give it one in practice).
On Thu, 30 Mar 2023 at 15:44, Jonathan Wakely wrote:
>
> On Thu, 30 Mar 2023 at 00:40, Jonathan Wakely via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
> >
> > Tested powerpc64le-linux. Pushed to trunk.
> >
> > -- >8 --
> >
> > Change ip::basic_endpoint to work in constant expressions, but only for
> > C++20 and later (due to the use of a union, which cannot change active
> > member in constexpr evaluation until C++20).
> >
> > During constant evaluation we cannot inspect the common initial sequence
> > of basic_endpoint's union members to check whether sin_family == AF_INET
> > or AF_INET6. This means we need to store an additional boolean member
> > that remembers whether we have a v4 or v6 address. The address type can
> > change behind our backs if a user copies an address to the data()
> > pointer and then calls resize(n), so we need to inspect the sa_family_t
> > member in the union after a resize and update the boolean. POSIX only
> > guarantees that the sa_family_t member of each protocol-specific address
> > structure is at the same offset and of the same type, not that there is
> > a common initial sequence. The check in resize is done using memcmp, so
> > that we avoid accessing an inactive member of the union if the
> > sockaddr_in and sockaddr_in6 structures do not have a common initial
> > sequence that includes the sa_family_t member.
>
> If we had https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/p2641r2.html
> then we wouldn't need the extra boolean. During constant evaluation we
> could use the new magic function to check which union member is
> active, and during runtime we could inspect the sa_family_t member
> (still using memcmp, because POSIX doesn't guarantee a common initial
> sequence, even though I think everybody does give it one in practice).
In fact, thinking about P2641 some more, I might revert this change.
Instead of adding an extra bool member to support constexpr, I think
I'll just remove the 'constexpr' keywords from basic_endpoint for now,
and implement it in terms of just inspecting the sa_family_t member of
the union members. And then later, once we have something like P2641,
we can re-add the constexpr keywords and use is_within_lifetime during
constant evaluation. That way we don't add a bool then need to take it
away again, changing the ABI each time.
Am Do., 30. März 2023 um 18:00 Uhr schrieb Jonathan Wakely via
Libstdc++ <libstdc++@gcc.gnu.org>:
>
[..]
>
> In fact, thinking about P2641 some more, I might revert this change.
> Instead of adding an extra bool member to support constexpr, I think
> I'll just remove the 'constexpr' keywords from basic_endpoint for now,
> and implement it in terms of just inspecting the sa_family_t member of
> the union members. And then later, once we have something like P2641,
> we can re-add the constexpr keywords and use is_within_lifetime during
> constant evaluation. That way we don't add a bool then need to take it
> away again, changing the ABI each time.
I was just going to make the same suggestion.
- Daniel
On Thu, 30 Mar 2023 at 17:01, Daniel Krügler wrote:
>
> Am Do., 30. März 2023 um 18:00 Uhr schrieb Jonathan Wakely via
> Libstdc++ <libstdc++@gcc.gnu.org>:
> >
> [..]
> >
> > In fact, thinking about P2641 some more, I might revert this change.
> > Instead of adding an extra bool member to support constexpr, I think
> > I'll just remove the 'constexpr' keywords from basic_endpoint for now,
> > and implement it in terms of just inspecting the sa_family_t member of
> > the union members. And then later, once we have something like P2641,
> > we can re-add the constexpr keywords and use is_within_lifetime during
> > constant evaluation. That way we don't add a bool then need to take it
> > away again, changing the ABI each time.
>
> I was just going to make the same suggestion.
Actually, as pointed out in Barry's P2641R1 paper, we can just use
GCC's __builtin_constant_p:
+ constexpr bool
+ _M_is_v6() const noexcept
+ {
+ // For constexpr eval we can just detect which union member is active.
+ // i.e. emulate P2641R1's std::is_active_member(&_M_data._M_v6)).
+ if (std::__is_constant_evaluated())
+ return __builtin_constant_p(_M_data._M_v6.sin6_family);
+ return _M_data._M_v6.sin6_family == AF_INET6;
+ }
On Fri, 31 Mar 2023 at 12:06, Jonathan Wakely wrote:
>
> On Thu, 30 Mar 2023 at 17:01, Daniel Krügler wrote:
> >
> > Am Do., 30. März 2023 um 18:00 Uhr schrieb Jonathan Wakely via
> > Libstdc++ <libstdc++@gcc.gnu.org>:
> > >
> > [..]
> > >
> > > In fact, thinking about P2641 some more, I might revert this change.
> > > Instead of adding an extra bool member to support constexpr, I think
> > > I'll just remove the 'constexpr' keywords from basic_endpoint for now,
> > > and implement it in terms of just inspecting the sa_family_t member of
> > > the union members. And then later, once we have something like P2641,
> > > we can re-add the constexpr keywords and use is_within_lifetime during
> > > constant evaluation. That way we don't add a bool then need to take it
> > > away again, changing the ABI each time.
> >
> > I was just going to make the same suggestion.
>
> Actually, as pointed out in Barry's P2641R1 paper, we can just use
> GCC's __builtin_constant_p:
>
> + constexpr bool
> + _M_is_v6() const noexcept
> + {
> + // For constexpr eval we can just detect which union member is active.
> + // i.e. emulate P2641R1's std::is_active_member(&_M_data._M_v6)).
> + if (std::__is_constant_evaluated())
> + return __builtin_constant_p(_M_data._M_v6.sin6_family);
> + return _M_data._M_v6.sin6_family == AF_INET6;
> + }
Done in commit 10e573e86c6a1ed11df529288ed8fba97f876032
@@ -977,7 +977,8 @@ namespace ip
{ return make_address(__str, __throw_on_error{"make_address"}); }
inline address
- make_address(const string& __str, error_code& __ec) noexcept; // TODO
+ make_address(const string& __str, error_code& __ec) noexcept
+ { return make_address(__str.c_str(), __ec); }
inline address
make_address(const string& __str)
@@ -1275,7 +1276,12 @@ namespace ip
constexpr address_v4
broadcast() const noexcept
- { return address_v4{_M_addr.to_uint() | (0xFFFFFFFFu >> _M_prefix_len)}; }
+ {
+ auto __b = _M_addr.to_uint();
+ if (_M_prefix_len < 32)
+ __b |= 0xFFFFFFFFu >> _M_prefix_len;
+ return address_v4{__b};
+ }
address_v4_range
hosts() const noexcept
@@ -1510,19 +1516,31 @@ namespace ip
basic_endpoint() noexcept : _M_data()
{ _M_data._M_v4.sin_family = protocol_type::v4().family(); }
- constexpr
+ _GLIBCXX20_CONSTEXPR
basic_endpoint(const protocol_type& __proto,
port_type __port_num) noexcept
: _M_data()
{
- __glibcxx_assert(__proto == protocol_type::v4()
- || __proto == protocol_type::v6());
-
- _M_data._M_v4.sin_family = __proto.family();
- _M_data._M_v4.sin_port = address_v4::_S_hton_16(__port_num);
+ if (__proto == protocol_type::v4())
+ {
+ _M_data._M_v4.sin_family = __proto.family();
+ _M_data._M_v4.sin_port = address_v4::_S_hton_16(__port_num);
+ }
+ else if (__proto == protocol_type::v6())
+ {
+ std::_Construct(&_M_data._M_v6);
+ _M_data._M_v6.sin6_family = __proto.family();
+ _M_data._M_v6.sin6_port = address_v4::_S_hton_16(__port_num);
+ _M_is_v6 = true;
+ }
+ else
+ {
+ __glibcxx_assert(__proto == protocol_type::v4()
+ || __proto == protocol_type::v6());
+ }
}
- constexpr
+ _GLIBCXX20_CONSTEXPR
basic_endpoint(const ip::address& __addr,
port_type __port_num) noexcept
: _M_data()
@@ -1535,37 +1553,42 @@ namespace ip
}
else
{
- _M_data._M_v6 = {};
+ std::_Construct(&_M_data._M_v6);
_M_data._M_v6.sin6_family = protocol_type::v6().family();
_M_data._M_v6.sin6_port = address_v4::_S_hton_16(__port_num);
- __builtin_memcpy(_M_data._M_v6.sin6_addr.s6_addr,
- __addr._M_v6._M_bytes.data(), 16);
+ uint8_t* __s6a = _M_data._M_v6.sin6_addr.s6_addr;
+ for (int __i = 0; __i < 16; ++__i)
+ __s6a[__i] = __addr._M_v6._M_bytes[__i];
_M_data._M_v6.sin6_scope_id = __addr._M_v6._M_scope_id;
+ _M_is_v6 = true;
}
}
// members:
+
constexpr protocol_type protocol() const noexcept
{
- return _M_is_v6() ? protocol_type::v6() : protocol_type::v4();
+ return _M_is_v6 ? protocol_type::v6() : protocol_type::v4();
}
constexpr ip::address
address() const noexcept
{
- ip::address __addr;
- if (_M_is_v6())
+ if (_M_is_v6)
{
- __builtin_memcpy(&__addr._M_v6._M_bytes,
- _M_data._M_v6.sin6_addr.s6_addr, 16);
- __addr._M_is_v4 = false;
+ address_v6 __v6;
+ const uint8_t* __s6a = _M_data._M_v6.sin6_addr.s6_addr;
+ for (int __i = 0; __i < 16; ++__i)
+ __v6._M_bytes[__i] = __s6a[__i];
+ __v6._M_scope_id = _M_data._M_v6.sin6_scope_id;
+ return __v6;
}
else
{
- __builtin_memcpy(&__addr._M_v4._M_addr,
- &_M_data._M_v4.sin_addr.s_addr, 4);
+ address_v4 __v4;
+ __v4._M_addr = _M_data._M_v4.sin_addr.s_addr;
+ return __v4;
}
- return __addr;
}
void
@@ -1573,37 +1596,62 @@ namespace ip
{
if (__addr.is_v6())
{
- _M_data._M_v6 = {};
+ std::_Construct(&_M_data._M_v6);
_M_data._M_v6.sin6_family = protocol_type::v6().family();
__builtin_memcpy(_M_data._M_v6.sin6_addr.s6_addr,
__addr._M_v6._M_bytes.data(), 16);
_M_data._M_v6.sin6_scope_id = __addr._M_v6._M_scope_id;
+ _M_is_v6 = true;
}
else
{
+ std::_Construct(&_M_data._M_v4);
_M_data._M_v4.sin_family = protocol_type::v4().family();
_M_data._M_v4.sin_addr.s_addr = __addr._M_v4._M_addr;
+ _M_is_v6 = false;
}
}
constexpr port_type
port() const noexcept
- { return address_v4::_S_ntoh_16(_M_data._M_v4.sin_port); }
+ {
+ port_type __p = 0;
+ if (_M_is_v6)
+ __p = _M_data._M_v6.sin6_port;
+ else
+ __p = _M_data._M_v4.sin_port;
+ return address_v4::_S_ntoh_16(__p);
+ }
void
port(port_type __port_num) noexcept
- { _M_data._M_v4.sin_port = address_v4::_S_hton_16(__port_num); }
+ {
+ __port_num = address_v4::_S_hton_16(__port_num);
+ if (_M_is_v6)
+ _M_data._M_v6.sin6_port = __port_num;
+ else
+ _M_data._M_v4.sin_port = __port_num;
+ }
void* data() noexcept { return &_M_data; }
const void* data() const noexcept { return &_M_data; }
- constexpr size_t size() const noexcept
- { return _M_is_v6() ? sizeof(sockaddr_in6) : sizeof(sockaddr_in); }
+ constexpr size_t
+ size() const noexcept
+ { return _M_is_v6 ? sizeof(sockaddr_in6) : sizeof(sockaddr_in); }
void
resize(size_t __s)
{
+ __glibcxx_assert(__s >= 0);
+ static_assert(offsetof(sockaddr_in6, sin6_family)
+ == offsetof(sockaddr_in, sin_family),
+ "sockaddr_in::sin_family and sockaddr_in6::sin6_family "
+ "must be at the same offset");
+ const sa_family_t __in6 = AF_INET6;
+ const auto* __ptr = (char*)&_M_data + offsetof(sockaddr_in, sin_family);
+ _M_is_v6 = __builtin_memcmp(&__in6, __ptr, sizeof(__in6)) == 0;
if (__s != size())
__throw_length_error("net::ip::basic_endpoint::resize");
}
@@ -1617,8 +1665,7 @@ namespace ip
sockaddr_in6 _M_v6;
} _M_data;
- constexpr bool _M_is_v6() const noexcept
- { return _M_data._M_v4.sin_family == AF_INET6; }
+ bool _M_is_v6 = false;
};
/** basic_endpoint comparisons
@@ -68,6 +68,15 @@ test03()
VERIFY( a1.to_bytes() == address_v4::bytes_type( 5, 6, 7, 8 ) );
}
+constexpr bool
+test_constexpr()
+{
+ test01();
+ test02();
+ test03();
+ return true;
+}
+
int
main()
{
@@ -75,10 +84,5 @@ main()
test02();
test03();
- constexpr bool c = []{
- test01();
- test02();
- test03();
- return true;
- };
+ static_assert( test_constexpr(), "valid in constant expressions" );
}
@@ -88,6 +88,14 @@ test03()
VERIFY( ec == std::errc::invalid_argument );
}
+constexpr bool
+test_constexpr()
+{
+ test01();
+ test02();
+ return true;
+}
+
int
main()
{
@@ -95,9 +103,5 @@ main()
test02();
test03();
- constexpr bool c = []{
- test01();
- test02();
- return true;
- };
+ static_assert( test_constexpr(), "valid in constant expressions" );
}
@@ -21,7 +21,10 @@ test_default()
VERIFY( t2.port() == 0 );
}
-constexpr void
+#if __cplusplus >= 202002
+constexpr
+#endif
+void
test_proto()
{
ip::tcp::endpoint t1(ip::tcp::v4(), 22);
@@ -35,7 +38,10 @@ test_proto()
VERIFY( t2.port() == 80 );
}
-constexpr void
+#if __cplusplus >= 202002
+constexpr
+#endif
+void
test_addr()
{
ip::address_v4 a1(ip::address_v4::bytes_type(1, 2, 3, 4));
@@ -51,16 +57,23 @@ test_addr()
VERIFY( t2.port() == 80 );
}
+constexpr bool
+test_constexpr()
+{
+ test_default();
+#if __cplusplus >= 202002
+ // Non-default basic_endpoint constructors are only constexpr in C++20.
+ test_proto();
+ test_addr();
+#endif
+ return true;
+}
+
int main()
{
test_default();
test_proto();
test_addr();
- constexpr bool c = [] {
- test_default();
- test_proto();
- test_addr();
- return true;
- };
+ static_assert( test_constexpr(), "valid in constant expressions" );
}
new file mode 100644
@@ -0,0 +1,47 @@
+// { dg-do run { target c++14 } }
+// { dg-require-effective-target net_ts_ip }
+// { dg-add-options net_ts }
+
+#include <experimental/internet>
+#include <cstring>
+#include <testsuite_hooks.h>
+
+using namespace std::experimental::net;
+
+void
+test_extensible()
+{
+ static_assert(ip::tcp::endpoint().capacity() == sizeof(sockaddr_in6),
+ "ip::tcp::endpoint::capacity() can store a sockaddr_in6");
+
+ ip::tcp::endpoint t1(ip::tcp::v4(), 22);
+ VERIFY(t1.size() == sizeof(sockaddr_in));
+ t1.resize(sizeof(sockaddr_in));
+ try {
+ t1.resize(2 * sizeof(sockaddr_in));
+ VERIFY(false);
+ } catch (const std::length_error&) {
+ }
+
+ ip::tcp::endpoint t2(ip::tcp::v6(), 80);
+ VERIFY(t2.size() == sizeof(sockaddr_in6));
+ t2.resize(sizeof(sockaddr_in6));
+ try {
+ t2.resize(2 * sizeof(sockaddr_in6));
+ VERIFY(false);
+ } catch (const std::length_error&) {
+ }
+
+ ip::tcp::endpoint t3;
+ std::memcpy(t3.data(), t1.data(), t1.size());
+ t3.resize(t1.size());
+ VERIFY( t3 == t1 );
+ std::memcpy(t3.data(), t2.data(), t2.size());
+ t3.resize(t2.size());
+ VERIFY( t3 == t2 );
+}
+
+int main()
+{
+ test_extensible();
+}
@@ -111,6 +111,16 @@ test03_errors()
}
}
+constexpr bool
+test_constexpr()
+{
+ test01();
+ test02();
+ test03();
+ return true;
+}
+
+
int
main()
{
@@ -120,10 +130,5 @@ main()
test03();
test03_errors();
- constexpr bool c = []{
- test01();
- test02();
- test03();
- return true;
- };
+ static_assert( test_constexpr(), "valid in constant expressions" );
}
@@ -166,6 +166,17 @@ test_to_string()
VERIFY( str2 == "87.65.43.21/4" );
}
+constexpr bool
+test_constexpr()
+{
+ test_netmask();
+ test_network();
+ test_broadcast();
+ test_canonical();
+ test_is_host();
+ return true;
+}
+
int main()
{
test_netmask();
@@ -175,12 +186,5 @@ int main()
test_is_host();
test_to_string();
- constexpr bool c = []{
- test_netmask();
- test_network();
- test_broadcast();
- test_canonical();
- test_is_host();
- return true;
- };
+ static_assert( test_constexpr(), "valid in constant expressions" );
}