[WIP] Fix macOS libcachesim installation

[laustam]

Problem

The problem is that the installation of libcachesim fails on macOS due to too many errors caught by Clang that are otherwise ignored by the less strict GCC. This MR addresses issues listed in Issue #182 and fixes further issues encountered along the way.

Issue 1: Code quality CI never triggers

The code-quality.yaml includes a clang-tidy formatter that seemed to not run previously due to its configuration. After ensuring that all C and C++ files are captured (c4f9bfb), the whole repository was reformatted (225b07e) to conform to clang-tidy. As the repository-wide reformatting affected a lot of files without changing their functionality, the commit is ignored from git blame (fd5388d).

Issue 2: Incorrect format specifiers

A common error reported by Clang was incorrect format specifiers used with the printf function. The solution for this was simply to correct the format specifier (32bad8f). An example of the error is shown here:

libCacheSim/cache/eviction/LIRS.c:696:53: error: format specifies type 'unsigned long' but the argument has type 'int64_t' (aka 'long long') [-Werror,-Wformat]
    printf("%ld(%lu, %s, %s)->", (long)obj->obj_id, obj->obj_size,
                ~~~                                 ^~~~~~~~~~~~~
                %lld

Issue 3: Unqualified calls to std::move

The file splaytree.hpp contained unqualified calls to std::move. The fix was simple and just required a change from invoking std::move instead of move (25b3b43). An example of the error is shown here:

libCacheSim/mrcProfiler/../dataStructure/splaytree.hpp:386:13: error: unqualified call to 'std::move' [-Werror,-Wunqualified-std-cast-call]
    root_ = move(n->right);
            ^
            std::

Issue 4: Use of deprecated sprintf function

The sprintf function is reported to be deprecated and insecure, so it is replaced by snprintf (5d2b445). An example of the error is shown here:

test/common.h:62:3: error: 'sprintf' is deprecated: This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of sprintf(3), it is highly recommended that you use snprintf(3) instead. [-Werror,-Wdeprecated-declarations]
  sprintf(data_path, "data/%s", data_name);

Issue 5: Unused variables

There are some unused variables that have been removed (fdf9406). An example of the error is shown here:

libCacheSim/traceAnalyzer/accessPattern.h:51:12: error: private field 'n_items' is not used [-Werror,-Wunused-private-field]
   int64_t n_items = 0;
           ^

Issue 6: C++ constructs used in C scope

The split_by_char function in bin/mrcProfiler/cli_parser.cpp returns an instance of type std::vector<std::string>. The problem here is that this function is declared within an extern "C" {...} block, so is treated as a C function type eventhough its return type is unsupported (only supported in C++). The fix here is to move the function outside of the extern block (7e835f5). An example of the error is shown here:

libCacheSim/bin/mrcProfiler/cli_parser.cpp:165:26: error: 'split_by_char' has C-linkage specified, but returns incomplete type 'std::vector<std::string>' (aka 'vector<basic_string<char>>') which could be incompatible with C [-Werror,-Wreturn-type-c-linkage]
std::vector<std::string> split_by_char(const char *input, const char &c) {
                         ^

Issue 7: Variadic argument list (...) with optional arguments is not handled correctly

The header file inclue/libCacheSim/macro.h defines some macros for assertions with optional arguments. The problem here is that this only works if __VA_ARGS__ is not empty. Otherwise, it leaves a dangling comma like CHECK_CONDITION(x, ==, NULL, FMT, ) and causes compilation errors on Clang (GCC silently ignores these). The solution is to wrap the comma in __VA_OPT__ like CHECK_CONDITION(x, ==, NULL, FMT __VA_OPT__(,) __VA_ARGS__), so the expansion only adds a comma when __VA_ARGS__ is not empty, else no dangling comma (da4b717). An example of the error is shown here:

libCacheSim-fork/libCacheSim/profiler/simulator.c:207:3: error: expected expression
  ASSERT_NOT_NULL(gthread_pool, "cannot create thread pool in simulator\n");
  ^

libCacheSim/profiler/../include/libCacheSim/macro.h:92:3: note: expanded from macro 'ASSERT_NOT_NULL'
  CHECK_CONDITION(x, ==, NULL, FMT, __VA_ARGS__)
  ^

Issue 7: Unsupported linker options for macOS

There are three linker options used in CMakeLists.txt files across the project that are unsupported by Clang: export-dynamic, whole-archive, and no-whole-archive. Thus, logic has been added around these instances to ensure these options are only linked when run on Linux systems (33b614f). An example of the error is shown here:

ld: unknown options: --export-dynamic --whole-archive --no-whole-archive 
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Issue 8: Specific GCC flags not recognised by Clang

The C file bin/cachesim/sim.c contains GCC-specific #pragma directives used to temporarily surpress the compiler warning -Wformat-truncation. The problem is that Clang does not recognize GCC-specific warnings. To fix this, preprocessor checks are applied to ensure the pragma is only applied for real GCC (ca5af76). An example of the error is shown here:

libCacheSim/bin/cachesim/sim.c:84:32: error: unknown warning group '-Wformat-truncation', ignored [-Werror,-Wunknown-warning-option]
#pragma GCC diagnostic ignored "-Wformat-truncation"

Issue 9: Linker issue with _create_cache function

The linker issue indicated that the definition of the _create_cache function could not be found. The problem here is that the C file plugin.c which contains the _create_cache function was never compiled and linked and caused an unresolved symbol error. To solve this, the cache/CMakeLists.txt was modified to include plugin.c in the source list for the cachelib target (5dca341). An example of the error is shown here:

Undefined symbols for architecture arm64:
  "_create_cache", referenced from:
      mrcProfiler::MRCProfilerMINISIM::run() in libmrcProfilerLib.a[2](mrcProfiler.cpp.o)
ld: symbol(s) not found for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Issue 10: GNU-style variadic arguments not supported by Clang

GNU-style variadic arguments are used in the project, which Clang warns about by default. To resolve this by suppressing these warnings about zero-argument variadic macros, the compiler flag -Wno-gnu-zero-variadic-macro-arguments is added to both CMAKE_C_FLAGS and CMAKE_CXX_FLAGS in CMakeLists.txt (37f80d2). An example of the error is shown here:

libCacheSim/profiler/simulator.c:207:75: error: must specify at least one argument for '...' parameter of variadic macro [-Werror,-Wgnu-zero-variadic-macro-arguments]
  ASSERT_NOT_NULL(gthread_pool, "cannot create thread pool in simulator\n");

Issue 11: Library zstd not found

The zstd library was not found on macOS due to the path in which the library exists. Since brew is used to install packages on macOS, the path is different than the default path. To correctly handle this difference, the package manager pkg-config is used to link the zstd library correctly (e05e0df). An example of the error is shown here:

ld: library 'zstd' not found
clang: error: linker command failed with exit code 1 (use -v to see invocation)

[laustam]

The SonarCloud failures are not my failures; these already existed but had not been triggered due to the CI configuration. They were now triggered due to the repository-wide reformat to conform to the clang-format style. Either way, the warnings shown on SonarCloud are not very useful, complaining about strlen being unsafe. Yeah, it can be unsafe, this is C code :)

[sonarqubecloud]

Quality Gate failed

Failed conditions
10 Security Hotspots
8.6% Duplication on New Code (required ≤ 3%)
E Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE