Skip to content

Commit

Permalink
Enable sanitizer based builds on travis.
Browse files Browse the repository at this point in the history
Includes two fixes:
Fix intended "memory leaks" in makeSharedMemory
Avoid asan "detected memory leaks" after TBB lazy initialization
  • Loading branch information
TheMarex authored and oxidase committed Oct 6, 2016
1 parent 49a28b4 commit 834fca0
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 38 deletions.
6 changes: 3 additions & 3 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ matrix:
apt:
sources: ['ubuntu-toolchain-r-test']
packages: ['g++-6', 'libbz2-dev', 'libstxxl-dev', 'libstxxl1', 'libxml2-dev', 'libzip-dev', 'lua5.1', 'liblua5.1-0-dev', 'libtbb-dev', 'libgdal-dev', 'libluabind-dev', 'libboost-all-dev', 'ccache']
env: CCOMPILER='gcc-6' CXXCOMPILER='g++-6' BUILD_TYPE='Debug' COVERAGE=ON
env: CCOMPILER='gcc-6' CXXCOMPILER='g++-6' BUILD_TYPE='Debug' TARGET_ARCH='x86_64-asan' ENABLE_COVERAGE=ON ENABLE_SANITIZER=ON

- os: linux
compiler: "clang-3.8-debug"
Expand Down Expand Up @@ -130,7 +130,7 @@ install:
fi
- mkdir build && pushd build
- export CC=${CCOMPILER} CXX=${CXXCOMPILER}
- cmake .. -DCMAKE_BUILD_TYPE=${BUILD_TYPE} -DBUILD_SHARED_LIBS=${BUILD_SHARED_LIBS:-OFF} -DCOVERAGE=${COVERAGE:-OFF} -DBUILD_TOOLS=ON -DBUILD_COMPONENTS=ON -DENABLE_CCACHE=ON
- cmake .. -DCMAKE_BUILD_TYPE=${BUILD_TYPE} -DBUILD_SHARED_LIBS=${BUILD_SHARED_LIBS:-OFF} -DENABLE_COVERAGE=${ENABLE_COVERAGE:-OFF} -DENABLE_SANITIZER=${ENABLE_SANITIZER:-OFF} -DBUILD_TOOLS=ON -DBUILD_COMPONENTS=ON -DENABLE_CCACHE=ON
- echo "travis_fold:start:MAKE"
- make osrm-extract --jobs=3
- make --jobs=${JOBS}
Expand Down Expand Up @@ -171,6 +171,6 @@ after_success:
./scripts/format.sh # we don't want to fail just yet
fi
- |
if [ -n "${COVERAGE}" ]; then
if [ -n "${ENABLE_COVERAGE}" ]; then
bash <(curl -s https://codecov.io/bash)
fi
16 changes: 6 additions & 10 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ endif()
option(ENABLE_CCACHE "Speed up incremental rebuilds via ccache" ON)
option(BUILD_TOOLS "Build OSRM tools" OFF)
option(BUILD_COMPONENTS "Build osrm-components" OFF)
option(ENABLE_ASSERTIONS OFF)
option(COVERAGE OFF)
option(SANITIZER OFF)
option(ENABLE_ASSERTIONS "Use assertions in release mode" OFF)
option(ENABLE_COVERAGE "Build with coverage instrumentalisation" OFF)
option(ENABLE_SANITIZER "Use memory sanitizer for Debug build" OFF)
option(ENABLE_LTO "Use LTO if available" ON)
option(ENABLE_FUZZING "Fuzz testing using LLVM's libFuzzer" OFF)
option(ENABLE_GOLD_LINKER "Use GNU gold linker if available" ON)
Expand Down Expand Up @@ -109,7 +109,6 @@ if(CMAKE_BUILD_TYPE MATCHES Debug)
if(NOT ${CMAKE_CXX_COMPILER_ID} STREQUAL "MSVC")

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-inline -fno-omit-frame-pointer")

if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Og -ggdb")
endif()
Expand Down Expand Up @@ -175,18 +174,15 @@ if(CMAKE_BUILD_TYPE MATCHES Release)
endif()

set(MAYBE_COVERAGE_LIBRARIES "")
if (COVERAGE)
if (ENABLE_COVERAGE)
if (NOT CMAKE_BUILD_TYPE MATCHES "Debug")
message(ERROR "COVERAGE=ON only make sense with a Debug build")
message(ERROR "ENABLE_COVERAGE=ON only make sense with a Debug build")
endif()
message(INFO "Enabling coverage")
set(MAYBE_COVERAGE_LIBRARIES "gcov")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -ftest-coverage -fprofile-arcs")
endif()
if (SANITIZER)
if (NOT CMAKE_BUILD_TYPE MATCHES "Debug")
message(ERROR "SANITIZER=ON only make sense with a Debug build")
endif()
if (ENABLE_SANITIZER)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address")
endif()

Expand Down
12 changes: 6 additions & 6 deletions include/engine/datafacade/shared_datafacade.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ class SharedDataFacade final : public BaseDataFacade

unsigned m_check_sum;
std::unique_ptr<QueryGraph> m_query_graph;
std::unique_ptr<storage::SharedMemory> m_data_timestamp_memory;
std::unique_ptr<storage::SharedMemory> m_layout_memory;
std::unique_ptr<storage::SharedMemory> m_large_memory;
std::string m_timestamp;
Expand Down Expand Up @@ -376,10 +377,9 @@ class SharedDataFacade final : public BaseDataFacade
throw util::exception(
"No shared memory blocks found, have you forgotten to run osrm-datastore?");
}
data_timestamp_ptr = static_cast<storage::SharedDataTimestamp *>(
storage::makeSharedMemory(
storage::CURRENT_REGIONS, sizeof(storage::SharedDataTimestamp), false, false)
->Ptr());

m_data_timestamp_memory = storage::makeSharedMemory(storage::CURRENT_REGIONS, sizeof(storage::SharedDataTimestamp), false, false);
data_timestamp_ptr = static_cast<storage::SharedDataTimestamp *>(m_data_timestamp_memory->Ptr());
CURRENT_LAYOUT = storage::LAYOUT_NONE;
CURRENT_DATA = storage::DATA_NONE;
CURRENT_TIMESTAMP = 0;
Expand Down Expand Up @@ -423,11 +423,11 @@ class SharedDataFacade final : public BaseDataFacade
CURRENT_TIMESTAMP = data_timestamp_ptr->timestamp;

util::SimpleLogger().Write(logDEBUG) << "Performing data reload";
m_layout_memory.reset(storage::makeSharedMemory(CURRENT_LAYOUT));
m_layout_memory = storage::makeSharedMemory(CURRENT_LAYOUT);

data_layout = static_cast<storage::SharedDataLayout *>(m_layout_memory->Ptr());

m_large_memory.reset(storage::makeSharedMemory(CURRENT_DATA));
m_large_memory = storage::makeSharedMemory(CURRENT_DATA);
shared_memory = (char *)(m_large_memory->Ptr());

const auto file_index_ptr = data_layout->GetBlockPtr<char>(
Expand Down
28 changes: 19 additions & 9 deletions include/storage/shared_memory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

#include <algorithm>
#include <exception>
#include <memory>

namespace osrm
{
Expand Down Expand Up @@ -86,7 +87,8 @@ class SharedMemory
const IdentifierT id,
const uint64_t size = 0,
bool read_write = false,
bool remove_prev = true)
bool remove_prev = true,
bool remove_at_dtor = false)
: key(lock_file.string().c_str(), id)
{
if (0 == size)
Expand Down Expand Up @@ -117,7 +119,10 @@ class SharedMemory
#endif
region = boost::interprocess::mapped_region(shm, boost::interprocess::read_write);

remover.SetID(shm.get_shmid());
if (remove_at_dtor)
{
remover.SetID(shm.get_shmid());
}
util::SimpleLogger().Write(logDEBUG) << "writeable memory allocated " << size
<< " bytes";
}
Expand Down Expand Up @@ -230,7 +235,8 @@ class SharedMemory
const int id,
const uint64_t size = 0,
bool read_write = false,
bool remove_prev = true)
bool remove_prev = true,
bool remove_at_dtor = false)
{
sprintf(key, "%s.%d", "osrm.lock", id);
if (0 == size)
Expand All @@ -254,7 +260,10 @@ class SharedMemory
shm.truncate(size);
region = boost::interprocess::mapped_region(shm, boost::interprocess::read_write);

remover.SetID(key);
if (remove_at_dtor)
{
remover.SetID(key);
}
util::SimpleLogger().Write(logDEBUG) << "writeable memory allocated " << size
<< " bytes";
}
Expand Down Expand Up @@ -327,10 +336,11 @@ class SharedMemory
#endif

template <typename IdentifierT, typename LockFileT = OSRMLockFile>
SharedMemory *makeSharedMemory(const IdentifierT &id,
const uint64_t size = 0,
bool read_write = false,
bool remove_prev = true)
std::unique_ptr<SharedMemory> makeSharedMemory(const IdentifierT &id,
const uint64_t size = 0,
bool read_write = false,
bool remove_prev = true,
bool remove_at_dtor = false)
{
try
{
Expand All @@ -346,7 +356,7 @@ SharedMemory *makeSharedMemory(const IdentifierT &id,
boost::filesystem::ofstream ofs(lock_file());
}
}
return new SharedMemory(lock_file(), id, size, read_write, remove_prev);
return std::make_unique<SharedMemory>(lock_file(), id, size, read_write, remove_prev, remove_at_dtor);
}
catch (const boost::interprocess::interprocess_exception &e)
{
Expand Down
5 changes: 5 additions & 0 deletions scripts/travis/before_install.x86_64-asan.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#!/bin/sh

sudo add-apt-repository ppa:jonathonf/binutils --yes || true
sudo apt-get update -qq --yes || true
sudo apt-get install -qq --yes --force-yes binutils
14 changes: 7 additions & 7 deletions src/extractor/extractor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,15 +108,15 @@ transformTurnLaneMapIntoArrays(const guidance::LaneDescriptionMap &turn_lane_map
*/
int Extractor::run(ScriptingEnvironment &scripting_environment)
{
{
util::LogPolicy::GetInstance().Unmute();
TIMER_START(extracting);
util::LogPolicy::GetInstance().Unmute();
TIMER_START(extracting);

const unsigned recommended_num_threads = tbb::task_scheduler_init::default_num_threads();
const auto number_of_threads =
std::min(recommended_num_threads, config.requested_num_threads);
tbb::task_scheduler_init init(number_of_threads);
const unsigned recommended_num_threads = tbb::task_scheduler_init::default_num_threads();
const auto number_of_threads =
std::min(recommended_num_threads, config.requested_num_threads);
tbb::task_scheduler_init init(number_of_threads);

{
util::SimpleLogger().Write() << "Input file: " << config.input_path.filename().string();
if (!config.profile_path.empty())
{
Expand Down
6 changes: 3 additions & 3 deletions src/storage/storage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ int Storage::Run()
}();

// Allocate a memory layout in shared memory, deallocate previous
auto *layout_memory = makeSharedMemory(layout_region, sizeof(SharedDataLayout));
auto layout_memory = makeSharedMemory(layout_region, sizeof(SharedDataLayout));
auto shared_layout_ptr = new (layout_memory->Ptr()) SharedDataLayout();
auto absolute_file_index_path = boost::filesystem::absolute(config.file_index_path);

Expand Down Expand Up @@ -406,7 +406,7 @@ int Storage::Run()
// allocate shared memory block
util::SimpleLogger().Write() << "allocating shared memory of "
<< shared_layout_ptr->GetSizeOfLayout() << " bytes";
auto *shared_memory = makeSharedMemory(data_region, shared_layout_ptr->GetSizeOfLayout());
auto shared_memory = makeSharedMemory(data_region, shared_layout_ptr->GetSizeOfLayout());
char *shared_memory_ptr = static_cast<char *>(shared_memory->Ptr());

// read actual data into shared memory object //
Expand Down Expand Up @@ -733,7 +733,7 @@ int Storage::Run()
}

// acquire lock
SharedMemory *data_type_memory =
auto data_type_memory =
makeSharedMemory(CURRENT_REGIONS, sizeof(SharedDataTimestamp), true, false);
SharedDataTimestamp *data_timestamp_ptr =
static_cast<SharedDataTimestamp *>(data_type_memory->Ptr());
Expand Down
5 changes: 5 additions & 0 deletions unit_tests/util/static_rtree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@
#include <utility>
#include <vector>

#include <tbb/task_scheduler_init.h>

// explicit TBB scheduler init to register resources cleanup at exit
tbb::task_scheduler_init init(2);

BOOST_AUTO_TEST_SUITE(static_rtree)

using namespace osrm;
Expand Down

0 comments on commit 834fca0

Please sign in to comment.