From 9b25cebdf11e6da25d83e513a2e4b6db0515abad Mon Sep 17 00:00:00 2001 From: Teemu Ollakka Date: Thu, 5 Dec 2019 13:29:42 +0200 Subject: [PATCH] codership/wsrep-lib#117 Fixed empty vector access. Access to empty vector by using operator[] may cause stdlib++ assertions to fail. Replaced the vector data access to use data() method which is valid operation even if the vector is empty. Added unit test to reproduce assertion with empty mutable_buffer access. Added -D_GLIBCXX_ASSERTIONS preprocessor option to debug builds to catch standard library misuse. Added gcc 8 and gcc9 into travis build matrix. --- .travis.yml | 56 ++++++++++++++++++++++++++++++++++++++ CMakeLists.txt | 5 ++++ include/wsrep/buffer.hpp | 22 +++++++++++++-- src/wsrep_provider_v26.cpp | 2 +- test/CMakeLists.txt | 1 + test/buffer_test.cpp | 28 +++++++++++++++++++ 6 files changed, 111 insertions(+), 3 deletions(-) create mode 100644 test/buffer_test.cpp diff --git a/.travis.yml b/.travis.yml index 1161ecb..757499f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -147,6 +147,62 @@ matrix: - libboost-filesystem-dev - libboost-thread-dev env: MATRIX_EVAL="CC=gcc-7 CXX=g++-7 TYPE=RelWithDebInfo STRICT=ON UNIT_TESTS=ON ASAN=OFF DBSIM=ON" + - os: linux + name: "GCC 8 Debug" + addons: + apt: + sources: + - ubuntu-toolchain-r-test + packages: + - g++-8 + - cmake + - libboost-test-dev + - libboost-program-options-dev + - libboost-filesystem-dev + - libboost-thread-dev + env: MATRIX_EVAL="CC=gcc-8 CXX=g++-8 TYPE=Debug STRICT=ON UNIT_TESTS=ON ASAN=OFF DBSIM=ON" + - os: linux + name: "GCC 8 RelWithDebInfo" + addons: + apt: + sources: + - ubuntu-toolchain-r-test + packages: + - g++-8 + - cmake + - libboost-test-dev + - libboost-program-options-dev + - libboost-filesystem-dev + - libboost-thread-dev + env: MATRIX_EVAL="CC=gcc-8 CXX=g++-8 TYPE=RelWithDebInfo STRICT=ON UNIT_TESTS=ON ASAN=OFF DBSIM=ON" + - os: linux + name: "GCC 9 Debug" + addons: + apt: + sources: + - ubuntu-toolchain-r-test + packages: + - g++-9 + - cmake + - libboost-test-dev + - libboost-program-options-dev + - libboost-filesystem-dev + - libboost-thread-dev + env: MATRIX_EVAL="CC=gcc-9 CXX=g++-9 TYPE=Debug STRICT=ON UNIT_TESTS=ON ASAN=OFF DBSIM=ON" + - os: linux + name: "GCC 9 RelWithDebInfo" + addons: + apt: + sources: + - ubuntu-toolchain-r-test + packages: + - g++-9 + - cmake + - libboost-test-dev + - libboost-program-options-dev + - libboost-filesystem-dev + - libboost-thread-dev + env: MATRIX_EVAL="CC=gcc-9 CXX=g++-9 TYPE=RelWithDebInfo STRICT=ON UNIT_TESTS=ON ASAN=OFF DBSIM=ON" - os: linux dist: trusty name: "Clang 3.6 Debug" diff --git a/CMakeLists.txt b/CMakeLists.txt index ae64386..7bb5869 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -80,6 +80,11 @@ if (WSREP_LIB_MAINTAINER_MODE) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror") endif() +# Enable extra libstdc++ assertions with debug build. +if (CMAKE_BUILD_TYPE STREQUAL "Debug") + add_definitions("-D_GLIBCXX_ASSERTIONS") +endif() + # Set up include directories include_directories("${CMAKE_CURRENT_SOURCE_DIR}/include") include_directories("${CMAKE_CURRENT_SOURCE_DIR}/wsrep-API") diff --git a/include/wsrep/buffer.hpp b/include/wsrep/buffer.hpp index 7eaea75..89dc822 100644 --- a/include/wsrep/buffer.hpp +++ b/include/wsrep/buffer.hpp @@ -85,8 +85,26 @@ namespace wsrep } size_t size() const { return buffer_.size(); } - char* data() { return &buffer_[0]; } - const char* data() const { return &buffer_[0]; } + + /** + * Return pointer to underlying data array. The returned pointer + * may or may not be null in case of empty buffer, it is up to + * user to check the size of the array before dereferencing the + * pointer. + * + * @return Pointer to underlying data array. + */ + char* data() { return buffer_.data(); } + + /** + * Return const pointer to underlying data array. The returned pointer + * may or may not be null in case of empty buffer, it is up to + * user to check the size of the array before dereferencing the + * pointer. + * + * @return Const pointer to underlying data array. + */ + const char* data() const { return buffer_.data(); } mutable_buffer& operator= (const mutable_buffer& other) { diff --git a/src/wsrep_provider_v26.cpp b/src/wsrep_provider_v26.cpp index 75e93ee..49ed841 100644 --- a/src/wsrep_provider_v26.cpp +++ b/src/wsrep_provider_v26.cpp @@ -884,7 +884,7 @@ wsrep::wsrep_provider_v26::enter_toi( return map_return_value(wsrep_->to_execute_start( wsrep_, client_id.get(), - &wsrep_keys[0], + wsrep_keys.data(), wsrep_keys.size(), &wsrep_buf, 1, diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index ed667f6..74dbc94 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -7,6 +7,7 @@ add_executable(wsrep-lib_test mock_high_priority_service.cpp mock_storage_service.cpp test_utils.cpp + buffer_test.cpp gtid_test.cpp id_test.cpp server_context_test.cpp diff --git a/test/buffer_test.cpp b/test/buffer_test.cpp new file mode 100644 index 0000000..84d4a48 --- /dev/null +++ b/test/buffer_test.cpp @@ -0,0 +1,28 @@ +/* + * Copyright (C) 2019 Codership Oy + * + * This file is part of wsrep-lib. + * + * Wsrep-lib is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 of the License, or + * (at your option) any later version. + * + * Wsrep-lib is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with wsrep-lib. If not, see . + */ + +#include "wsrep/buffer.hpp" +#include + +BOOST_AUTO_TEST_CASE(buffer_test_empty_access) +{ + wsrep::mutable_buffer buf; + BOOST_REQUIRE(buf.size() == 0); + (void)buf.data(); +}