Fix comparator bugs which are not compliant to strict weak ordering.

See a full explanation of the problem on this blog [1] post about changing
std::sort in LLVM and relative issues uncovered.

The CompareNetwork function was violating the 4th rule of "strict weak
ordering" (Transitivity of incomparability: x == y and y == z imply x == z, where x == y means x < y and y < x are both false).

[1] - https://danlark.org/2022/04/20/changing-stdsort-at-googles-scale-and-beyond/

Bug: None
Change-Id: I7e893f0a30da31403766284823f75c45c4db91c3
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251681
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36711}

rtc_base/network.cc

@@ -63,16 +63,6 @@ struct AddressList {
   std::vector<InterfaceAddress> ips;
 };
 
-bool CompareNetworks(const std::unique_ptr<Network>& a,
-                     const std::unique_ptr<Network>& b) {
-  if (a->prefix_length() == b->prefix_length()) {
-    if (a->name() == b->name()) {
-      return a->prefix() < b->prefix();
-    }
-  }
-  return a->name() < b->name();
-}
-
 bool SortNetworks(const Network* a, const Network* b) {
   // Network types will be preferred above everything else while sorting
   // Networks.
@@ -195,6 +185,19 @@ const char kPublicIPv4Host[] = "8.8.8.8";
 const char kPublicIPv6Host[] = "2001:4860:4860::8888";
 const int kPublicPort = 53;  // DNS port.
 
+namespace webrtc_network_internal {
+bool CompareNetworks(const std::unique_ptr<Network>& a,
+                     const std::unique_ptr<Network>& b) {
+  if (a->prefix_length() != b->prefix_length()) {
+    return a->prefix_length() < b->prefix_length();
+  }
+  if (a->name() != b->name()) {
+    return a->name() < b->name();
+  }
+  return a->prefix() < b->prefix();
+}
+}  // namespace webrtc_network_internal
+
 std::string MakeNetworkKey(absl::string_view name,
                            const IPAddress& prefix,
                            int prefix_length) {
@@ -337,7 +340,7 @@ void NetworkManagerBase::MergeNetworkList(
   // AddressList in this map will track IP addresses for all Networks
   // with the same key.
   std::map<std::string, AddressList> consolidated_address_list;
-  absl::c_sort(new_networks, CompareNetworks);
+  absl::c_sort(new_networks, rtc::webrtc_network_internal::CompareNetworks);
   // First, build a set of network-keys to the ipaddresses.
   for (auto& network : new_networks) {
     bool might_add_to_merged_list = false;

rtc_base/network.h

@@ -53,6 +53,11 @@ class Thread;
 // By default, ignore loopback interfaces on the host.
 const int kDefaultNetworkIgnoreMask = ADAPTER_TYPE_LOOPBACK;
 
+namespace webrtc_network_internal {
+bool CompareNetworks(const std::unique_ptr<Network>& a,
+                     const std::unique_ptr<Network>& b);
+}  // namespace webrtc_network_internal
+
 // Makes a string key for this network. Used in the network manager's maps.
 // Network objects are keyed on interface name, network prefix and the
 // length of that prefix.

rtc_base/network_unittest.cc

@@ -1533,4 +1533,93 @@ TEST_F(NetworkTest, HardcodedVpn) {
   EXPECT_FALSE(NetworkManagerBase::IsVpnMacAddress(nullptr));
 }
 
+TEST(CompareNetworks, IrreflexivityTest) {
+  // x < x is false
+  auto network = std::make_unique<Network>(
+      "test_eth0", "Test Network Adapter 1", IPAddress(0x12345600U), 24);
+  EXPECT_FALSE(webrtc_network_internal::CompareNetworks(network, network));
+}
+
+TEST(CompareNetworks, AsymmetryTest) {
+  // x < y and y < x cannot be both true
+  auto network_a = std::make_unique<Network>(
+      "test_eth0", "Test Network Adapter 1", IPAddress(0x12345600U), 24);
+  auto network_b = std::make_unique<Network>(
+      "test_eth1", "Test Network Adapter 1", IPAddress(0x12345600U), 24);
+  EXPECT_TRUE(webrtc_network_internal::CompareNetworks(network_a, network_b));
+  EXPECT_FALSE(webrtc_network_internal::CompareNetworks(network_b, network_a));
+
+  auto network_c = std::make_unique<Network>(
+      "test_eth0", "Test Network Adapter 1", IPAddress(0x12345500U), 24);
+  auto network_d = std::make_unique<Network>(
+      "test_eth0", "Test Network Adapter 1", IPAddress(0x12345600U), 24);
+  EXPECT_TRUE(webrtc_network_internal::CompareNetworks(network_c, network_d));
+  EXPECT_FALSE(webrtc_network_internal::CompareNetworks(network_d, network_c));
+}
+
+TEST(CompareNetworks, TransitivityTest) {
+  // x < y and y < z imply x < z
+  auto network_a = std::make_unique<Network>(
+      "test_eth0", "Test Network Adapter 1", IPAddress(0x12345600U), 24);
+  auto network_b = std::make_unique<Network>(
+      "test_eth1", "Test Network Adapter 1", IPAddress(0x12345600U), 24);
+  auto network_c = std::make_unique<Network>(
+      "test_eth2", "Test Network Adapter 1", IPAddress(0x12345600U), 24);
+  EXPECT_TRUE(webrtc_network_internal::CompareNetworks(network_a, network_b));
+  EXPECT_TRUE(webrtc_network_internal::CompareNetworks(network_b, network_c));
+
+  auto network_d = std::make_unique<Network>(
+      "test_eth0", "Test Network Adapter 1", IPAddress(0x12345600U), 24);
+  auto network_e = std::make_unique<Network>(
+      "test_eth0", "Test Network Adapter 1", IPAddress(0x12345700U), 24);
+  auto network_f = std::make_unique<Network>(
+      "test_eth0", "Test Network Adapter 1", IPAddress(0x12345800U), 24);
+  EXPECT_TRUE(webrtc_network_internal::CompareNetworks(network_d, network_e));
+  EXPECT_TRUE(webrtc_network_internal::CompareNetworks(network_e, network_f));
+  EXPECT_TRUE(webrtc_network_internal::CompareNetworks(network_d, network_f));
+  EXPECT_TRUE(webrtc_network_internal::CompareNetworks(network_a, network_c));
+}
+
+TEST(CompareNetworks, TransitivityOfIncomparabilityTest) {
+  // x == y and y == z imply x == z,
+  // where x == y means x < y and y < x are both false
+  auto network_a = std::make_unique<Network>(
+      "test_eth0", "Test Network Adapter 1", IPAddress(0x12345600U), 23);
+  auto network_b = std::make_unique<Network>(
+      "test_eth0", "Test Network Adapter 1", IPAddress(0x12345600U), 24);
+  auto network_c = std::make_unique<Network>(
+      "test_eth0", "Test Network Adapter 1", IPAddress(0x12345700U), 24);
+
+  // network_a < network_b
+  EXPECT_TRUE(webrtc_network_internal::CompareNetworks(network_a, network_b));
+  EXPECT_FALSE(webrtc_network_internal::CompareNetworks(network_b, network_a));
+
+  // network_b < network_c
+  EXPECT_TRUE(webrtc_network_internal::CompareNetworks(network_b, network_c));
+  EXPECT_FALSE(webrtc_network_internal::CompareNetworks(network_c, network_b));
+
+  // network_a < network_c
+  EXPECT_TRUE(webrtc_network_internal::CompareNetworks(network_a, network_c));
+  EXPECT_FALSE(webrtc_network_internal::CompareNetworks(network_c, network_a));
+
+  auto network_d = std::make_unique<Network>(
+      "test_eth0", "Test Network Adapter 1", IPAddress(0x12345600U), 24);
+  auto network_e = std::make_unique<Network>(
+      "test_eth0", "Test Network Adapter 1", IPAddress(0x12345600U), 24);
+  auto network_f = std::make_unique<Network>(
+      "test_eth0", "Test Network Adapter 1", IPAddress(0x12345600U), 24);
+
+  // network_d == network_e
+  EXPECT_FALSE(webrtc_network_internal::CompareNetworks(network_d, network_e));
+  EXPECT_FALSE(webrtc_network_internal::CompareNetworks(network_e, network_d));
+
+  // network_e == network_f
+  EXPECT_FALSE(webrtc_network_internal::CompareNetworks(network_e, network_f));
+  EXPECT_FALSE(webrtc_network_internal::CompareNetworks(network_f, network_e));
+
+  // network_d == network_f
+  EXPECT_FALSE(webrtc_network_internal::CompareNetworks(network_d, network_f));
+  EXPECT_FALSE(webrtc_network_internal::CompareNetworks(network_f, network_d));
+}
+
 }  // namespace rtc