diff options
Diffstat (limited to 'clang-tools-extra/docs')
3 files changed, 137 insertions, 0 deletions
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index e5be18202c9..fc5336602c8 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -79,6 +79,15 @@ Improvements to clang-tidy Finds obvious infinite loops (loops where the condition variable is not changed at all). +- New :doc:`bugprone-not-null-terminated-result + <clang-tidy/checks/bugprone-not-null-terminated-result>` check + + Finds function calls where it is possible to cause a not null-terminated + result. Usually the proper length of a string is ``strlen(str) + 1`` or equal + length of this expression, because the null terminator needs an extra space. + Without the null terminator it can result in undefined behaviour when the + string is read. + - New :doc:`cppcoreguidelines-init-variables <clang-tidy/checks/cppcoreguidelines-init-variables>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst new file mode 100644 index 00000000000..d6f797200e1 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst @@ -0,0 +1,127 @@ +.. title:: clang-tidy - bugprone-not-null-terminated-result + +bugprone-not-null-terminated-result +=================================== + +Finds function calls where it is possible to cause a not null-terminated result. +Usually the proper length of a string is ``strlen(src) + 1`` or equal length of +this expression, because the null terminator needs an extra space. Without the +null terminator it can result in undefined behaviour when the string is read. + +The following and their respective ``wchar_t`` based functions are checked: + +``memcpy``, ``memcpy_s``, ``memchr``, ``memmove``, ``memmove_s``, +``strerror_s``, ``strncmp``, ``strxfrm`` + +The following is a real-world example where the programmer forgot to increase +the passed third argument, which is ``size_t length``. That is why the length +of the allocated memory is not enough to hold the null terminator. + + .. code-block:: c + + static char *stringCpy(const std::string &str) { + char *result = reinterpret_cast<char *>(malloc(str.size())); + memcpy(result, str.data(), str.size()); + return result; + } + +In addition to issuing warnings, fix-it rewrites all the necessary code. It also +tries to adjust the capacity of the destination array: + + .. code-block:: c + + static char *stringCpy(const std::string &str) { + char *result = reinterpret_cast<char *>(malloc(str.size() + 1)); + strcpy(result, str.data()); + return result; + } + +Note: It cannot guarantee to rewrite every of the path-sensitive memory + allocations. + +.. _MemcpyTransformation: + +Transformation rules of 'memcpy()' +---------------------------------- + +It is possible to rewrite the ``memcpy()`` and ``memcpy_s()`` calls as the +following four functions: ``strcpy()``, ``strncpy()``, ``strcpy_s()``, +``strncpy_s()``, where the latter two are the safer versions of the former two. +It rewrites the ``wchar_t`` based memory handler functions respectively. + +Rewrite based on the destination array +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +- If copy to the destination array cannot overflow [1] the new function should + be the older copy function (ending with ``cpy``), because it is more + efficient than the safe version. + +- If copy to the destination array can overflow [1] and + ``AreSafeFunctionsAvailable`` is set to ``Yes``, ``y`` or non-zero and it is + possible to obtain the capacity of the destination array then the new function + could be the safe version (ending with ``cpy_s``). + +- If the new function is could be safe version and C++ files are analysed and + the destination array is plain ``char``/``wchar_t`` without ``un/signed`` then + the length of the destination array can be omitted. + +- If the new function is could be safe version and the destination array is + ``un/signed`` it needs to be casted to plain ``char *``/``wchar_t *``. + +[1] It is possible to overflow: + - If the capacity of the destination array is unknown. + - If the given length is equal to the destination array's capacity. + +Rewrite based on the length of the source string +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +- If the given length is ``strlen(source)`` or equal length of this expression + then the new function should be the older copy function (ending with ``cpy``), + as it is more efficient than the safe version (ending with ``cpy_s``). + +- Otherwise we assume that the programmer wanted to copy 'N' characters, so the + new function is ``ncpy``-like which copies 'N' characters. + +Transformations with 'strlen()' or equal length of this expression +------------------------------------------------------------------ + +It transforms the ``wchar_t`` based memory and string handler functions +respectively (where only ``strerror_s`` does not have ``wchar_t`` based alias). + +Memory handler functions +^^^^^^^^^^^^^^^^^^^^^^^^ + +- ``memcpy``: Visit the + :ref:`Transformation rules of 'memcpy()'<MemcpyTransformation>` section. + +- ``memchr``: + - Usually there is a C-style cast and it is needed to be removed, because the + new function ``strchr``'s return type is correct. + - The given length is going to be removed. + +- ``memmove``: + - If safe functions are available the new function is ``memmove_s``, which has + a new second argument which is the length of the destination array, it is + adjusted, and the length of the source string is incremented by one. + - If safe functions are not available the given length is incremented by one. + +- ``memmove_s``: given length is incremented by one. + +String handler functions +^^^^^^^^^^^^^^^^^^^^^^^^ + +- ``strerror_s``: given length is incremented by one. + +- ``strncmp``: If the third argument is the first or the second argument's + ``length + 1`` it has to be truncated without the ``+ 1`` operation. + +- ``strxfrm``: given length is incremented by one. + +Options +------- + +.. option:: WantToUseSafeFunctions + + An integer non-zero value specifying if the target environment is considered + to implement '_s' suffixed memory and string handler functions which are + safer than older versions (e.g. 'memcpy_s()'). The default value is ``1``. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index f0f35bd9e62..5d3bedc603b 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -60,6 +60,7 @@ Clang-Tidy Checks bugprone-misplaced-widening-cast bugprone-move-forwarding-reference bugprone-multiple-statement-macro + bugprone-not-null-terminated-result bugprone-parent-virtual-call bugprone-posix-return bugprone-sizeof-container |
