Skip to content

Commit

Permalink
common: Re-enable limit_malloc sanitizer tests (RobotLocomotion#10830)
Browse files Browse the repository at this point in the history
The limit_malloc death tests are still excluded from all sanitizer and memcheck
builds, but the non-death tests all pass now.
  • Loading branch information
jwnimmer-tri authored Mar 6, 2019
1 parent d8b31a4 commit a9efd5c
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 22 deletions.
40 changes: 27 additions & 13 deletions common/test_utilities/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ drake_cc_library(
testonly = 1,
srcs = ["limit_malloc.cc"],
hdrs = ["limit_malloc.h"],
copts = [
# We shouldn't call __tsan_func_entry from allocations during dl_init.
"-fno-sanitize=thread",
],
tags = [
# The limit_malloc library be used sparingly, and not by default.
# Don't add this library into the ":test_utilities" package library.
Expand Down Expand Up @@ -135,24 +139,34 @@ drake_cc_library(

drake_cc_googletest(
name = "limit_malloc_test",
args = [
# By default, we will skip the death tests because they only pass on
# non-Apple, non-Sanitizer, non-Memcheck builds. We will explicitly
# run them (below) for the configurations that succeed.
"--gtest_filter=-*DeathTest*",
],
deps = [
":limit_malloc",
],
)

# N.B. All sh_tests are excluded from dynamic_analysis (sanitizer and memcheck)
# build by default; these death tests are skipped under those configurations.
sh_test(
name = "limit_malloc_death_test",
srcs = [":limit_malloc_test"],
args = select({
"//tools/cc_toolchain:apple": [
# The code to enforce malloc limits is not implemented on macOS.
"--gtest_filter=-*DeathTest*",
"--gtest_filter=-*",
],
"//conditions:default": [
# Run only the death tests, not the plain tests.
"--gtest_filter=*DeathTest*",
],
"//conditions:default": [],
}),
tags = [
# TODO(jwnimmer-tri) Clang ASan and TSan either segfault or abort with
# LimitMalloc. Investigate, fix, and re-enable this test.
"no_asan",
"no_tsan",
# TODO(jwnimmer-tri) The DeathTests fail their test conditions when run
# under Valgrind. Investigate, fix, and re-enable this test.
"no_memcheck",
],
deps = [
":limit_malloc",
data = [
":limit_malloc_test",
],
)

Expand Down
42 changes: 33 additions & 9 deletions common/test_utilities/limit_malloc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,22 @@
#include <type_traits>
#include <utility>

// Functions that are called during dl_init must not use the sanitizer runtime.
#ifdef __clang__
#define DRAKE_NO_SANITIZE __attribute__((no_sanitize("address")))
#else
#define DRAKE_NO_SANITIZE
#endif

namespace drake {
namespace test {
namespace {

// This variable is used as an early short-circuit for our malloc hooks. When
// false, we execute as minimal code footprint as possible. This keeps dl_init
// happy when its invoke malloc and we can't yet execute our C++ code.
volatile bool g_any_monitor_has_ever_been_installed = false;

// A set of limits and current tallies.
class Monitor {
public:
Expand Down Expand Up @@ -72,6 +84,7 @@ class ActiveMonitor {
// Swaps the currently-active monitor, returning the prior one.
static std::shared_ptr<Monitor> exchange(std::shared_ptr<Monitor> next) {
auto& self = singleton();
g_any_monitor_has_ever_been_installed = true;
std::lock_guard<std::mutex> guard(self.mutex_);
self.active_.swap(next);
return next;
Expand All @@ -84,22 +97,31 @@ class ActiveMonitor {
}

// To be called by our hooks when an allocation attempt occurs,
DRAKE_NO_SANITIZE
static void malloc(size_t size) {
auto monitor = singleton().load();
if (monitor) {
monitor->malloc(size);
if (g_any_monitor_has_ever_been_installed) {
auto monitor = load();
if (monitor) {
monitor->malloc(size);
}
}
}
DRAKE_NO_SANITIZE
static void calloc(size_t nmemb, size_t size) {
auto monitor = singleton().load();
if (monitor) {
monitor->calloc(nmemb, size);
if (g_any_monitor_has_ever_been_installed) {
auto monitor = load();
if (monitor) {
monitor->calloc(nmemb, size);
}
}
}
DRAKE_NO_SANITIZE
static void realloc(void* ptr, size_t size, bool is_noop) {
auto monitor = singleton().load();
if (monitor) {
monitor->realloc(ptr, size, is_noop);
if (g_any_monitor_has_ever_been_installed) {
auto monitor = load();
if (monitor) {
monitor->realloc(ptr, size, is_noop);
}
}
}

Expand Down Expand Up @@ -187,3 +209,5 @@ void* realloc(void* ptr, size_t size) {
return result;
}
#endif

#undef DRAKE_NO_SANITIZE

0 comments on commit a9efd5c

Please sign in to comment.