Skip to content

Commit

Permalink
[#8748] Every executable should link tcmalloc directly
Browse files Browse the repository at this point in the history
Summary:
Every executable should link tcmalloc directly. This is necessary to ensure that no memory allocations happen with the default allocator that would later be deallocated with tcmalloc, causing a crash.

Also fixing a warning that causes a compilation failure on GCC 9.

Test Plan: Jenkins

Reviewers: bogdan, steve.varnau, sergei

Reviewed By: sergei

Subscribers: ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D12245
  • Loading branch information
mbautin committed Jul 13, 2021
1 parent e10d5c9 commit 070d104
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 6 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ if (USING_LINUXBREW)
ADD_GLOBAL_RPATH_ENTRY("/usr/lib64")
endif()

message("Linker flags: ${CMAKE_EXE_LINKER_FLAGS}")
message("Linker flags for executables: ${CMAKE_EXE_LINKER_FLAGS}")

# Define full paths to libpq and libyb_pgbackend shared libraries. These libraries are built as part
# of building PostgreSQL. We need to declare these as by-products of running the PostgreSQL build
Expand Down
3 changes: 3 additions & 0 deletions cmake_modules/YugabyteFindThirdParty.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,9 @@ if ("${YB_TCMALLOC_ENABLED}" STREQUAL "1")
SHARED_LIB "${PROFILER_SHARED_LIB}")
list(APPEND YB_BASE_LIBS tcmalloc profiler)
ADD_CXX_FLAGS("-DTCMALLOC_ENABLED")
# Each executable should link with tcmalloc directly so that it does not allocate memory using
# system malloc before loading a library that depends on tcmalloc.
ADD_EXE_LINKER_FLAGS("-ltcmalloc")
else()
message("Not using tcmalloc, YB_TCMALLOC_ENABLED is '${YB_TCMALLOC_ENABLED}'")
endif()
Expand Down
15 changes: 11 additions & 4 deletions cmake_modules/YugabyteFunctions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,13 @@ function(ADD_CXX_FLAGS FLAGS)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${FLAGS}" PARENT_SCOPE)
endfunction()

function(ADD_EXE_LINKER_FLAGS FLAGS)
if ($ENV{YB_VERBOSE})
message("Adding executable linking flags: ${FLAGS}")
endif()
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${FLAGS}" PARENT_SCOPE)
endfunction()

function(YB_INCLUDE_EXTENSIONS)
file(RELATIVE_PATH CUR_REL_LIST_FILE "${YB_SRC_ROOT}" "${CMAKE_CURRENT_LIST_FILE}")
get_filename_component(CUR_REL_LIST_NAME_NO_EXT "${CUR_REL_LIST_FILE}" NAME_WE)
Expand Down Expand Up @@ -421,9 +428,9 @@ macro(YB_SETUP_SANITIZER)
ADD_CXX_FLAGS("-DADDRESS_SANITIZER")

# Compile and link against the thirdparty ASAN instrumented libstdcxx.
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address")
ADD_EXE_LINKER_FLAGS("-fsanitize=address")
if("${COMPILER_FAMILY}" STREQUAL "gcc")
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -lubsan -ldl")
ADD_EXE_LINKER_FLAGS("-lubsan -ldl")
ADD_CXX_FLAGS("-Wno-error=maybe-uninitialized")
endif()
elseif("${YB_BUILD_TYPE}" STREQUAL "tsan")
Expand All @@ -436,12 +443,12 @@ macro(YB_SETUP_SANITIZER)
ADD_CXX_FLAGS("-DTHREAD_SANITIZER")

# Compile and link against the thirdparty TSAN instrumented libstdcxx.
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=thread")
ADD_EXE_LINKER_FLAGS("-fsanitize=thread")
if("${COMPILER_FAMILY}" STREQUAL "clang" AND
"${COMPILER_VERSION}" VERSION_GREATER_EQUAL "10.0.0")
# To avoid issues with missing libunwind symbols:
# https://gist.githubusercontent.com/mbautin/5bc53ed2d342eab300aec7120eb42996/raw
set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -lunwind")
ADD_EXE_LINKER_FLAGS("-lunwind")
endif()
else()
message(FATAL_ERROR "Invalid build type for YB_SETUP_SANITIZER: '${YB_BUILD_TYPE}'")
Expand Down
2 changes: 1 addition & 1 deletion src/yb/rocksdb/db/deletefile_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ Result<std::vector<LiveFileMetaData>> DeleteFileTest::AddFiles(
}
std::vector<LiveFileMetaData> metadata;
db_->GetLiveFilesMetaData(&metadata);
return std::move(metadata);
return metadata;
}

size_t DeleteFileTest::TryDeleteFiles(
Expand Down

0 comments on commit 070d104

Please sign in to comment.