From 21869679878a3d578adb6bc4ed8eaacb9f86ac3c Mon Sep 17 00:00:00 2001 From: Aliaksey Kandratsenka Date: Wed, 21 Jun 2023 10:39:35 -0400 Subject: [PATCH] fix heap checker unittest We had shell wrapper for heap checker unittest, but it failed to deal with heap-checker-debug variant. So we now posix_spawn from .cc test instead. --- CMakeLists.txt | 5 +- Makefile.am | 20 +------ src/tests/heap-checker_unittest.cc | 41 ++++++++++++++ src/tests/heap-checker_unittest.sh | 89 ------------------------------ 4 files changed, 45 insertions(+), 110 deletions(-) delete mode 100755 src/tests/heap-checker_unittest.sh diff --git a/CMakeLists.txt b/CMakeLists.txt index 9c17337..fd9c562 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1232,8 +1232,7 @@ if(GPERFTOOLS_BUILD_HEAP_CHECKER OR GPERFTOOLS_BUILD_HEAP_PROFILER) ${HEAP_CHECKER_UNITTEST_INCLUDES}) add_executable(heap_checker_unittest ${heap_checker_unittest_SOURCES}) target_link_libraries(heap_checker_unittest ${TCMALLOC_FLAGS} tcmalloc logging Threads::Threads) - add_test(NAME heap-checker_unittest.sh - COMMAND "${CMAKE_CURRENT_SOURCE_DIR}/src/tests/heap-checker_unittest.sh" heap_checker_unittest) + add_test(heap-checker_unittest heap_checker_unittest) add_test(NAME heap-checker-death_unittest.sh COMMAND "${CMAKE_CURRENT_SOURCE_DIR}/src/tests/heap-checker-death_unittest.sh") endif() @@ -1289,7 +1288,7 @@ if(GPERFTOOLS_BUILD_DEBUGALLOC) if(GPERFTOOLS_BUILD_HEAP_CHECKER) add_executable(heap_checker_debug_unittest ${heap_checker_unittest_SOURCES}) target_link_libraries(heap_checker_debug_unittest ${TCMALLOC_FLAGS} tcmalloc_debug logging Threads::Threads) - add_test(heap-checker_debug_unittest.sh "${CMAKE_CURRENT_SOURCE_DIR}/src/tests/heap-checker_unittest.sh" heap-checker_debug_unittest) + add_test(heap_checker_debug_unittest heap_checker_debug_unittest) endif() endif() endif() diff --git a/Makefile.am b/Makefile.am index 307c151..4b3bde7 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1119,14 +1119,6 @@ endif WITH_HEAP_PROFILER if WITH_HEAP_CHECKER -TESTS += heap-checker_unittest.sh$(EXEEXT) -heap_checker_unittest_sh_SOURCES = src/tests/heap-checker_unittest.sh -noinst_SCRIPTS += $(heap_checker_unittest_sh_SOURCES) -heap-checker_unittest.sh$(EXEEXT): $(top_srcdir)/$(heap_checker_unittest_sh_SOURCES) \ - heap-checker_unittest - rm -f $@ - cp -p $(top_srcdir)/$(heap_checker_unittest_sh_SOURCES) $@ - TESTS += heap-checker-death_unittest.sh$(EXEEXT) heap_checker_death_unittest_sh_SOURCES = src/tests/heap-checker-death_unittest.sh noinst_SCRIPTS += $(top_srcdir)/$(heap_checker_death_unittest_sh_SOURCES) @@ -1136,7 +1128,7 @@ heap-checker-death_unittest.sh$(EXEEXT): $(heap_checker_death_unittest_sh_SOURCE cp -p $(top_srcdir)/$(heap_checker_death_unittest_sh_SOURCES) $@ # These are sub-programs used by heap-checker_unittest.sh -noinst_PROGRAMS += heap-checker_unittest +TESTS += heap-checker_unittest HEAP_CHECKER_UNITTEST_INCLUDES = src/config_for_unittests.h \ src/memory_region_map.h \ src/base/commandlineflags.h \ @@ -1236,15 +1228,7 @@ endif WITH_HEAP_PROFILER if WITH_HEAP_CHECKER -TESTS += heap-checker_debug_unittest.sh$(EXEEXT) -heap_checker_debug_unittest_sh_SOURCES = src/tests/heap-checker_unittest.sh -heap-checker_debug_unittest.sh$(EXEEXT): $(top_srcdir)/$(heap_checker_unittest_sh_SOURCES) \ - heap-checker_debug_unittest - rm -f $@ - cp -p $(top_srcdir)/$(heap_checker_unittest_sh_SOURCES) $@ - -# These are sub-programs used by heap-checker_debug_unittest.sh -noinst_PROGRAMS += heap-checker_debug_unittest +TESTS += heap-checker_debug_unittest heap_checker_debug_unittest_SOURCES = $(heap_checker_unittest_SOURCES) heap_checker_debug_unittest_CXXFLAGS = $(heap_checker_unittest_CXXFLAGS) heap_checker_debug_unittest_LDFLAGS = $(heap_checker_unittest_LDFLAGS) diff --git a/src/tests/heap-checker_unittest.cc b/src/tests/heap-checker_unittest.cc index b4bcb52..d86eebc 100644 --- a/src/tests/heap-checker_unittest.cc +++ b/src/tests/heap-checker_unittest.cc @@ -86,6 +86,8 @@ #ifdef HAVE_PWD_H #include #endif +#include // for posix_spawn +#include // waitpid etc #include #include // for cout @@ -1377,7 +1379,46 @@ static int Pass() { return 0; } +bool spawn_subtest(const char* mode, char** argv) { + putenv(strdup((std::string{"HEAPCHECK="} + std::string(mode)).c_str())); + int pid; + printf("Spawning heapcheck test with HEAPCHECK=%s...\n", mode); + + int rv = posix_spawn(&pid, "/proc/self/exe", nullptr, nullptr, argv, environ); + if (rv != 0) { + errno = rv; + perror("posix_spawn"); + abort(); + } + do { + if (waitpid(pid, &rv, 0) < 0) { + if (errno == EINTR) { + continue; // try again + } + perror("waitpid"); + abort(); + } + } while (false); + if (!WIFEXITED(rv)) { + LOGF << std::hex << "weird waitpid status: " << rv; + LOG(FATAL, "\n"); + } + + rv = WEXITSTATUS(rv); + printf("Test in mode %s finished with status %d\n", mode, rv); + + return (rv == 0); +} + int main(int argc, char** argv) { + if (getenv("HEAPCHECK") == nullptr) { + CHECK(!HeapLeakChecker::IsActive()); + + bool ok = (spawn_subtest("", argv) && spawn_subtest("local", argv) + && spawn_subtest("normal", argv) && spawn_subtest("strict", argv)); + return ok ? 0 : 1; + } + run_hidden_ptr = DoRunHidden; wipe_stack_ptr = DoWipeStack; if (!HeapLeakChecker::IsActive()) { diff --git a/src/tests/heap-checker_unittest.sh b/src/tests/heap-checker_unittest.sh deleted file mode 100755 index 3c9c0e9..0000000 --- a/src/tests/heap-checker_unittest.sh +++ /dev/null @@ -1,89 +0,0 @@ -#!/bin/sh - -# Copyright (c) 2005, Google Inc. -# All rights reserved. -# -# Redistribution and use in source and binary forms, with or without -# modification, are permitted provided that the following conditions are -# met: -# -# * Redistributions of source code must retain the above copyright -# notice, this list of conditions and the following disclaimer. -# * Redistributions in binary form must reproduce the above -# copyright notice, this list of conditions and the following disclaimer -# in the documentation and/or other materials provided with the -# distribution. -# * Neither the name of Google Inc. nor the names of its -# contributors may be used to endorse or promote products derived from -# this software without specific prior written permission. -# -# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - -# --- -# Author: Craig Silverstein -# -# Runs the heap-checker unittest with various environment variables. -# This is necessary because we turn on features like the heap profiler -# and heap checker via environment variables. This test makes sure -# they all play well together. - -# We expect BINDIR and PPROF_PATH to be set in the environment. -# If not, we set them to some reasonable values -BINDIR="${BINDIR:-.}" -PPROF_PATH="${PPROF_PATH:-$BINDIR/src/pprof}" - -if [ "x$1" = "x-h" -o "$1" = "x--help" ]; then - echo "USAGE: $0 [unittest dir] [path to pprof]" - echo " By default, unittest_dir=$BINDIR, pprof_path=$PPROF_PATH" - exit 1 -fi - -HEAP_CHECKER="${1:-$BINDIR/heap-checker_unittest}" -PPROF_PATH="${2:-$PPROF_PATH}" - -TMPDIR=/tmp/heap_check_info -rm -rf $TMPDIR || exit 2 -mkdir $TMPDIR || exit 3 - -# $1: value of heap-check env. var. -run_check() { - export PPROF_PATH="$PPROF_PATH" - [ -n "$1" ] && export HEAPCHECK="$1" || unset HEAPPROFILE - - echo -n "Testing $HEAP_CHECKER with HEAPCHECK=$1 ... " - if $HEAP_CHECKER > $TMPDIR/output 2>&1; then - echo "OK" - else - echo "FAILED" - echo "Output from the failed run:" - echo "----" - cat $TMPDIR/output - echo "----" - exit 4 - fi - - # If we set HEAPPROFILE, then we expect it to actually have emitted - # a profile. Check that it did. - if [ -n "$HEAPPROFILE" ]; then - [ -e "$HEAPPROFILE.0001.heap" ] || exit 5 - fi -} - -run_check "" -run_check "local" -run_check "normal" -run_check "strict" - -rm -rf $TMPDIR # clean up - -echo "PASS"