Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Compilation for Inventory Module on Windows Using MSVC #167

Closed
5 tasks
cborla opened this issue Sep 21, 2024 · 5 comments · Fixed by #208
Closed
5 tasks

Support Compilation for Inventory Module on Windows Using MSVC #167

cborla opened this issue Sep 21, 2024 · 5 comments · Fixed by #208
Assignees
Labels

Comments

@cborla
Copy link
Member

cborla commented Sep 21, 2024

Related Issue
#17

Description

Currently, several functions are mocked to enable the Wazuh agent to compile with the inventory module on Windows using MSVC. The code needs to be updated to replace the mocked functions with working implementations and ensure compatibility with MSVC.

The objective is to complete the simulated parts and allow the agent inventory module to successfully compile using MSVC on Windows, and have the same functionality as is available on the other operating systems.

Current Compilation Status:

At this point, the code compiles but remains non-functional, as many functions are mocked or replaced with placeholders. The goal is to finalize the compilation process while addressing MSVC-specific issues and resolving the mocked functionalities.

Tasks:

  1. Replace Mocked Functions: Implement or find equivalents for the following mocked functions and types:

    • Time-related functions (localtime_r)
    • POSIX file system functions (opendir, readdir, closedir, etc.)
    • Directory functions (dirname, PathFindFileNameA, PathRemoveFileSpec)
    • Sockets and network functions (ssize_t, NETIOAPI_API, etc.)
    • String handling (strtok_r)
  2. CMake Adjustments: Update the CMakeLists.txt files to handle MSVC-specific configurations, replacing GCC/MinGW flags and pragmas that are incompatible with MSVC.

  3. Normalize Preprocessor Directives: Standardize conditional behavior based on platform or compiler (WIN32, MSVC, GCC, etc.). Awaiting a definition

  4. Handle Circular Inclusion Issues: Resolve the recursive circular inclusion problem in MSVC, which may require restructuring shared headers.

  5. Testing: Re-enable tests for AgentTest and InventoryTest once the implementation is complete.

  6. Document Compiler Flags: Update the documentation to reflect the required MSVC flags and any platform-specific settings.

Steps to Compile:

git submodule update --init --recursive
mkdir build
cd build
cmake .. -DBUILD_TESTS=1 -G "Visual Studio 17 2022" -A x64
cmake --build .
@ncvicchi
Copy link
Member

ncvicchi commented Oct 2, 2024

Compiler level

The identified differences at a compiler level are:

  • Attributes: The __attribute__ GCC option is not compatible with MSVC.
  • Flags: GCC use flags that are prefixed with "-", while MSVC uses "/". And most of these flags are not directly interchangeable
  • pragmas: pragmas unavalaible at MSVC are used

Attributes

GCC attributes are used for several uses in GCC based code, that are either incompatible or have no alternative in MSVC.
Although these attributes are a compiler difference, they are present in the source code.

Some possiblities are:

  1. ad-hoc: This is what was done in in the PR that allowed the compilation step. It is not a recommended, tidy, nor an scalable option
  2. Common include file: All differentes are solved in a file compiler-attributes.h, that will be included in every file that requires.
  3. Compiler-dependant common include file: All differences are acommodated in files like gcc-attribues.h, msvc-attribues.h, etc.
  4. All attributes are completely avoided, implementing that behavior differently: Although this seems like the worse option, it must be taken into account that using attributes in one compiler that are not available in the other might give a false sensation of fiability.
  5. Others?

Flags

In the case of flags, thechanges must be done at the cmakelist.txt files. Currently, there is no convention on how to apply them:

  1. As a compiler-based decision (GCC, GNU, MSVC, etc)
    If this decision is taken into account, it will be a tidier approach, although it might not be easily extendable to other compilers. In this case, how are handled differences between Linux and MacOS?
  2. As a platform-based decision (Windows, Linux, MacOS). In this case, no considerations on the compilers are made.

Currently, a mixture of both exits in all cmakelists.txt.
This decision affects also how the attributes are handle in the source.

pragmas

Although pragmas are compiler options, these are used at a source code level. Their usage should consider if the pragma exists for both compilers, or use the same considerations as for the attributes.

Source code level

Some incompatibilites at source code level were identified:

  • Language unsupported or incompatible features
  • Unavailable libraries

Language unsupported or incompatible features

There are some language features that are supported or even exclusive to GCC that are not supported by MSVC.
Example of these are for example the declaration of vectors with variable indexes (even if the variable has a const or constexpr definition). This probed to be not supported by MSVC.
These features should be avoided, preferring to use compatible features over them. In the above case, malloc or was used to replace them.
It is preferable to avoid the usage of preprocessor for these situations.

Unavailable libraries

Several libraries were provided by MinGW (POSIX for example), that are not available at MSVC. Different approaches can be presented to provide compatibility for the lack of libraries:

  1. Ad-hoc: similar to the one implemented in the PR that allowed the compilation step. Easily and fast to implement, but not scalable. It seems not to be the best fit for a clean restart of a project. It is more suitable for fast&dirty or quick fixes. Mocked functions and defintiones are implemented with wathever is available, emulating the expected behavior.
#if platformA
#include <library.h>
 int function(int param){
    return libraryFunction(param);
 }
 #else
#include <different.h>
int libraryFunction(int param){
    void *p;
    p = differentFunction(&param);
    if(p)
        return 0;
    return -1;
}
int function(int param){
     return libraryFunction(param);
 }
 #endif

 ...
 function(p);
 ...
  1. In-file conditional compiling: Whenever a definition or function is missing, a pre-processor-conditioned code is present to solve the situation for every platform.
 #if platformA
 int functionForPlatformA(int param){
 
 }

 #else
 void *funtionForPlatformB(int *p){

 }
 #endif
  1. A file for every platform: Codes that are too tied to a platform implementation are replicated for every platform, with the same signature. In the cmakelist.txt files, it is decided which file will be compiled.

platformA_component.c

bool component_function(int *a){
    int result;
    result = platformA_function(*a);
    if(result == 0)
        return true;
    return false;
}

platformB_component.c

bool component_function(int *a){
    FILE *handler;
    handler = platformA_function(a);
    if(result == NULL)
        return false;
    return true;
}
  1. Platform abstraction layer (PAL): Platform dependent code is identified, and are implemented using platform independent functions. These functions, methods, classes or definitoins then are implemented on platform dependent libraries, and compiled accordingly. Easier to test, and tidier to implement. Ideally, libraries provided by MSVC should be used, and if that's not case, libraries available in vcpkg should be used.

higher_level_code.c

#include "pal.h"
int result, int param;
result = palFunction(palParameter);

pal_platformA.c

#include <platA.h>
int palFunction(int param){
    FILE *p;
    p = platAFunc(param);
    if(p)
        return 0;
    else
        return -1;
}

pal_platformB.c

#include <platB.h>
int palFunction(int param){
    file_t p;
    p = platBFunc(param, FIXED_FLAGS);

    if(p != 0)
        return 0;
    else
        return -1;
}
  1. Multi-platform libraries (available in vcpkg): This would be the best approach, but it requires that all missing libraries be used in the original code, and thus extensive analysis and testing will be necessary, incrementing the necessary.

Conclusion

The best approach, from my point of view, would be using the 4th strategie, since it is a tidier approach than the first 3, but doesn't increment the necessary time as the fifth strategy.
A gradual migration from the 4th to the 5th strategy could be implemented as time permits.

@wazuhci wazuhci moved this to In progress in Release 5.0.0 Oct 3, 2024
@sdvendramini
Copy link
Member

Hi @ncvicchi , I share with you the changes with which I was able to continue using the agent in windows.

Changes
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 036c90e8f..24ab04d69 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -7,11 +7,6 @@ project(wazuh-agent)
 include(cmake/CommonSettings.cmake)
 set_common_settings()
 
-if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /WX-")
-    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /WX-")
-    add_compile_options(/w)
-endif()
 
 add_subdirectory(common)
 add_subdirectory(modules)
diff --git a/src/agent/include/agent.hpp b/src/agent/include/agent.hpp
index 91efb4fca..399084db0 100644
--- a/src/agent/include/agent.hpp
+++ b/src/agent/include/agent.hpp
@@ -29,5 +29,5 @@ private:
     configuration::ConfigurationParser m_configurationParser;
     communicator::Communicator m_communicator;
     command_handler::CommandHandler m_commandHandler;
-    ModuleManager m_moduleManager;
+    //ModuleManager m_moduleManager;
 };
diff --git a/src/agent/src/agent.cpp b/src/agent/src/agent.cpp
index 9aa6f2c05..eb0700cc5 100644
--- a/src/agent/src/agent.cpp
+++ b/src/agent/src/agent.cpp
@@ -20,7 +20,7 @@ Agent::Agent(std::unique_ptr<ISignalHandler> signalHandler)
                      m_agentInfo.GetKey(),
                      [this](std::string table, std::string key) -> std::string
                      { return m_configurationParser.GetConfig<std::string>(std::move(table), std::move(key)); })
-    , m_moduleManager(m_messageQueue, m_configurationParser)
+    //, m_moduleManager(m_messageQueue, m_configurationParser)
 {
     m_taskManager.Start(std::thread::hardware_concurrency());
 }
@@ -51,13 +51,13 @@ void Agent::Run()
         [this]() { return GetCommandFromQueue(m_messageQueue); },
         [this]() { return PopCommandFromQueue(m_messageQueue); },
         [this](module_command::CommandEntry& cmd)
-        { return DispatchCommand(cmd, m_moduleManager.GetModule(cmd.Module), m_messageQueue); }));
+        { return DispatchCommand(cmd,nullptr, m_messageQueue); }));
 
-    m_moduleManager.AddModule(Inventory::Instance());
-    m_moduleManager.Setup();
-    m_taskManager.EnqueueTask([this]() { m_moduleManager.Start(); });
+    //m_moduleManager.AddModule(Inventory::Instance());
+    //m_moduleManager.Setup();
+    //m_taskManager.EnqueueTask([this]() { m_moduleManager.Start(); });

     m_signalHandler->WaitForSignal();
-    m_moduleManager.Stop();
+    //m_moduleManager.Stop();
     m_communicator.Stop();
 }
diff --git a/src/common/utils/include/builder.hpp b/src/common/utils/include/builder.hpp
index 55c4369b5..03bac6672 100644
--- a/src/common/utils/include/builder.hpp
+++ b/src/common/utils/include/builder.hpp
@@ -12,8 +12,10 @@
 #ifndef _BUILDER_PATTERN_HPP
 #define _BUILDER_PATTERN_HPP

+#ifndef _WIN32
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wunused-function"
+#endif

 namespace Utils
 {
@@ -49,7 +51,9 @@ namespace Utils
     };
 }

+#ifndef _WIN32
 #pragma GCC diagnostic pop
+#endif

 #endif // _BUILDER_PATTERN_HPP

diff --git a/src/modules/inventory/CMakeLists.txt b/src/modules/inventory/CMakeLists.txt
index 0bec7bbc8..ae7aba6fd 100644
--- a/src/modules/inventory/CMakeLists.txt
+++ b/src/modules/inventory/CMakeLists.txt
@@ -5,6 +5,12 @@ project(Inventory)
 include(../../cmake/CommonSettings.cmake)
 set_common_settings()

+if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /WX-")
+    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} /WX-")
+    add_compile_options(/w)
+endif()
+
 get_filename_component(SRC_FOLDER       ${CMAKE_CURRENT_SOURCE_DIR}/../../ ABSOLUTE)
 get_filename_component(COMMON_FOLDER    ${SRC_FOLDER}/common/ ABSOLUTE)


Changes to the CMakefiles.txt files were made to make warnings (treated as errors) from clang-format and clang-tidy checks work throughout the rest of the project.You can leave them inside the invetory module until they are not needed.

To avoid previous problems related to these warnings treated as errors what I did was to install locally LLVM 18.1.8 which is the version used by the github runner. And I also installed cmake through Chocolatey which downloads cmake 3.30.3, in the same way as it is done in the windows action.

CC: @cborla

@cborla cborla linked a pull request Oct 9, 2024 that will close this issue
@cborla
Copy link
Member Author

cborla commented Oct 21, 2024

Moved to on hold until the release testing stage is completed.

@wazuhci wazuhci moved this from In progress to On hold in Release 5.0.0 Oct 21, 2024
@wazuhci wazuhci moved this from On hold to In progress in Release 5.0.0 Nov 5, 2024
@ncvicchi
Copy link
Member

On hold to work on higher priority tasks (release tasks)

@wazuhci wazuhci moved this from In progress to On hold in Release 5.0.0 Nov 11, 2024
@wazuhci wazuhci moved this from On hold to In progress in Release 5.0.0 Nov 19, 2024
@cborla
Copy link
Member Author

cborla commented Nov 25, 2024

On hold to work on higher release tasks

@wazuhci wazuhci moved this from In progress to On hold in Release 5.0.0 Nov 25, 2024
@wazuhci wazuhci moved this from On hold to In progress in Release 5.0.0 Nov 27, 2024
@wazuhci wazuhci moved this from In progress to In review in Release 5.0.0 Dec 9, 2024
@wazuhci wazuhci moved this from In review to In progress in Release 5.0.0 Dec 12, 2024
@wazuhci wazuhci moved this from In progress to Done in Release 5.0.0 Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants