Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve configurations for sanitized builds #6911

Merged
merged 20 commits into from
Oct 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
5f09115
Refactor sanitizer flags for different build types:
kiburtse Aug 9, 2023
3c34674
Fix usage of moved object for fuzz tester
kiburtse Aug 18, 2023
edec9a8
Add missing source from s2
kiburtse Aug 18, 2023
b97650c
Check asan/tsan on macos x64/arm64
kiburtse Aug 18, 2023
4b7415a
Check asan with msvc 2019
kiburtse Aug 18, 2023
abf55d4
Remove Jenkins sanitized builders replaced by evergreen configs
kiburtse Aug 24, 2023
ead6435
Work-around stack-use-after-scope with msvc2019 and mpark
kiburtse Aug 25, 2023
25441b7
Fix crash on check with staled ColKeys
kiburtse Aug 25, 2023
c033380
tsan fixes (#6930)
ironage Aug 28, 2023
871bb09
Add some logger related test fixes
kiburtse Aug 28, 2023
c2fd050
Work around catch2 limmitation with not thread safe asserts and TSAN …
kiburtse Aug 28, 2023
30a649b
Merge branch 'master' into kb/sanitizer_build_configs_improve
kiburtse Sep 8, 2023
e4da3fd
Run multiprocesses tests under sanitizers
kiburtse Sep 8, 2023
f765af0
Merge branch 'master' into kb/sanitizer_build_configs_improve
kiburtse Sep 25, 2023
b82419a
Merge branch 'master' into kb/sanitizer_build_configs_improve
kiburtse Oct 6, 2023
b8dcd1e
add assert for an error reported by undefined sanitizer
kiburtse Oct 19, 2023
a576686
Merge branch 'master' into kb/sanitizer_build_configs_improve
kiburtse Oct 19, 2023
e6c54bc
Workaround uv scheduler main thread only constraint for callbacks cal…
kiburtse Oct 20, 2023
3affdae
Merge branch 'master' into kb/sanitizer_build_configs_improve
kiburtse Oct 27, 2023
d667940
add changelog entries
kiburtse Oct 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

### Fixed
* <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
* None.
* Potential stack-use-after-scope issue on changesets integration with msvc-2019 and mpack code ([PR #6911](https://github.com/realm/realm-core/pull/6911))

### Breaking changes
* None.
Expand All @@ -17,7 +17,7 @@
-----------

### Internals
* None.
* REALM_[ATMU]SAN cmake flags no longer override compilation options and can be combined with Debug|RelWithDebInfo|etc. build types. Rel[ATMU]SAN build type shortcuts are now all slightly optimized debug-based builds with sanitizers. REALM_ASAN now works with msvc (2019/2022) builds. ([PR #6911](https://github.com/realm/realm-core/pull/6911))

----------------------------------------------

Expand Down
59 changes: 0 additions & 59 deletions Jenkinsfile
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,6 @@ jobWrapper {
buildAndroidArm64Debug : doAndroidBuildInDocker('arm64-v8a', 'Debug'),
buildAndroidTestsArmeabi: doAndroidBuildInDocker('armeabi-v7a', 'Debug', TestAction.Build),
buildEmscripten : doBuildEmscripten('Debug'),
threadSanitizer : doCheckSanity(buildOptions + [enableSync: true, sanitizeMode: 'thread']),
addressSanitizer : doCheckSanity(buildOptions + [enableSync: true, sanitizeMode: 'address']),
]
if (releaseTesting) {
extendedChecks = [
Expand Down Expand Up @@ -312,63 +310,6 @@ def doCheckInDocker(Map options = [:]) {
}
}

def doCheckSanity(Map options = [:]) {
def privileged = '';

def cmakeOptions = [
CMAKE_BUILD_TYPE: options.buildType,
REALM_MAX_BPNODE_SIZE: options.maxBpNodeSize,
REALM_ENABLE_SYNC: options.enableSync,
]

if (options.sanitizeMode.contains('thread')) {
cmakeOptions << [
REALM_TSAN: "ON",
]
}
else if (options.sanitizeMode.contains('address')) {
privileged = '--privileged'
cmakeOptions << [
REALM_ASAN: "ON",
]
}

def cmakeDefinitions = cmakeOptions.collect { k,v -> "-D$k=$v" }.join(' ')

return {
rlmNode('docker') {
getArchive()

def environment = environment() + [
'CC=clang',
'CXX=clang++',
'UNITTEST_XML=unit-test-report.xml',
"UNITTEST_SUITE_NAME=Linux-${options.buildType}",
"TSAN_OPTIONS=\"suppressions=${WORKSPACE}/test/tsan.suppress\""
]
buildDockerEnv('linux.Dockerfile').inside(privileged) {
withEnv(environment) {
try {
dir('build-dir') {
sh "cmake ${cmakeDefinitions} -G Ninja .."
runAndCollectWarnings(
script: 'ninja',
parser: "clang",
name: "linux-clang-${options.buildType}-${options.sanitizeMode}",
filters: warningFilters,
)
sh "${ctest_cmd}"
}

} finally {
junit testResults: 'build-dir/test/unit-test-report.xml'
}
}
}
}
}
}

def doBuildLinux(String buildType) {
return {
rlmNode('docker') {
Expand Down
8 changes: 6 additions & 2 deletions bindgen/src/realm_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,12 @@ struct Helpers {
static void simulate_sync_error(SyncSession& session, const int& code, const std::string& message,
const std::string& type, bool is_fatal)
{
sync::SessionErrorInfo error(Status{type == "realm::sync::ProtocolError" ? ErrorCodes::SyncClientResetRequired : ErrorCodes::UnknownError, message}, sync::IsFatal(is_fatal));
error.server_requests_action = code == 211 ? sync::ProtocolErrorInfo::Action::ClientReset : sync::ProtocolErrorInfo::Action::Warning;
sync::SessionErrorInfo error(Status{type == "realm::sync::ProtocolError" ? ErrorCodes::SyncClientResetRequired
: ErrorCodes::UnknownError,
message},
sync::IsFatal(is_fatal));
error.server_requests_action =
code == 211 ? sync::ProtocolErrorInfo::Action::ClientReset : sync::ProtocolErrorInfo::Action::Warning;
SyncSession::OnlyForTesting::handle_error(session, std::move(error));
}

Expand Down
86 changes: 86 additions & 0 deletions evergreen/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1214,6 +1214,7 @@ buildvariants:
cxx_compiler: "./clang_binaries/bin/clang++"
run_with_encryption: On
enable_tsan: On
cmake_build_type: RelWithDebInfo
tasks:
- name: core_tests_group
distros:
Expand All @@ -1232,6 +1233,7 @@ buildvariants:
llvm_symbolizer: "./clang_binaries/bin/llvm-symbolizer"
run_with_encryption: On
enable_asan: On
cmake_build_type: RelWithDebInfo
tasks:
- name: core_tests_group
distros:
Expand Down Expand Up @@ -1269,6 +1271,7 @@ buildvariants:
cmake_bindir: "./cmake_binaries/bin"
fetch_missing_dependencies: On
enable_asan: On
cmake_build_type: RelWithDebInfo
c_compiler: "./clang_binaries/bin/clang"
cxx_compiler: "./clang_binaries/bin/clang++"
llvm_symbolizer: "./clang_binaries/bin/llvm-symbolizer"
Expand All @@ -1287,6 +1290,7 @@ buildvariants:
cmake_bindir: "./cmake_binaries/bin"
fetch_missing_dependencies: On
enable_tsan: On
cmake_build_type: RelWithDebInfo
c_compiler: "./clang_binaries/bin/clang"
cxx_compiler: "./clang_binaries/bin/clang++"
tasks:
Expand All @@ -1301,6 +1305,7 @@ buildvariants:
cmake_bindir: "./cmake_binaries/bin"
fetch_missing_dependencies: On
enable_ubsan: On
cmake_build_type: RelWithDebInfo
c_compiler: "./clang_binaries/bin/clang"
cxx_compiler: "./clang_binaries/bin/clang++"
tasks:
Expand Down Expand Up @@ -1448,6 +1453,70 @@ buildvariants:
- name: long-running-tests
- name: swift-build-and-test

- name: macos-1100-x64-asan
display_name: "MacOS 11.0 x86_64 (ASAN)"
run_on: macos-1100
expansions:
cmake_url: "https://s3.amazonaws.com/static.realm.io/evergreen-assets/cmake-3.20.3-macos-universal.tar.gz"
cmake_bindir: "./cmake_binaries/CMake.app/Contents/bin"
cmake_toolchain_file: "./tools/cmake/xcode.toolchain.cmake"
cmake_generator: Xcode
max_jobs: $(sysctl -n hw.logicalcpu)
xcode_developer_dir: /Applications/Xcode13.1.app/Contents/Developer
extra_flags: -DCMAKE_SYSTEM_NAME=Darwin -DCMAKE_OSX_ARCHITECTURES=x86_64
cmake_build_type: RelWithDebInfo
enable_asan: On
tasks:
- name: compile_test

- name: macos-1100-x64-tsan
display_name: "MacOS 11.0 x86_64 (TSAN)"
run_on: macos-1100
expansions:
cmake_url: "https://s3.amazonaws.com/static.realm.io/evergreen-assets/cmake-3.20.3-macos-universal.tar.gz"
cmake_bindir: "./cmake_binaries/CMake.app/Contents/bin"
cmake_toolchain_file: "./tools/cmake/xcode.toolchain.cmake"
cmake_generator: Xcode
max_jobs: $(sysctl -n hw.logicalcpu)
xcode_developer_dir: /Applications/Xcode13.1.app/Contents/Developer
extra_flags: -DCMAKE_SYSTEM_NAME=Darwin -DCMAKE_OSX_ARCHITECTURES=x86_64
cmake_build_type: RelWithDebInfo
enable_tsan: On
tasks:
- name: compile_test

- name: macos-1100-arm64-asan
display_name: "MacOS 11 arm64 (ASAN)"
run_on: macos-1100-arm64
expansions:
cmake_url: "https://s3.amazonaws.com/static.realm.io/evergreen-assets/cmake-3.20.3-macos-universal.tar.gz"
cmake_bindir: "./cmake_binaries/CMake.app/Contents/bin"
cmake_toolchain_file: "./tools/cmake/xcode.toolchain.cmake"
cmake_generator: Xcode
max_jobs: $(sysctl -n hw.logicalcpu)
xcode_developer_dir: /Applications/Xcode13.1.app/Contents/Developer
extra_flags: -DCMAKE_SYSTEM_NAME=Darwin -DCMAKE_OSX_ARCHITECTURES=arm64
cmake_build_type: RelWithDebInfo
enable_asan: On
tasks:
- name: compile_test

- name: macos-1100-arm64-tsan
display_name: "MacOS 11 arm64 (TSAN)"
run_on: macos-1100-arm64
expansions:
cmake_url: "https://s3.amazonaws.com/static.realm.io/evergreen-assets/cmake-3.20.3-macos-universal.tar.gz"
cmake_bindir: "./cmake_binaries/CMake.app/Contents/bin"
cmake_toolchain_file: "./tools/cmake/xcode.toolchain.cmake"
cmake_generator: Xcode
max_jobs: $(sysctl -n hw.logicalcpu)
xcode_developer_dir: /Applications/Xcode13.1.app/Contents/Developer
extra_flags: -DCMAKE_SYSTEM_NAME=Darwin -DCMAKE_OSX_ARCHITECTURES=arm64
cmake_build_type: RelWithDebInfo
enable_tsan: On
tasks:
- name: compile_test

- name: macos-coverage
display_name: "MacOS 11 arm64 (Code Coverage)"
run_on: macos-1100-arm64
Expand Down Expand Up @@ -1529,3 +1598,20 @@ buildvariants:
python3: "/cygdrive/c/python/python37/python.exe"
tasks:
- name: compile_test

- name: windows-64-vs2019-asan
display_name: "Windows x86_64 (VS 2019 ASAN)"
run_on: windows-vsCurrent-large
expansions:
cmake_url: "https://s3.amazonaws.com/static.realm.io/evergreen-assets/cmake-3.20.3-windows-x86_64.zip"
cmake_bindir: "./cmake-3.20.3-windows-x86_64/bin"
cmake_generator: "Visual Studio 16 2019"
extra_flags: "-A x64"
cmake_build_type: "Debug"
enable_asan: On
max_jobs: $(($(grep -c proc /proc/cpuinfo) / 2))
fetch_missing_dependencies: On
curl_base: "/cygdrive/c/curl"
python3: "/cygdrive/c/python/python37/python.exe"
tasks:
- name: compile_test
1 change: 1 addition & 0 deletions src/external/s2/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ set(S2_SOURCES
s1interval.cc
s2cap.cc
s2cell.cc
s2cellunion.cc
s2edgeindex.cc
s2edgeutil.cc
s2latlngrect.cc
Expand Down
8 changes: 5 additions & 3 deletions src/realm/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,6 @@ set_target_properties(Storage PROPERTIES
OUTPUT_NAME "realm"
)

target_compile_options(Storage PUBLIC ${REALM_SANITIZER_FLAGS})
if(REALM_USE_FAST_LINKER)
target_link_options(Storage PUBLIC ${REALM_USE_FAST_LINKER})
endif()
Expand All @@ -315,9 +314,12 @@ target_compile_definitions(Storage PUBLIC
$<$<CONFIG:Debug>:REALM_DEBUG=1>
)

if(NOT MSVC)
if (REALM_SANITIZER_FLAGS)
target_compile_options(Storage PUBLIC ${REALM_SANITIZER_FLAGS})
target_link_options(Storage PUBLIC ${REALM_SANITIZER_FLAGS})

if (REALM_SANITIZER_LINK_FLAGS)
target_link_options(Storage PUBLIC ${REALM_SANITIZER_LINK_FLAGS})
endif()
endif()

target_include_directories(Storage INTERFACE
Expand Down
1 change: 1 addition & 0 deletions src/realm/sync/noinst/client_history_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1194,6 +1194,7 @@ void ClientHistory::update_from_ref_and_version(ref_type ref, version_type versi
m_arrays->init_from_ref(ref);
}
else {
REALM_ASSERT_RELEASE(m_group);
m_arrays.emplace(m_db->get_alloc(), *m_group, ref);
}

Expand Down
14 changes: 8 additions & 6 deletions src/realm/sync/transform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -997,16 +997,18 @@ struct MergeUtils {
bool same_path_element(const Instruction::Path::Element& left,
const Instruction::Path::Element& right) const noexcept
{
const auto& pred = util::overload{
[&](uint32_t lhs, uint32_t rhs) {
auto pred = util::overload{
kiburtse marked this conversation as resolved.
Show resolved Hide resolved
[](uint32_t lhs, uint32_t rhs) {
return lhs == rhs;
},
[&](InternString lhs, InternString rhs) {
[this](InternString lhs, InternString rhs) {
return same_string(lhs, rhs);
},
[&](const auto&, const auto&) {
// FIXME: Paths contain incompatible element types. Should we raise an
// error here?
// FIXME: Paths contain incompatible element types. Should we raise an error here?
[](InternString, uint32_t) {
return false;
},
[](uint32_t, InternString) {
return false;
},
};
Expand Down
12 changes: 6 additions & 6 deletions src/realm/table.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -903,10 +903,10 @@ class ColKeyIterator {

private:
friend class ColKeys;
const Table* m_table;
ConstTableRef m_table;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is probably harmless, but using ColKeys as anything other than a temporary that doesn't need a strong reference to the table seems very suspicious and is probably broken even if it doesn't hit UB.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it was caught by a few new asan configuraions. How it wasn't fixed in the test in 5 years i don't understand really. Now just it will at least check this situation with the ref.

size_t m_pos;

ColKeyIterator(const Table* t, size_t p)
ColKeyIterator(const ConstTableRef& t, size_t p)
: m_table(t)
, m_pos(p)
{
Expand All @@ -915,8 +915,8 @@ class ColKeyIterator {

class ColKeys {
public:
ColKeys(const Table* t)
: m_table(t)
ColKeys(ConstTableRef&& t)
: m_table(std::move(t))
{
}

Expand Down Expand Up @@ -947,7 +947,7 @@ class ColKeys {
}

private:
const Table* m_table;
ConstTableRef m_table;
};

// Class used to collect a chain of links when building up a Query following links.
Expand Down Expand Up @@ -1079,7 +1079,7 @@ class LinkChain {

inline ColKeys Table::get_column_keys() const
{
return ColKeys(this);
return ColKeys(ConstTableRef(this, m_alloc.get_instance_version()));
}

inline uint_fast64_t Table::get_content_version() const noexcept
Expand Down
6 changes: 3 additions & 3 deletions src/realm/timestamp.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#ifndef REALM_TIMESTAMP_HPP
#define REALM_TIMESTAMP_HPP

#include <array>
#include <cstdint>
#include <ostream>
#include <chrono>
Expand Down Expand Up @@ -182,8 +183,7 @@ class Timestamp {
return size_t(m_seconds) ^ size_t(m_nanoseconds);
}

// Buffer must be at least 32 bytes long
const char* to_string(char* buffer) const;
const char* to_string(std::array<char, 32>& buffer) const;

template <class Ch, class Tr>
friend std::basic_ostream<Ch, Tr>& operator<<(std::basic_ostream<Ch, Tr>& out, const Timestamp&);
Expand All @@ -199,7 +199,7 @@ class Timestamp {
template <class C, class T>
inline std::basic_ostream<C, T>& operator<<(std::basic_ostream<C, T>& out, const Timestamp& d)
{
char buffer[32];
std::array<char, 32> buffer{};
out << d.to_string(buffer);
return out;
}
Expand Down
2 changes: 2 additions & 0 deletions src/realm/util/logger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ const std::string_view Logger::level_to_string(Level level) noexcept

void StderrLogger::do_log(Level level, const std::string& message)
{
static Mutex mutex;
LockGuard l(mutex);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StderrLogger should be wrapped with ThreadSafeLogger when this behavior is desired rather than adding it to each logger.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, i'll try to revert this back on next pr in order to not let this one to stale for longer. It's just that wrapping StderrLogger is not the only thing. The very same instance should be then shared everywhere on every config. It's not the case now. Also, the problem is with a few places where logger is initialized from realm default, which doesn't know anything about other parts. So, it's easy to misuse as it turned out during these fixes here.

// std::cerr is unbuffered, so no need to flush
std::cerr << get_level_prefix(level) << message << '\n'; // Throws
}
Expand Down
Loading