From 7f8495c44b48b2995e45419bdd4149896eec536d Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Sun, 7 May 2023 13:32:46 +1000 Subject: [PATCH] CMake: add WITH_STRSIZE_DEBUG option, RNA support Support string size debug so it can be used for regular development. It works be writing values into strings, ensuring the buffer size given is actually available. Developers can use this with memory checking tools such as ASAN/valgrind to force an error when the value used for the size of a buffer is larger than the buffer. Resolve remaining issue with RNA using BLI_strncpy* in generated callback functions where the size argument didn't represent the size of the destination buffer. This is automatically enabled along with ASAN for the blender_developer.cmake configuration. Ref PR !107602. --- CMakeLists.txt | 6 +++++ .../cmake/config/blender_developer.cmake | 1 + source/blender/makesrna/intern/makesrna.c | 27 +++++++------------ source/creator/CMakeLists.txt | 4 +++ 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 93fddfb1d15..d32692f1fbd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -651,6 +651,12 @@ if(WIN32) set(CPACK_INSTALL_PREFIX ${CMAKE_GENERIC_PROGRAM_FILES}/${}) endif() +option(WITH_STRSIZE_DEBUG "\ +Ensure string operations on fixed size buffers \ +(works well with with \"WITH_COMPILER_ASAN\" & valgrind to detect incorrect buffer size arguments)" + OFF) +mark_as_advanced(WITH_STRSIZE_DEBUG) + # Compiler tool-chain. if(UNIX) if(CMAKE_COMPILER_IS_GNUCC) diff --git a/build_files/cmake/config/blender_developer.cmake b/build_files/cmake/config/blender_developer.cmake index ebc5727a79c..8c954feefbe 100644 --- a/build_files/cmake/config/blender_developer.cmake +++ b/build_files/cmake/config/blender_developer.cmake @@ -12,6 +12,7 @@ set(WITH_BUILDINFO OFF CACHE BOOL "" FORCE) # developer profile for now. if(NOT WIN32) set(WITH_COMPILER_ASAN ON CACHE BOOL "" FORCE) + set(WITH_STRSIZE_DEBUG ON CACHE BOOL "" FORCE) endif() set(WITH_CYCLES_NATIVE_ONLY ON CACHE BOOL "" FORCE) set(WITH_DOC_MANPAGE OFF CACHE BOOL "" FORCE) diff --git a/source/blender/makesrna/intern/makesrna.c b/source/blender/makesrna/intern/makesrna.c index 33777bbc33d..47cfb6f8fbf 100644 --- a/source/blender/makesrna/intern/makesrna.c +++ b/source/blender/makesrna/intern/makesrna.c @@ -726,18 +726,13 @@ static char *rna_def_property_get_func( switch (prop->type) { case PROP_STRING: { StringPropertyRNA *sprop = (StringPropertyRNA *)prop; + UNUSED_VARS_NDEBUG(sprop); fprintf(f, "void %s(PointerRNA *ptr, char *value)\n", func); fprintf(f, "{\n"); if (manualfunc) { fprintf(f, " %s(ptr, value);\n", manualfunc); } else { - const PropertySubType subtype = prop->subtype; - const char *string_copy_func = - ELEM(subtype, PROP_FILEPATH, PROP_DIRPATH, PROP_FILENAME, PROP_BYTESTRING) ? - "BLI_strncpy" : - "BLI_strncpy_utf8"; - rna_print_data_get(f, dp); if (dp->dnapointerlevel == 1) { @@ -746,28 +741,24 @@ static char *rna_def_property_get_func( fprintf(f, " *value = '\\0';\n"); fprintf(f, " return;\n"); fprintf(f, " }\n"); - fprintf(f, - " %s(value, data->%s, strlen(data->%s) + 1);\n", - string_copy_func, - dp->dnaname, - dp->dnaname); + fprintf(f, " strcpy(value, data->%s);\n", dp->dnaname); } else { /* Handle char array properties. */ + +#ifndef NDEBUG /* Assert lengths never exceed their maximum expected value. */ if (sprop->maxlength) { - fprintf(f, - " %s(value, data->%s, %d);\n", - string_copy_func, - dp->dnaname, - sprop->maxlength); + fprintf(f, " BLI_assert(strlen(data->%s) < %d);\n", dp->dnaname, sprop->maxlength); } else { fprintf(f, - " %s(value, data->%s, sizeof(data->%s));\n", - string_copy_func, + " BLI_assert(strlen(data->%s) < sizeof(data->%s));\n", dp->dnaname, dp->dnaname); } +#endif + + fprintf(f, " strcpy(value, data->%s);\n", dp->dnaname); } } fprintf(f, "}\n\n"); diff --git a/source/creator/CMakeLists.txt b/source/creator/CMakeLists.txt index 14bb46d60c3..e1aa7d72a66 100644 --- a/source/creator/CMakeLists.txt +++ b/source/creator/CMakeLists.txt @@ -26,6 +26,10 @@ if(HAVE_FEENABLEEXCEPT) add_definitions(-DHAVE_FEENABLEEXCEPT) endif() +if(WITH_STRSIZE_DEBUG) + add_definitions(-DDEBUG_STRSIZE) +endif() + if(WITH_TBB) # Force TBB libraries to be in front of MKL (part of `OpenImageDenoise`), so # that it is initialized before MKL and static library initialization order issues are avoided.