-
Daniel Lenski authored
The `safe_strcpy()` function was added in https://github.com/mariadb/server/commit/567b68129943#diff-23f88d0b52735bf79b7eb76e2ddbbebc96f3b1ca16e784a347525a9c43134d77 Unfortunately, its current implementation triggers many GCC 8+ string truncation and array bounds warnings, particularly due to the potential for a false positive `-Warray-bounds`. For example, the line `safe_strcpy(delimiter, sizeof(delimiter), ";")` in `client/mysqldump.c` causes the following warning: [1669/1914] Building C object client/CMakeFiles/mariadb-dump.dir/mysqldump.c.o In file included from /PATH/include/my_sys.h:20, from /PATH/mysqldump.c:51: In function ?safe_strcpy?, inlined from ?dump_events_for_db.isra? at /PATH/client/mysqldump.c:2595:3: /PATH/include/m_string.h:258:39: warning: array subscript 1535 is outside array bounds of ?const char[2]? [-Warray-bounds=] 258 | if (dst[dst_size - 2] != '\0' && src[dst_size - 1] != '\0') | ~~~^~~~~~~~~~~~~~ GCC is reporting that the `safe_strcpy` function *could* cause an out-of-bounds read from the constant *source* string `";"`, however this warning is unhelpful and confusing because it can only happen if the size of the *destination* buffer is incorrectly specified, which is not the case here. In https://github.com/MariaDB/server/pull/2640, Andrew Hutchings proposed fixing this by disabling the `-Warray-bounds` check in this function (specifically in https://github.com/MariaDB/server/pull/2640/commits/be382d01d08739d081f6cf40f350f7414f29b49d#diff-23f88d0b52735bf79b7eb76e2ddbbebc96f3b1ca16e784a347525a9c43134d77R255-R262). However, this was rejected because it also disables the *helpful* `-Warray-bounds` check on the destination buffer. Cherry-picking the commit https://github.com/MariaDB/server/commit/a7adfd4c52307876d68ad3386cefd3757ee66e92 from 11.2 by Monty Widenius solves the first two problems: 1. It reimplements `safe_strcpy` a bit more efficiently, skipping the `memset(dst, 0, dst_size)`. This is unnecessary since `strncpy` already pads `dst` with 0 bytes. 2. It will not trigger the `-Warray-bounds` warning, because `src` is not read based on an offset determined from `dst_size`. There is a third problem, however. Using `strncpy` triggers the `-Wstringop-truncation` warning (https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstringop-truncation), so we need to disable that. However, that is a much less broadly and generally-useful warning so there is no loss of static analysis value caused by disabling it. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
2ba5c387