[vcpkg] Turn on tests and PREfast in CI, and fix tests to pass. (#11239)

Co-authored-by: nicole mazzuca <mazzucan@outlook.com>

15 files changed, 87 insertions, 71 deletions

diff --git a/toolsrc/src/vcpkg-test/plan.cpp b/toolsrc/src/vcpkg-test/plan.cpp
index 75e8acd58..a39c2b4a4 100644
--- a/toolsrc/src/vcpkg-test/plan.cpp
+++ b/toolsrc/src/vcpkg-test/plan.cpp
@@ -369,9 +369,8 @@ TEST_CASE ("basic feature test 7", "[plan]")
     remove_plan_check(plan.remove_actions.at(0), "x");
     remove_plan_check(plan.remove_actions.at(1), "b");
 
-    // TODO: order here may change but A < X, and B anywhere
-    features_check(plan.install_actions.at(0), "b", {"core", "b1"});
-    features_check(plan.install_actions.at(1), "a", {"core"});
+    features_check(plan.install_actions.at(0), "a", {"core"});
+    features_check(plan.install_actions.at(1), "b", {"core", "b1"});
     features_check(plan.install_actions.at(2), "x", {"core"});
 }
 
diff --git a/toolsrc/src/vcpkg-test/system.cpp b/toolsrc/src/vcpkg-test/system.cpp
index 70424f0b5..6b9dfaf95 100644
--- a/toolsrc/src/vcpkg-test/system.cpp
+++ b/toolsrc/src/vcpkg-test/system.cpp
@@ -1,3 +1,4 @@
+#define _POSIX_C_SOURCE 200112L
 #include <catch2/catch.hpp>
 
 #include <string>
@@ -27,7 +28,7 @@ using vcpkg::System::CPUArchitecture;
 
 namespace
 {
-    void set_environment_variable(StringView varname, Optional<std::string> value)
+    void set_environment_variable(ZStringView varname, Optional<std::string> value)
     {
 #if defined(_WIN32)
         const auto w_varname = vcpkg::Strings::to_utf16(varname);
@@ -45,19 +46,14 @@ namespace
 
         check_exit(VCPKG_LINE_INFO, exit_code != 0);
 #else  // ^^^ defined(_WIN32) / !defined(_WIN32) vvv
-        std::string tmp;
-        tmp.append(varname.data(), varname.size());
-        tmp.push_back('=');
         if (auto v = value.get())
         {
-            tmp.append(*v);
+            check_exit(VCPKG_LINE_INFO, setenv(varname.c_str(), v->c_str(), 1) == 0);
+        }
+        else
+        {
+            check_exit(VCPKG_LINE_INFO, unsetenv(varname.c_str()) == 0);
         }
-
-        // putenv expects the string to never go out of scope
-        char* env_string = new char[tmp.size() + 1]; // overflow checked by tmp's null allocation
-        memcpy(env_string, tmp.data(), tmp.size());
-        const int exit_code = putenv(env_string);
-        check_exit(VCPKG_LINE_INFO, exit_code == 0);
 #endif // defined(_WIN32)
     }
 
diff --git a/toolsrc/src/vcpkg-test/util.cpp b/toolsrc/src/vcpkg-test/util.cpp
index c615f6bb2..8ead355f1 100644
--- a/toolsrc/src/vcpkg-test/util.cpp
+++ b/toolsrc/src/vcpkg-test/util.cpp
@@ -126,7 +126,7 @@ namespace vcpkg::Test
         if (status == ERROR_SUCCESS && data == 1) {
             return AllowSymlinks::Yes;
         } else {
-            std::clog << "Symlinks are not allowed on this system\n";
+            std::cout << "Symlinks are not allowed on this system\n";
             return AllowSymlinks::No;
         }
 #endif
diff --git a/toolsrc/src/vcpkg.cpp b/toolsrc/src/vcpkg.cpp
index 39f588915..a249a3d9d 100644
--- a/toolsrc/src/vcpkg.cpp
+++ b/toolsrc/src/vcpkg.cpp
@@ -1,14 +1,7 @@
-#if defined(_MSC_VER) && _MSC_VER < 1911
-// [[nodiscard]] is not recognized before VS 2017 version 15.3
-#pragma warning(disable : 5030)
-#endif
-
-#if defined(__GNUC__) && __GNUC__ < 7
-// [[nodiscard]] is not recognized before GCC version 7
-#pragma GCC diagnostic ignored "-Wattributes"
-#endif
+#include <vcpkg/base/pragmas.h>
 
 #if defined(_WIN32)
+#define NOMINMAX
 #define WIN32_LEAN_AND_MEAN
 #include <Windows.h>
 
diff --git a/toolsrc/src/vcpkg/base/downloads.cpp b/toolsrc/src/vcpkg/base/downloads.cpp
index 995e2d4f3..c46fe4288 100644
--- a/toolsrc/src/vcpkg/base/downloads.cpp
+++ b/toolsrc/src/vcpkg/base/downloads.cpp
@@ -33,6 +33,7 @@ namespace vcpkg::Downloads
                            url_path,
                            target_file_path,
                            std::to_string(err));
+        _Analysis_assume_(f != nullptr);
 
         auto hSession = WinHttpOpen(L"vcpkg/1.0",
                                     IsWindows8Point1OrGreater() ? WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY
diff --git a/toolsrc/src/vcpkg/base/files.cpp b/toolsrc/src/vcpkg/base/files.cpp
index 8e554e4bc..2d12b1757 100644
--- a/toolsrc/src/vcpkg/base/files.cpp
+++ b/toolsrc/src/vcpkg/base/files.cpp
@@ -73,7 +73,7 @@ namespace vcpkg::Files
                 constexpr auto all_write = perms::group_write | perms::owner_write | perms::others_write;
                 permissions = perms::all & ~all_write;
             }
-            else if (ft != file_type::none && ft != file_type::none)
+            else if (ft != file_type::none)
             {
                 permissions = perms::all;
             }
diff --git a/toolsrc/src/vcpkg/base/strings.cpp b/toolsrc/src/vcpkg/base/strings.cpp
index 46e78a363..d9dc4c088 100644
--- a/toolsrc/src/vcpkg/base/strings.cpp
+++ b/toolsrc/src/vcpkg/base/strings.cpp
@@ -7,7 +7,7 @@
 namespace vcpkg::Strings::details
 {
     // To disambiguate between two overloads
-    static bool is_space(const char c) { return std::isspace(c) != 0; }
+    static bool is_space(const char c) { return std::isspace(static_cast<unsigned char>(c)) != 0; }
 
     // Avoids C4244 warnings because of char<->int conversion that occur when using std::tolower()
     static char tolower_char(const char c) { return (c < 'A' || c > 'Z') ? c : c - 'A' + 'a'; }
diff --git a/toolsrc/src/vcpkg/base/system.process.cpp b/toolsrc/src/vcpkg/base/system.process.cpp
index 4c7f712e5..74c4390cc 100644
--- a/toolsrc/src/vcpkg/base/system.process.cpp
+++ b/toolsrc/src/vcpkg/base/system.process.cpp
@@ -30,7 +30,7 @@ namespace vcpkg
     {
         struct CtrlCStateMachine
         {
-            CtrlCStateMachine() : m_number_of_external_processes(0), m_global_job(NULL), m_in_interactive(0) {}
+            CtrlCStateMachine() : m_number_of_external_processes(0), m_global_job(NULL), m_in_interactive(0) { }
 
             void transition_to_spawn_process() noexcept
             {
@@ -327,29 +327,47 @@ namespace vcpkg
 #if defined(_WIN32)
     struct ProcessInfo
     {
-        constexpr ProcessInfo() : proc_info{} {}
+        constexpr ProcessInfo() noexcept : proc_info{} { }
+        ProcessInfo(ProcessInfo&& other) noexcept : proc_info(other.proc_info)
+        {
+            other.proc_info.hProcess = nullptr;
+            other.proc_info.hThread = nullptr;
+        }
+        ~ProcessInfo()
+        {
+            if (proc_info.hThread)
+            {
+                CloseHandle(proc_info.hThread);
+            }
+            if (proc_info.hProcess)
+            {
+                CloseHandle(proc_info.hProcess);
+            }
+        }
+
+        ProcessInfo& operator=(ProcessInfo&& other) noexcept
+        {
+            ProcessInfo{std::move(other)}.swap(*this);
+            return *this;
+        }
 
-        unsigned int wait_and_close_handles()
+        void swap(ProcessInfo& other) noexcept
         {
-            CloseHandle(proc_info.hThread);
+            std::swap(proc_info.hProcess, other.proc_info.hProcess);
+            std::swap(proc_info.hThread, other.proc_info.hThread);
+        }
+
+        friend void swap(ProcessInfo& lhs, ProcessInfo& rhs) noexcept { lhs.swap(rhs); }
 
+        unsigned int wait()
+        {
             const DWORD result = WaitForSingleObject(proc_info.hProcess, INFINITE);
             Checks::check_exit(VCPKG_LINE_INFO, result != WAIT_FAILED, "WaitForSingleObject failed");
-
             DWORD exit_code = 0;
             GetExitCodeProcess(proc_info.hProcess, &exit_code);
-
-            CloseHandle(proc_info.hProcess);
-
             return exit_code;
         }
 
-        void close_handles()
-        {
-            CloseHandle(proc_info.hThread);
-            CloseHandle(proc_info.hProcess);
-        }
-
         PROCESS_INFORMATION proc_info;
     };
 
@@ -365,16 +383,23 @@ namespace vcpkg
 
         // Flush stdout before launching external process
         fflush(nullptr);
-        bool succeeded = TRUE == CreateProcessW(nullptr,
-                                                Strings::to_utf16(cmd_line).data(),
-                                                nullptr,
-                                                nullptr,
-                                                TRUE,
-                                                IDLE_PRIORITY_CLASS | CREATE_UNICODE_ENVIRONMENT | dwCreationFlags,
-                                                (void*)(env.m_env_data.empty() ? nullptr : env.m_env_data.data()),
-                                                nullptr,
-                                                &startup_info,
-                                                &process_info.proc_info);
+
+VCPKG_MSVC_WARNING(suppress : 6335) // Leaking process information handle 'process_info.proc_info.hProcess'
+                            // /analyze can't tell that we transferred ownership here
+        bool succeeded =
+            TRUE == CreateProcessW(nullptr,
+                                   Strings::to_utf16(cmd_line).data(),
+                                   nullptr,
+                                   nullptr,
+                                   TRUE,
+                                   IDLE_PRIORITY_CLASS | CREATE_UNICODE_ENVIRONMENT | dwCreationFlags,
+                                   env.m_env_data.empty()
+                                       ? nullptr
+                                       : const_cast<void*>(static_cast<const void*>(env.m_env_data.data())),
+                                   nullptr,
+                                   &startup_info,
+                                   &process_info.proc_info);
+
         if (succeeded)
             return process_info;
         else
@@ -413,7 +438,7 @@ namespace vcpkg
 
             CloseHandle(child_stdout);
 
-            return proc_info.wait_and_close_handles();
+            return proc_info.wait();
         }
     };
 
@@ -467,11 +492,7 @@ namespace vcpkg
         auto timer = Chrono::ElapsedTimer::create_started();
 
         auto process_info = windows_create_process(cmd_line, {}, DETACHED_PROCESS | CREATE_BREAKAWAY_FROM_JOB);
-        if (auto p = process_info.get())
-        {
-            p->close_handles();
-        }
-        else
+        if (!process_info.get())
         {
             Debug::print("cmd_execute_no_wait() failed with error code ", process_info.error(), "\n");
         }
@@ -523,7 +544,7 @@ namespace vcpkg
         auto proc_info = windows_create_process(cmd_line, env, NULL);
         auto long_exit_code = [&]() -> unsigned long {
             if (auto p = proc_info.get())
-                return p->wait_and_close_handles();
+                return p->wait();
             else
                 return proc_info.error();
         }();
@@ -646,6 +667,6 @@ namespace vcpkg
         SetConsoleCtrlHandler(reinterpret_cast<PHANDLER_ROUTINE>(ctrl_handler), TRUE);
     }
 #else
-    void System::register_console_ctrl_handler() {}
+    void System::register_console_ctrl_handler() { }
 #endif
 }
diff --git a/toolsrc/src/vcpkg/binarycaching.cpp b/toolsrc/src/vcpkg/binarycaching.cpp
index 87738c039..2ad061e2c 100644
--- a/toolsrc/src/vcpkg/binarycaching.cpp
+++ b/toolsrc/src/vcpkg/binarycaching.cpp
@@ -288,12 +288,12 @@ ExpectedS<std::unique_ptr<IBinaryProvider>> vcpkg::create_binary_provider_from_c
                         auto n = match_until([](char32_t ch) { return ch == ',' || ch == '`' || ch == ';'; });

                         Strings::append(segment, n);

                         auto ch = cur();

-                        if (ch == '\0' || ch == ',' || ch == ';')

+                        if (ch == Unicode::end_of_file || ch == ',' || ch == ';')

                             break;

                         else if (ch == '`')

                         {

                             ch = next();

-                            if (ch == '\0')

+                            if (ch == Unicode::end_of_file)

                                 add_error("unexpected eof: trailing unescaped backticks (`) are not allowed");

                             else

                                 Unicode::utf8_append_code_point(segment, ch);

@@ -305,7 +305,7 @@ ExpectedS<std::unique_ptr<IBinaryProvider>> vcpkg::create_binary_provider_from_c
                     segments.emplace_back(std::move(loc), std::move(segment));

 

                     auto ch = cur();

-                    if (ch == '\0' || ch == ';')

+                    if (ch == Unicode::end_of_file || ch == ';')

                         break;

                     else if (ch == ',')

                     {

diff --git a/toolsrc/src/vcpkg/binaryparagraph.cpp b/toolsrc/src/vcpkg/binaryparagraph.cpp
index ad18b71c6..305ae2ae9 100644
--- a/toolsrc/src/vcpkg/binaryparagraph.cpp
+++ b/toolsrc/src/vcpkg/binaryparagraph.cpp
@@ -89,9 +89,11 @@ namespace vcpkg
         , version(spgh.version)
         , description(spgh.description)
         , maintainer(spgh.maintainer)
+        , feature()
+        , default_features(spgh.default_features)
+        , depends()
         , abi(abi_tag)
         , type(spgh.type)
-        , default_features(spgh.default_features)
     {
         this->depends = Util::fmap(deps, [](const FeatureSpec& spec) { return spec.spec().name(); });
         Util::sort_unique_erase(this->depends);
@@ -106,8 +108,10 @@ namespace vcpkg
         , description(fpgh.description)
         , maintainer()
         , feature(fpgh.name)
-        , type(spgh.type)
         , default_features()
+        , depends()
+        , abi()
+        , type(spgh.type)
     {
         this->depends = Util::fmap(deps, [](const FeatureSpec& spec) { return spec.spec().name(); });
         Util::sort_unique_erase(this->depends);
diff --git a/toolsrc/src/vcpkg/build.cpp b/toolsrc/src/vcpkg/build.cpp
index 5d40e8c24..a1a520970 100644
--- a/toolsrc/src/vcpkg/build.cpp
+++ b/toolsrc/src/vcpkg/build.cpp
@@ -90,6 +90,7 @@ namespace vcpkg::Build::Command
         }

 

         Checks::check_exit(VCPKG_LINE_INFO, action != nullptr);

+        _Analysis_assume_(action != nullptr);

 

         action->build_options = build_package_options;

 

@@ -149,6 +150,7 @@ namespace vcpkg::Build::Command
         const auto* scfl = provider.get_control_file(port_name).get();

 

         Checks::check_exit(VCPKG_LINE_INFO, scfl != nullptr, "Error: Couldn't find port '%s'", port_name);

+        _Analysis_assume_(scfl != nullptr);

 

         perform_and_exit_ex(spec, *scfl, provider, *binaryprovider, paths);

     }

diff --git a/toolsrc/src/vcpkg/commands.integrate.cpp b/toolsrc/src/vcpkg/commands.integrate.cpp
index b427cb247..e2ae967aa 100644
--- a/toolsrc/src/vcpkg/commands.integrate.cpp
+++ b/toolsrc/src/vcpkg/commands.integrate.cpp
@@ -79,7 +79,7 @@ namespace vcpkg::Commands::Integrate
         dir_id.erase(1, 1); // Erasing the ":"
 
         // NuGet id cannot have invalid characters. We will only use alphanumeric and dot.
-        Util::erase_remove_if(dir_id, [](char c) { return !isalnum(c) && (c != '.'); });
+        Util::erase_remove_if(dir_id, [](char c) { return !isalnum(static_cast<unsigned char>(c)) && (c != '.'); });
 
         const std::string nuget_id = "vcpkg." + dir_id;
         return nuget_id;
diff --git a/toolsrc/src/vcpkg/dependencies.cpp b/toolsrc/src/vcpkg/dependencies.cpp
index 316429515..c57f44c0e 100644
--- a/toolsrc/src/vcpkg/dependencies.cpp
+++ b/toolsrc/src/vcpkg/dependencies.cpp
@@ -39,7 +39,7 @@ namespace vcpkg::Dependencies
 
         struct ClusterInstallInfo
         {
-            std::unordered_map<std::string, std::vector<FeatureSpec>> build_edges;
+            std::map<std::string, std::vector<FeatureSpec>> build_edges;
             bool defaults_requested = false;
         };
 
@@ -294,7 +294,7 @@ namespace vcpkg::Dependencies
         auto end() const { return m_graph.end(); }
 
     private:
-        std::unordered_map<PackageSpec, Cluster> m_graph;
+        std::map<PackageSpec, Cluster> m_graph;
         const PortFileProvider::PortFileProvider& m_port_provider;
     };
 
@@ -352,7 +352,7 @@ namespace vcpkg::Dependencies
     InstallPlanAction::InstallPlanAction(const PackageSpec& spec,
                                          const SourceControlFileLocation& scfl,
                                          const RequestType& request_type,
-                                         std::unordered_map<std::string, std::vector<FeatureSpec>>&& dependencies)
+                                         std::map<std::string, std::vector<FeatureSpec>>&& dependencies)
         : spec(spec)
         , source_control_file_location(scfl)
         , plan_type(InstallPlanType::BUILD_AND_INSTALL)
@@ -857,7 +857,7 @@ namespace vcpkg::Dependencies
             {
                 auto&& scfl = p_cluster->m_scfl;
 
-                std::unordered_map<std::string, std::vector<FeatureSpec>> computed_edges;
+                std::map<std::string, std::vector<FeatureSpec>> computed_edges;
                 for (auto&& kv : info_ptr->build_edges)
                 {
                     std::set<FeatureSpec> fspecs;
diff --git a/toolsrc/src/vcpkg/metrics.cpp b/toolsrc/src/vcpkg/metrics.cpp
index 253ea121d..d85d58cd3 100644
--- a/toolsrc/src/vcpkg/metrics.cpp
+++ b/toolsrc/src/vcpkg/metrics.cpp
@@ -248,7 +248,7 @@ namespace vcpkg::Metrics
         ;
     static bool g_should_print_metrics = false;
 
-    bool get_compiled_metrics_enabled() { return VCPKG_DISABLE_METRICS == 0; }
+    bool get_compiled_metrics_enabled() { return !VCPKG_DISABLE_METRICS; }
 
     std::string get_MAC_user()
     {
diff --git a/toolsrc/src/vcpkg/statusparagraph.cpp b/toolsrc/src/vcpkg/statusparagraph.cpp
index ef8715ec2..2f29fb6c6 100644
--- a/toolsrc/src/vcpkg/statusparagraph.cpp
+++ b/toolsrc/src/vcpkg/statusparagraph.cpp
@@ -86,11 +86,11 @@ namespace vcpkg
         }
     }
 
-    std::unordered_map<std::string, std::vector<FeatureSpec>> InstalledPackageView::feature_dependencies() const
+    std::map<std::string, std::vector<FeatureSpec>> InstalledPackageView::feature_dependencies() const
     {
         auto extract_deps = [&](const std::string& name) { return FeatureSpec{{name, spec().triplet()}, "core"}; };
 
-        std::unordered_map<std::string, std::vector<FeatureSpec>> deps;
+        std::map<std::string, std::vector<FeatureSpec>> deps;
 
         deps.emplace("core", Util::fmap(core->package.depends, extract_deps));