Hi, thanks for review!
See my 2 comments below.
Branch: https://github.com/tarantool/tarantool/tree/artemreyt/gh-4966-gcc10-warns
Issue:
https://github.com/tarantool/tarantool/issues/4966
Hi! Thanks for the patch!
I have the following thoughts:
1. It's not obvious for me that if a function does not allocate of free memory
then it can't have influence on memory access. For example, deletion from tree
can break tree structure and returned pages are not correct page anymore.
I think we should dig more.
1. They may be not correct pages in a tree, but in this function (tt_bitset_set) it doesn't matter
for displayed warning.
The most important thing - is it enough memory was allocated for
returned page AFTER
(uint8_t *) page + sizeof(tt_bitset_page).
I will add a definition of tt_bitset_page
for visibility:
struct tt_bitset_page { size_t first_pos; rb_node(struct tt_bitset_page) node; size_t cardinality; uint8_t data[0]; };
This is not trivial problem. I made a new ticket for this:
https://github.com/tarantool/tarantool/issues/5564
2. I'm sure we should not switch off the check for entire project because of one file.
At least we should use smth like set_source_file_properties
2. I agree with that, so i switched off the check only for bitset.c referred to the created ticket:
diff --git a/src/lib/bitset/CMakeLists.txt
b/src/lib/bitset/CMakeLists.txt
index 1339a02ef..0aba49a47 100644
--- a/src/lib/bitset/CMakeLists.txt
+++ b/src/lib/bitset/CMakeLists.txt
@@ -6,6 +6,11 @@ set(lib_sources
index.c
)
+if (CMAKE_COMPILER_IS_GNUCC AND CMAKE_C_COMPILER_VERSION
VERSION_GREATER_EQUAL 10)
+ # gh-5564: bitset: writing 1 byte into a region of size 0
(gcc 10)
+
set_source_files_properties(${CMAKE_CURRENT_SOURCE_DIR}/bitset.c
+ PROPERTIES COMPILE_OPTIONS -Wno-stringop-overflow)
+endif()
set_source_files_compile_flags(${lib_sources})
add_library(bitset STATIC ${lib_sources})
target_link_libraries(bitset bit)
On 11/20/20 5:23 AM, Artem Starshov wrote:
GCC 10 reports: "warning: writing 1 byte into
a region of size 0 [-Wstringop-overflow=]"
It's false positive. In order to fix build
i tried to add ignoring gcc -Wstringop-overflow via
preprocessor directive `pragma GCC ignored`.
But it doesn't work, so added -Wnostringop-overflow
flag in complier.cmake for gcc version equal or greather 10.
Explaining of false positive:
The problem is:
In file included from /root/ttar/src/lib/bitset/bitset.h:45,
from /root/ttar/src/lib/bitset/bitset.c:32:
In function ‘bit_set’,
inlined from ‘tt_bitset_set’ at /root/ttar/src/lib/bitset/bitset.c:112:14:
/root/ttar/src/lib/bit/bit.h:230:15: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
230 | ldata[chunk] |= (1UL << offset);
| ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
I made a research and found out that if to explicitly init bitset->realloc = realloc
than a warning is still triggers, but if not to call rb_tree functions (tt_bitset_pages_search and
tt_bitset_pages_insert) it's ok. But these functions don't allocate or free a memory, so they can't
influence on memory region which `page` variable points to.
Part-of #4966
---
cmake/compiler.cmake | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/cmake/compiler.cmake b/cmake/compiler.cmake
index db2ae6227..5db636812 100644
--- a/cmake/compiler.cmake
+++ b/cmake/compiler.cmake
@@ -430,3 +430,17 @@ else()
set(CMAKE_HOST_C_COMPILER ${CMAKE_C_COMPILER})
set(CMAKE_HOST_CXX_COMPILER ${CMAKE_CXX_COMPILER})
endif()
+
+if (CMAKE_COMPILER_IS_GNUCC AND CMAKE_C_COMPILER_VERSION VERSION_GREATER_EQUAL 10)
+# In file included from src/lib/bitset/bitset.h:45,
+# from src/lib/bitset/bitset.c:32:
+# In function ‘bit_set’,
+# inlined from ‘tt_bitset_set’ at src/lib/bitset/bitset.c:111:14:
+# src/lib/bit/bit.h:230:15: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
+# 230 | ldata[chunk] |= (1UL << offset);
+# | ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
+#
+# false positive for gcc version 10
+# macro 'GCC diagnostic ignored "-Wstringop-overflow"' doesn't help
+ add_compile_flags("C" "-Wno-stringop-overflow")
+endif()