Windows: enable tests and envoy-static.exe pdb file (#13688)

Introduce debugging output for msvc-cl and clang-cl opt binaries in
order to be able to unwind stack traces from windows core files.

Mark additional tests fails on windows based on clang-cl build
observed test failures

Work around problematic googletests with clang-cl compilation

Solve problematic missing override declarations of mocks by temporarilly
adding -Wno-inconsistent-missing-override for clang. (It appears
googletest on clang-cl isn't handling something that our gcc, clang and
msvc-cl builds all ignore without the warning override.

Workaround THROW tests which googletest with clang-cl are framing as
as -Wdiscard errors.

Workaround nodiscard next() method error

Follow up about discarded results of singleton construction and other
discarded odd or apparently irrelevant function call results.

Holding on adding clang-cl CI until LLVM 11.0.0 can be used on Windows
with Bazel (requires Bazel 3.7.1 and 4.0.0 releases)

Risk Level: Low (only affects Windows build)
Testing: Verified locally
Docs Changes: N/A
Release Notes: N/A
Platform Specific Features: N/A
Related to #11974 (partially addresses)

Co-authored-by: Sunjay Bhatia <[email protected]>
Signed-off-by: Sunjay Bhatia <[email protected]>
Co-authored-by: William A Rowe Jr <[email protected]>
Signed-off-by: William A Rowe Jr <[email protected]>

.bazelrc

@@ -322,6 +322,12 @@ build:clang-cl --define clang_cl=1
 # Override determinism flags (DATE etc) is valid on clang-cl compiler
 build:clang-cl --copt="-Wno-macro-redefined"
 build:clang-cl --copt="-Wno-builtin-macro-redefined"
+# Workaround problematic missing override declarations of mocks
+# TODO: resolve this class of problematic mocks, e.g.
+# ./test/mocks/http/stream.h(16,21): error: 'addCallbacks'
+#     overrides a member function but is not marked 'override'
+#   MOCK_METHOD(void, addCallbacks, (StreamCallbacks & callbacks));
+build:clang-cl --copt="-Wno-inconsistent-missing-override"
 build:clang-cl --action_env=USE_CLANG_CL=1
 
 # Defaults to 'auto' - Off for windows, so override to linux behavior

bazel/BUILD

@@ -84,6 +84,14 @@ config_setting(
     },
 )
 
+config_setting(
+    name = "clang_cl_build",
+    values = {
+        "cpu": "x64_windows",
+        "define": "clang_cl=1",
+    },
+)
+
 config_setting(
     name = "clang_cl_opt_build",
     values = {

bazel/envoy_binary.bzl

@@ -60,7 +60,7 @@ def _envoy_linkopts():
             "-pagezero_size 10000",
             "-image_base 100000000",
         ],
-        "@envoy//bazel:clang_cl_opt_build": [
+        "@envoy//bazel:windows_opt_build": [
             "-DEFAULTLIB:ws2_32.lib",
             "-DEFAULTLIB:iphlpapi.lib",
             "-DEBUG:FULL",

bazel/envoy_internal.bzl

@@ -34,37 +34,44 @@ def envoy_copts(repository, test = False):
         "-DNOIME",
         "-DNOCRYPT",
         # Ignore unguarded gcc pragmas in quiche (unrecognized by MSVC)
-        # TODO(wrowe,sunjayBhatia): Drop this change when fixed in bazel/external/quiche.genrule_cmd
         "-wd4068",
         # Silence incorrect MSVC compiler warnings when converting between std::optional
         # data types (while conversions between primitive types are producing no error)
         "-wd4244",
         # Allow inline functions to be undefined
         "-wd4506",
-        # Allow 'nodiscard' function return values to be discarded
-        # TODO(wrowe,sunjayBhatia): Drop this option when all causes are fixed
-        "-wd4834",
     ]
 
     return select({
                repository + "//bazel:windows_x86_64": msvc_options,
                "//conditions:default": posix_options,
            }) + select({
-               # Bazel adds an implicit -DNDEBUG for opt.
+               # Simplify the amount of symbolic debug info for test binaries, since
+               # debugging info detailing some 1600 test binaries would be wasteful.
+               # targets listed in order from generic to increasing specificity.
+               # Bazel adds an implicit -DNDEBUG for opt targets.
                repository + "//bazel:opt_build": [] if test else ["-ggdb3", "-gsplit-dwarf"],
                repository + "//bazel:fastbuild_build": [],
                repository + "//bazel:dbg_build": ["-ggdb3", "-gsplit-dwarf"],
-               repository + "//bazel:windows_opt_build": [],
+               repository + "//bazel:windows_opt_build": [] if test else ["-Z7"],
                repository + "//bazel:windows_fastbuild_build": [],
                repository + "//bazel:windows_dbg_build": [],
                repository + "//bazel:clang_cl_opt_build": [] if test else ["-Z7", "-fstandalone-debug"],
                repository + "//bazel:clang_cl_fastbuild_build": ["-fno-standalone-debug"],
                repository + "//bazel:clang_cl_dbg_build": ["-fstandalone-debug"],
            }) + select({
-               repository + "//bazel:clang_build": ["-fno-limit-debug-info", "-Wgnu-conditional-omitted-operand", "-Wc++2a-extensions", "-Wrange-loop-analysis"],
+               # Toggle expected features and warnings by compiler
+               repository + "//bazel:clang_build": [
+                   "-fno-limit-debug-info",
+                   "-Wgnu-conditional-omitted-operand",
+                   "-Wc++2a-extensions",
+                   "-Wrange-loop-analysis",
+               ],
                repository + "//bazel:gcc_build": ["-Wno-maybe-uninitialized"],
-               # TODO: Replace with /Zc:preprocessor for cl.exe versions >= 16.5
-               repository + "//bazel:windows_x86_64": ["-experimental:preprocessor", "-Wv:19.4"],
+               # Allow 'nodiscard' function results values to be discarded for test code only
+               # TODO(envoyproxy/windows-dev): Replace with /Zc:preprocessor for cl.exe versions >= 16.5
+               repository + "//bazel:windows_x86_64": ["-wd4834", "-experimental:preprocessor", "-Wv:19.4"] if test else ["-experimental:preprocessor", "-Wv:19.4"],
+               repository + "//bazel:clang_cl_build": ["-Wno-unused-result"] if test else [],
                "//conditions:default": [],
            }) + select({
                repository + "//bazel:no_debug_info": ["-g0"],

bazel/repository_locations.bzl

@@ -537,11 +537,11 @@ REPOSITORY_LOCATIONS_SPEC = dict(
         project_desc = "Bazel rules for the C++ language",
         project_url = "https://github.com/bazelbuild/rules_cc",
         # TODO(lizan): pin to a point releases when there's a released version.
-        version = "818289e5613731ae410efb54218a4077fb9dbb03",
-        sha256 = "9d48151ea71b3e225adfb6867e6d2c7d0dce46cbdc8710d9a9a628574dfd40a0",
+        version = "b1c40e1de81913a3c40e5948f78719c28152486d",
+        sha256 = "71d037168733f26d2a9648ad066ee8da4a34a13f51d24843a42efa6b65c2420f",
         strip_prefix = "rules_cc-{version}",
         urls = ["https://github.com/bazelbuild/rules_cc/archive/{version}.tar.gz"],
-        release_date = "2020-05-13",
+        release_date = "2020-11-11",
         use_category = ["build"],
     ),
     rules_foreign_cc = dict(

ci/windows_ci_steps.sh

@@ -79,6 +79,6 @@ bazel "${BAZEL_STARTUP_OPTIONS[@]}" test "${BAZEL_BUILD_OPTIONS[@]}" //test/...
 bazel "${BAZEL_STARTUP_OPTIONS[@]}" build "${BAZEL_BUILD_OPTIONS[@]}" //test/... --test_tag_filters=-skip_on_windows,fails_on_windows,flaky_on_windows --build_tests_only
 
 # Summarize tests bypasssed to monitor the progress of porting to Windows
-echo "Tests bypassed as skip_on_windows: $(bazel query 'kind(".*test rule", attr("tags", "skip_on_windows", //test/...))' 2>/dev/null | sort | wc -l) known unbuildable or inapplicable tests"
-echo "Tests bypassed as fails_on_windows: $(bazel query 'kind(".*test rule", attr("tags", "fails_on_windows", //test/...))' 2>/dev/null | sort | wc -l) known incompatible tests"
-echo "Tests bypassed as flaky_on_windows: $(bazel query 'kind(".*test rule", attr("tags", "flaky_on_windows", //test/...))' 2>/dev/null | sort | wc -l) known unstable tests"
+echo "Tests bypassed as skip_on_windows: $(bazel "${BAZEL_STARTUP_OPTIONS[@]}" query 'kind(".*test rule", attr("tags", "skip_on_windows", //test/...))' 2>/dev/null | sort | wc -l) known unbuildable or inapplicable tests"
+echo "Tests bypassed as fails_on_windows: $(bazel "${BAZEL_STARTUP_OPTIONS[@]}" query 'kind(".*test rule", attr("tags", "fails_on_windows", //test/...))' 2>/dev/null | sort | wc -l) known incompatible tests"
+echo "Tests bypassed as flaky_on_windows: $(bazel "${BAZEL_STARTUP_OPTIONS[@]}" query 'kind(".*test rule", attr("tags", "flaky_on_windows", //test/...))' 2>/dev/null | sort | wc -l) known unstable tests"

source/extensions/clusters/redis/redis_cluster.cc

@@ -274,8 +274,7 @@ void RedisCluster::RedisDiscoverySession::startResolveRedis() {
   Upstream::HostSharedPtr host;
   if (parent_.hosts_.empty()) {
     const int rand_idx = parent_.random_.random() % discovery_address_list_.size();
-    auto it = discovery_address_list_.begin();
-    std::next(it, rand_idx);
+    auto it = std::next(discovery_address_list_.begin(), rand_idx);
     host = Upstream::HostSharedPtr{
         new RedisHost(parent_.info(), "", *it, parent_, true, parent_.timeSource())};
   } else {

test/common/network/BUILD

@@ -286,10 +286,8 @@ envoy_cc_test(
 envoy_cc_test(
     name = "udp_listener_impl_batch_writer_test",
     srcs = ["udp_listener_impl_batch_writer_test.cc"],
-    tags = [
-        # Skipping as quiche quic_gso_batch_writer.h does not exist on Windows
-        "skip_on_windows",
-    ],
+    # Skipping as quiche quic_gso_batch_writer.h does not exist on Windows
+    tags = ["skip_on_windows"],
     deps = [
         ":udp_listener_impl_test_base_lib",
         "//source/common/event:dispatcher_lib",

test/common/network/cidr_range_test.cc

@@ -68,7 +68,6 @@ TEST(TruncateIpAddressAndLength, Various) {
       {{"ffff::ffff", 128}, {"ffff::ffff", 128}},
       {{"ffff::ffff", 999}, {"ffff::ffff", 128}},
   };
-  test_cases.size();
   for (const auto& kv : test_cases) {
     InstanceConstSharedPtr inPtr = Utility::parseInternetAddress(kv.first.first);
     EXPECT_NE(inPtr, nullptr) << kv.first.first;

test/common/network/socket_option_factory_test.cc

@@ -24,8 +24,8 @@ class SocketOptionFactoryTest : public testing::Test {
   TestThreadsafeSingletonInjector<Api::OsSysCallsImpl> os_calls_{[this]() {
     // Before injecting OsSysCallsImpl, make sure validateIpv{4,6}Supported is called so the static
     // bool is initialized without requiring to mock ::socket and ::close. :( :(
-    std::make_unique<Address::Ipv4Instance>("1.2.3.4", 5678);
-    std::make_unique<Address::Ipv6Instance>("::1:2:3:4", 5678);
+    (void)std::make_unique<Address::Ipv4Instance>("1.2.3.4", 5678);
+    (void)std::make_unique<Address::Ipv6Instance>("::1:2:3:4", 5678);
     return &os_sys_calls_mock_;
   }()};
 

test/common/network/socket_option_test.h

@@ -65,8 +65,8 @@ class SocketOptionTest : public testing::Test {
   TestThreadsafeSingletonInjector<Api::OsSysCallsImpl> os_calls_{[this]() {
     // Before injecting OsSysCallsImpl, make sure validateIpv{4,6}Supported is called so the static
     // bool is initialized without requiring to mock ::socket and ::close.
-    std::make_unique<Address::Ipv4Instance>("1.2.3.4", 5678);
-    std::make_unique<Address::Ipv6Instance>("::1:2:3:4", 5678);
+    (void)std::make_unique<Address::Ipv4Instance>("1.2.3.4", 5678);
+    (void)std::make_unique<Address::Ipv6Instance>("::1:2:3:4", 5678);
     return &os_sys_calls_;
   }()};
 

test/extensions/filters/http/dynamic_forward_proxy/BUILD

@@ -51,6 +51,8 @@ envoy_extension_cc_test(
         "//test/config/integration/certs",
     ],
     extension_name = "envoy.filters.http.dynamic_forward_proxy",
+    # TODO(envoyproxy/windows-dev): Diagnose flake in msvc-cl pipeline, observed timeout in
+    # IpVersions/ProxyFilterIntegrationTest.ReloadClusterAndAttachToCache/IPv6
     tags = ["flaky_on_windows"],
     deps = [
         "//source/extensions/clusters/dynamic_forward_proxy:cluster",

test/extensions/filters/http/ratelimit/BUILD

@@ -51,8 +51,6 @@ envoy_extension_cc_test(
     name = "ratelimit_integration_test",
     srcs = ["ratelimit_integration_test.cc"],
     extension_name = "envoy.filters.http.ratelimit",
-    # TODO(envoyproxy/windows-dev): Apparently resolved by Level Events PR #13787
-    tags = ["flaky_on_windows"],
     deps = [
         "//source/common/buffer:zero_copy_input_stream_lib",
         "//source/common/grpc:codec_lib",

test/extensions/filters/http/tap/BUILD

@@ -52,6 +52,9 @@ envoy_extension_cc_test(
     name = "tap_filter_integration_test",
     srcs = ["tap_filter_integration_test.cc"],
     extension_name = "envoy.filters.http.tap",
+    # TODO(envoyproxy/windows-dev): Diagnose flake, observed to hang at;
+    # IpVersions/TapIntegrationTest.AdminLdsReload/IPv6
+    tags = ["flaky_on_windows"],
     deps = [
         "//source/extensions/filters/http/tap:config",
         "//test/extensions/common/tap:common",

test/extensions/quic_listeners/quiche/BUILD

@@ -164,10 +164,7 @@ envoy_cc_test(
 envoy_cc_test(
     name = "active_quic_listener_test",
     srcs = ["active_quic_listener_test.cc"],
-    tags = [
-        "fails_on_windows",
-        "nofips",
-    ],
+    tags = ["nofips"],
     deps = [
         ":quic_test_utils_for_envoy_lib",
         ":test_utils_lib",

test/extensions/stats_sinks/metrics_service/BUILD

@@ -45,8 +45,6 @@ envoy_extension_cc_test(
     name = "metrics_service_integration_test",
     srcs = ["metrics_service_integration_test.cc"],
     extension_name = "envoy.stat_sinks.metrics_service",
-    # TODO(envoyproxy/windows-dev): Apparently resolved by Level Events PR #13787
-    tags = ["flaky_on_windows"],
     deps = [
         "//source/common/buffer:zero_copy_input_stream_lib",
         "//source/common/grpc:codec_lib",