From d6b822a7e712e6d699eda08a8bee626ac6bbfb3b Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 4 Dec 2019 13:16:33 -0500 Subject: [PATCH 01/13] timezonespec: add NUL to the end of the CSV data This is passed to a `std::string` constructor which ends up reading off the end of the array looking for the terminating `NUL`. --- smtk/common/timezonespec.cxx | 13 +++++++------ smtk/common/timezonespec.h | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/smtk/common/timezonespec.cxx b/smtk/common/timezonespec.cxx index e5bee1ca04..c5b87d3453 100644 --- a/smtk/common/timezonespec.cxx +++ b/smtk/common/timezonespec.cxx @@ -12,10 +12,11 @@ // This file is a hexdump of the boost data_time_zonespec.csv file, // which is a database of 380+ time zone names, offsets, DST adjustments, etc. -// The content below was generated using "xxd -i date_time_zonespec.csv". -// The date_time_zonespec.csv file is in the boost folder libs/date_time/data -// erased the trailing newline to avoid an error about "invalid uuid string" -// Currently from boost 1.66 +// The content below was generated using "xxd -i date_time_zonespec.csv". The +// array should be appended with a `0x00` entry to make it into a valid C +// string. The date_time_zonespec.csv file is in the boost folder +// libs/date_time/data erased the trailing newline to avoid an error about +// "invalid uuid string" Currently from boost 1.66 namespace smtk { @@ -23,7 +24,7 @@ namespace common { // clang-format off -const char timezonespec_csv[35854] = { +const char timezonespec_csv[35855] = { 0x22, 0x49, 0x44, 0x22, 0x2c, 0x22, 0x53, 0x54, 0x44, 0x20, 0x41, 0x42, 0x42, 0x52, 0x22, 0x2c, 0x22, 0x53, 0x54, 0x44, 0x20, 0x4e, 0x41, 0x4d, 0x45, 0x22, 0x2c, 0x22, 0x44, 0x53, 0x54, 0x20, 0x41, 0x42, 0x42, 0x52, @@ -3011,7 +3012,7 @@ const char timezonespec_csv[35854] = { 0x22, 0x2c, 0x22, 0x2b, 0x31, 0x30, 0x3a, 0x30, 0x30, 0x3a, 0x30, 0x30, 0x22, 0x2c, 0x22, 0x2b, 0x30, 0x30, 0x3a, 0x30, 0x30, 0x3a, 0x30, 0x30, 0x22, 0x2c, 0x22, 0x22, 0x2c, 0x22, 0x22, 0x2c, 0x22, 0x22, 0x2c, 0x22, - 0x2b, 0x30, 0x30, 0x3a, 0x30, 0x30, 0x3a, 0x30, 0x30, 0x22 + 0x2b, 0x30, 0x30, 0x3a, 0x30, 0x30, 0x3a, 0x30, 0x30, 0x22, 0x00 }; // clang-format on } // namespace common diff --git a/smtk/common/timezonespec.h b/smtk/common/timezonespec.h index 84db86acf7..09b4bb1580 100644 --- a/smtk/common/timezonespec.h +++ b/smtk/common/timezonespec.h @@ -20,7 +20,7 @@ namespace common // Derived from boost, see cxx: SMTKCORE_EXPORT -extern const char timezonespec_csv[35854]; +extern const char timezonespec_csv[35855]; } // namespace common } // namespace smtk -- GitLab From 78ca215c75adce2417e0249588a7b7597d160a75 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 4 Dec 2019 13:33:21 -0500 Subject: [PATCH 02/13] mesh: fix off-by-one errors in intervals HandleInterval is closed while the pointer addition ends up with a half-open interval. Subtract one from the upper bound to fix the mismatch. --- smtk/mesh/testing/cxx/UnitTestCellSet.cxx | 4 ++-- smtk/mesh/testing/cxx/UnitTestModelToMesh3D.cxx | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/smtk/mesh/testing/cxx/UnitTestCellSet.cxx b/smtk/mesh/testing/cxx/UnitTestCellSet.cxx index 61924bd489..03d605d4bf 100644 --- a/smtk/mesh/testing/cxx/UnitTestCellSet.cxx +++ b/smtk/mesh/testing/cxx/UnitTestCellSet.cxx @@ -566,7 +566,7 @@ public: this->numCellsVisited++; this->numPointsSeen += numPts; this->pointsSeen.insert( - smtk::mesh::HandleInterval(*this->pointIds(), *(this->pointIds() + numPts))); + smtk::mesh::HandleInterval(*this->pointIds(), *(this->pointIds() + numPts - 1))); this->cellTypesSeen[static_cast(cellType)] = true; } @@ -599,7 +599,7 @@ void verify_cellset_for_each(const smtk::mesh::ResourcePtr& mr) while (pc.fetchNextCell(numPts, points)) { numPointsSeen += numPts; - pointsFromConnectivity.insert(smtk::mesh::HandleInterval(*points, *(points + numPts))); + pointsFromConnectivity.insert(smtk::mesh::HandleInterval(*points, *(points + numPts - 1))); } //verify that point connectivity iteration and cell for_each visit diff --git a/smtk/mesh/testing/cxx/UnitTestModelToMesh3D.cxx b/smtk/mesh/testing/cxx/UnitTestModelToMesh3D.cxx index 1b804e2352..c0f5c88b40 100644 --- a/smtk/mesh/testing/cxx/UnitTestModelToMesh3D.cxx +++ b/smtk/mesh/testing/cxx/UnitTestModelToMesh3D.cxx @@ -55,7 +55,7 @@ public: this->numCellsVisited++; this->numPointsSeen += numPts; this->pointsSeen.insert( - smtk::mesh::HandleInterval(*this->pointIds(), *(this->pointIds() + numPts))); + smtk::mesh::HandleInterval(*this->pointIds(), *(this->pointIds() + numPts - 1))); } int numberOCellsVisited() const { return numCellsVisited; } -- GitLab From 3c816a403ce9be597a904e7f5a291bc6d784b6a7 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 4 Dec 2019 16:59:55 -0500 Subject: [PATCH 03/13] projectManagerMeshSessionTest: use the instanced logger --- smtk/project/testing/python/projectManagerMeshSessionTest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smtk/project/testing/python/projectManagerMeshSessionTest.py b/smtk/project/testing/python/projectManagerMeshSessionTest.py index 098a107392..426b36cb8b 100644 --- a/smtk/project/testing/python/projectManagerMeshSessionTest.py +++ b/smtk/project/testing/python/projectManagerMeshSessionTest.py @@ -194,7 +194,7 @@ class TestProjectManager(unittest.TestCase): def open_project(self): path = os.path.join(smtk.testing.TEMP_DIR, PROJECT1) # project = self.pm.openProject(path, smtk.io.Logger.instance()) - logger = smtk.io.Logger() + logger = smtk.io.Logger.instance() project = self.pm.openProject(path, logger) self.assertIsNotNone(project, msg=logger.convertToString()) -- GitLab From c29499a28e1fdecb99aaf939a2eeab53a4332a81 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 4 Dec 2019 17:43:00 -0500 Subject: [PATCH 04/13] projectManagerMeshSessionTest: pass the logger down It seems that the default argument here is not working. I suspect there's a lifetime issue somewhere. --- smtk/project/testing/python/projectManagerMeshSessionTest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/smtk/project/testing/python/projectManagerMeshSessionTest.py b/smtk/project/testing/python/projectManagerMeshSessionTest.py index 426b36cb8b..54521c90c0 100644 --- a/smtk/project/testing/python/projectManagerMeshSessionTest.py +++ b/smtk/project/testing/python/projectManagerMeshSessionTest.py @@ -53,7 +53,7 @@ class TestProjectManager(unittest.TestCase): pm = smtk.project.Manager.create(rm, om) spec = pm.getProjectSpecification() logger = smtk.io.Logger.instance() - project = pm.createProject(spec) + project = pm.createProject(spec, logger=logger) self.assertIsNone(project) def init_project_manager(self): -- GitLab From 7b8a941bd3197ac59f89b05a434ab662051f1b31 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 4 Dec 2019 10:30:57 -0500 Subject: [PATCH 05/13] gitlab-ci: factor out test exclusion lists --- .gitlab/ci/ctest_exclusions.cmake | 10 ++++++++++ .gitlab/ci/ctest_test.cmake | 12 +----------- 2 files changed, 11 insertions(+), 11 deletions(-) create mode 100644 .gitlab/ci/ctest_exclusions.cmake diff --git a/.gitlab/ci/ctest_exclusions.cmake b/.gitlab/ci/ctest_exclusions.cmake new file mode 100644 index 0000000000..8a8a19e476 --- /dev/null +++ b/.gitlab/ci/ctest_exclusions.cmake @@ -0,0 +1,10 @@ +set(test_exclusions + # Issue #296. + "elevateMeshOnStructuredGridPy" + "pv.OpenExodusFile" + "TestReadWrite" +) +string(REPLACE ";" "|" test_exclusions "${test_exclusions}") +if (test_exclusions) + set(test_exclusions "(${test_exclusions})") +endif () diff --git a/.gitlab/ci/ctest_test.cmake b/.gitlab/ci/ctest_test.cmake index e15e34ba0a..891006c29d 100644 --- a/.gitlab/ci/ctest_test.cmake +++ b/.gitlab/ci/ctest_test.cmake @@ -12,17 +12,7 @@ ctest_start(APPEND) include(ProcessorCount) ProcessorCount(nproc) -set(test_exclusions - # Issue #296. - "elevateMeshOnStructuredGridPy" - "pv.OpenExodusFile" - "TestReadWrite" -) -string(REPLACE ";" "|" test_exclusions "${test_exclusions}") -if (test_exclusions) - set(test_exclusions "(${test_exclusions})") -endif () - +include("${CMAKE_CURRENT_LIST_DIR}/ctest_exclusions.cmake") ctest_test( PARALLEL_LEVEL "${nproc}" RETURN_VALUE test_result -- GitLab From 9649fe47524fd5f5437099136438d42ef74ee8d3 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 4 Dec 2019 10:39:24 -0500 Subject: [PATCH 06/13] ci: add asan, ubsan, and valgrind to the paraview container --- .gitlab/ci/docker/fedora31-paraview/install_deps.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.gitlab/ci/docker/fedora31-paraview/install_deps.sh b/.gitlab/ci/docker/fedora31-paraview/install_deps.sh index 23660fc50f..05194f641f 100755 --- a/.gitlab/ci/docker/fedora31-paraview/install_deps.sh +++ b/.gitlab/ci/docker/fedora31-paraview/install_deps.sh @@ -19,4 +19,10 @@ dnf install -y \ ninja-build \ make +# Install memcheck tools +dnf install -y \ + libasan \ + libubsan \ + valgrind + dnf clean all -- GitLab From c2645bdc5354e228a9343d0f1d1ed8218fae9719 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 4 Dec 2019 12:02:34 -0500 Subject: [PATCH 07/13] cmake: add support for running sanitizers --- CMake/SMTKSanitize.cmake | 34 ++++++++++++++++++++++++++++++++++ CMakeLists.txt | 3 +++ 2 files changed, 37 insertions(+) create mode 100644 CMake/SMTKSanitize.cmake diff --git a/CMake/SMTKSanitize.cmake b/CMake/SMTKSanitize.cmake new file mode 100644 index 0000000000..f5b9be45da --- /dev/null +++ b/CMake/SMTKSanitize.cmake @@ -0,0 +1,34 @@ +#========================================================================= +# +# This software is distributed WITHOUT ANY WARRANTY; without even +# the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR +# PURPOSE. See the above copyright notice for more information. +# +#========================================================================= + +# This code has been adapted from remus (https://gitlab.kitware.com/cmb/remus) + +if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang" OR + CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang") + set(CMAKE_COMPILER_IS_CLANGXX 1) +endif() + +if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_COMPILER_IS_CLANGXX) + + #Add option for enabling sanitizers + option(SMTK_ENABLE_SANITIZER "Build with sanitizer support." OFF) + mark_as_advanced(SMTK_ENABLE_SANITIZER) + + if(SMTK_ENABLE_SANITIZER) + set(SMTK_SANITIZER "address" + CACHE STRING "The sanitizer to use") + mark_as_advanced(SMTK_SANITIZER) + + #We're setting the CXX flags and C flags beacuse they're propagated down + #independent of build type. + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=${SMTK_SANITIZER}") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=${SMTK_SANITIZER}") + set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=${SMTK_SANITIZER}") + set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -fsanitize=${SMTK_SANITIZER}") + endif() +endif() diff --git a/CMakeLists.txt b/CMakeLists.txt index cc23b71356..93ef7d9f51 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -66,6 +66,9 @@ include(CMake/SMTKTestingMacros.cmake) # Add options for performing code coverage tests. include(CMake/SMTKCoverage.cmake) +# Add options for performing sanitization. +include(CMake/SMTKSanitize.cmake) + # Include mechanism for determining function and bind support include(Function) -- GitLab From 12d3fce7fc85cc9977848ad35fbf0700fc6d3f75 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 4 Dec 2019 16:35:56 -0500 Subject: [PATCH 08/13] cmake: disable contract tests with sanitizers --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 93ef7d9f51..b1eeaccdd0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -760,7 +760,7 @@ endif() # for an example of a plugin contract file. ################################################################################ -if (SMTK_ENABLE_TESTING AND SMTK_PLUGIN_CONTRACT_FILE_URLS) +if (SMTK_ENABLE_TESTING AND SMTK_PLUGIN_CONTRACT_FILE_URLS AND NOT SMTK_ENABLE_SANITIZER) include(CMake/SMTKPluginTestingMacros.cmake) list(REMOVE_DUPLICATES SMTK_PLUGIN_CONTRACT_FILE_URLS) -- GitLab From e771136cd252654594f10b17ea9fb596054f4e3c Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 4 Dec 2019 12:03:03 -0500 Subject: [PATCH 09/13] tutorials: disable for sanitization as well --- doc/tutorials/CMakeLists.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/tutorials/CMakeLists.txt b/doc/tutorials/CMakeLists.txt index 24586a9b67..7353e1317a 100644 --- a/doc/tutorials/CMakeLists.txt +++ b/doc/tutorials/CMakeLists.txt @@ -1,8 +1,8 @@ # Now add tutorial directories -# If building with coverage, the tutorials will fail to build because they -# don't know to use the coverage flags. -if (SMTK_ENABLE_COVERAGE) +# If building with coverage or sanitizers, the tutorials will fail to build +# because they don't know to use the appropriate flags. +if (SMTK_ENABLE_COVERAGE OR SMTK_ENABLE_SANITIZER) return () endif () -- GitLab From fe24a9241572c58c77d13a9e731fe1c043e43f11 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 4 Dec 2019 15:36:24 -0500 Subject: [PATCH 10/13] python: preload the ASan library for Python tests --- CMakeLists.txt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index b1eeaccdd0..e250a2ce5d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -520,6 +520,24 @@ if(SMTK_ENABLE_PYTHON_WRAPPING) set(smtk_python_test_environment "PYTHONPATH=${smtk_pythonpath_env}${envsep}${CMAKE_RUNTIME_OUTPUT_DIRECTORY}") + if (SMTK_ENABLE_SANITIZER) + set(preload_libraries) + if (SMTK_SANITIZER MATCHES "address") + find_library(SMTK_ASAN_LIBRARY NAMES libasan.so.5 DOC "ASan library") + mark_as_advanced(SMTK_ASAN_LIBRARY) + + list(APPEND preload_libraries + "${SMTK_ASAN_LIBRARY}") + endif () + + if (preload_libraries) + if (UNIX AND NOT APPLE) + list(APPEND smtk_python_test_environment + "LD_PRELOAD=${preload_libraries}") + endif () + endif () + endif () + if (libpath_envvar) list(APPEND smtk_python_test_environment "${libpath_envvar}=${smtk_libpath_env}") -- GitLab From f30cc6c0acfd0235b6dd738ee2d454c1e76f54c5 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 4 Dec 2019 10:40:03 -0500 Subject: [PATCH 11/13] gitlab-ci: update paraview container --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 9d7e858f27..13b194bf56 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -56,7 +56,7 @@ .fedora31_paraview: &fedora31_paraview extends: .fedora31_vtk - image: "kitware/cmb:ci-smtk-fedora31-paraview-20191125" + image: "kitware/cmb:ci-smtk-fedora31-paraview-20191204" variables: CMAKE_CONFIGURATION: fedora31_paraview -- GitLab From a8e30febefba428cd7304316f500e650c6504fe3 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 4 Dec 2019 10:31:22 -0500 Subject: [PATCH 12/13] gitlab-ci: add a memcheck script --- .gitlab/ci/ctest_memcheck.cmake | 34 +++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 .gitlab/ci/ctest_memcheck.cmake diff --git a/.gitlab/ci/ctest_memcheck.cmake b/.gitlab/ci/ctest_memcheck.cmake new file mode 100644 index 0000000000..2b798d9d86 --- /dev/null +++ b/.gitlab/ci/ctest_memcheck.cmake @@ -0,0 +1,34 @@ +cmake_minimum_required(VERSION 3.8) + +include("${CMAKE_CURRENT_LIST_DIR}/gitlab_ci.cmake") + +# Read the files from the build directory. +ctest_read_custom_files("${CTEST_BINARY_DIRECTORY}") +include("${CMAKE_CURRENT_LIST_DIR}/ctest_submit_multi.cmake") + +# Pick up from where the configure left off. +ctest_start(APPEND) + +include(ProcessorCount) +ProcessorCount(nproc) + +set(CTEST_MEMORYCHECK_TYPE "$ENV{CTEST_MEMORYCHECK_TYPE}") +set(CTEST_MEMORYCHECK_SANITIZER_OPTIONS "$ENV{CTEST_MEMORYCHECK_SANITIZER_OPTIONS}") + +include("${CMAKE_CURRENT_LIST_DIR}/ctest_exclusions.cmake") +ctest_memcheck( + PARALLEL_LEVEL "${nproc}" + RETURN_VALUE test_result + EXCLUDE "${test_exclusions}" + DEFECT_COUNT defects) +ctest_submit_multi(PARTS Memcheck) + +if (test_result) + message(FATAL_ERROR + "Failed to test") +endif () + +if (defects) + message(FATAL_ERROR + "Found ${defects} memcheck defects") +endif () -- GitLab From c45119c39c239db9c19c81c78bad86eb3c9b4a95 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 4 Dec 2019 10:31:55 -0500 Subject: [PATCH 13/13] gitlab-ci: add an ASAN builder --- .gitlab-ci.yml | 61 ++++++++++++++++++++++++ .gitlab/ci/configure_fedora31_asan.cmake | 4 ++ 2 files changed, 65 insertions(+) create mode 100644 .gitlab/ci/configure_fedora31_asan.cmake diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 13b194bf56..a6d7d4417b 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -68,6 +68,22 @@ variables: CMAKE_CONFIGURATION: fedora31_vtk_python2 +.fedora31_memcheck: &fedora31_memcheck + extends: .fedora31_paraview + + variables: + CMAKE_BUILD_TYPE: RelWithDebInfo + +.fedora31_asan: &fedora31_asan + extends: .fedora31_memcheck + + variables: + CMAKE_CONFIGURATION: fedora31_asan + CTEST_MEMORYCHECK_TYPE: AddressSanitizer + # Disable LeakSanitizer for now. It's catching all kinds of errors that + # need investigated or suppressed. + CTEST_MEMORYCHECK_SANITIZER_OPTIONS: detect_leaks=0 + .fedora31_coverage: &fedora31_coverage extends: .fedora31_paraview @@ -174,6 +190,37 @@ - linux - x11 +.cmake_memcheck_unix: &cmake_memcheck_unix + stage: test + only: *only_settings + tags: + - build + - cmb + - docker + - linux + + script: + - "$LAUNCHER $CTEST -VV -S .gitlab/ci/ctest_memcheck.cmake" + + interruptible: true + +.cmake_memcheck_unix_x11: &cmake_memcheck_unix_x11 + <<: *cmake_memcheck_unix + tags: + - cmb + - docker + - linux + - x11 + +.cmake_memcheck_unix_x11_priv: &cmake_memcheck_unix_x11_priv + <<: *cmake_memcheck_unix + tags: + - cmb + - docker + - linux + - privileged + - x11 + .cmake_coverage: &cmake_coverage stage: analyze only: *only_settings @@ -250,6 +297,20 @@ test:fedora31-vtk-python2: needs: - build:fedora31-vtk-python2 +build:fedora31-asan: + <<: + - *fedora31_asan + - *cmake_build_unix + +test:fedora31-asan: + <<: + - *fedora31_asan + - *cmake_memcheck_unix_x11_priv + dependencies: + - build:fedora31-asan + needs: + - build:fedora31-asan + build:fedora31-coverage: <<: - *fedora31_coverage diff --git a/.gitlab/ci/configure_fedora31_asan.cmake b/.gitlab/ci/configure_fedora31_asan.cmake new file mode 100644 index 0000000000..1b19e8a111 --- /dev/null +++ b/.gitlab/ci/configure_fedora31_asan.cmake @@ -0,0 +1,4 @@ +set(SMTK_ENABLE_SANITIZER ON CACHE BOOL "") +set(SMTK_SANITIZER "address" CACHE STRING "") + +include("${CMAKE_CURRENT_LIST_DIR}/configure_fedora31_paraview.cmake") -- GitLab