summaryrefslogtreecommitdiff
path: root/indra
diff options
context:
space:
mode:
authorNat Goodspeed <nat@lindenlab.com>2017-05-08 09:09:22 -0400
committerNat Goodspeed <nat@lindenlab.com>2017-05-08 09:09:22 -0400
commit322c4c6bec54b4968d0105cf1bb28bb62c6dfcbc (patch)
treef0e7258b12c232a72cbd3618cc54f7f74de7de3a /indra
parenta4467e9ffe9dc378451d7bb6b8c70f6c0c42a814 (diff)
DRTVWR-418: Fix -std=c++11 llinstancetracker_test crash.
LLInstanceTracker<T> performs validation in ~LLInstanceTracker(). Normally validation failure logs an error and terminates the program, which is fine. In the test executable, though, we want validation failure to throw an exception instead so we can catch it and continue testing other failure conditions. But since destructors in C++11 are implicitly noexcept(true), that exception never made it out of ~LLInstanceTracker(): it crashed the test program instead. Declaring ~LLInstanceTracker() noexcept(false) solves that, allowing the test program to catch the exception and continue. However, if we unconditionally declare that, then every destructor anywhere in the inheritance hierarchy for any LLInstanceTracker subclass must also be noexcept(false)! That's way too pervasive, especially for functionality we only need (or want) in a specific test executable. Instead, make the CMake macros LL_ADD_PROJECT_UNIT_TESTS() and LL_ADD_INTEGRATION_TEST() -- with which we define all viewer build-time tests -- define two new command-line macros: LL_TEST=testname and LL_TEST_testname. That way, preprocessor logic in a header file can detect whether it's being compiled for production code or for a test executable. (While at it, encapsulate in a new GET_OPT_SOURCE_FILE_PROPERTY() CMake macro an ugly repetitive pattern. The builtin GET_SOURCE_FILE_PROPERTY() sets the target variable to "NOTFOUND" -- rather than an empty string -- if the specified property wasn't set. Every call to GET_SOURCE_FILE_PROPERTY() in LL_ADD_PROJECT_UNIT_TESTS() was followed by a test for NOTFOUND and an assignment to "". Wrap all that in a macro whose 'unset' value is "".) Now llinstancetracker.h can detect when we're building the LLInstanceTracker unit test executable, and *only then* declare ~LLInstanceTracker() as noexcept(false). We #define LLINSTANCETRACKER_DTOR_NOEXCEPT to expand either empty or noexcept(false), also detecting clang in C++11 mode. (It all works fine without noexcept(false) until we turn on C++11 mode.) We also use that macro for the StatBase class in lltrace.h. Turns out some of the infrastructure headers required for tests in general, including the LLInstanceTracker test, use LLInstanceTracker. Fortunately that appears to be the only other class we must annotate this way for the LLInstanceTracker tests.
Diffstat (limited to 'indra')
-rw-r--r--indra/cmake/LLAddBuildTest.cmake73
-rw-r--r--indra/llcommon/llinstancetracker.h24
-rw-r--r--indra/llcommon/lltrace.h2
3 files changed, 65 insertions, 34 deletions
diff --git a/indra/cmake/LLAddBuildTest.cmake b/indra/cmake/LLAddBuildTest.cmake
index 96d3e39a06..024bfe14a1 100644
--- a/indra/cmake/LLAddBuildTest.cmake
+++ b/indra/cmake/LLAddBuildTest.cmake
@@ -3,6 +3,9 @@ include(LLTestCommand)
include(GoogleMock)
include(Tut)
+#*****************************************************************************
+# LL_ADD_PROJECT_UNIT_TESTS
+#*****************************************************************************
MACRO(LL_ADD_PROJECT_UNIT_TESTS project sources)
# Given a project name and a list of sourcefiles (with optional properties on each),
# add targets to build and run the tests specified.
@@ -74,19 +77,17 @@ INCLUDE(GoogleMock)
# Per-codefile additional / external source, header, and include dir property extraction
#
# Source
- GET_SOURCE_FILE_PROPERTY(${name}_test_additional_SOURCE_FILES ${source} LL_TEST_ADDITIONAL_SOURCE_FILES)
- IF(${name}_test_additional_SOURCE_FILES MATCHES NOTFOUND)
- SET(${name}_test_additional_SOURCE_FILES "")
- ENDIF(${name}_test_additional_SOURCE_FILES MATCHES NOTFOUND)
- SET(${name}_test_SOURCE_FILES ${source} tests/${name}_test.${extension} ${alltest_SOURCE_FILES} ${${name}_test_additional_SOURCE_FILES} )
+ GET_OPT_SOURCE_FILE_PROPERTY(${name}_test_additional_SOURCE_FILES ${source} LL_TEST_ADDITIONAL_SOURCE_FILES)
+ SET(${name}_test_SOURCE_FILES
+ ${source}
+ tests/${name}_test.${extension}
+ ${alltest_SOURCE_FILES}
+ ${${name}_test_additional_SOURCE_FILES} )
IF(LL_TEST_VERBOSE)
MESSAGE("LL_ADD_PROJECT_UNIT_TESTS ${name}_test_SOURCE_FILES ${${name}_test_SOURCE_FILES}")
ENDIF(LL_TEST_VERBOSE)
# Headers
- GET_SOURCE_FILE_PROPERTY(${name}_test_additional_HEADER_FILES ${source} LL_TEST_ADDITIONAL_HEADER_FILES)
- IF(${name}_test_additional_HEADER_FILES MATCHES NOTFOUND)
- SET(${name}_test_additional_HEADER_FILES "")
- ENDIF(${name}_test_additional_HEADER_FILES MATCHES NOTFOUND)
+ GET_OPT_SOURCE_FILE_PROPERTY(${name}_test_additional_HEADER_FILES ${source} LL_TEST_ADDITIONAL_HEADER_FILES)
SET(${name}_test_HEADER_FILES ${name}.h ${${name}_test_additional_HEADER_FILES})
set_source_files_properties(${${name}_test_HEADER_FILES} PROPERTIES HEADER_FILE_ONLY TRUE)
LIST(APPEND ${name}_test_SOURCE_FILES ${${name}_test_HEADER_FILES})
@@ -94,10 +95,7 @@ INCLUDE(GoogleMock)
MESSAGE("LL_ADD_PROJECT_UNIT_TESTS ${name}_test_HEADER_FILES ${${name}_test_HEADER_FILES}")
ENDIF(LL_TEST_VERBOSE)
# Include dirs
- GET_SOURCE_FILE_PROPERTY(${name}_test_additional_INCLUDE_DIRS ${source} LL_TEST_ADDITIONAL_INCLUDE_DIRS)
- IF(${name}_test_additional_INCLUDE_DIRS MATCHES NOTFOUND)
- SET(${name}_test_additional_INCLUDE_DIRS "")
- ENDIF(${name}_test_additional_INCLUDE_DIRS MATCHES NOTFOUND)
+ GET_OPT_SOURCE_FILE_PROPERTY(${name}_test_additional_INCLUDE_DIRS ${source} LL_TEST_ADDITIONAL_INCLUDE_DIRS)
INCLUDE_DIRECTORIES(${alltest_INCLUDE_DIRS} ${${name}_test_additional_INCLUDE_DIRS} )
IF(LL_TEST_VERBOSE)
MESSAGE("LL_ADD_PROJECT_UNIT_TESTS ${name}_test_additional_INCLUDE_DIRS ${${name}_test_additional_INCLUDE_DIRS}")
@@ -113,15 +111,9 @@ INCLUDE(GoogleMock)
#
# WARNING: it's REALLY IMPORTANT to not mix these. I guarantee it will not work in the future. + poppy 2009-04-19
# Projects
- GET_SOURCE_FILE_PROPERTY(${name}_test_additional_PROJECTS ${source} LL_TEST_ADDITIONAL_PROJECTS)
- IF(${name}_test_additional_PROJECTS MATCHES NOTFOUND)
- SET(${name}_test_additional_PROJECTS "")
- ENDIF(${name}_test_additional_PROJECTS MATCHES NOTFOUND)
+ GET_OPT_SOURCE_FILE_PROPERTY(${name}_test_additional_PROJECTS ${source} LL_TEST_ADDITIONAL_PROJECTS)
# Libraries
- GET_SOURCE_FILE_PROPERTY(${name}_test_additional_LIBRARIES ${source} LL_TEST_ADDITIONAL_LIBRARIES)
- IF(${name}_test_additional_LIBRARIES MATCHES NOTFOUND)
- SET(${name}_test_additional_LIBRARIES "")
- ENDIF(${name}_test_additional_LIBRARIES MATCHES NOTFOUND)
+ GET_OPT_SOURCE_FILE_PROPERTY(${name}_test_additional_LIBRARIES ${source} LL_TEST_ADDITIONAL_LIBRARIES)
IF(LL_TEST_VERBOSE)
MESSAGE("LL_ADD_PROJECT_UNIT_TESTS ${name}_test_additional_PROJECTS ${${name}_test_additional_PROJECTS}")
MESSAGE("LL_ADD_PROJECT_UNIT_TESTS ${name}_test_additional_LIBRARIES ${${name}_test_additional_LIBRARIES}")
@@ -129,13 +121,14 @@ INCLUDE(GoogleMock)
# Add to project
TARGET_LINK_LIBRARIES(PROJECT_${project}_TEST_${name} ${alltest_LIBRARIES} ${alltest_DEP_TARGETS} ${${name}_test_additional_PROJECTS} ${${name}_test_additional_LIBRARIES} )
# Compile-time Definitions
- GET_SOURCE_FILE_PROPERTY(${name}_test_additional_CFLAGS ${source} LL_TEST_ADDITIONAL_CFLAGS)
- IF(NOT ${name}_test_additional_CFLAGS MATCHES NOTFOUND)
- SET_TARGET_PROPERTIES(PROJECT_${project}_TEST_${name} PROPERTIES COMPILE_FLAGS ${${name}_test_additional_CFLAGS} )
- IF(LL_TEST_VERBOSE)
- MESSAGE("LL_ADD_PROJECT_UNIT_TESTS ${name}_test_additional_CFLAGS ${${name}_test_additional_CFLAGS}")
- ENDIF(LL_TEST_VERBOSE)
- ENDIF(NOT ${name}_test_additional_CFLAGS MATCHES NOTFOUND)
+ GET_OPT_SOURCE_FILE_PROPERTY(${name}_test_additional_CFLAGS ${source} LL_TEST_ADDITIONAL_CFLAGS)
+ SET_TARGET_PROPERTIES(PROJECT_${project}_TEST_${name}
+ PROPERTIES
+ COMPILE_FLAGS "${${name}_test_additional_CFLAGS}"
+ COMPILE_DEFINITIONS "LL_TEST=${name};LL_TEST_${name}")
+ IF(LL_TEST_VERBOSE)
+ MESSAGE("LL_ADD_PROJECT_UNIT_TESTS ${name}_test_additional_CFLAGS ${${name}_test_additional_CFLAGS}")
+ ENDIF(LL_TEST_VERBOSE)
#
# Setup test targets
@@ -175,6 +168,19 @@ INCLUDE(GoogleMock)
ADD_DEPENDENCIES(${project} ${project}_tests)
ENDMACRO(LL_ADD_PROJECT_UNIT_TESTS)
+#*****************************************************************************
+# GET_OPT_SOURCE_FILE_PROPERTY
+#*****************************************************************************
+MACRO(GET_OPT_SOURCE_FILE_PROPERTY var filename property)
+ GET_SOURCE_FILE_PROPERTY(${var} "${filename}" "${property}")
+ IF("${${var}}" MATCHES NOTFOUND)
+ SET(${var} "")
+ ENDIF("${${var}}" MATCHES NOTFOUND)
+ENDMACRO(GET_OPT_SOURCE_FILE_PROPERTY)
+
+#*****************************************************************************
+# LL_ADD_INTEGRATION_TEST
+#*****************************************************************************
FUNCTION(LL_ADD_INTEGRATION_TEST
testname
additional_source_files
@@ -184,7 +190,7 @@ FUNCTION(LL_ADD_INTEGRATION_TEST
if(TEST_DEBUG)
message(STATUS "Adding INTEGRATION_TEST_${testname} - debug output is on")
endif(TEST_DEBUG)
-
+
SET(source_files
tests/${testname}_test.cpp
${CMAKE_SOURCE_DIR}/test/test.cpp
@@ -206,7 +212,11 @@ FUNCTION(LL_ADD_INTEGRATION_TEST
message(STATUS "ADD_EXECUTABLE(INTEGRATION_TEST_${testname} ${source_files})")
endif(TEST_DEBUG)
ADD_EXECUTABLE(INTEGRATION_TEST_${testname} ${source_files})
- SET_TARGET_PROPERTIES(INTEGRATION_TEST_${testname} PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${EXE_STAGING_DIR}")
+ SET_TARGET_PROPERTIES(INTEGRATION_TEST_${testname}
+ PROPERTIES
+ RUNTIME_OUTPUT_DIRECTORY "${EXE_STAGING_DIR}"
+ COMPILE_DEFINITIONS "LL_TEST=${testname};LL_TEST_${testname}"
+ )
if(USESYSTEMLIBS)
SET_TARGET_PROPERTIES(INTEGRATION_TEST_${testname} PROPERTIES COMPILE_FLAGS -I"${TUT_INCLUDE_DIR}")
@@ -268,6 +278,9 @@ FUNCTION(LL_ADD_INTEGRATION_TEST
ENDFUNCTION(LL_ADD_INTEGRATION_TEST)
+#*****************************************************************************
+# SET_TEST_PATH
+#*****************************************************************************
MACRO(SET_TEST_PATH LISTVAR)
IF(WINDOWS)
# We typically build/package only Release variants of third-party
diff --git a/indra/llcommon/llinstancetracker.h b/indra/llcommon/llinstancetracker.h
index 9783644e66..69c712b656 100644
--- a/indra/llcommon/llinstancetracker.h
+++ b/indra/llcommon/llinstancetracker.h
@@ -35,6 +35,24 @@
#include <boost/iterator/transform_iterator.hpp>
#include <boost/iterator/indirect_iterator.hpp>
+// As of 2017-05-06, as far as nat knows, only clang supports __has_feature().
+#if defined(LL_TEST_llinstancetracker) && defined(__clang__) && __has_feature(cxx_noexcept)
+// ~LLInstanceTracker() performs llassert_always() validation. That's fine in
+// production code, since the llassert_always() is implemented as an LL_ERRS
+// message, which will crash-with-message. In our integration test executable,
+// though, this llassert_always() throws an exception instead so we can test
+// error conditions and continue running the test. However -- as of C++11,
+// destructors are implicitly noexcept(true). Unless we mark
+// ~LLInstanceTracker() noexcept(false), the test executable crashes even on
+// the ATTEMPT to throw.
+#define LLINSTANCETRACKER_DTOR_NOEXCEPT noexcept(false)
+#else
+// If we're building for production, or in fact building *any other* test, or
+// we're using a compiler that doesn't support __has_feature(), or we're not
+// compiling with a C++ version that supports noexcept -- don't specify it.
+#define LLINSTANCETRACKER_DTOR_NOEXCEPT
+#endif
+
/**
* Base class manages "class-static" data that must actually have singleton
* semantics: one instance per process, rather than one instance per module as
@@ -198,11 +216,11 @@ protected:
getStatic();
add_(key);
}
- virtual ~LLInstanceTracker()
+ virtual ~LLInstanceTracker() LLINSTANCETRACKER_DTOR_NOEXCEPT
{
// it's unsafe to delete instances of this type while all instances are being iterated over.
llassert_always(getStatic().getDepth() == 0);
- remove_();
+ remove_();
}
virtual void setKey(KEY key) { remove_(); add_(key); }
virtual const KEY& getKey() const { return mInstanceKey; }
@@ -335,7 +353,7 @@ protected:
getStatic();
getSet_().insert(static_cast<T*>(this));
}
- virtual ~LLInstanceTracker()
+ virtual ~LLInstanceTracker() LLINSTANCETRACKER_DTOR_NOEXCEPT
{
// it's unsafe to delete instances of this type while all instances are being iterated over.
llassert_always(getStatic().getDepth() == 0);
diff --git a/indra/llcommon/lltrace.h b/indra/llcommon/lltrace.h
index 5f1289dad8..79ff55b739 100644
--- a/indra/llcommon/lltrace.h
+++ b/indra/llcommon/lltrace.h
@@ -57,7 +57,7 @@ class StatBase
{
public:
StatBase(const char* name, const char* description);
- virtual ~StatBase() {};
+ virtual ~StatBase() LLINSTANCETRACKER_DTOR_NOEXCEPT {}
virtual const char* getUnitLabel() const;
const std::string& getName() const { return mName; }