Enable SDL related windows compiler flags (#2964)

---------

Co-authored-by: Trawinski, Dariusz <[email protected]>

Author: michalkulakowski

.bazelrc

@@ -176,9 +176,13 @@ build:monolithic --define framework_shared_object=false
 build:monolithic --define tsl_protobuf_header_only=false
 # On windows, we still link everything into a single DLL - required by TF
 build:windows --config=monolithic
-
-build:windows --copt=/W0
+# Warnings are enabled in common_settings
+build:windows --copt=/W0 
 build:windows --host_copt=/W0
+build:windows --copt=/external:anglebrackets
+build:windows --host_copt=/external:anglebrackets
+build:windows --copt=/external:W0
+build:windows --host_copt=/external:W0
 
 # On Windows, `__cplusplus` is wrongly defined without this switch
 # See https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/

common_settings.bzl

@@ -122,7 +122,37 @@ COMMON_STATIC_LIBS_COPTS = select({
                     "-Werror",
                 ],
                 "//src:windows" : [
-                    "-Wall",
+                        "/W4",
+                        "/WX",
+                        "/external:anglebrackets",
+                        "/external:W0",
+                        "/sdl",
+                        "/analyze",
+                        "/Gy",
+                        "/DYNAMICBASE",
+                        "/Qspectre",
+                        "/wd4018", # level 3
+                        "/wd4068", # level 1
+                        "/wd4458", # level 4
+                        "/wd4100", # level 4
+                        "/wd4267", # level 1
+                        "/wd4389", # level 4
+                        "/wd4127", # level 4
+                        "/wd4456", # level 4
+                        "/wd4673", # level 4
+                        "/wd4670", # level 4
+                        "/wd4244", # level 3
+                        "/wd4457", # level 4
+                        "/wd4505", # level 4
+                        "/wd6246", # level 3
+                        "/wd4702", # level 4
+                        "/wd4101", # level 3/4
+                        "/wd6387", # level 4
+                        "/wd6308", # level 4
+                        "/wd6319", # level 3/4
+                        "/wd4297", # level 3/4
+                        "/wd4701",
+                        "/wd4804",
                     ],
                 })
 
@@ -136,8 +166,8 @@ COMMON_STATIC_TEST_COPTS = select({
                     "-fvisibility=hidden",# Needed for pybind targets
                 ],
                 "//src:windows" : [
-                    "-W0",
-                    "-Isrc",
+                        "-W0",
+                        "-Isrc",
                     ],
                 })
 
@@ -150,7 +180,8 @@ COMMON_STATIC_LIBS_COPTS_VISIBLE = select({
                     "-Werror",
                 ],
                 "//src:windows" : [
-                    "-Wall",
+                        "-W0",
+                        "-Isrc",
                     ],
                 }) 
 
@@ -164,8 +195,10 @@ COMMON_STATIC_LIBS_LINKOPTS = select({
                     # "-lovms_shared",  # Use for dynamic linking when necessary
                 ],
                 "//src:windows" : [
-                    "",
-                    ],
+                    "/NXCOMPAT",
+                    "/LTCG",
+                    "/QConditional-branch:Retpoline",
+                ],
                 })
 COPTS_PYTHON = select({
     "//conditions:default": ["-DPYTHON_DISABLE=1"],

src/capi_frontend/capi.cpp

@@ -18,12 +18,14 @@
 #include <iterator>
 #include <memory>
 #include <string>
-
+#pragma warning(push)
+#pragma warning(disable : 6313)
 #include <rapidjson/document.h>
 #include <rapidjson/pointer.h>
 #include <rapidjson/rapidjson.h>
 #include <rapidjson/stringbuffer.h>
 #include <rapidjson/writer.h>
+#pragma warning(pop)
 
 #include "../dags/pipeline.hpp"
 #include "../dags/pipelinedefinition.hpp"
@@ -75,7 +77,8 @@ using std::chrono::microseconds;
 #ifdef __cplusplus
 extern "C" {
 #endif
-
+#pragma warning(push)
+#pragma warning(disable : 4005)
 #ifdef __linux__
 #define DLL_PUBLIC __attribute__((visibility("default")))
 #define DLL_LOCAL __attribute__((visibility("hidden")))
@@ -84,6 +87,7 @@ extern "C" {
 // #define DLL_PUBLIC __declspec(dllexport)
 #define DLL_PUBLIC
 #endif
+#pragma warning(pop)
 
 DLL_PUBLIC OVMS_Status* OVMS_ApiVersion(uint32_t* major, uint32_t* minor) {
     if (major == nullptr)
@@ -208,7 +212,11 @@ DLL_PUBLIC OVMS_Status* OVMS_MetadataFieldByPointer(OVMS_Metadata* metadata, con
     if (!val) {
         return reinterpret_cast<OVMS_Status*>(new Status(StatusCode::JSON_SERIALIZATION_ERROR, "value not found"));
     }
+#ifdef __linux__
     *value = strdup(val->GetString());
+#elif _WIN32
+    *value = _strdup(val->GetString());
+#endif
     *size = val->GetStringLength();
     return nullptr;
 }
@@ -229,7 +237,11 @@ DLL_PUBLIC OVMS_Status* OVMS_SerializeMetadataToString(OVMS_Metadata* metadata,
 
     rapidjson::Writer<rapidjson::StringBuffer> writer(strbuf);
     doc->Accept(writer);
+#ifdef __linux__
     *json = strdup(strbuf.GetString());
+#elif _WIN32
+    *json = _strdup(strbuf.GetString());
+#endif
     *size = strbuf.GetSize();
 
     return nullptr;
@@ -878,7 +890,8 @@ enum : uint32_t {
     TIMER_CALLBACK,
     TIMER_END
 };
-
+#pragma warning(push)
+#pragma warning(disable : 4190)  // TODO verify
 static Status getModelManager(Server& server, ModelManager** modelManager) {
     if (!server.isLive()) {
         return ovms::Status(ovms::StatusCode::SERVER_NOT_READY, "not live");
@@ -926,7 +939,7 @@ static Status getPipelineDefinition(Server& server, const std::string& servableN
     }
     return (*pipelineDefinition)->waitForLoaded(unloadGuard, 0);
 }
-
+#pragma warning(pop)
 }  // namespace
 
 DLL_PUBLIC OVMS_Status* OVMS_Inference(OVMS_Server* serverPtr, OVMS_InferenceRequest* request, OVMS_InferenceResponse** response) {

src/cli_parser.cpp

@@ -194,13 +194,15 @@ void CLIParser::parse(int argc, char** argv) {
         exit(3);
 #endif
         }
-
+#pragma warning(push)
+#pragma warning(disable : 4129)
         if (result->count("version")) {
             std::string project_name(PROJECT_NAME);
             std::string project_version(PROJECT_VERSION);
             std::cout << project_name + " " + project_version << std::endl;
             std::cout << "OpenVINO backend " << OPENVINO_NAME << std::endl;
             std::cout << "Bazel build flags: " << BAZEL_BUILD_FLAGS << std::endl;
+#pragma warning(pop)
 #ifdef __linux__
             exit(EX_OK);
 #elif _WIN32

src/customloaderconfig.hpp

@@ -16,11 +16,13 @@
 #pragma once
 
 #include <string>
-
+#pragma warning(push)
+#pragma warning(disable : 6313)
 #include <rapidjson/document.h>
 #include <rapidjson/istreamwrapper.h>
 #include <rapidjson/stringbuffer.h>
 #include <rapidjson/writer.h>
+#pragma warning(pop)
 #include <spdlog/spdlog.h>
 
 #include "filesystem.hpp"

src/dags/custom_node.hpp

@@ -28,9 +28,9 @@
 
 namespace ovms {
 
-class NodeLibrary;
+struct NodeLibrary;
 class Status;
-class CNLIMWrapper;
+struct CNLIMWrapper;
 
 class CustomNode : public Node {
     NodeLibrary library;

src/dags/customnodesession.hpp

@@ -36,13 +36,13 @@
 #include "pipelineeventqueue.hpp"
 #include "tensormap.hpp"
 
-class CustomNodeTensor;
-class CustomNodeParam;
+struct CustomNodeTensor;
+struct CustomNodeParam;
 
 namespace ovms {
 
 class Node;
-class NodeLibrary;
+struct NodeLibrary;
 class Status;
 
 class CustomNodeSession : public NodeSession {

src/dags/entry_node.cpp

@@ -35,11 +35,14 @@
 #include "../tfs_frontend/tfs_utils.hpp"
 #include "nodesession.hpp"
 
+#pragma warning(push)
+#pragma warning(disable : 4624 6001 6385 6386 6326 6011)
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wall"
 #include "tensorflow/core/framework/tensor.h"
 #include "tensorflow_serving/apis/prediction_service.grpc.pb.h"
 #pragma GCC diagnostic pop
+#pragma warning(pop)
 
 namespace ovms {
 

src/dags/entry_node.hpp

@@ -62,8 +62,6 @@ class EntryNode : public Node {
         throw std::logic_error("This node cannot have dependency");
     }
 
-    Status isInputBinary(const std::string& name, bool& isBinary) const;
-
     const Status validate();
 };
 

src/dags/exit_node.cpp

@@ -22,12 +22,14 @@
 #include "../logging.hpp"
 #include "../ov_utils.hpp"
 #include "../serialization.hpp"
-
+#pragma warning(push)
+#pragma warning(disable : 4624 6001 6385 6386 6326 6011)
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wall"
 #pragma GCC diagnostic ignored "-Wunused-but-set-variable"
 #include "tensorflow/core/framework/tensor.h"
 #pragma GCC diagnostic pop
+#pragma warning(pop)
 
 #include "exitnodesession.hpp"
 

src/dags/exitnodesession.hpp

@@ -24,7 +24,7 @@
 #include "tensormap.hpp"
 
 namespace ovms {
-class CollapseDetails;
+struct CollapseDetails;
 class NodeSessionMetadata;
 
 template <typename ResponseType>

src/dags/gathernodeinputhandler.hpp

@@ -33,7 +33,7 @@ namespace ovms {
 
 using shard_map_t = std::unordered_map<session_id_t, ov::Tensor>;
 
-class CollapseDetails;
+struct CollapseDetails;
 
 class GatherNodeInputHandler : public NodeInputHandler {
     std::unordered_map<std::string, shard_map_t> shardsStorage;

src/dags/node.cpp

@@ -112,7 +112,7 @@ Status Node::setInputs(const Node& dependency, TensorWithSourceMap& inputs, Node
     try {
         static const std::set<std::string> emptySet;
         shardId = metadata.getShardId(gatherFrom.value_or(emptySet));
-    } catch (const std::exception& e) {
+    } catch (const std::exception&) {
         SPDLOG_LOGGER_ERROR(dag_executor_logger, "Failed to get shardId for node: {}", getName());
         return StatusCode::INTERNAL_ERROR;
     }
@@ -158,7 +158,7 @@ NodeSession* Node::getNodeSession(const NodeSessionMetadata& metadata) {
     if (gatherFrom) {
         try {
             sessionKey = metadata.getSessionKey(gatherFrom.value());
-        } catch (const std::exception& e) {
+        } catch (const std::exception&) {
             SPDLOG_LOGGER_ERROR(dag_executor_logger, "Failed to create collapsed metadata session key for node: {}, incoming session key: {}",
                 getName(), metadata.getSessionKey());
             return nullptr;
@@ -177,7 +177,7 @@ NodeSession* Node::getNodeSession(const NodeSessionMetadata& metadata) {
     if (gatherFrom) {
         try {
             std::tie(newSessionMetadata, collapsingDetails) = metadata.getCollapsedSessionMetadata(gatherFrom.value());
-        } catch (const std::exception& e) {
+        } catch (const std::exception&) {
             SPDLOG_LOGGER_ERROR(dag_executor_logger, "Failed to create collapsed metadata for node: {}", getName());
             return nullptr;
         }
@@ -210,7 +210,7 @@ Status Node::demultiplyOutputs(SessionResults& nodeSessionOutputs) {
     }
     auto& [metadata, tensorMap] = nodeSessionOutputs.begin()->second;
     auto firstTensorShape = tensorMap.begin()->second.getActualTensor().get_shape();
-    uint32_t resultsDemultiplyCount = firstTensorShape[0];
+    size_t resultsDemultiplyCount = firstTensorShape[0];
     if (firstTensorShape[0] > DEMULTIPLY_LIMIT) {
         SPDLOG_LOGGER_ERROR(dag_executor_logger, "Node: {} - too large dim[0] size: {} of tensor: {}. Maximum allowed is: {}",
             getName(), firstTensorShape[0], tensorMap.begin()->first, DEMULTIPLY_LIMIT);

src/dags/nodeinfo.hpp

@@ -64,7 +64,7 @@ struct NodeInfo {
     std::string modelName;
     std::optional<model_version_t> modelVersion;
     std::unordered_map<std::string, std::string> outputNameAliases;
-    std::optional<int32_t> demultiplyCount;
+    std::optional<size_t> demultiplyCount;
     std::set<std::string> gatherFromNode;
     NodeLibrary library;
     parameters_t parameters;

src/dags/nodesession.hpp

@@ -22,7 +22,7 @@
 #include "nodesessionmetadata.hpp"
 
 namespace ovms {
-struct NodeInputHandler;
+class NodeInputHandler;
 struct NodeOutputHandler;
 class Status;
 class TensorWithSource;

src/dags/nodestreamidguard.hpp

@@ -24,7 +24,8 @@ class OVInferRequestsQueue;
 class ModelMetricReporter;
 class OVInferRequestsQueue;
 
-struct NodeStreamIdGuard {
+class NodeStreamIdGuard {
+public:
     NodeStreamIdGuard(OVInferRequestsQueue& inferRequestsQueue, ModelMetricReporter& reporter);
     ~NodeStreamIdGuard();
 

src/dags/pipeline.cpp

@@ -164,13 +164,13 @@ Status Pipeline::execute(ExecutionContext context) {
             DeferredNodeSessions tmpDeferredNodeSessions;
             for (auto& nextNode : nextNodesFromFinished) {
                 auto readySessions = nextNode.get().getReadySessions();
-                for (auto& sessionKey : readySessions) {
-                    SPDLOG_LOGGER_DEBUG(dag_executor_logger, "Started execution of pipeline: {} node: {} session: {}", getName(), nextNode.get().getName(), sessionKey);
-                    startedSessions.emplace(nextNode.get().getName() + sessionKey);
-                    status = nextNode.get().execute(sessionKey, finishedNodeQueue);
+                for (auto& readySessionKey : readySessions) {
+                    SPDLOG_LOGGER_DEBUG(dag_executor_logger, "Started execution of pipeline: {} node: {} session: {}", getName(), nextNode.get().getName(), readySessionKey);
+                    startedSessions.emplace(nextNode.get().getName() + readySessionKey);
+                    status = nextNode.get().execute(readySessionKey, finishedNodeQueue);
                     if (status == StatusCode::PIPELINE_STREAM_ID_NOT_READY_YET) {
-                        SPDLOG_LOGGER_DEBUG(dag_executor_logger, "Node: {} session: {} not ready for execution yet", nextNode.get().getName(), sessionKey);
-                        tmpDeferredNodeSessions.emplace_back(nextNode.get(), sessionKey);
+                        SPDLOG_LOGGER_DEBUG(dag_executor_logger, "Node: {} session: {} not ready for execution yet", nextNode.get().getName(), readySessionKey);
+                        tmpDeferredNodeSessions.emplace_back(nextNode.get(), readySessionKey);
                         status = StatusCode::OK;
                     }
                     CHECK_AND_LOG_ERROR(nextNode.get())
@@ -192,18 +192,18 @@ Status Pipeline::execute(ExecutionContext context) {
                 if (finishedNodeQueue.size() > 0) {
                     break;
                 }
-                auto& [nodeRef, sessionKey] = *it;
+                auto& [nodeRef, deferredSessionKey] = *it;
                 auto& node = nodeRef.get();
-                SPDLOG_LOGGER_DEBUG(dag_executor_logger, "Trying to trigger node: {} session: {} execution", node.getName(), sessionKey);
-                status = node.execute(sessionKey, finishedNodeQueue);
+                SPDLOG_LOGGER_DEBUG(dag_executor_logger, "Trying to trigger node: {} session: {} execution", node.getName(), deferredSessionKey);
+                status = node.execute(deferredSessionKey, finishedNodeQueue);
                 if (status.ok()) {
-                    SPDLOG_LOGGER_DEBUG(dag_executor_logger, "Node: {} session: {} is ready", node.getName(), sessionKey);
+                    SPDLOG_LOGGER_DEBUG(dag_executor_logger, "Node: {} session: {} is ready", node.getName(), deferredSessionKey);
                     it = deferredNodeSessions.erase(it);
                     continue;
                 }
                 it++;
                 if (status == StatusCode::PIPELINE_STREAM_ID_NOT_READY_YET) {
-                    SPDLOG_LOGGER_DEBUG(dag_executor_logger, "Node: {} session: {} not ready for execution yet", node.getName(), sessionKey);
+                    SPDLOG_LOGGER_DEBUG(dag_executor_logger, "Node: {} session: {} not ready for execution yet", node.getName(), deferredSessionKey);
                     status = StatusCode::OK;
                 } else {
                     CHECK_AND_LOG_ERROR(node)