cmake : add sanitizer flags for llama.cpp #11279
Conversation
common/CMakeLists.txt
Outdated
| if (NOT MSVC) | ||
| if (LLAMA_SANITIZE_THREAD) | ||
| add_compile_options(-fsanitize=thread) | ||
| link_libraries (-fsanitize=thread) | ||
| endif() | ||
|
|
||
| if (LLAMA_SANITIZE_ADDRESS) | ||
| add_compile_options(-fsanitize=address -fno-omit-frame-pointer) | ||
| link_libraries (-fsanitize=address) | ||
| endif() | ||
|
|
||
| if (LLAMA_SANITIZE_UNDEFINED) | ||
| add_compile_options(-fsanitize=undefined) | ||
| link_libraries (-fsanitize=undefined) | ||
| endif() | ||
| endif() |
There was a problem hiding this comment.
If this is done in the top-level CMakeLists.txt it should automatically apply to all included subdirectories.
There was a problem hiding this comment.
I moved them to the llama_add_compile_flags() to be consistent with the existing LLAMA_FATAL_WARNINGS and LLAMA_ALL_WARNINGS options.
Moved a single call of llama_add_compile_flags() to the top-level CMakeLists.txt. It's OK for now, but in general I think that having separate calls in the sub-trees is better, because it allows for example to have 3rd-party code in the examples tree that would not be a subject to the llama.cpp compile flags.
There was a problem hiding this comment.
The sanitizer flags should still be applied to 3rd-party code regardless, but maybe not other flags like warnings.
There was a problem hiding this comment.
Only the sanitizer flags are now at the top level. Also removed the GGML_SANITIZE_* options as they are not actually useful, because all the code must be compiled with sanitizers.
| @@ -99,7 +99,7 @@ static bool expect_context_not_null(const enum handcrafted_file_type hft) { | |||
|
|
|||
| typedef std::pair<enum ggml_type, std::array<int64_t, GGML_MAX_DIMS>> tensor_config_t; | |||
|
|
|||
| std::vector<tensor_config_t> get_tensor_configs(std::mt19937 & rng) { | |||
| static std::vector<tensor_config_t> get_tensor_configs(std::mt19937 & rng) { | |||
There was a problem hiding this comment.
Curious, compiler warnings normally pick this up, but not here it seems
|
The reason the tests are failing is another bug in |
|
@JohannesGaessler Thanks for taking a look. I've cherry-picked your commit into this branch. Let's see if the CI passes now. |
* cmake : add sanitizer flags for llama.cpp ggml-ci * tests : fix compile warnings ggml-ci * cmake : move sanitizer flags to llama_add_compile_flags ggml-ci * cmake : move llama.cpp compile flags to top level lists ggml-ci * cmake : apply only sanitizer flags at top level ggml-ci * tests : fix gguf context use in same_tensor_data * gguf-test: tensor data comparison * dummy : trigger ggml-ci * unicode : silence gcc warnings ggml-ci * ci : use sanitizer builds only in Debug mode ggml-ci * cmake : add status messages [no ci] --------- Co-authored-by: Johannes Gäßler <johannesg@5d6.de>
ref #11252 (comment)
Also, enable compile flags for
tests.