You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by to...@apache.org on 2016/03/05 22:24:19 UTC
incubator-kudu git commit: Fix and improve code coverage
Repository: incubator-kudu
Updated Branches:
refs/heads/master e8969ff9b -> 48799d3fe
Fix and improve code coverage
- Switches the build to use clang coverage instead of gcc.
Now that we have a reasonably up-to-date clang, this actually works better
than gcc's coverage, and provides a more standard compiler that we know
everyone will have. I suspect that the coverage implementation may be
slightly lower overhead in clang as well, but couldn't find any public
benchmarks.
Another big advantage of clang coverage is that it properly chains together
the 'flush' calls across multiple shared objects. So, we can now use the
shared library debug build instead of static-linked one, decreasing
compile times for the coverage build.
- Adds a simple wrapper for 'llvm-cov gcov' because the clang coverage output
isn't 100% compatible with gcov's. 'gcovr' requires the wrapper script in
order to use llvm-cov.
- Changes our glog setup code to add hooks to flush coverage on crash or on
LOG(FATAL). I verified by running some coverage builds that this now includes
coverage data for files like fault_injection.cc which only run just before a
LOG(FATAL). This increased our measured coverage by a percentage point or
two.
Change-Id: If0273fe095d8f160f0068ec421fceb54eda588de
Reviewed-on: http://gerrit.cloudera.org:8080/2448
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/incubator-kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-kudu/commit/48799d3f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-kudu/tree/48799d3f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-kudu/diff/48799d3f
Branch: refs/heads/master
Commit: 48799d3fed53c79a5163fd9c4cd9cbae80a9f2d6
Parents: e8969ff
Author: Todd Lipcon <to...@apache.org>
Authored: Thu Mar 3 15:55:24 2016 -0800
Committer: Todd Lipcon <to...@apache.org>
Committed: Sat Mar 5 21:04:04 2016 +0000
----------------------------------------------------------------------
CMakeLists.txt | 19 +++-------
README.adoc | 19 ++++------
build-support/jenkins/build-and-test.sh | 14 ++++++-
build-support/llvm-gcov-wrapper | 23 ++++++++++++
src/kudu/server/generic_service.cc | 24 +++++-------
src/kudu/util/debug-util.cc | 26 +++++++++++++
src/kudu/util/debug-util.h | 8 ++++
src/kudu/util/logging.cc | 56 ++++++++++++++++++++++++++++
8 files changed, 148 insertions(+), 41 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/48799d3f/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 0c0c8f8..04cb3fb 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -374,22 +374,13 @@ endif()
# Code coverage
if ("${KUDU_GENERATE_COVERAGE}")
- if("${CMAKE_CXX_COMPILER}" MATCHES ".*clang.*")
- # There appears to be some bugs in clang 3.3 which cause code coverage
- # to have link errors, not locating the llvm_gcda_* symbols.
- # This should be fixed in llvm 3.4 with http://llvm.org/viewvc/llvm-project?view=revision&revision=184666
- message(SEND_ERROR "Cannot currently generate coverage with clang")
- endif()
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} --coverage -DCOVERAGE_BUILD")
- # For coverage to work properly, we need to use static linkage. Otherwise,
- # __gcov_flush() doesn't properly flush coverage from every module.
- # See http://stackoverflow.com/questions/28164543/using-gcov-flush-within-a-library-doesnt-force-the-other-modules-to-yield-gc
- if("${KUDU_LINK}" STREQUAL "a")
- message("Using static linking for coverage build")
- set(KUDU_LINK "s")
- elseif("${KUDU_LINK}" STREQUAL "d")
- message(SEND_ERROR "Cannot use coverage with static linking")
+ if(NOT "${COMPILER_FAMILY}" STREQUAL "clang")
+ # Use clang for coverage builds so that we can standardize on a single
+ # compiler. clang also handles flushing coverage from shared libraries
+ # better than gcc.
+ message(SEND_ERROR "Must use clang for coverage build")
endif()
endif()
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/48799d3f/README.adoc
----------------------------------------------------------------------
diff --git a/README.adoc b/README.adoc
index bb83b38..816fabe 100644
--- a/README.adoc
+++ b/README.adoc
@@ -225,26 +225,23 @@ and use the following flags:
----
$ mkdir -p build/coverage
$ cd build/coverage
-$ cmake -DKUDU_GENERATE_COVERAGE=1 ../..
+$ CC=../../thirdparty/clang-toolchain/bin/clang \
+ CXX=../../thirdparty/clang-toolchain/bin/clang++ \
+ cmake -DKUDU_GENERATE_COVERAGE=1 ../..
$ make -j4
$ ctest -j4
----
This will generate the code coverage files with extensions .gcno and .gcda. You can then
-use a tool like `lcov` or `gcovr` to visualize the results. For example, using gcovr:
+use a tool like `gcovr` or `llvm-cov gcov` to visualize the results. For example, using
+gcovr:
[source,bash]
----
$ mkdir cov_html
-$ ./thirdparty/gcovr-3.0/scripts/gcovr -r src/
-----
-
-Or using `lcov` (which seems to produce better HTML output):
-
-[source,bash]
-----
-$ lcov --capture --directory src --output-file coverage.info
-$ genhtml coverage.info --output-directory out
+$ cd build/coverage
+$ ../../thirdparty/gcovr-3.0/scripts/gcovr -r ../../src/ \
+ --gcov-executable=../../build-support/llvm-gcov-wrapper
----
=== Running lint checks
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/48799d3f/build-support/jenkins/build-and-test.sh
----------------------------------------------------------------------
diff --git a/build-support/jenkins/build-and-test.sh b/build-support/jenkins/build-and-test.sh
index a00ea5b..91eaca5 100755
--- a/build-support/jenkins/build-and-test.sh
+++ b/build-support/jenkins/build-and-test.sh
@@ -177,7 +177,13 @@ elif [ "$BUILD_TYPE" = "TSAN" ]; then
elif [ "$BUILD_TYPE" = "COVERAGE" ]; then
DO_COVERAGE=1
BUILD_TYPE=debug
- $SOURCE_ROOT/build-support/enable_devtoolset.sh "$THIRDPARTY_BIN/cmake -DKUDU_GENERATE_COVERAGE=1 $SOURCE_ROOT"
+
+ # We currently dont capture coverage for Java or Python.
+ BUILD_JAVA=0
+ BUILD_PYTHON=0
+
+ $SOURCE_ROOT/build-support/enable_devtoolset.sh \
+ "env CC=$CLANG CXX=$CLANG++ $THIRDPARTY_BIN/cmake -DKUDU_GENERATE_COVERAGE=1 $SOURCE_ROOT"
elif [ "$BUILD_TYPE" = "LINT" ]; then
# Create empty test logs or else Jenkins fails to archive artifacts, which
# results in the build failing.
@@ -296,7 +302,11 @@ if [ "$DO_COVERAGE" == "1" ]; then
echo
echo Generating coverage report...
echo ------------------------------------------------------------
- if ! $SOURCE_ROOT/thirdparty/gcovr-3.0/scripts/gcovr -r $SOURCE_ROOT --xml \
+ if ! $SOURCE_ROOT/thirdparty/gcovr-3.0/scripts/gcovr \
+ -r $SOURCE_ROOT \
+ --gcov-filter='.*src#kudu.*' \
+ --gcov-executable=$SOURCE_ROOT/build-support/llvm-gcov-wrapper \
+ --xml \
> $BUILD_ROOT/coverage.xml ; then
EXIT_STATUS=1
FAILURES="$FAILURES"$'Coverage report failed\n'
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/48799d3f/build-support/llvm-gcov-wrapper
----------------------------------------------------------------------
diff --git a/build-support/llvm-gcov-wrapper b/build-support/llvm-gcov-wrapper
new file mode 100755
index 0000000..0b24685
--- /dev/null
+++ b/build-support/llvm-gcov-wrapper
@@ -0,0 +1,23 @@
+#!/bin/bash -e
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you 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.
+
+# This script acts as a wrapper around 'llvm-cov gcov' so that
+# gcovr can call it.
+
+SOURCE_ROOT=$(cd $(dirname "$BASH_SOURCE")/..; pwd)
+exec $SOURCE_ROOT/thirdparty/clang-toolchain/bin/llvm-cov gcov "$@"
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/48799d3f/src/kudu/server/generic_service.cc
----------------------------------------------------------------------
diff --git a/src/kudu/server/generic_service.cc b/src/kudu/server/generic_service.cc
index 4d1d9f3..160e109 100644
--- a/src/kudu/server/generic_service.cc
+++ b/src/kudu/server/generic_service.cc
@@ -26,6 +26,7 @@
#include "kudu/server/clock.h"
#include "kudu/server/hybrid_clock.h"
#include "kudu/server/server_base.h"
+#include "kudu/util/debug-util.h"
#include "kudu/util/flag_tags.h"
DECLARE_bool(use_mock_wall_clock);
@@ -34,11 +35,6 @@ DECLARE_bool(use_hybrid_clock);
using std::string;
using std::unordered_set;
-#ifdef COVERAGE_BUILD
-extern "C" void __gcov_flush(void);
-#endif
-
-
namespace kudu {
namespace server {
@@ -101,15 +97,15 @@ void GenericServiceImpl::SetFlag(const SetFlagRequestPB* req,
void GenericServiceImpl::FlushCoverage(const FlushCoverageRequestPB* req,
FlushCoverageResponsePB* resp,
rpc::RpcContext* rpc) {
-#ifdef COVERAGE_BUILD
- __gcov_flush();
- LOG(INFO) << "Flushed coverage info. (request from " << rpc->requestor_string() << ")";
- resp->set_success(true);
-#else
- LOG(WARNING) << "Non-coverage build cannot flush coverage (request from "
- << rpc->requestor_string() << ")";
- resp->set_success(false);
-#endif
+ if (IsCoverageBuild()) {
+ TryFlushCoverage();
+ LOG(INFO) << "Flushed coverage info. (request from " << rpc->requestor_string() << ")";
+ resp->set_success(true);
+ } else {
+ LOG(WARNING) << "Non-coverage build cannot flush coverage (request from "
+ << rpc->requestor_string() << ")";
+ resp->set_success(false);
+ }
rpc->RespondSuccess();
}
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/48799d3f/src/kudu/util/debug-util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/debug-util.cc b/src/kudu/util/debug-util.cc
index daabc8a..3adfdd0 100644
--- a/src/kudu/util/debug-util.cc
+++ b/src/kudu/util/debug-util.cc
@@ -37,6 +37,13 @@
typedef sig_t sighandler_t;
#endif
+// In coverage builds, this symbol will be defined and allows us to flush coverage info
+// to disk before exiting.
+extern "C" {
+__attribute__((weak))
+void __gcov_flush();
+}
+
// Evil hack to grab a few useful functions from glog
namespace google {
@@ -69,6 +76,25 @@ static base::SpinLock g_dumper_thread_lock(base::LINKER_INITIALIZED);
namespace kudu {
+bool IsCoverageBuild() {
+ return __gcov_flush != nullptr;
+}
+
+void TryFlushCoverage() {
+ static base::SpinLock lock(base::LINKER_INITIALIZED);
+
+ // Flushing coverage is not reentrant or thread-safe.
+ if (!__gcov_flush || !lock.TryLock()) {
+ return;
+ }
+
+ __gcov_flush();
+
+ lock.Unlock();
+}
+
+
+
namespace {
// Global structure used to communicate between the signal handler
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/48799d3f/src/kudu/util/debug-util.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/debug-util.h b/src/kudu/util/debug-util.h
index e61f8f0..2617520 100644
--- a/src/kudu/util/debug-util.h
+++ b/src/kudu/util/debug-util.h
@@ -27,6 +27,14 @@
namespace kudu {
+// Return true if coverage is enabled.
+bool IsCoverageBuild();
+
+// Try to flush coverage info. If another thread is already flushing
+// coverage, this returns without doing anything, since flushing coverage
+// is not thread-safe or re-entrant.
+void TryFlushCoverage();
+
// Return a list of all of the thread IDs currently running in this process.
// Not async-safe.
Status ListThreads(std::vector<pid_t>* tids);
http://git-wip-us.apache.org/repos/asf/incubator-kudu/blob/48799d3f/src/kudu/util/logging.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/logging.cc b/src/kudu/util/logging.cc
index 8edd20a..cfe57f8 100644
--- a/src/kudu/util/logging.cc
+++ b/src/kudu/util/logging.cc
@@ -32,6 +32,7 @@
#include <boost/uuid/uuid.hpp>
#include <boost/uuid/uuid_io.hpp>
#include <boost/uuid/uuid_generators.hpp>
+#include <mutex>
#include <sstream>
#include <stdio.h>
#include <iostream>
@@ -40,6 +41,7 @@
#include "kudu/gutil/callback.h"
#include "kudu/gutil/spinlock.h"
+#include "kudu/util/debug-util.h"
#include "kudu/util/flag_tags.h"
DEFINE_string(log_filename, "",
@@ -111,6 +113,7 @@ SimpleSink* registered_sink = nullptr;
// Protected by 'logging_mutex'.
int initial_stderr_severity;
+
void UnregisterLoggingCallbackUnlocked() {
CHECK(logging_mutex.IsHeld());
CHECK(registered_sink);
@@ -123,6 +126,48 @@ void UnregisterLoggingCallbackUnlocked() {
registered_sink = nullptr;
}
+void FlushCoverageOnExit() {
+ // Coverage flushing is not re-entrant, but this might be called from a
+ // crash signal context, so avoid re-entrancy.
+ static __thread bool in_call = false;
+ if (in_call) return;
+ in_call = true;
+
+ // The failure writer will be called multiple times per exit.
+ // We only need to flush coverage once. We use a 'once' here so that,
+ // if another thread is already flushing, we'll block and wait for them
+ // to finish before allowing this thread to call abort().
+ static std::once_flag once;
+ std::call_once(once, [] {
+ static const char msg[] = "Flushing coverage data before crash...\n";
+ write(STDERR_FILENO, msg, arraysize(msg));
+ TryFlushCoverage();
+ });
+ in_call = false;
+}
+
+// On SEGVs, etc, glog will call this function to write the error to stderr. This
+// implementation is copied from glog with the exception that we also flush coverage
+// the first time it's called.
+//
+// NOTE: this is only used in coverage builds!
+void FailureWriterWithCoverage(const char* data, int size) {
+ FlushCoverageOnExit();
+
+ // Original implementation from glog:
+ if (write(STDERR_FILENO, data, size) < 0) {
+ // Ignore errors.
+ }
+}
+
+// GLog "failure function". This is called in the case of LOG(FATAL) to
+// ensure that we flush coverage even on crashes.
+//
+// NOTE: this is only used in coverage builds!
+void FlushCoverageAndAbort() {
+ FlushCoverageOnExit();
+ abort();
+}
} // anonymous namespace
void InitGoogleLoggingSafe(const char* arg) {
@@ -165,6 +210,17 @@ void InitGoogleLoggingSafe(const char* arg) {
google::InitGoogleLogging(arg);
+ // In coverage builds, we should flush coverage before exiting on crash.
+ // This way, fault injection tests still capture coverage of the daemon
+ // that "crashed".
+ if (IsCoverageBuild()) {
+ // We have to use both the "failure writer" and the "FailureFunction".
+ // This allows us to handle both LOG(FATAL) and unintended crashes like
+ // SEGVs.
+ google::InstallFailureWriter(FailureWriterWithCoverage);
+ google::InstallFailureFunction(FlushCoverageAndAbort);
+ }
+
// Needs to be done after InitGoogleLogging
if (FLAGS_log_filename.empty()) {
CHECK_STRNE(google::ProgramInvocationShortName(), "UNKNOWN")