ENABLE ROCKSDB - Fix clang-5+ and gcc-5+ compilation issues (facebook…

…#1076)

Summary:
1. Suppress warnings only if compiler supports "-Wno-xxxxxx".
2. Fix issues described at https://jira.percona.com/browse/PS-6054 and https://jira.percona.com/browse/PS-6058
3. Fix linking issues for clang-7 and clang-8:
```
/usr/bin/ld.gold: error: /usr/lib/llvm-8/lib/clang/8.0.1/lib/linux/libclang_rt.fuzzer-x86_64.a(FuzzerDataFlowTrace.cpp.o): invalid section group 9 refers to earlier section 3
/usr/bin/ld.gold: error: /usr/lib/llvm-8/lib/clang/8.0.1/lib/linux/libclang_rt.fuzzer-x86_64.a(FuzzerDataFlowTrace.cpp.o): invalid section group 10 refers to earlier section 4
/usr/bin/ld.gold: error: /usr/lib/llvm-8/lib/clang/8.0.1/lib/linux/libclang_rt.fuzzer-x86_64.a(FuzzerDataFlowTrace.cpp.o): invalid section group 11 refers to earlier section 5
```
4. Fix `-Werror=ignored-qualifiers` warnings e.g.
```
/fb-8.0.17/storage/rocksdb/ha_rocksdb.cc:882:70: error: type qualifiers ignored on cast result type [-Werror=ignored-qualifiers]
  882 |       static_cast<const rocksdb::InfoLogLevel>(rocksdb_info_log_level));
```
5. Fix a `-Wundefined-reinterpret-cast` warning:
```
/fb-8.0.17/storage/rocksdb/ha_rocksdb.cc:3777:11: error: dereference of type 'myrocks::Rdb_transaction **' that was reinterpret_cast from type 'void **' has undefined behavior [-Werror,-Wundefined-reinterpret-cast]
  return *reinterpret_cast<Rdb_transaction **>(
```
6. Fix a `-Werror=attributes` warning:
```
/fb-8.0.17/include/my_compiler.h:100:40: error: ‘nonnull’ attribute only applies to function types [-Werror=attributes]
  100 | #define MY_ATTRIBUTE(A) __attribute__(A)
```
7. Fix
```
/fb-8.0.17/sql/dd/impl/types/schema_impl.cc:100:1: error: base class ‘class dd::Weak_object’ should be explicitly initialized in the copy constructor [-Werror=extra]
 Schema_impl::Schema_impl(const Schema_impl &src)
```
Pull Request resolved: facebook#1076

Reviewed By: lth

Differential Revision: D19301431

Pulled By: lth

fbshipit-source-id: d302e8d

CMakeLists.txt

@@ -531,6 +531,7 @@ INCLUDE(mysql_add_executable)
 INCLUDE(curl)
 INCLUDE(rapidjson)
 INCLUDE(fprofile)
+INCLUDE(prepend_append_cflags_if_supported)
 
 OPTION(DISABLE_SHARED
  "Don't build shared libraries, compile code as position-dependent" OFF)

cmake/maintainer.cmake

@@ -94,6 +94,11 @@ IF(MY_COMPILER_IS_CLANG)
   STRING_APPEND(MY_CXX_WARNING_FLAGS " -Wno-unused-private-field")
   STRING_APPEND(MY_CXX_WARNING_FLAGS " -Wno-unused-command-line-argument")
 
+  # clang-5 or older: disable "suggest braces around initialization of subobject" warnings
+  IF(CMAKE_CXX_COMPILER_VERSION VERSION_LESS 6)
+    STRING_APPEND(MY_CXX_WARNING_FLAGS " -Wno-missing-braces")
+  ENDIF()
+
   STRING_APPEND(MY_CXX_WARNING_FLAGS " -Wconditional-uninitialized")
   STRING_APPEND(MY_CXX_WARNING_FLAGS " -Wdeprecated")
   STRING_APPEND(MY_CXX_WARNING_FLAGS " -Wextra-semi")

cmake/prepend_append_cflags_if_supported.cmake

@@ -0,0 +1,84 @@
+# Copyright (c) 2017, Percona and/or its affiliates. All rights reserved.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301  USA */
+
+# helper macros to prepend or append c and cxx flags if supported by compiler
+
+INCLUDE (CheckCCompilerFlag)
+INCLUDE (CheckCXXCompilerFlag)
+
+MACRO (prepend_cflags_if_supported)
+  FOREACH (flag ${ARGN})
+    STRING (REGEX REPLACE "-" "_" temp_flag ${flag})
+    check_c_compiler_flag (${flag} HAVE_C_${temp_flag})
+    IF (HAVE_C_${temp_flag})
+      SET (CMAKE_C_FLAGS "${flag} ${CMAKE_C_FLAGS}")
+    ENDIF ()
+    check_cxx_compiler_flag (${flag} HAVE_CXX_${temp_flag})
+    IF (HAVE_CXX_${temp_flag})
+      SET (CMAKE_CXX_FLAGS "${flag} ${CMAKE_CXX_FLAGS}")
+    ENDIF ()
+  ENDFOREACH (flag)
+ENDMACRO (prepend_cflags_if_supported)
+
+MACRO (append_cflags_if_supported)
+  FOREACH (flag ${ARGN})
+    STRING (REGEX REPLACE "-" "_" temp_flag ${flag})
+    check_c_compiler_flag (${flag} HAVE_C_${temp_flag})
+    IF (HAVE_C_${temp_flag})
+      SET (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${flag}")
+    ENDIF ()
+    check_cxx_compiler_flag (${flag} HAVE_CXX_${temp_flag})
+    IF (HAVE_CXX_${temp_flag})
+      SET (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${flag}")
+    ENDIF ()
+  ENDFOREACH (flag)
+ENDMACRO (append_cflags_if_supported)
+
+MACRO (remove_compile_flags)
+  FOREACH (flag ${ARGN})
+    IF(CMAKE_C_FLAGS MATCHES ${flag})
+      STRING(REGEX REPLACE "${flag}( |$)" "" CMAKE_C_FLAGS "${CMAKE_C_FLAGS}")
+    ENDIF(CMAKE_C_FLAGS MATCHES ${flag})
+    IF(CMAKE_CXX_FLAGS MATCHES ${flag})
+      STRING(REGEX REPLACE "${flag}( |$)" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}")
+    ENDIF(CMAKE_CXX_FLAGS MATCHES ${flag})
+  ENDFOREACH (flag)
+ENDMACRO (remove_compile_flags)
+
+## ADD_CXX_COMPILE_FLAGS_TO_FILES(<flags> FILES <source files>)
+## Add compile flags to given c++ files for all supported flags
+MACRO(ADD_CXX_COMPILE_FLAGS_TO_FILES)
+  SET(FILES "")
+  SET(FLAGS "")
+  SET(FILES_SEEN)
+  FOREACH(ARG ${ARGV})
+    IF("x${ARG}" STREQUAL "xFILES")
+      SET(FILES_SEEN 1)
+    ELSEIF(FILES_SEEN)
+      LIST(APPEND FILES ${ARG})
+    ELSE()
+      LIST(APPEND FLAGS ${ARG})
+    ENDIF()
+  ENDFOREACH()
+  SET(CHECKED_FLAGS "")
+  FOREACH (flag ${FLAGS})
+    STRING (REGEX REPLACE "-" "_" temp_flag ${flag})
+    MY_CHECK_CXX_COMPILER_FLAG(${flag} HAVE_CXX_${temp_flag})
+    IF (HAVE_CXX_${temp_flag})
+      STRING_APPEND(CHECKED_FLAGS "${flag}")
+    ENDIF()
+  ENDFOREACH(flag)
+  ADD_COMPILE_FLAGS(${FILES} COMPILE_FLAGS ${CHECKED_FLAGS})
+ENDMACRO(ADD_CXX_COMPILE_FLAGS_TO_FILES)

mysql-test/t/func_group.test

@@ -904,7 +904,7 @@ SELECT 1 FROM t1 GROUP BY (SELECT SLEEP(0) FROM t1 ORDER BY AVG(DISTINCT a) );
 DROP TABLE t1;
 
 #
-# Bug #30715: Assertion failed: item_field->field->real_maybe_null(), file
+# Bug #30715: Assertion failed: item_field->field->is_nullable(), file
 # .\opt_sum.cc, line
 #
 

router/CMakeLists.txt

@@ -31,6 +31,13 @@ IF(WITH_JEMALLOC)
   STRING(REPLACE "-ljemalloc" "" CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS}")
 ENDIF()
 
+# Disable ld.gold for clang-7 and clang-8
+IF(USE_LD_LLD AND CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND
+   NOT CMAKE_CXX_COMPILER_VERSION VERSION_LESS 7.0 AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 9.0)
+  STRING(REPLACE "-fuse-ld=gold" "" CMAKE_C_LINK_FLAGS "${CMAKE_C_LINK_FLAGS}")
+  STRING(REPLACE "-fuse-ld=gold" "" CMAKE_CXX_LINK_FLAGS "${CMAKE_CXX_LINK_FLAGS}")
+ENDIF()
+
 INCLUDE(cmake/version.cmake)
 SET(CMAKE_MODULE_PATH
   ${CMAKE_MODULE_PATH}

router/src/http/src/CMakeLists.txt

@@ -180,6 +180,13 @@ TARGET_INCLUDE_DIRECTORIES(http_server PRIVATE
   ${CMAKE_CURRENT_SOURCE_DIR}
   ${CMAKE_CURRENT_SOURCE_DIR}/../include/)
 
+# Suppress warnings for gcc-5 or older
+IF(CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 6.0)
+  MY_CHECK_CXX_COMPILER_FLAG("-Wno-unused-value" HAVE_NO_UNUSED_VALUE)
+  IF(HAVE_NO_UNUSED_VALUE)
+    ADD_COMPILE_FLAGS(tls_server_context.cc COMPILE_FLAGS "-Wno-unused-value")
+  ENDIF()
+ENDIF()
 
 ADD_LIBRARY(http_client
   SHARED

sql/rpl_gtid.h

@@ -28,6 +28,7 @@
 #include <map>
 #include <string>
 
+#undef ZSTD  // defined in storage/rocksdb/CMakeLists.txt
 #include "libbinlogevents/include/compression/base.h"
 #include "libbinlogevents/include/uuid.h"
 #include "map_helpers.h"

storage/rocksdb/CMakeLists.txt

@@ -1,7 +1,5 @@
 # TODO: Copyrights
 
-RETURN() # MyRocks is disabled
-
 CHECK_FUNCTION_EXISTS(sched_getcpu  HAVE_SCHED_GETCPU)
 IF(HAVE_SCHED_GETCPU)
   ADD_DEFINITIONS(-DHAVE_SCHED_GETCPU=1 -DROCKSDB_SCHED_GETCPU_PRESENT)
@@ -53,15 +51,30 @@ INCLUDE_DIRECTORIES(
 )
 
 
-SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-conditional-uninitialized")
-SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-undefined-reinterpret-cast")
-# TODO(yzha): Fix warnings in RocksDB
-SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-extra-semi")
-SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-inconsistent-missing-destructor-override")
-SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-deprecated")
-SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-shadow-field")
-# platform007 (tbb, etc) has throw() which is deprecated
-SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-deprecated-dynamic-exception-spec")
+# Suppress warnings for all compilers
+remove_compile_flags(-Wcast-qual)
+
+# Suppress warnings for all clang versions
+IF(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+  remove_compile_flags(-Winconsistent-missing-destructor-override -Wdeprecated -Wextra-semi)
+  ADD_CXX_COMPILE_FLAGS_TO_FILES(-Wno-shadow-field FILES ../../rocksdb/memtable/hash_skiplist_rep.cc)
+  ADD_CXX_COMPILE_FLAGS_TO_FILES(-Wno-conditional-uninitialized FILES ../../rocksdb/env/fs_posix.cc)
+
+  # platform007 (tbb, etc) has throw() which is deprecated
+  append_cflags_if_supported(-Wno-deprecated-dynamic-exception-spec)
+ENDIF()
+
+# Suppress warnings for clang-10 or newer
+IF(CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND NOT CMAKE_CXX_COMPILER_VERSION VERSION_LESS 10.0)
+  ADD_CXX_COMPILE_FLAGS_TO_FILES(-Wno-range-loop-construct FILES ../../rocksdb/options/options_helper.cc)
+ENDIF()
+
+# Suppress warnings for all gcc versions
+IF(CMAKE_COMPILER_IS_GNUCXX)
+  # "cmake/maintainer.cmake" sets "-Wmissing-format-attribute" which cause warnings with RocksDB
+  remove_compile_flags(-Wmissing-format-attribute)
+ENDIF()
+
 
 IF(UNIX)
   IF(CMAKE_SYSTEM_NAME STREQUAL "Linux")

storage/rocksdb/ha_rocksdb.cc

@@ -56,6 +56,7 @@
 #include "sql/sql_class.h"
 #include "sql/sql_lex.h"
 #include "sql/sql_table.h"
+#include "sql/sql_thd_internal_api.h"
 
 /* RocksDB includes */
 #include "monitoring/histogram.h"
@@ -159,20 +160,20 @@ class explicit_snapshot {
 class Rdb_explicit_snapshot : public explicit_snapshot {
  public:
   static std::shared_ptr<Rdb_explicit_snapshot> create(
-      snapshot_info_st *ss_info, rocksdb::DB *db,
+      snapshot_info_st *ssinfo, rocksdb::DB *db,
       const rocksdb::Snapshot *snapshot) {
     std::lock_guard<std::mutex> lock(explicit_snapshot_mutex);
     auto s = std::unique_ptr<rocksdb::ManagedSnapshot>(
         new rocksdb::ManagedSnapshot(db, snapshot));
     if (!s) {
       return nullptr;
     }
-    ss_info->snapshot_id = ++explicit_snapshot_counter;
-    auto ret = std::make_shared<Rdb_explicit_snapshot>(*ss_info, std::move(s));
+    ssinfo->snapshot_id = ++explicit_snapshot_counter;
+    auto ret = std::make_shared<Rdb_explicit_snapshot>(*ssinfo, std::move(s));
     if (!ret) {
       return nullptr;
     }
-    explicit_snapshots[ss_info->snapshot_id] = ret;
+    explicit_snapshots[ssinfo->snapshot_id] = ret;
     return ret;
   }
 
@@ -204,9 +205,9 @@ class Rdb_explicit_snapshot : public explicit_snapshot {
 
   rocksdb::ManagedSnapshot *get_snapshot() { return snapshot.get(); }
 
-  Rdb_explicit_snapshot(snapshot_info_st ss_info,
+  Rdb_explicit_snapshot(snapshot_info_st ssinfo,
                         std::unique_ptr<rocksdb::ManagedSnapshot> &&snapshot)
-      : explicit_snapshot(ss_info), snapshot(std::move(snapshot)) {}
+      : explicit_snapshot(ssinfo), snapshot(std::move(snapshot)) {}
 
   virtual ~Rdb_explicit_snapshot() {
     std::lock_guard<std::mutex> lock(explicit_snapshot_mutex);
@@ -927,7 +928,7 @@ static void rocksdb_set_rocksdb_info_log_level(
   RDB_MUTEX_LOCK_CHECK(rdb_sysvars_mutex);
   rocksdb_info_log_level = *static_cast<const uint64_t *>(save);
   rocksdb_db_options->info_log->SetInfoLogLevel(
-      static_cast<const rocksdb::InfoLogLevel>(rocksdb_info_log_level));
+      static_cast<rocksdb::InfoLogLevel>(rocksdb_info_log_level));
   RDB_MUTEX_UNLOCK_CHECK(rdb_sysvars_mutex);
 }
 
@@ -939,8 +940,7 @@ static void rocksdb_set_rocksdb_stats_level(
 
   RDB_MUTEX_LOCK_CHECK(rdb_sysvars_mutex);
   rocksdb_db_options->statistics->set_stats_level(
-      static_cast<const rocksdb::StatsLevel>(
-          *static_cast<const uint64_t *>(save)));
+      static_cast<rocksdb::StatsLevel>(*static_cast<const uint64_t *>(save)));
   // Actual stats level is defined at rocksdb dbopt::statistics::stats_level_
   // so adjusting rocksdb_stats_level here to make sure it points to
   // the correct stats level.
@@ -8402,7 +8402,7 @@ int ha_rocksdb::read_row_from_secondary_key(uchar *const buf,
                                             const Rdb_key_def &kd,
                                             bool move_forward) {
   int rc = 0;
-  uint pk_size;
+  uint pk_size = 0;
 
   /* Get the key columns and primary key value */
   const rocksdb::Slice &rkey = m_scan_it->key();
@@ -9636,8 +9636,8 @@ bool ha_rocksdb::do_bulk_commit(Rdb_transaction *const tx) {
   does not contain a primary key. (In which case we generate a hidden
   'auto-incremented' pk.)
 */
-bool ha_rocksdb::has_hidden_pk(const TABLE *const table) const {
-  return Rdb_key_def::table_has_hidden_pk(table);
+bool ha_rocksdb::has_hidden_pk(const TABLE *const tabl) const {
+  return Rdb_key_def::table_has_hidden_pk(tabl);
 }
 
 /*
@@ -11478,7 +11478,7 @@ void ha_rocksdb::update_table_stats_if_needed() {
   uint64 n_rows = m_tbl_def->m_tbl_stats.m_stat_n_rows;
 
   if (counter > std::max(rocksdb_table_stats_recalc_threshold_count,
-                         static_cast<uint64>(
+                         static_cast<unsigned long long>(
                              n_rows * rocksdb_table_stats_recalc_threshold_pct /
                              100.0))) {
     // Add the table to the recalc queue

storage/rocksdb/rdb_cf_options.h

@@ -71,8 +71,8 @@ class Rdb_cf_options {
       const std::string &cf_name);
 
   void get_cf_options(const std::string &cf_name,
-                      rocksdb::ColumnFamilyOptions *const opts
-                          MY_ATTRIBUTE((__nonnull__)));
+                      rocksdb::ColumnFamilyOptions *const opts)
+      MY_ATTRIBUTE((__nonnull__));
 
   static bool parse_cf_options(const std::string &cf_options,
                                Name_to_config_t *option_map,

storage/rocksdb/rdb_converter.cc

@@ -71,7 +71,7 @@ int Rdb_convert_to_record_value_decoder::decode(uchar *const buf, uint *offset,
   uint field_offset = field->ptr - table->record[0];
   *offset = field_offset;
   uint null_offset = field->null_offset();
-  bool maybe_null = field->real_maybe_null();
+  bool maybe_null = field->is_nullable();
   field->move_field(buf + field_offset,
                     maybe_null ? buf + null_offset : nullptr, field->null_bit);
 
@@ -463,7 +463,7 @@ void Rdb_converter::setup_field_encoders() {
     m_encoder_arr[i].m_field_index = i;
     m_encoder_arr[i].m_pack_length_in_rec = field->pack_length_in_rec();
 
-    if (field->real_maybe_null()) {
+    if (field->is_nullable()) {
       m_encoder_arr[i].m_null_mask = cur_null_mask;
       m_encoder_arr[i].m_null_offset = null_bytes_length;
       if (cur_null_mask == 0x80) {