[yugabyte#5203] Analyze C/C++ code with PVS Studio and fix some of th…

…e warnings

Summary:
PVS-Studio is a static analysis tool that can help detect certain bugs. Adding infrastructure for running PVS-Studio on our codebase. This includes a new debug-mode build type ("pvs") preconfigured for this kind of analysis:
- Automatically generate the compilation database (compile_commands.json file).
- Disabling remote compiler invocation.
- Disabling building any of the test code.

Also adding a yugabytedb.pvsconfig configuration file that suppresses some of the PVS-Studio false positives on YugabyteDB code.

Fixing some of the warnings found by PVS-Studio, mostly fields left uninitialized in constructors.

To re-run PVS-Studio analysis, do the build and then run: build-support/run_pvs_studio_analyzer --build_root build/pvs-gcc-dynamic-ninja

Other changes:
- Refactor compilation database generation scripts.
- Simplify wrapping libpq and yb_pgbackend (the shared library with PostgreSQL backend code) in CMake libraries.
- Fixing a dependency bug with libpq not having been built prior to building tablet code that depends on it due to YSQL backfill. This produces a warning `ld: warning: directory not found for option` which is now being treated as an error.
- Changing the environment variable that triggers compile_commands.json generation from CMAKE_EXPORT_COMPILE_COMMANDS  to YB_EXPORT_COMPILE_COMMANDS for clarity, to avoid confusion with CMake's variable (https://cmake.org/cmake/help/latest/variable/CMAKE_EXPORT_COMPILE_COMMANDS.html).
- remote_build.py now supports auto-detecting the remote build directory based on patterns specified in the profile file, and automatically cloning the repo remotely if necessary.
- Introducing the YbBuildToolBase class and using it in build_postgres.py and run_pvs_studio_analyzer.py.

One more detail is that we currently run PVS-Studio with devtoolset-8's gcc8 (see yugabyte#4996) and without Linuxbrew, to minimize false positives where standard headers inside Linuxbrew are not being recognized as such by PVS-Studio. So for any of the above to work, currently the following needs to be done first:
```
. /opt/rh/devtoolset-8/enable
export YB_DISABLE_LINUXBREW=1
```

Test Plan: Jenkins

Reviewers: bogdan, steve.varnau

Reviewed By: steve.varnau

Subscribers: kannan, ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D9395

CMakeLists.txt

@@ -574,7 +574,7 @@ endif()
 # Generate a Clang compile_commands.json "compilation database" file for use
 # with various development tools, such as Vim's YouCompleteMe plugin.
 # See http://clang.llvm.org/docs/JSONCompilationDatabase.html
-if ("$ENV{CMAKE_EXPORT_COMPILE_COMMANDS}" STREQUAL "1" OR
+if ("$ENV{YB_EXPORT_COMPILE_COMMANDS}" STREQUAL "1" OR
     "$ENV{YB_RUN_AFFECTED_TESTS_ONLY}" STREQUAL "1")
   set(CMAKE_EXPORT_COMPILE_COMMANDS 1)
 endif()
@@ -1108,6 +1108,30 @@ function(ADD_YB_LIBRARY LIB_NAME)
   endif()
 endfunction()
 
+# Import a shared library (e.g. libpq) that is built as part of PostgreSQL as a CMake target.
+# See https://github.com/yugabyte/yugabyte-db/issues/5853: we currently have to run this function
+# separately for each library from each CMakeLists.txt file that uses that library.
+function(ADD_POSTGRES_SHARED_LIBRARY LIB_NAME SHARED_LIB_PATH)
+  if ("${SHARED_LIB_PATH}" STREQUAL "")
+    message(FATAL_ERROR
+            "Shared library path cannot be empty. "
+            "LIB_NAME=${LIB_NAME} "
+            "CMAKE_CURRENT_LIST_DIR=${CMAKE_CURRENT_LIST_DIR}")
+  endif()
+  add_library(${LIB_NAME} SHARED IMPORTED)
+  set_target_properties(${LIB_NAME} PROPERTIES IMPORTED_LOCATION "${SHARED_LIB_PATH}")
+  # "postgres" is the target that actually builds this shared library.
+  add_dependencies(${LIB_NAME} postgres)
+  message("Added target ${LIB_NAME} for a shared library built as part of PostgreSQL code: "
+          "${SHARED_LIB_PATH} (invoked from ${CMAKE_CURRENT_LIST_FILE})")
+endfunction()
+
+function(allow_using_postgres_libraries)
+  include_directories(${YB_BUILD_ROOT}/postgres/include)
+  add_postgres_shared_library(pq "${LIBPQ_SHARED_LIB}")
+  add_postgres_shared_library(yb_pgbackend "${YB_PGBACKEND_SHARED_LIB}")
+endfunction()
+
 ############################################################
 # Testing
 ############################################################
@@ -1736,7 +1760,11 @@ endif()
 
 message("Linker flags: ${CMAKE_EXE_LINKER_FLAGS}")
 
-set(PQ_SHARED_LIB
+# Define full paths to libpq and libyb_pgbackend shared libraries. These libraries are built as part
+# of building PostgreSQL. We need to declare these as by-products of running the PostgreSQL build
+# command, as well as wrap them into CMake targets so that other libraries and executables can
+# depend on them.
+set(LIBPQ_SHARED_LIB
     "${YB_BUILD_ROOT}/postgres/lib/libpq${YB_SHARED_LIBRARY_SUFFIX}")
 set(YB_PGBACKEND_SHARED_LIB
     "${YB_BUILD_ROOT}/postgres/lib/libyb_pgbackend${YB_SHARED_LIBRARY_SUFFIX}")

bin/remote_build.py

@@ -17,11 +17,13 @@
 import argparse
 import os
 import sys
+import logging
 
 sys.path.insert(0, os.path.join(
     os.path.dirname(os.path.dirname(os.path.abspath(__file__))), 'python'))  # noqa
 
 from yb import remote
+from yb.common_util import init_env
 
 
 def add_extra_ybd_args(ybd_args, extra_args):
@@ -48,7 +50,9 @@ def main():
                               'variable.').format(remote.REMOTE_BUILD_HOST_ENV_VAR))
     home = os.path.expanduser('~')
     cwd = os.getcwd()
-    default_path = '~/{0}'.format(cwd[len(home) + 1:] if cwd.startswith(home) else 'code/yugabyte')
+    default_path = '~/{0}'.format(
+        cwd[len(home) + 1:] if cwd.startswith(home + '/') else 'code/yugabyte'
+    )
 
     # Note: don't specify default arguments here, because they may come from the "profile".
     parser.add_argument('--remote-path', type=str,
@@ -64,6 +68,9 @@ def main():
     parser.add_argument('--profile',
                         help='Use a "profile" specified in the {} file'.format(
                             remote.CONFIG_FILE_PATH))
+    parser.add_argument('--verbose',
+                        action='store_true',
+                        help='Verbose output')
     parser.add_argument('build_args', nargs=argparse.REMAINDER,
                         help='arguments for yb_build.sh')
 
@@ -72,8 +79,9 @@ def main():
         # after remote_build.py.
         sys.argv[1:2] = ['--']
     args = parser.parse_args()
+    init_env(verbose=args.verbose)
 
-    remote.load_profile(['host', 'remote_path', 'branch'], args, args.profile)
+    remote.load_profile(args, args.profile)
 
     # ---------------------------------------------------------------------------------------------
     # Default arguments go here.

bin/remote_release.py

@@ -53,7 +53,7 @@ def main():
 
     args = parser.parse_args()
 
-    remote.load_profile(['host', 'remote_path', 'branch'], args, args.profile)
+    remote.load_profile(args, args.profile)
 
     # ---------------------------------------------------------------------------------------------
     # Default arguments go here.

build-support/common-build-env.sh

@@ -140,6 +140,7 @@ readonly -a VALID_BUILD_TYPES=(
   release
   tsan
   tsan_slow
+  pvs
 )
 make_regex_from_list VALID_BUILD_TYPES "${VALID_BUILD_TYPES[@]}"
 
@@ -537,9 +538,15 @@ set_cmake_build_type_and_compiler_type() {
     ;;
     compilecmds)
       cmake_build_type=debug
-      export CMAKE_EXPORT_COMPILE_COMMANDS=1
       export YB_EXPORT_COMPILE_COMMANDS=1
     ;;
+    pvs)
+      cmake_build_type=debug
+      export YB_EXPORT_COMPILE_COMMANDS=1
+      export YB_DO_NOT_BUILD_TESTS=1
+      export YB_SKIP_INITIAL_SYS_CATALOG_SNAPSHOT=1
+      export YB_REMOTE_COMPILATION=0
+    ;;
     idebug|ifastdebug|irelease)
       cmake_build_type=${build_type:1}
       cmake_opts+=( "-DYB_INSTRUMENT_FUNCTIONS=1" )

build-support/compiler-wrappers/compiler-wrapper.sh

@@ -578,19 +578,32 @@ set_default_compiler_type
 find_or_download_thirdparty
 find_compiler_by_type "$YB_COMPILER_TYPE"
 
+if [[ $cc_or_cxx == "compiler-wrapper.sh" && $compiler_args_str == "--version" ]]; then
+  # Allow invoking this script not through a symlink but directly in one special case: when trying
+  # to determine the compiler version.
+  cc_or_cxx=cc
+fi
+
 case "$cc_or_cxx" in
-  cc) compiler_executable="$cc_executable" ;;
-  c++) compiler_executable="$cxx_executable" ;;
-  default)
-    echo "The $SCRIPT_NAME script should be invoked through a symlink named 'cc' or 'c++', " \
-         "found: $cc_or_cxx" >&2
-    exit 1
+  cc) compiler_executable=$cc_executable ;;
+  c++) compiler_executable=$cxx_executable ;;
+  *)
+    fatal "The $SCRIPT_NAME script should be invoked through a symlink named 'cc' or 'c++', " \
+          "found: $cc_or_cxx." \
+          "Just in case:" \
+          "cc_executable=${cc_executable:-undefined}," \
+          "cxx_executable=${cxx_executable:-undefined}."
 esac
 
+if [[ -z ${compiler_executable:-} ]]; then
+  fatal "[Host $(hostname)] The compiler_executable variable is not defined." \
+        "Command line: $compiler_args_str"
+fi
+
 if [[ ! -x $compiler_executable ]]; then
   log_diagnostics_about_local_thirdparty
   fatal "[Host $(hostname)] Compiler executable does not exist or is not executable:" \
-        "$compiler_executable"
+        "$compiler_executable. Command line: $compiler_args_str"
 fi
 
 # We use ccache if it is available and YB_NO_CCACHE is not set.
@@ -831,6 +844,11 @@ if [[ $compiler_exit_code -ne 0 ]]; then
   exit "$compiler_exit_code"
 fi
 
+if grep -Eq 'ld: warning: directory not found for option' "$stderr_path"; then
+  log "Linker failed to find a directory (probably a library directory) that should exist."
+  exit 1
+fi
+
 if is_clang &&
     [[ ${YB_ENABLE_STATIC_ANALYZER:-0} == "1" ]] &&
     ! is_thirdparty_build &&

build-support/run_pvs_studio_analyzer

@@ -0,0 +1,20 @@
+#!/usr/bin/env bash
+#
+# Copyright (c) Yugabyte, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except
+# in compliance with the License.  You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software distributed under the License
+# is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+# or implied.  See the License for the specific language governing permissions and limitations
+# under the License.
+#
+set -euo pipefail
+. "${BASH_SOURCE%/*}/common-build-env.sh"
+
+activate_virtualenv
+export PYTHONPATH=$YB_SRC_ROOT/python:${PYTHONPATH:-}
+"$YB_SRC_ROOT"/python/yb/run_pvs_studio_analyzer.py "$@"

ent/src/yb/master/catalog_manager_ent.cc

@@ -1089,12 +1089,12 @@ Status CatalogManager::ImportTableEntry(const NamespaceMap& namespace_map,
   }
 
   TRACE("Locking table");
-  auto l = table->LockForRead();
+  auto table_lock = table->LockForRead();
   vector<scoped_refptr<TabletInfo>> new_tablets;
   table->GetAllTablets(&new_tablets);
 
   for (const scoped_refptr<TabletInfo>& tablet : new_tablets) {
-    auto l = tablet->LockForRead();
+    auto tablet_lock = tablet->LockForRead();
     const PartitionPB& partition_pb = tablet->metadata().state().pb.partition();
     const ExternalTableSnapshotData::PartitionKeys key(
         partition_pb.partition_key_start(), partition_pb.partition_key_end());

ent/src/yb/tools/CMakeLists-include.txt

@@ -37,6 +37,4 @@ set(TOOLS_EXTENSIONS_TEST_LINK_LIBS
 set(TOOLS_EXTENSIONS_TESTS
   yb-admin-test_ent
   yb-backup-test_ent
-  PARENT_SCOPE)
-
-link_directories(${YB_BUILD_ROOT}/postgres/lib)
+  PARENT_SCOPE)

mypy.ini

@@ -0,0 +1,7 @@
+[mypy]
+disallow_untyped_defs = True
+disallow_untyped_calls = True
+disallow_incomplete_defs = True
+check_untyped_defs = True
+disallow_untyped_decorators = True
+disallow_any_unimported = True