Details
-
Task
-
Status: Closed (View Workflow)
-
Major
-
Resolution: Fixed
Description
IF (CMAKE_SYSTEM_PROCESSOR MATCHES "x86_64" OR
|
CMAKE_SYSTEM_PROCESSOR MATCHES "i386")
|
INCLUDE(CheckCXXCompilerFlag)
|
CHECK_CXX_COMPILER_FLAG("-fno-builtin-memcmp" HAVE_NO_BUILTIN_MEMCMP)
|
IF (HAVE_NO_BUILTIN_MEMCMP)
|
# Work around http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43052
|
SET_SOURCE_FILES_PROPERTIES(${CMAKE_CURRENT_SOURCE_DIR}/rem/rem0cmp.cc
|
PROPERTIES COMPILE_FLAGS -fno-builtin-memcmp)
|
ENDIF()
|
ENDIF()
|
ENDIF()
|
This was added to mitigate poor compiler optimization. As I can see, it was fixed in gcc-4.6 https://godbolt.org/z/7G64qo For 10.5 we support at least gcc-4.8 so this CMake code can be removed. For older versions it should be replaced with some preprocessor condition to check compiler name and version.
Attachments
Activity
The 10.5 version of the change introduced a WITH_UBSAN failure, by removing the zero check on len before invoking memcmp(). The function parameters can be null pointers in this case, and memcmp(nullptr,nullptr,0) is undefined behaviour, and it will allow GCC 8 (or later) to infer that the pointer arguments are nonnull, and to optimize away further nullness checks for them. That actually happened in MDEV-15587.
diff --git a/storage/innobase/rem/rem0cmp.cc b/storage/innobase/rem/rem0cmp.cc
|
index c0e82bd3ad7..b7e835767df 100644
|
--- a/storage/innobase/rem/rem0cmp.cc
|
+++ b/storage/innobase/rem/rem0cmp.cc
|
@@ -451,8 +451,7 @@ cmp_data(
|
}
|
|
ulint len = std::min(len1, len2);
|
-
|
- int cmp = memcmp(data1, data2, len);
|
+ int cmp = len ? memcmp(data1, data2, len) : 0;
|
|
if (cmp) {
|
return (cmp); |
With this change, WITH_UBSAN should be happy. The following tests cover this condition:
vcol.vcol_keys_innodb gcol.gcol_keys_innodb main.func_group_innodb innodb.innodb_bug53592
|
The minor regression (with no observed effects outside WITH_UBSAN) is now fixed in the 10.5 branch. It was not part of any release yet.
Thank you! It looks good to me.