From f201addbaf3f4543a3bdb3a8800a7a14ac7edf88 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Sun, 20 Jan 2019 03:20:16 -0800 Subject: [PATCH 01/15] Use Ninja cmake generator on appveyor CI --- appveyor.yml | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 6555400a..f5c3cd41 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -20,13 +20,11 @@ init: - echo %NUMBER_OF_PROCESSORS% - echo %PLATFORM% - echo %APPVEYOR_BUILD_WORKER_IMAGE% -- set arch= -- if "%PLATFORM%"=="x64" ( set arch= Win64) -- if "%APPVEYOR_BUILD_WORKER_IMAGE%"=="Visual Studio 2013" (set generator=Visual Studio 12 2013%arch%) -- if "%APPVEYOR_BUILD_WORKER_IMAGE%"=="Visual Studio 2015" (set generator=Visual Studio 14 2015%arch%) -- if "%APPVEYOR_BUILD_WORKER_IMAGE%"=="Visual Studio 2017" (set generator=Visual Studio 15 2017%arch%) -- if "%APPVEYOR_BUILD_WORKER_IMAGE%"=="Visual Studio 2017 Preview" (set generator=Visual Studio 15 2017%arch%) -- echo %generator% +- set arch=32 +- if "%PLATFORM%"=="x64" ( set arch=64) +- set VisualStudioVersion=2017 +- if "%APPVEYOR_BUILD_WORKER_IMAGE%" == "Visual Studio 2017 Preview" ( set VisualStudioVersion=Preview) +- cmd: call "C:\Program Files (x86)\Microsoft Visual Studio\%VisualStudioVersion%\Community\VC\Auxiliary\Build\vcvars%arch%.bat" install: - choco install -y dokany --version 1.1.0.2000 --installargs INSTALLDEVFILES=1 @@ -35,17 +33,16 @@ install: build_script: - cmd: mkdir build - cmd: cd build - - cmd: cmake .. -G "%generator%" -DBUILD_TESTING=on -DBOOST_ROOT="C:/Libraries/boost_1_65_1" -DDOKAN_PATH="C:/Program Files/Dokan/DokanLibrary-1.1.0" - # TODO Make build parallel + - cmd: cmake .. -G "Ninja" -DBUILD_TESTING=on -DBOOST_ROOT="C:/Libraries/boost_1_65_1" -DDOKAN_PATH="C:/Program Files/Dokan/DokanLibrary-1.1.0" - cmd: cmake --build . --config %CONFIGURATION% - - cmd: .\test\gitversion\%CONFIGURATION%\gitversion-test.exe - - cmd: cd .\test\cpp-utils\%CONFIGURATION%\ && .\cpp-utils-test.exe && cd ..\..\.. - #- cmd: .\test\fspp\%CONFIGURATION%\fspp-test.exe - - cmd: .\test\parallelaccessstore\%CONFIGURATION%\parallelaccessstore-test.exe - - cmd: .\test\blockstore\%CONFIGURATION%\blockstore-test.exe - - cmd: .\test\blobstore\%CONFIGURATION%\blobstore-test.exe - - cmd: .\test\cryfs\%CONFIGURATION%\cryfs-test.exe - #- cmd: .\test\cryfs-cli\%CONFIGURATION%\cryfs-cli-test.exe + - cmd: .\test\gitversion\gitversion-test.exe + - cmd: cd .\test\cpp-utils\ && .\cpp-utils-test.exe && cd ..\.. + #- cmd: .\test\fspp\fspp-test.exe + - cmd: .\test\parallelaccessstore\parallelaccessstore-test.exe + - cmd: .\test\blockstore\blockstore-test.exe + - cmd: .\test\blobstore\blobstore-test.exe + - cmd: .\test\cryfs\cryfs-test.exe + #- cmd: .\test\cryfs-cli\cryfs-cli-test.exe - cmd: cpack -C %CONFIGURATION% --verbose -G WIX From 29f7f06ca91ecf087d1df2caf8657c0eba0ea012 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Sun, 20 Jan 2019 03:21:20 -0800 Subject: [PATCH 02/15] Implement set_thread_name and get_thread_name for debugging purposes --- appveyor.yml | 3 +- src/cpp-utils/CMakeLists.txt | 2 + src/cpp-utils/thread/debugging.h | 16 +++ src/cpp-utils/thread/debugging_nonwindows.cpp | 55 +++++++++ src/cpp-utils/thread/debugging_windows.cpp | 111 ++++++++++++++++++ test/cpp-utils/CMakeLists.txt | 1 + test/cpp-utils/thread/debugging_test.cpp | 62 ++++++++++ 7 files changed, 249 insertions(+), 1 deletion(-) create mode 100644 src/cpp-utils/thread/debugging.h create mode 100644 src/cpp-utils/thread/debugging_nonwindows.cpp create mode 100644 src/cpp-utils/thread/debugging_windows.cpp create mode 100644 test/cpp-utils/thread/debugging_test.cpp diff --git a/appveyor.yml b/appveyor.yml index f5c3cd41..469b15c9 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -36,7 +36,8 @@ build_script: - cmd: cmake .. -G "Ninja" -DBUILD_TESTING=on -DBOOST_ROOT="C:/Libraries/boost_1_65_1" -DDOKAN_PATH="C:/Program Files/Dokan/DokanLibrary-1.1.0" - cmd: cmake --build . --config %CONFIGURATION% - cmd: .\test\gitversion\gitversion-test.exe - - cmd: cd .\test\cpp-utils\ && .\cpp-utils-test.exe && cd ..\.. + # cpp-utils-test disables ThreadDebuggingTest_ThreadName.*_thenIsCorrect because the appveyor image is too old to support the API needed for that + - cmd: cd .\test\cpp-utils\ && .\cpp-utils-test.exe --gtest_filter=-ThreadDebuggingTest_ThreadName.*_thenIsCorrect && cd ..\.. #- cmd: .\test\fspp\fspp-test.exe - cmd: .\test\parallelaccessstore\parallelaccessstore-test.exe - cmd: .\test\blockstore\blockstore-test.exe diff --git a/src/cpp-utils/CMakeLists.txt b/src/cpp-utils/CMakeLists.txt index 07ca7416..2d5db214 100644 --- a/src/cpp-utils/CMakeLists.txt +++ b/src/cpp-utils/CMakeLists.txt @@ -24,6 +24,8 @@ set(SOURCES io/pipestream.cpp thread/LoopThread.cpp thread/ThreadSystem.cpp + thread/debugging_nonwindows.cpp + thread/debugging_windows.cpp random/Random.cpp random/RandomGeneratorThread.cpp random/OSRandomGenerator.cpp diff --git a/src/cpp-utils/thread/debugging.h b/src/cpp-utils/thread/debugging.h new file mode 100644 index 00000000..9965649c --- /dev/null +++ b/src/cpp-utils/thread/debugging.h @@ -0,0 +1,16 @@ +#pragma once +#ifndef MESSMER_CPPUTILS_DEBUGGING_H +#define MESSMER_CPPUTILS_DEBUGGING_H + +#include +#include + +namespace cpputils { + +void set_thread_name(const char* name); +std::string get_thread_name(); +std::string get_thread_name(std::thread* thread); + +} + +#endif diff --git a/src/cpp-utils/thread/debugging_nonwindows.cpp b/src/cpp-utils/thread/debugging_nonwindows.cpp new file mode 100644 index 00000000..3ed543ff --- /dev/null +++ b/src/cpp-utils/thread/debugging_nonwindows.cpp @@ -0,0 +1,55 @@ +#if !defined(_MSC_VER) + +#include "debugging.h" +#include +#include +#include +#include + +namespace cpputils { + +namespace { +constexpr size_t MAX_NAME_LEN = 16; // this length includes the terminating null character at the end +} + +void set_thread_name(const char* name) { + std::string name_(name); + if (name_.size() > MAX_NAME_LEN - 1) { + name_.resize(MAX_NAME_LEN - 1); + } +#if defined(__APPLE__) + int result = pthread_setname_np(name_.c_str()); +#else + int result = pthread_setname_np(pthread_self(), name_.c_str()); +#endif + if (0 != result) { + throw std::runtime_error("Error setting thread name with pthread_setname_np. Code: " + std::to_string(result)); + } +} + +namespace { +std::string get_thread_name(pthread_t thread) { + char name[MAX_NAME_LEN]; + int result = pthread_getname_np(thread, name, MAX_NAME_LEN); + if (0 != result) { + throw std::runtime_error("Error getting thread name with pthread_getname_np. Code: " + std::to_string(result)); + } + // pthread_getname_np returns a null terminated string with maximum 16 bytes. + // but just to be safe against a buggy implementation, let's set the last byte to zero. + name[MAX_NAME_LEN - 1] = '\0'; + return name; +} +} + +std::string get_thread_name() { + return get_thread_name(pthread_self()); +} + +std::string get_thread_name(std::thread* thread) { + ASSERT(thread->joinable(), "Thread not running"); + return get_thread_name(thread->native_handle()); +} + +} + +#endif diff --git a/src/cpp-utils/thread/debugging_windows.cpp b/src/cpp-utils/thread/debugging_windows.cpp new file mode 100644 index 00000000..4033b638 --- /dev/null +++ b/src/cpp-utils/thread/debugging_windows.cpp @@ -0,0 +1,111 @@ +#if defined(_MSC_VER) + +#include +#include "debugging.h" +#include +#include + +using std::string; +using std::wstring; +using std::wstring_convert; + +namespace cpputils { + +namespace { +struct NameData final { + wchar_t *name = nullptr; + + ~NameData() { + if (nullptr != LocalFree(name)) { + throw std::runtime_error("Error releasing thread description memory. Error code: " + std::to_string(GetLastError())); + } + } +}; + +struct ModuleHandle final { + HMODULE module; + + ModuleHandle(const char* dll) { + bool success = GetModuleHandleExA(0, dll, &module); + if (!success) { + throw std::runtime_error(string() + "Error loading dll: " + dll + ". Error code: " + std::to_string(GetLastError())); + } + } + + ~ModuleHandle() { + bool success = FreeLibrary(module); + if (!success) { + throw std::runtime_error("Error unloading dll. Error code: " + std::to_string(GetLastError())); + } + } +}; +template +class APIFunction final { +private: + ModuleHandle module_; + Fn func_; + +public: + APIFunction(const char* dll, const char* function) + : module_(dll), func_(reinterpret_cast(GetProcAddress(module_.module, function))) { + } + + bool valid() const { + return func_ != nullptr; + } + + Fn func() const { + return func_; + } +}; + +std::string get_thread_name(HANDLE thread) { + // The GetThreadDescription API was brought in version 1607 of Windows 10. + typedef HRESULT(WINAPI* GetThreadDescriptionFn)(HANDLE hThread, PWSTR* ppszThreadDescription); + static APIFunction get_thread_description_func("Kernel32.dll", "GetThreadDescription"); + + if (get_thread_description_func.valid()) { + NameData name_data; + + HRESULT status = get_thread_description_func.func()(thread, &name_data.name); + if (FAILED(status)) { + throw std::runtime_error("Error getting thread description. Error code: " + std::to_string(status)); + } + return wstring_convert>().to_bytes(name_data.name); + } + else { + // GetThreadDescription API is not available. + return ""; + } +} +} + +void set_thread_name(const char* name) { + // The GetThreadDescription API was brought in version 1607 of Windows 10. + typedef HRESULT(WINAPI* SetThreadDescriptionFn)(HANDLE hThread, PCWSTR lpThreadDescription); + static APIFunction set_thread_description_func("Kernel32.dll", "SetThreadDescription"); + + if (set_thread_description_func.valid()) { + wstring wname = wstring_convert>().from_bytes(name); + HRESULT status = set_thread_description_func.func()(GetCurrentThread(), wname.c_str()); + if (FAILED(status)) { + throw std::runtime_error("Error setting thread description. Error code: " + std::to_string(status)); + } + } + else { + // intentionally empty. SetThreadDescription API is not available. + } +} + +std::string get_thread_name() { + return get_thread_name(GetCurrentThread()); +} + +std::string get_thread_name(std::thread* thread) { + ASSERT(thread->joinable(), "Thread not running"); + return get_thread_name(static_cast(thread->native_handle())); +} + +} + +#endif diff --git a/test/cpp-utils/CMakeLists.txt b/test/cpp-utils/CMakeLists.txt index 52873bc6..d8efa2e6 100644 --- a/test/cpp-utils/CMakeLists.txt +++ b/test/cpp-utils/CMakeLists.txt @@ -54,6 +54,7 @@ set(SOURCES system/MemoryTest.cpp system/HomedirTest.cpp system/EnvTest.cpp + thread/debugging_test.cpp value_type/ValueTypeTest.cpp ) diff --git a/test/cpp-utils/thread/debugging_test.cpp b/test/cpp-utils/thread/debugging_test.cpp new file mode 100644 index 00000000..7df169ef --- /dev/null +++ b/test/cpp-utils/thread/debugging_test.cpp @@ -0,0 +1,62 @@ +#include +#include +#include +#include + +using namespace cpputils; +using std::string; + +TEST(ThreadDebuggingTest_ThreadName, givenMainThread_whenSettingAndGetting_thenDoesntCrash) { + set_thread_name("my_thread_name"); + get_thread_name(); +} + +TEST(ThreadDebuggingTest_ThreadName, givenChildThread_whenSettingAndGetting_thenDoesntCrash) { + ConditionBarrier nameIsChecked; + + bool child_didnt_crash = false; + std::thread child([&] { + set_thread_name("my_thread_name"); + get_thread_name(); + child_didnt_crash = true; + nameIsChecked.wait(); + }); + get_thread_name(&child); + nameIsChecked.release(); // getting the name of a not-running thread would cause errors, so let's make sure we only exit after getting the name + child.join(); + EXPECT_TRUE(child_didnt_crash); +} + +TEST(ThreadDebuggingTest_ThreadName, givenMainThread_whenGettingFromInside_thenIsCorrect) { + set_thread_name("my_thread_name"); + string name = get_thread_name(); + EXPECT_EQ("my_thread_name", name); +} + +TEST(ThreadDebuggingTest_ThreadName, givenChildThread_whenGettingFromInside_thenIsCorrect) { + std::thread child([] { + set_thread_name("my_thread_name"); + string name = get_thread_name(); + EXPECT_EQ("my_thread_name", name); + }); + child.join(); +} + +TEST(ThreadDebuggingTest_ThreadName, givenChildThread_whenGettingFromOutside_thenIsCorrect) { + ConditionBarrier nameIsSet; + ConditionBarrier nameIsChecked; + + std::thread child([&] { + set_thread_name("my_thread_name"); + nameIsSet.release(); + nameIsChecked.wait(); + }); + + nameIsSet.wait(); + string name = get_thread_name(&child); + EXPECT_EQ("my_thread_name", name); + + nameIsChecked.release(); + child.join(); +} + From 8d09fb4c46623ddb6908b5225501730fb27275f0 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Sun, 20 Jan 2019 03:25:21 -0800 Subject: [PATCH 03/15] Set meaningful thread names for debugging purposes --- .../caching/CachingBlockStore2.cpp | 2 +- .../implementations/caching/cache/Cache.h | 6 +-- .../caching/cache/PeriodicTask.cpp | 4 +- .../caching/cache/PeriodicTask.h | 2 +- .../random/RandomGeneratorThread.cpp | 2 +- src/cpp-utils/thread/LoopThread.cpp | 5 ++- src/cpp-utils/thread/LoopThread.h | 3 +- src/cpp-utils/thread/ThreadSystem.cpp | 15 ++++--- src/cpp-utils/thread/ThreadSystem.h | 5 ++- src/cryfs-cli/CallAfterTimeout.h | 6 +-- src/cryfs-cli/Cli.cpp | 4 +- .../cachingfsblobstore/CachingFsBlobStore.h | 2 +- src/fspp/fuse/Fuse.cpp | 45 +++++++++++++++++++ .../cache/CacheTest_MoveConstructor.cpp | 2 +- .../caching/cache/CacheTest_RaceCondition.cpp | 2 +- .../caching/cache/PeriodicTaskTest.cpp | 6 +-- .../caching/cache/testutils/CacheTest.h | 2 +- test/cryfs-cli/CallAfterTimeoutTest.cpp | 2 +- 18 files changed, 84 insertions(+), 31 deletions(-) diff --git a/src/blockstore/implementations/caching/CachingBlockStore2.cpp b/src/blockstore/implementations/caching/CachingBlockStore2.cpp index e9e9733c..b7437364 100644 --- a/src/blockstore/implementations/caching/CachingBlockStore2.cpp +++ b/src/blockstore/implementations/caching/CachingBlockStore2.cpp @@ -45,7 +45,7 @@ void CachingBlockStore2::CachedBlock::write(Data data) { } CachingBlockStore2::CachingBlockStore2(cpputils::unique_ref baseBlockStore) -: _baseBlockStore(std::move(baseBlockStore)), _cachedBlocksNotInBaseStoreMutex(), _cachedBlocksNotInBaseStore(), _cache() { +: _baseBlockStore(std::move(baseBlockStore)), _cachedBlocksNotInBaseStoreMutex(), _cachedBlocksNotInBaseStore(), _cache("blockstore") { } bool CachingBlockStore2::tryCreate(const BlockId &blockId, const Data &data) { diff --git a/src/blockstore/implementations/caching/cache/Cache.h b/src/blockstore/implementations/caching/cache/Cache.h index 588aa3b4..eb00dfa3 100644 --- a/src/blockstore/implementations/caching/cache/Cache.h +++ b/src/blockstore/implementations/caching/cache/Cache.h @@ -24,7 +24,7 @@ public: static constexpr double PURGE_INTERVAL = 0.5; // With this interval, we check for entries to purge static constexpr double MAX_LIFETIME_SEC = PURGE_LIFETIME_SEC + PURGE_INTERVAL; // This is the oldest age an entry can reach (given purging works in an ideal world, i.e. with the ideal interval and in zero time) - Cache(); + Cache(const std::string& cacheName); ~Cache(); uint32_t size() const; @@ -56,10 +56,10 @@ template constexpr double Cache constexpr double Cache::MAX_LIFETIME_SEC; template -Cache::Cache(): _mutex(), _currentlyFlushingEntries(), _cachedBlocks(), _timeoutFlusher(nullptr) { +Cache::Cache(const std::string& cacheName): _mutex(), _currentlyFlushingEntries(), _cachedBlocks(), _timeoutFlusher(nullptr) { //Don't initialize timeoutFlusher in the initializer list, //because it then might already call Cache::popOldEntries() before Cache is done constructing. - _timeoutFlusher = std::make_unique(std::bind(&Cache::_deleteOldEntriesParallel, this), PURGE_INTERVAL); + _timeoutFlusher = std::make_unique(std::bind(&Cache::_deleteOldEntriesParallel, this), PURGE_INTERVAL, "flush_" + cacheName); } template diff --git a/src/blockstore/implementations/caching/cache/PeriodicTask.cpp b/src/blockstore/implementations/caching/cache/PeriodicTask.cpp index 9145111f..ce4d68a9 100644 --- a/src/blockstore/implementations/caching/cache/PeriodicTask.cpp +++ b/src/blockstore/implementations/caching/cache/PeriodicTask.cpp @@ -7,10 +7,10 @@ using namespace cpputils::logging; namespace blockstore { namespace caching { -PeriodicTask::PeriodicTask(function task, double intervalSec) : +PeriodicTask::PeriodicTask(function task, double intervalSec, std::string threadName) : _task(task), _interval(static_cast(UINT64_C(1000000000) * intervalSec)), - _thread(std::bind(&PeriodicTask::_loopIteration, this)) { + _thread(std::bind(&PeriodicTask::_loopIteration, this), std::move(threadName)) { _thread.start(); } diff --git a/src/blockstore/implementations/caching/cache/PeriodicTask.h b/src/blockstore/implementations/caching/cache/PeriodicTask.h index 7663e293..eecdefce 100644 --- a/src/blockstore/implementations/caching/cache/PeriodicTask.h +++ b/src/blockstore/implementations/caching/cache/PeriodicTask.h @@ -11,7 +11,7 @@ namespace caching { class PeriodicTask final { public: - PeriodicTask(std::function task, double intervalSec); + PeriodicTask(std::function task, double intervalSec, std::string threadName); private: bool _loopIteration(); diff --git a/src/cpp-utils/random/RandomGeneratorThread.cpp b/src/cpp-utils/random/RandomGeneratorThread.cpp index 418f7124..8d436dd5 100644 --- a/src/cpp-utils/random/RandomGeneratorThread.cpp +++ b/src/cpp-utils/random/RandomGeneratorThread.cpp @@ -8,7 +8,7 @@ namespace cpputils { _buffer(buffer), _minSize(minSize), _maxSize(maxSize), - _thread(std::bind(&RandomGeneratorThread::_loopIteration, this)) { + _thread(std::bind(&RandomGeneratorThread::_loopIteration, this), "RandomGeneratorThread") { ASSERT(_maxSize >= _minSize, "Invalid parameters"); } diff --git a/src/cpp-utils/thread/LoopThread.cpp b/src/cpp-utils/thread/LoopThread.cpp index 12489433..31227938 100644 --- a/src/cpp-utils/thread/LoopThread.cpp +++ b/src/cpp-utils/thread/LoopThread.cpp @@ -6,7 +6,8 @@ using boost::none; namespace cpputils { - LoopThread::LoopThread(function loopIteration): _loopIteration(std::move(loopIteration)), _runningHandle(none) { + LoopThread::LoopThread(function loopIteration, std::string threadName) + : _loopIteration(std::move(loopIteration)), _runningHandle(none), _threadName(std::move(threadName)) { } LoopThread::~LoopThread() { @@ -16,7 +17,7 @@ namespace cpputils { } void LoopThread::start() { - _runningHandle = ThreadSystem::singleton().start(_loopIteration); + _runningHandle = ThreadSystem::singleton().start(_loopIteration, _threadName); } void LoopThread::stop() { diff --git a/src/cpp-utils/thread/LoopThread.h b/src/cpp-utils/thread/LoopThread.h index a0cad1a9..ca4cefe1 100644 --- a/src/cpp-utils/thread/LoopThread.h +++ b/src/cpp-utils/thread/LoopThread.h @@ -14,7 +14,7 @@ namespace cpputils { class LoopThread final { public: // The loopIteration callback returns true, if more iterations should be run, and false, if the thread should be terminated. - LoopThread(std::function loopIteration); + LoopThread(std::function loopIteration, std::string threadName); ~LoopThread(); void start(); void stop(); @@ -22,6 +22,7 @@ namespace cpputils { private: std::function _loopIteration; boost::optional _runningHandle; + std::string _threadName; DISALLOW_COPY_AND_ASSIGN(LoopThread); }; diff --git a/src/cpp-utils/thread/ThreadSystem.cpp b/src/cpp-utils/thread/ThreadSystem.cpp index 4451a668..dc5a62a6 100644 --- a/src/cpp-utils/thread/ThreadSystem.cpp +++ b/src/cpp-utils/thread/ThreadSystem.cpp @@ -1,7 +1,9 @@ #include "ThreadSystem.h" #include "../logging/logging.h" +#include "debugging.h" using std::function; +using std::string; using namespace cpputils::logging; namespace cpputils { @@ -21,10 +23,10 @@ namespace cpputils { #endif } - ThreadSystem::Handle ThreadSystem::start(function loopIteration) { + ThreadSystem::Handle ThreadSystem::start(function loopIteration, string threadName) { boost::unique_lock lock(_mutex); - auto thread = _startThread(loopIteration); - _runningThreads.push_back(RunningThread{std::move(loopIteration), std::move(thread)}); + auto thread = _startThread(loopIteration, threadName); + _runningThreads.push_back(RunningThread{std::move(threadName), std::move(loopIteration), std::move(thread)}); return std::prev(_runningThreads.end()); } @@ -59,13 +61,14 @@ namespace cpputils { void ThreadSystem::_restartAllThreads() { for (RunningThread &thread : _runningThreads) { - thread.thread = _startThread(thread.loopIteration); + thread.thread = _startThread(thread.loopIteration, thread.threadName); } _mutex.unlock(); // Was locked in the before-fork handler } - boost::thread ThreadSystem::_startThread(function loopIteration) { - return boost::thread([loopIteration = std::move(loopIteration)] { + boost::thread ThreadSystem::_startThread(function loopIteration, const string& threadName) { + return boost::thread([loopIteration = std::move(loopIteration), threadName] { + cpputils::set_thread_name(threadName.c_str()); ThreadSystem::_runThread(loopIteration); }); } diff --git a/src/cpp-utils/thread/ThreadSystem.h b/src/cpp-utils/thread/ThreadSystem.h index b9a7b419..796a84d6 100644 --- a/src/cpp-utils/thread/ThreadSystem.h +++ b/src/cpp-utils/thread/ThreadSystem.h @@ -13,6 +13,7 @@ namespace cpputils { class ThreadSystem final { private: struct RunningThread { + std::string threadName; std::function loopIteration; // The loopIteration callback returns true, if more iterations should be run, and false, if the thread should be terminated. boost::thread thread; // boost::thread because we need it to be interruptible. }; @@ -21,7 +22,7 @@ namespace cpputils { static ThreadSystem &singleton(); - Handle start(std::function loopIteration); + Handle start(std::function loopIteration, std::string threadName); void stop(Handle handle); private: @@ -34,7 +35,7 @@ namespace cpputils { //TODO Rename to _doOnBeforeFork and _doAfterFork or similar, because they also handle locking _mutex for fork(). void _stopAllThreadsForRestart(); void _restartAllThreads(); - boost::thread _startThread(std::function loopIteration); + boost::thread _startThread(std::function loopIteration, const std::string& threadName); std::list _runningThreads; // std::list, because we give out iterators as handles boost::mutex _mutex; diff --git a/src/cryfs-cli/CallAfterTimeout.h b/src/cryfs-cli/CallAfterTimeout.h index 16cfe48c..6052a38c 100644 --- a/src/cryfs-cli/CallAfterTimeout.h +++ b/src/cryfs-cli/CallAfterTimeout.h @@ -9,7 +9,7 @@ namespace cryfs_cli { class CallAfterTimeout final { public: - CallAfterTimeout(boost::chrono::milliseconds timeout, std::function callback); + CallAfterTimeout(boost::chrono::milliseconds timeout, std::function callback, const std::string& timeoutName); void resetTimer(); private: bool _checkTimeoutThreadIteration(); @@ -25,8 +25,8 @@ namespace cryfs_cli { DISALLOW_COPY_AND_ASSIGN(CallAfterTimeout); }; - inline CallAfterTimeout::CallAfterTimeout(boost::chrono::milliseconds timeout, std::function callback) - :_callback(std::move(callback)), _timeout(timeout), _start(), _checkTimeoutThread(std::bind(&CallAfterTimeout::_checkTimeoutThreadIteration, this)) { + inline CallAfterTimeout::CallAfterTimeout(boost::chrono::milliseconds timeout, std::function callback, const std::string& timeoutName) + :_callback(std::move(callback)), _timeout(timeout), _start(), _checkTimeoutThread(std::bind(&CallAfterTimeout::_checkTimeoutThreadIteration, this), "timeout_" + timeoutName) { resetTimer(); _checkTimeoutThread.start(); } diff --git a/src/cryfs-cli/Cli.cpp b/src/cryfs-cli/Cli.cpp index 8f3dd291..b4e7a508 100644 --- a/src/cryfs-cli/Cli.cpp +++ b/src/cryfs-cli/Cli.cpp @@ -26,6 +26,7 @@ #include #include "Environment.h" #include +#include //TODO Many functions accessing the ProgramOptions object. Factor out into class that stores it as a member. //TODO Factor out class handling askPassword @@ -297,7 +298,7 @@ namespace cryfs_cli { return none; } uint64_t millis = std::llround(60000 * (*minutes)); - return make_unique_ref(milliseconds(millis), callback); + return make_unique_ref(milliseconds(millis), callback, "idlecallback"); } void Cli::_initLogfile(const ProgramOptions &options) { @@ -396,6 +397,7 @@ namespace cryfs_cli { int Cli::main(int argc, const char *argv[], unique_ref httpClient, std::function onMounted) { cpputils::showBacktraceOnCrash(); + cpputils::set_thread_name("cryfs"); try { _showVersion(std::move(httpClient)); diff --git a/src/cryfs/filesystem/cachingfsblobstore/CachingFsBlobStore.h b/src/cryfs/filesystem/cachingfsblobstore/CachingFsBlobStore.h index 6bba337a..5e75131f 100644 --- a/src/cryfs/filesystem/cachingfsblobstore/CachingFsBlobStore.h +++ b/src/cryfs/filesystem/cachingfsblobstore/CachingFsBlobStore.h @@ -50,7 +50,7 @@ namespace cryfs { inline CachingFsBlobStore::CachingFsBlobStore(cpputils::unique_ref baseBlobStore) - : _baseBlobStore(std::move(baseBlobStore)), _cache() { + : _baseBlobStore(std::move(baseBlobStore)), _cache("fsblobstore") { } inline CachingFsBlobStore::~CachingFsBlobStore() { diff --git a/src/fspp/fuse/Fuse.cpp b/src/fspp/fuse/Fuse.cpp index 68489964..6303b28b 100644 --- a/src/fspp/fuse/Fuse.cpp +++ b/src/fspp/fuse/Fuse.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include "InvalidFilesystem.h" @@ -23,7 +24,9 @@ namespace bf = boost::filesystem; using namespace cpputils::logging; using std::make_shared; using std::shared_ptr; +using std::string; using namespace fspp::fuse; +using cpputils::set_thread_name; namespace { bool is_valid_fspp_path(const bf::path& path) { @@ -32,6 +35,18 @@ bool is_valid_fspp_path(const bf::path& path) { && !path.has_root_name() // on Windows, it shouldn't have a device specifier (i.e. no "C:") && (path.string() == path.generic_string()); // must use portable '/' as directory separator } + +class ThreadNameForDebugging final { +public: + ThreadNameForDebugging(const string& threadName) { + std::string name = "fspp_" + threadName; + set_thread_name(name.c_str()); + } + + ~ThreadNameForDebugging() { + set_thread_name("fspp_idle"); + } +}; } #define FUSE_OBJ (static_cast(fuse_get_context()->private_data)) @@ -335,6 +350,7 @@ void Fuse::unmount(const bf::path& mountdir, bool force) { } int Fuse::getattr(const bf::path &path, fspp::fuse::STAT *stbuf) { + ThreadNameForDebugging _threadName("getattr"); #ifdef FSPP_LOG LOG(DEBUG, "getattr({}, _, _)", path); #endif @@ -357,6 +373,7 @@ int Fuse::getattr(const bf::path &path, fspp::fuse::STAT *stbuf) { } int Fuse::fgetattr(const bf::path &path, fspp::fuse::STAT *stbuf, fuse_file_info *fileinfo) { + ThreadNameForDebugging _threadName("fgetattr"); #ifdef FSPP_LOG LOG(DEBUG, "fgetattr({}, _, _)\n", path); #endif @@ -389,6 +406,7 @@ int Fuse::fgetattr(const bf::path &path, fspp::fuse::STAT *stbuf, fuse_file_info } int Fuse::readlink(const bf::path &path, char *buf, size_t size) { + ThreadNameForDebugging _threadName("readlink"); #ifdef FSPP_LOG LOG(DEBUG, "readlink({}, _, {})", path, size); #endif @@ -414,11 +432,13 @@ int Fuse::mknod(const bf::path &path, ::mode_t mode, dev_t rdev) { UNUSED(rdev); UNUSED(mode); UNUSED(path); + ThreadNameForDebugging _threadName("mknod"); LOG(WARN, "Called non-implemented mknod({}, {}, _)", path, mode); return ENOSYS; } int Fuse::mkdir(const bf::path &path, ::mode_t mode) { + ThreadNameForDebugging _threadName("mkdir"); #ifdef FSPP_LOG LOG(DEBUG, "mkdir({}, {})", path, mode); #endif @@ -442,6 +462,7 @@ int Fuse::mkdir(const bf::path &path, ::mode_t mode) { } int Fuse::unlink(const bf::path &path) { + ThreadNameForDebugging _threadName("unlink"); #ifdef FSPP_LOG LOG(DEBUG, "unlink({})", path); #endif @@ -464,6 +485,7 @@ int Fuse::unlink(const bf::path &path) { } int Fuse::rmdir(const bf::path &path) { + ThreadNameForDebugging _threadName("rmdir"); #ifdef FSPP_LOG LOG(DEBUG, "rmdir({})", path); #endif @@ -486,6 +508,7 @@ int Fuse::rmdir(const bf::path &path) { } int Fuse::symlink(const bf::path &to, const bf::path &from) { + ThreadNameForDebugging _threadName("symlink"); #ifdef FSPP_LOG LOG(DEBUG, "symlink({}, {})", to, from); #endif @@ -509,6 +532,7 @@ int Fuse::symlink(const bf::path &to, const bf::path &from) { } int Fuse::rename(const bf::path &from, const bf::path &to) { + ThreadNameForDebugging _threadName("rename"); #ifdef FSPP_LOG LOG(DEBUG, "rename({}, {})", from, to); #endif @@ -533,6 +557,7 @@ int Fuse::rename(const bf::path &from, const bf::path &to) { //TODO int Fuse::link(const bf::path &from, const bf::path &to) { + ThreadNameForDebugging _threadName("link"); LOG(WARN, "NOT IMPLEMENTED: link({}, {})", from, to); //auto real_from = _impl->RootDir() / from; //auto real_to = _impl->RootDir() / to; @@ -542,6 +567,7 @@ int Fuse::link(const bf::path &from, const bf::path &to) { } int Fuse::chmod(const bf::path &path, ::mode_t mode) { + ThreadNameForDebugging _threadName("chmod"); #ifdef FSPP_LOG LOG(DEBUG, "chmod({}, {})", path, mode); #endif @@ -564,6 +590,7 @@ int Fuse::chmod(const bf::path &path, ::mode_t mode) { } int Fuse::chown(const bf::path &path, ::uid_t uid, ::gid_t gid) { + ThreadNameForDebugging _threadName("chown"); #ifdef FSPP_LOG LOG(DEBUG, "chown({}, {}, {})", path, uid, gid); #endif @@ -586,6 +613,7 @@ int Fuse::chown(const bf::path &path, ::uid_t uid, ::gid_t gid) { } int Fuse::truncate(const bf::path &path, int64_t size) { + ThreadNameForDebugging _threadName("truncate"); #ifdef FSPP_LOG LOG(DEBUG, "truncate({}, {})", path, size); #endif @@ -608,6 +636,7 @@ int Fuse::truncate(const bf::path &path, int64_t size) { } int Fuse::ftruncate(const bf::path &path, int64_t size, fuse_file_info *fileinfo) { + ThreadNameForDebugging _threadName("ftruncate"); #ifdef FSPP_LOG LOG(DEBUG, "ftruncate({}, {})", path, size); #endif @@ -630,6 +659,7 @@ int Fuse::ftruncate(const bf::path &path, int64_t size, fuse_file_info *fileinfo } int Fuse::utimens(const bf::path &path, const timespec times[2]) { + ThreadNameForDebugging _threadName("utimens"); #ifdef FSPP_LOG LOG(DEBUG, "utimens({}, _)", path); #endif @@ -652,6 +682,7 @@ int Fuse::utimens(const bf::path &path, const timespec times[2]) { } int Fuse::open(const bf::path &path, fuse_file_info *fileinfo) { + ThreadNameForDebugging _threadName("open"); #ifdef FSPP_LOG LOG(DEBUG, "open({}, _)", path); #endif @@ -674,6 +705,7 @@ int Fuse::open(const bf::path &path, fuse_file_info *fileinfo) { } int Fuse::release(const bf::path &path, fuse_file_info *fileinfo) { + ThreadNameForDebugging _threadName("release"); #ifdef FSPP_LOG LOG(DEBUG, "release({}, _)", path); #endif @@ -696,6 +728,7 @@ int Fuse::release(const bf::path &path, fuse_file_info *fileinfo) { } int Fuse::read(const bf::path &path, char *buf, size_t size, int64_t offset, fuse_file_info *fileinfo) { + ThreadNameForDebugging _threadName("read"); #ifdef FSPP_LOG LOG(DEBUG, "read({}, _, {}, {}, _)", path, size, offset); #endif @@ -717,6 +750,7 @@ int Fuse::read(const bf::path &path, char *buf, size_t size, int64_t offset, fus } int Fuse::write(const bf::path &path, const char *buf, size_t size, int64_t offset, fuse_file_info *fileinfo) { + ThreadNameForDebugging _threadName("write"); #ifdef FSPP_LOG LOG(DEBUG, "write({}, _, {}, {}, _)", path, size, offsset); #endif @@ -739,6 +773,7 @@ int Fuse::write(const bf::path &path, const char *buf, size_t size, int64_t offs } int Fuse::statfs(const bf::path &path, struct ::statvfs *fsstat) { + ThreadNameForDebugging _threadName("statfs"); #ifdef FSPP_LOG LOG(DEBUG, "statfs({}, _)", path); #endif @@ -762,6 +797,7 @@ int Fuse::statfs(const bf::path &path, struct ::statvfs *fsstat) { } int Fuse::flush(const bf::path &path, fuse_file_info *fileinfo) { + ThreadNameForDebugging _threadName("flush"); #ifdef FSPP_LOG LOG(WARN, "flush({}, _)", path); #endif @@ -784,6 +820,7 @@ int Fuse::flush(const bf::path &path, fuse_file_info *fileinfo) { } int Fuse::fsync(const bf::path &path, int datasync, fuse_file_info *fileinfo) { + ThreadNameForDebugging _threadName("fsync"); #ifdef FSPP_LOG LOG(DEBUG, "fsync({}, {}, _)", path, datasync); #endif @@ -812,12 +849,14 @@ int Fuse::fsync(const bf::path &path, int datasync, fuse_file_info *fileinfo) { int Fuse::opendir(const bf::path &path, fuse_file_info *fileinfo) { UNUSED(path); UNUSED(fileinfo); + ThreadNameForDebugging _threadName("opendir"); //LOG(DEBUG, "opendir({}, _)", path); //We don't need opendir, because readdir works directly on the path return 0; } int Fuse::readdir(const bf::path &path, void *buf, fuse_fill_dir_t filler, int64_t offset, fuse_file_info *fileinfo) { + ThreadNameForDebugging _threadName("readdir"); #ifdef FSPP_LOG LOG(DEBUG, "readdir({}, _, _, {}, _)", path, offest); #endif @@ -863,6 +902,7 @@ int Fuse::readdir(const bf::path &path, void *buf, fuse_fill_dir_t filler, int64 int Fuse::releasedir(const bf::path &path, fuse_file_info *fileinfo) { UNUSED(path); UNUSED(fileinfo); + ThreadNameForDebugging _threadName("releasedir"); //LOG(DEBUG, "releasedir({}, _)", path); //We don't need releasedir, because readdir works directly on the path return 0; @@ -873,12 +913,14 @@ int Fuse::fsyncdir(const bf::path &path, int datasync, fuse_file_info *fileinfo) UNUSED(fileinfo); UNUSED(datasync); UNUSED(path); + ThreadNameForDebugging _threadName("fsyncdir"); //LOG(WARN, "Called non-implemented fsyncdir({}, {}, _)", path, datasync); return 0; } void Fuse::init(fuse_conn_info *conn) { UNUSED(conn); + ThreadNameForDebugging _threadName("init"); _fs = _init(this); LOG(INFO, "Filesystem started."); @@ -892,6 +934,7 @@ void Fuse::init(fuse_conn_info *conn) { } void Fuse::destroy() { + ThreadNameForDebugging _threadName("destroy"); _fs = make_shared(); LOG(INFO, "Filesystem stopped."); _running = false; @@ -899,6 +942,7 @@ void Fuse::destroy() { } int Fuse::access(const bf::path &path, int mask) { + ThreadNameForDebugging _threadName("access"); #ifdef FSPP_LOG LOG(DEBUG, "access({}, {})", path, mask); #endif @@ -921,6 +965,7 @@ int Fuse::access(const bf::path &path, int mask) { } int Fuse::create(const bf::path &path, ::mode_t mode, fuse_file_info *fileinfo) { + ThreadNameForDebugging _threadName("create"); #ifdef FSPP_LOG LOG(DEBUG, "create({}, {}, _)", path, mode); #endif diff --git a/test/blockstore/implementations/caching/cache/CacheTest_MoveConstructor.cpp b/test/blockstore/implementations/caching/cache/CacheTest_MoveConstructor.cpp index e69a278b..c4377ab0 100644 --- a/test/blockstore/implementations/caching/cache/CacheTest_MoveConstructor.cpp +++ b/test/blockstore/implementations/caching/cache/CacheTest_MoveConstructor.cpp @@ -13,7 +13,7 @@ using ::testing::Test; //Test that Cache uses a move constructor for Value if possible class CacheTest_MoveConstructor: public Test { public: - CacheTest_MoveConstructor(): cache(make_unique_ref>()) { + CacheTest_MoveConstructor(): cache(make_unique_ref>("test")) { CopyableMovableValueType::numCopyConstructorCalled = 0; } unique_ref> cache; diff --git a/test/blockstore/implementations/caching/cache/CacheTest_RaceCondition.cpp b/test/blockstore/implementations/caching/cache/CacheTest_RaceCondition.cpp index 7bc76c3b..d41fd3cc 100644 --- a/test/blockstore/implementations/caching/cache/CacheTest_RaceCondition.cpp +++ b/test/blockstore/implementations/caching/cache/CacheTest_RaceCondition.cpp @@ -36,7 +36,7 @@ private: class CacheTest_RaceCondition: public ::testing::Test { public: - CacheTest_RaceCondition(): cache(), destructorStarted(), destructorFinished(false) {} + CacheTest_RaceCondition(): cache("test"), destructorStarted(), destructorFinished(false) {} static constexpr unsigned int MAX_ENTRIES = 100; diff --git a/test/blockstore/implementations/caching/cache/PeriodicTaskTest.cpp b/test/blockstore/implementations/caching/cache/PeriodicTaskTest.cpp index 46843015..5dc7c739 100644 --- a/test/blockstore/implementations/caching/cache/PeriodicTaskTest.cpp +++ b/test/blockstore/implementations/caching/cache/PeriodicTaskTest.cpp @@ -37,7 +37,7 @@ class PeriodicTaskTest: public Test { }; TEST_F(PeriodicTaskTest, DoesntDeadlockInDestructorWhenDestructedImmediately) { - PeriodicTask task([](){}, 1); + PeriodicTask task([](){}, 1, "test"); } TEST_F(PeriodicTaskTest, CallsCallbackAtLeast10Times) { @@ -45,7 +45,7 @@ TEST_F(PeriodicTaskTest, CallsCallbackAtLeast10Times) { PeriodicTask task([&counter](){ counter.decrease(); - }, 0.001); + }, 0.001, "test"); counter.waitForZero(); } @@ -55,7 +55,7 @@ TEST_F(PeriodicTaskTest, DoesntCallCallbackAfterDestruction) { { PeriodicTask task([&callCount](){ callCount += 1; - }, 0.001); + }, 0.001, "test"); } int callCountDirectlyAfterDestruction = callCount; boost::this_thread::sleep_for(boost::chrono::seconds(1)); diff --git a/test/blockstore/implementations/caching/cache/testutils/CacheTest.h b/test/blockstore/implementations/caching/cache/testutils/CacheTest.h index f4c520b1..937609c9 100644 --- a/test/blockstore/implementations/caching/cache/testutils/CacheTest.h +++ b/test/blockstore/implementations/caching/cache/testutils/CacheTest.h @@ -13,7 +13,7 @@ // Furthermore, the class checks that there are no memory leaks left after destructing the QueueMap (by counting leftover instances of Keys/Values). class CacheTest: public ::testing::Test { public: - CacheTest(): _cache() {} + CacheTest(): _cache("test") {} void push(int key, int value); boost::optional pop(int key); diff --git a/test/cryfs-cli/CallAfterTimeoutTest.cpp b/test/cryfs-cli/CallAfterTimeoutTest.cpp index 2ba7eec1..5d11ed0c 100644 --- a/test/cryfs-cli/CallAfterTimeoutTest.cpp +++ b/test/cryfs-cli/CallAfterTimeoutTest.cpp @@ -15,7 +15,7 @@ public: CallAfterTimeoutTest(): called(false) {} unique_ref callAfterTimeout(milliseconds timeout) { - return make_unique_ref(timeout, [this] {called = true;}); + return make_unique_ref(timeout, [this] {called = true;}, "test"); } std::atomic called; From ec0561396855566f3435babe9ee91bf7942fca19 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Sun, 20 Jan 2019 19:57:23 -0800 Subject: [PATCH 04/15] Mark 0.9.10 as released --- ChangeLog.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog.txt b/ChangeLog.txt index 5655f461..7ef4b3d9 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -19,7 +19,7 @@ Fixed bugs: * On Mac OS X, Finder shows the correct name for the mount directory -Version 0.9.10 (unreleased) +Version 0.9.10 -------------- Fixed bugs: * Fixed occasional deadlock (https://github.com/cryfs/cryfs/issues/64) From 6801e6a3d9d3c8fb2adef0e116b88704c4c30698 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Sun, 20 Jan 2019 20:20:18 -0800 Subject: [PATCH 05/15] Add release builds to appveyor --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 469b15c9..d5fa3de9 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -12,7 +12,7 @@ platform: configuration: - Debug - RelWithDebInfo -# - Release + - Release version: '{branch}-{build}' From db6ed6ec99bee27ee5816c807d111aa25f3fc890 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Mon, 21 Jan 2019 00:12:07 -0800 Subject: [PATCH 06/15] Use more deterring language in question if file system should be migrated --- src/cryfs/config/CryConfigLoader.cpp | 2 +- test/cryfs/config/CryConfigLoaderTest.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cryfs/config/CryConfigLoader.cpp b/src/cryfs/config/CryConfigLoader.cpp index abdf022b..8476c30e 100644 --- a/src/cryfs/config/CryConfigLoader.cpp +++ b/src/cryfs/config/CryConfigLoader.cpp @@ -71,7 +71,7 @@ void CryConfigLoader::_checkVersion(const CryConfig &config, bool allowFilesyste } } if (!allowFilesystemUpgrade && gitversion::VersionCompare::isOlderThan(config.Version(), CryConfig::FilesystemFormatVersion)) { - if (!_console->askYesNo("This filesystem is for CryFS " + config.Version() + " (or a later version with the same storage format). You're running a CryFS version using storage format " + CryConfig::FilesystemFormatVersion + ". It can be migrated, but afterwards couldn't be opened anymore with older versions. Please make a backup of your data before attempting a migration. Do you want to migrate it now?", false)) { + if (!_console->askYesNo("This filesystem is for CryFS " + config.Version() + " (or a later version with the same storage format). You're running a CryFS version using storage format " + CryConfig::FilesystemFormatVersion + ". It is recommended to create a new filesystem with CryFS 0.10 and copy your files into it. If you don't want to do that, we can also attempt to migrate the existing filesystem, but that can take a long time, you won't be getting some of the performance advantages of the 0.10 release series, and if the migration fails, your data may be lost. If you decide to continue, please make sure you have a backup of your data. Do you want to attempt a migration now?", false)) { throw CryfsException("This filesystem is for CryFS " + config.Version() + " (or a later version with the same storage format). It has to be migrated.", ErrorCode::TooOldFilesystemFormat); } } diff --git a/test/cryfs/config/CryConfigLoaderTest.cpp b/test/cryfs/config/CryConfigLoaderTest.cpp index d31efc3f..1c2b4197 100644 --- a/test/cryfs/config/CryConfigLoaderTest.cpp +++ b/test/cryfs/config/CryConfigLoaderTest.cpp @@ -307,7 +307,7 @@ TEST_F(CryConfigLoaderTest, AsksWhenLoadingNewerFilesystem_AnswerNo) { } TEST_F(CryConfigLoaderTest, AsksWhenMigratingOlderFilesystem) { - EXPECT_CALL(*console, askYesNo(HasSubstr("Do you want to migrate it now?"), false)).Times(1).WillOnce(Return(true)); + EXPECT_CALL(*console, askYesNo(HasSubstr("Do you want to attempt a migration now?"), false)).Times(1).WillOnce(Return(true)); string version = olderVersion(); CreateWithVersion(version, version); @@ -315,14 +315,14 @@ TEST_F(CryConfigLoaderTest, AsksWhenMigratingOlderFilesystem) { } TEST_F(CryConfigLoaderTest, DoesNotAskForMigrationWhenCorrectVersion) { - EXPECT_CALL(*console, askYesNo(HasSubstr("Do you want to migrate it now?"), _)).Times(0); + EXPECT_CALL(*console, askYesNo(HasSubstr("Do you want to attempt a migration now?"), _)).Times(0); CreateWithVersion(gitversion::VersionString(), CryConfig::FilesystemFormatVersion); EXPECT_NE(boost::none, Load()); } TEST_F(CryConfigLoaderTest, DontMigrateWhenAnsweredNo) { - EXPECT_CALL(*console, askYesNo(HasSubstr("Do you want to migrate it now?"), false)).Times(1).WillOnce(Return(false)); + EXPECT_CALL(*console, askYesNo(HasSubstr("Do you want to attempt a migration now?"), false)).Times(1).WillOnce(Return(false)); string version = olderVersion(); CreateWithVersion(version, version); From 6251793b871b2c67442d4678172fc9c0440d2395 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Mon, 21 Jan 2019 00:29:37 -0800 Subject: [PATCH 07/15] Fix typo --- src/cryfs/filesystem/fsblobstore/FsBlobStore.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cryfs/filesystem/fsblobstore/FsBlobStore.cpp b/src/cryfs/filesystem/fsblobstore/FsBlobStore.cpp index 6d116358..502d1ebe 100644 --- a/src/cryfs/filesystem/fsblobstore/FsBlobStore.cpp +++ b/src/cryfs/filesystem/fsblobstore/FsBlobStore.cpp @@ -54,7 +54,7 @@ boost::optional> FsBlobStore::load(const blockstore::BlockId dir.AppendChildrenTo(&children); for (const auto &child : children) { auto childEntry = dir.GetChild(child.name); - ASSERT(childEntry != none, "Couldn't load child, although it was returned as a child in the lsit."); + ASSERT(childEntry != none, "Couldn't load child, although it was returned as a child in the list."); auto childBlob = _baseBlobStore->load(childEntry->blockId()); ASSERT(childBlob != none, "Couldn't load child blob"); _migrate(std::move(*childBlob), dir.blockId()); From 38397a2d86f5df4e8164c212425efcaa05fdc38f Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Mon, 21 Jan 2019 03:38:20 -0800 Subject: [PATCH 08/15] Cherry-pick some commits from Crypto++ to make OpenMP for scrypt work on Windows --- vendor/README | 5 ++++ .../cryptopp/vendor_cryptopp/cryptest.nmake | 1 + vendor/cryptopp/vendor_cryptopp/salsa.cpp | 5 ++++ vendor/cryptopp/vendor_cryptopp/scrypt.cpp | 28 ++++++++++++++++--- vendor/cryptopp/vendor_cryptopp/scrypt.h | 4 ++- 5 files changed, 38 insertions(+), 5 deletions(-) diff --git a/vendor/README b/vendor/README index 771c571b..aadcf953 100644 --- a/vendor/README +++ b/vendor/README @@ -5,3 +5,8 @@ spdlog: https://github.com/gabime/spdlog/tree/v0.16.3/include/spdlog cryptopp: https://github.com/weidai11/cryptopp/tree/CRYPTOPP_8_0_0 - changed: added CMakeLists.txt and cryptopp-config.cmake from https://github.com/noloader/cryptopp-cmake/tree/CRYPTOPP_8_0_0 - changed: commented out line including winapifamily.h in CMakeLists.txt + - cherry-picked commits to get OpenMP for scrypt on Windows: + - https://github.com/weidai11/cryptopp/commit/aa043b38a7930725c31a0cd7016986d1c581c573 + - https://github.com/weidai11/cryptopp/commit/672f5c7f3dad8ae12b2d0ce0940ccb7c8e257bf8 + - https://github.com/weidai11/cryptopp/commit/7e96a283a3192d29aac5b60e5b4ff19248f00d9a + - https://github.com/weidai11/cryptopp/commit/ca32b63038d5f7b13e2e00809cd9184a1efe8c24 diff --git a/vendor/cryptopp/vendor_cryptopp/cryptest.nmake b/vendor/cryptopp/vendor_cryptopp/cryptest.nmake index e2a5db72..a15fc7b0 100644 --- a/vendor/cryptopp/vendor_cryptopp/cryptest.nmake +++ b/vendor/cryptopp/vendor_cryptopp/cryptest.nmake @@ -139,6 +139,7 @@ LDLIBS = # CXXFLAGS = $(CXXFLAGS) /DDEBUG /D_DEBUG /Oi /Oy- /Od /MTd # Release build. Add /OPT:REF to linker CXXFLAGS = $(CXXFLAGS) /DNDEBUG /D_NDEBUG /Oi /Oy /O2 /MT +# Linker flags. LDFLAGS = $(LDFLAGS) /OPT:REF # Attempt to detect when and are available diff --git a/vendor/cryptopp/vendor_cryptopp/salsa.cpp b/vendor/cryptopp/vendor_cryptopp/salsa.cpp index 148d970a..fb2dc03e 100644 --- a/vendor/cryptopp/vendor_cryptopp/salsa.cpp +++ b/vendor/cryptopp/vendor_cryptopp/salsa.cpp @@ -90,9 +90,14 @@ void Salsa20_Core(word32* data, unsigned int rounds) x[15] ^= rotlConstant<18>(x[14]+x[13]); } +#ifdef _MSC_VER + for (size_t i = 0; i < 16; ++i) + data[i] += x[i]; +#else #pragma omp simd for (size_t i = 0; i < 16; ++i) data[i] += x[i]; +#endif } std::string Salsa20_Policy::AlgorithmProvider() const diff --git a/vendor/cryptopp/vendor_cryptopp/scrypt.cpp b/vendor/cryptopp/vendor_cryptopp/scrypt.cpp index 3566c3e1..69f486d1 100644 --- a/vendor/cryptopp/vendor_cryptopp/scrypt.cpp +++ b/vendor/cryptopp/vendor_cryptopp/scrypt.cpp @@ -14,6 +14,8 @@ #include "sha.h" #include +#include + #ifdef _OPENMP # include #endif @@ -53,9 +55,14 @@ static inline void BlockCopy(byte* dest, byte* src, size_t len) static inline void BlockXOR(byte* dest, byte* src, size_t len) { +#ifdef _MSC_VER + for (size_t i = 0; i < len; ++i) + dest[i] ^= src[i]; +#else #pragma omp simd for (size_t i = 0; i < len; ++i) dest[i] ^= src[i]; +#endif } static inline void PBKDF2_SHA256(byte* buf, size_t dkLen, @@ -171,6 +178,16 @@ void Scrypt::ValidateParameters(size_t derivedLen, word64 cost, word64 blockSize } } + // https://github.com/weidai11/cryptopp/issues/787 + CRYPTOPP_ASSERT(parallelization <= std::numeric_limits::max()); + if (parallelization > static_cast(std::numeric_limits::max())) + { + std::ostringstream oss; + oss << " parallelization " << parallelization << " is larger than "; + oss << std::numeric_limits::max(); + throw InvalidArgument("Scrypt: " + oss.str()); + } + CRYPTOPP_ASSERT(IsPowerOf2(cost)); if (IsPowerOf2(cost) == false) throw InvalidArgument("Scrypt: cost must be a power of 2"); @@ -245,10 +262,13 @@ size_t Scrypt::DeriveKey(byte*derived, size_t derivedLen, const byte*secret, siz // 1: (B_0 ... B_{p-1}) <-- PBKDF2(P, S, 1, p * MFLen) PBKDF2_SHA256(B, B.size(), secret, secretLen, salt, saltLen, 1); + // Visual Studio and OpenMP 2.0 fixup. We must use int, not size_t. + int maxParallel=0; + if (!SafeConvert(parallel, maxParallel)) + maxParallel = std::numeric_limits::max(); + #ifdef _OPENMP - int threads = STDMIN(omp_get_max_threads(), - static_cast(STDMIN(static_cast(parallel), - static_cast(std::numeric_limits::max())))); + int threads = STDMIN(omp_get_max_threads(), maxParallel); #endif // http://stackoverflow.com/q/49604260/608639 @@ -260,7 +280,7 @@ size_t Scrypt::DeriveKey(byte*derived, size_t derivedLen, const byte*secret, siz // 2: for i = 0 to p - 1 do #pragma omp for - for (size_t i = 0; i < static_cast(parallel); ++i) + for (int i = 0; i < maxParallel; ++i) { // 3: B_i <-- MF(B_i, N) const ptrdiff_t offset = static_cast(blockSize*i*128); diff --git a/vendor/cryptopp/vendor_cryptopp/scrypt.h b/vendor/cryptopp/vendor_cryptopp/scrypt.h index 129c5dc3..8c6f394f 100644 --- a/vendor/cryptopp/vendor_cryptopp/scrypt.h +++ b/vendor/cryptopp/vendor_cryptopp/scrypt.h @@ -76,7 +76,9 @@ public: /// \details The parameter blockSize ("r" in the documents) specifies the block /// size. /// \details The parallelization parameter ("p" in the documents) is a positive - /// integer less than or equal to ((2^32-1) * 32) / (128 * r). + /// integer less than or equal to ((2^32-1) * 32) / (128 * r). Due to Microsoft + /// and its OpenMP 2.0 implementation parallelization is limited to + /// std::numeric_limits::max(). /// \details Scrypt always returns 1 because it only performs 1 iteration. Other /// derivation functions, like PBKDF's, will return more interesting values. /// \details The Crypto++ implementation of Scrypt is limited by C++ datatypes. For From 081ea8d622a45ec65582cb5401253dfbec7ef7d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Me=C3=9Fmer?= Date: Mon, 21 Jan 2019 11:23:52 -0800 Subject: [PATCH 09/15] Update README.md - Add installation instructions for Windows and osx --- README.md | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index fc2a26f0..85109f51 100644 --- a/README.md +++ b/README.md @@ -6,10 +6,29 @@ See [https://www.cryfs.org](https://www.cryfs.org). Install latest release ====================== +Linux +------ + This only works for Ubuntu 17.04 and later, and Debian Stretch and later. You can also use CryFS on older versions of these distributions by following the **Building from source** instructions below. sudo apt install cryfs + +OSX +---- + +CryFS is distributed via Homebrew. Just do + + brew install cryfs + +Windows (experimental) +---------------------- + +CryFS has experimental Windows support since the 0.10 release series. To install it, do: + +1. Install [DokanY](https://github.com/dokan-dev/dokany/releases) +2. Install [Microsoft Visual C++ Redistributable for Visual Studio 2017](https://support.microsoft.com/en-us/help/2977003/the-latest-supported-visual-c-downloads) +3. Install CryFS GUI === @@ -72,12 +91,14 @@ You can pass the following variables to the *cmake* command (using *-Dvariablena - **-DCRYFS_UPDATE_CHECKS**=off: Build a CryFS that doesn't check online for updates and security vulnerabilities. Building on Windows (experimental) ---------------- +---------------------------------- Build with Visual Studio 2017 and pass in the following flags to CMake: - -DDOKAN_PATH=[dokan library location, e.g. "C:\Program Files\Dokan\DokanLibrary-1.1.0"] - -DBOOST_ROOT=[path to root of boost installation] + -DDOKAN_PATH=[dokan library location, e.g. "C:\Program Files\Dokan\DokanLibrary-1.1.0"] + -DBOOST_ROOT=[path to root of boost installation] + +If you set these variables correctly in the `CMakeSettings.json` file, you should be able to open the cryfs source folder with Visual Studio 2017. Troubleshooting --------------- @@ -141,7 +162,7 @@ There are additional requirements if you want to create packages. They are: 2. Build $ mkdir cmake && cd cmake - $ cmake .. -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=off + $ cmake .. -DCMAKE_BUILD_TYPE=RelWithDebInfo -DBUILD_TESTING=off $ make package From fe0d5493b2df96e61944e9c7b01436f37b04eb5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Me=C3=9Fmer?= Date: Mon, 21 Jan 2019 11:24:41 -0800 Subject: [PATCH 10/15] Update README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 85109f51..3dd10af4 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ CryFS has experimental Windows support since the 0.10 release series. To install 1. Install [DokanY](https://github.com/dokan-dev/dokany/releases) 2. Install [Microsoft Visual C++ Redistributable for Visual Studio 2017](https://support.microsoft.com/en-us/help/2977003/the-latest-supported-visual-c-downloads) -3. Install CryFS +3. Install [CryFS](https://www.cryfs.org/#download) GUI === From e06a0f97438c7ec86f9f28fa3bca590074cfc5ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Me=C3=9Fmer?= Date: Mon, 21 Jan 2019 11:25:31 -0800 Subject: [PATCH 11/15] Update README.md --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 3dd10af4..e4b9cd85 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,7 @@ OSX CryFS is distributed via Homebrew. Just do + brew cask install osxfuse brew install cryfs Windows (experimental) From 6042d4ba6b8c04d34a6faa21575364f9e8488632 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Mon, 21 Jan 2019 11:47:16 -0800 Subject: [PATCH 12/15] Build with OpenMP on Windows --- vendor/cryptopp/CMakeLists.txt | 132 ++++++++++++++++----------------- 1 file changed, 64 insertions(+), 68 deletions(-) diff --git a/vendor/cryptopp/CMakeLists.txt b/vendor/cryptopp/CMakeLists.txt index 9316493a..ef7a3e10 100644 --- a/vendor/cryptopp/CMakeLists.txt +++ b/vendor/cryptopp/CMakeLists.txt @@ -9,76 +9,72 @@ target_compile_definitions(cryptopp PUBLIC $<$:CRYPTOPP_DEBUG>) # add_compile_options($<$:-DCRYPTOPP_DEBUG>) # add to stuff built in subdirectories (like the actual library) if(NOT DISABLE_OPENMP) - if (MSVC) - message(WARNING "MSVC does not support the OpenMP 4.0 standard used by Crypto++. Disabling OpenMP. This can cause degraded performance.") - else() - find_package(OpenMP) + find_package(OpenMP) - if (OPENMP_FOUND OR OPENMP_CXX_FOUND) - message(STATUS "Found libomp without any special flags") - endif() - - # If OpenMP wasn't found, try if we can find it in the default Macports location - if((NOT OPENMP_FOUND) AND (NOT OPENMP_CXX_FOUND) AND EXISTS "/opt/local/lib/libomp/libomp.dylib") # older cmake uses OPENMP_FOUND, newer cmake also sets OPENMP_CXX_FOUND, homebrew installations seem only to get the latter set. - set(OpenMP_CXX_FLAGS "-Xpreprocessor -fopenmp -I/opt/local/include/libomp/") - set(OpenMP_CXX_LIB_NAMES omp) - set(OpenMP_omp_LIBRARY /opt/local/lib/libomp/libomp.dylib) - - find_package(OpenMP) - if (OPENMP_FOUND OR OPENMP_CXX_FOUND) - message(STATUS "Found libomp in macports default location.") - else() - message(FATAL_ERROR "Didn't find libomp. Tried macports default location but also didn't find it.") - endif() - endif() - - # If OpenMP wasn't found, try if we can find it in the default Homebrew location - if((NOT OPENMP_FOUND) AND (NOT OPENMP_CXX_FOUND) AND EXISTS "/usr/local/opt/libomp/lib/libomp.dylib") - set(OpenMP_CXX_FLAGS "-Xpreprocessor -fopenmp -I/usr/local/opt/libomp/include") - set(OpenMP_CXX_LIB_NAMES omp) - set(OpenMP_omp_LIBRARY /usr/local/opt/libomp/lib/libomp.dylib) - - find_package(OpenMP) - if (OPENMP_FOUND OR OPENMP_CXX_FOUND) - message(STATUS "Found libomp in homebrew default location.") - else() - message(FATAL_ERROR "Didn't find libomp. Tried homebrew default location but also didn't find it.") - endif() - endif() - - set(Additional_OpenMP_Libraries_Workaround "") - - # Workaround because older cmake on apple doesn't support FindOpenMP - if((NOT OPENMP_FOUND) AND (NOT OPENMP_CXX_FOUND)) - if((APPLE AND ((CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang") OR (CMAKE_CXX_COMPILER_ID STREQUAL "Clang"))) - AND ((CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "7.0") AND (CMAKE_VERSION VERSION_LESS "3.12.0"))) - message(STATUS "Applying workaround for OSX OpenMP with old cmake that doesn't have FindOpenMP") - set(OpenMP_CXX_FLAGS "-Xclang -fopenmp") - set(Additional_OpenMP_Libraries_Workaround "-lomp") - else() - message(FATAL_ERROR "Did not find OpenMP. Build with -DDISABLE_OPENMP=ON if you want to allow this and are willing to take the performance hit.") - endif() - endif() - - if(NOT TARGET OpenMP::OpenMP_CXX) - # We're on cmake < 3.9, handle behavior of the old FindOpenMP implementation - message(STATUS "Applying workaround for old CMake that doesn't define FindOpenMP using targets") - add_library(OpenMP_TARGET INTERFACE) - add_library(OpenMP::OpenMP_CXX ALIAS OpenMP_TARGET) - target_compile_options(OpenMP_TARGET INTERFACE ${OpenMP_CXX_FLAGS}) # add to all targets depending on this - find_package(Threads REQUIRED) - target_link_libraries(OpenMP_TARGET INTERFACE Threads::Threads) - target_link_libraries(OpenMP_TARGET INTERFACE ${OpenMP_CXX_FLAGS} ${Additional_OpenMP_Libraries_Workaround}) - endif() - - target_link_libraries(cryptopp PUBLIC ${OpenMP_CXX_FLAGS}) # Workaround for Ubuntu 18.04 that otherwise doesn't set -fopenmp for linking - target_link_libraries(cryptopp PUBLIC OpenMP::OpenMP_CXX) - - # also add these flags to the third party Crypto++ build setup that is built in a subdirectory - message(STATUS "OpenMP flags: ${OpenMP_CXX_FLAGS}") - string(REPLACE " " ";" REPLACED_FLAGS ${OpenMP_CXX_FLAGS}) - add_compile_options(${REPLACED_FLAGS}) + if (OPENMP_FOUND OR OPENMP_CXX_FOUND) + message(STATUS "Found libomp without any special flags") endif() + + # If OpenMP wasn't found, try if we can find it in the default Macports location + if((NOT OPENMP_FOUND) AND (NOT OPENMP_CXX_FOUND) AND EXISTS "/opt/local/lib/libomp/libomp.dylib") # older cmake uses OPENMP_FOUND, newer cmake also sets OPENMP_CXX_FOUND, homebrew installations seem only to get the latter set. + set(OpenMP_CXX_FLAGS "-Xpreprocessor -fopenmp -I/opt/local/include/libomp/") + set(OpenMP_CXX_LIB_NAMES omp) + set(OpenMP_omp_LIBRARY /opt/local/lib/libomp/libomp.dylib) + + find_package(OpenMP) + if (OPENMP_FOUND OR OPENMP_CXX_FOUND) + message(STATUS "Found libomp in macports default location.") + else() + message(FATAL_ERROR "Didn't find libomp. Tried macports default location but also didn't find it.") + endif() + endif() + + # If OpenMP wasn't found, try if we can find it in the default Homebrew location + if((NOT OPENMP_FOUND) AND (NOT OPENMP_CXX_FOUND) AND EXISTS "/usr/local/opt/libomp/lib/libomp.dylib") + set(OpenMP_CXX_FLAGS "-Xpreprocessor -fopenmp -I/usr/local/opt/libomp/include") + set(OpenMP_CXX_LIB_NAMES omp) + set(OpenMP_omp_LIBRARY /usr/local/opt/libomp/lib/libomp.dylib) + + find_package(OpenMP) + if (OPENMP_FOUND OR OPENMP_CXX_FOUND) + message(STATUS "Found libomp in homebrew default location.") + else() + message(FATAL_ERROR "Didn't find libomp. Tried homebrew default location but also didn't find it.") + endif() + endif() + + set(Additional_OpenMP_Libraries_Workaround "") + + # Workaround because older cmake on apple doesn't support FindOpenMP + if((NOT OPENMP_FOUND) AND (NOT OPENMP_CXX_FOUND)) + if((APPLE AND ((CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang") OR (CMAKE_CXX_COMPILER_ID STREQUAL "Clang"))) + AND ((CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL "7.0") AND (CMAKE_VERSION VERSION_LESS "3.12.0"))) + message(STATUS "Applying workaround for OSX OpenMP with old cmake that doesn't have FindOpenMP") + set(OpenMP_CXX_FLAGS "-Xclang -fopenmp") + set(Additional_OpenMP_Libraries_Workaround "-lomp") + else() + message(FATAL_ERROR "Did not find OpenMP. Build with -DDISABLE_OPENMP=ON if you want to allow this and are willing to take the performance hit.") + endif() + endif() + + if(NOT TARGET OpenMP::OpenMP_CXX) + # We're on cmake < 3.9, handle behavior of the old FindOpenMP implementation + message(STATUS "Applying workaround for old CMake that doesn't define FindOpenMP using targets") + add_library(OpenMP_TARGET INTERFACE) + add_library(OpenMP::OpenMP_CXX ALIAS OpenMP_TARGET) + target_compile_options(OpenMP_TARGET INTERFACE ${OpenMP_CXX_FLAGS}) # add to all targets depending on this + find_package(Threads REQUIRED) + target_link_libraries(OpenMP_TARGET INTERFACE Threads::Threads) + target_link_libraries(OpenMP_TARGET INTERFACE ${OpenMP_CXX_FLAGS} ${Additional_OpenMP_Libraries_Workaround}) + endif() + + target_link_libraries(cryptopp PUBLIC ${OpenMP_CXX_FLAGS}) # Workaround for Ubuntu 18.04 that otherwise doesn't set -fopenmp for linking + target_link_libraries(cryptopp PUBLIC OpenMP::OpenMP_CXX) + + # also add these flags to the third party Crypto++ build setup that is built in a subdirectory + message(STATUS "OpenMP flags: ${OpenMP_CXX_FLAGS}") + string(REPLACE " " ";" REPLACED_FLAGS ${OpenMP_CXX_FLAGS}) + add_compile_options(${REPLACED_FLAGS}) else() message(WARNING "OpenMP is disabled. This can cause degraded performance.") endif() From 5d9192caaae0f305916812b1e4dd3c5c011abe6f Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Mon, 21 Jan 2019 23:40:14 -0800 Subject: [PATCH 13/15] Remove Visual Studio preview build from appveyor CI --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index d5fa3de9..22176d39 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -2,7 +2,7 @@ image: #- Visual Studio 2013 #- Visual Studio 2015 - Visual Studio 2017 -- Visual Studio 2017 Preview +#- Visual Studio 2017 Preview platform: - x64 From ff19a9e128160f40cc5338d8666bce883d7e5c46 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Wed, 23 Jan 2019 16:45:30 -0800 Subject: [PATCH 14/15] Release appveyor builds are actually release builds --- appveyor.yml | 3 +- test/cpp-utils/assert/assert_debug_test.cpp | 13 ++++++++ test/cpp-utils/assert/assert_release_test.cpp | 19 +++++++++++- test/cpp-utils/assert/backtrace_test.cpp | 30 ++++++++++++++++++- 4 files changed, 62 insertions(+), 3 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 22176d39..6cfcdd52 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -33,7 +33,8 @@ install: build_script: - cmd: mkdir build - cmd: cd build - - cmd: cmake .. -G "Ninja" -DBUILD_TESTING=on -DBOOST_ROOT="C:/Libraries/boost_1_65_1" -DDOKAN_PATH="C:/Program Files/Dokan/DokanLibrary-1.1.0" + # note: The cmake+ninja workflow requires us to set build type in both cmake commands ('cmake' and 'cmake --build'), otherwise the cryfs.exe will depend on debug versions of the visual studio c++ runtime (i.e. msvcp140d.dll) + - cmd: cmake .. -G "Ninja" -DCMAKE_BUILD_TYPE=%CONFIGURATION% -DBUILD_TESTING=on -DBOOST_ROOT="C:/Libraries/boost_1_65_1" -DDOKAN_PATH="C:/Program Files/Dokan/DokanLibrary-1.1.0" - cmd: cmake --build . --config %CONFIGURATION% - cmd: .\test\gitversion\gitversion-test.exe # cpp-utils-test disables ThreadDebuggingTest_ThreadName.*_thenIsCorrect because the appveyor image is too old to support the API needed for that diff --git a/test/cpp-utils/assert/assert_debug_test.cpp b/test/cpp-utils/assert/assert_debug_test.cpp index 97c60268..8a214992 100644 --- a/test/cpp-utils/assert/assert_debug_test.cpp +++ b/test/cpp-utils/assert/assert_debug_test.cpp @@ -1,6 +1,10 @@ #include #include +#ifdef NDEBUG +#define _REAL_NDEBUG +#endif + //Include the ASSERT macro for a debug build #undef NDEBUG #include "cpp-utils/assert/assert.h" @@ -29,9 +33,18 @@ constexpr const char* EXPECTED = R"(Assertion \[2==5\] failed in .*assert_debug_ ); } +#if !(defined(_MSC_VER) && defined(_REAL_NDEBUG)) TEST(AssertTest_DebugBuild, AssertMessageContainsBacktrace) { EXPECT_DEATH( ASSERT(2==5, "my message"), "cpputils::" ); } +#else +TEST(AssertTest_DebugBuild, AssertMessageContainsBacktrace) { + EXPECT_DEATH( + ASSERT(2==5, "my message"), + "#1" + ); +} +#endif diff --git a/test/cpp-utils/assert/assert_release_test.cpp b/test/cpp-utils/assert/assert_release_test.cpp index 3b814e68..17a43fb3 100644 --- a/test/cpp-utils/assert/assert_release_test.cpp +++ b/test/cpp-utils/assert/assert_release_test.cpp @@ -2,6 +2,10 @@ #include #include +#ifdef NDEBUG +#define _REAL_NDEBUG +#endif + //Include the ASSERT macro for a release build #ifndef NDEBUG #define NDEBUG @@ -31,10 +35,11 @@ TEST(AssertTest_ReleaseBuild, AssertMessage) { /*EXPECT_THAT(e.what(), MatchesRegex( R"(Assertion \[2==5\] failed in .*assert_release_test.cpp:27: my message)" ));*/ - EXPECT_TRUE(std::regex_search(e.what(), std::regex(R"(Assertion \[2==5\] failed in .*assert_release_test.cpp:26: my message)"))); + EXPECT_TRUE(std::regex_search(e.what(), std::regex(R"(Assertion \[2==5\] failed in .*assert_release_test.cpp:30: my message)"))); } } +#if !(defined(_MSC_VER) && defined(_REAL_NDEBUG)) TEST(AssertTest_ReleaseBuild, AssertMessageContainsBacktrace) { try { ASSERT(2==5, "my message"); @@ -45,3 +50,15 @@ TEST(AssertTest_ReleaseBuild, AssertMessageContainsBacktrace) { )); } } +#else +TEST(AssertTest_ReleaseBuild, AssertMessageContainsBacktrace) { + try { + ASSERT(2==5, "my message"); + FAIL(); + } catch (const cpputils::AssertFailed &e) { + EXPECT_THAT(e.what(), HasSubstr( + "#1" + )); + } +} +#endif diff --git a/test/cpp-utils/assert/backtrace_test.cpp b/test/cpp-utils/assert/backtrace_test.cpp index deb77c14..769ae8a2 100644 --- a/test/cpp-utils/assert/backtrace_test.cpp +++ b/test/cpp-utils/assert/backtrace_test.cpp @@ -19,6 +19,12 @@ namespace { } } +TEST(BacktraceTest, ContainsBacktrace) { + string backtrace = cpputils::backtrace(); + EXPECT_THAT(backtrace, HasSubstr("#1")); +} + +#if !(defined(_MSC_VER) && defined(NDEBUG)) TEST(BacktraceTest, ContainsExecutableName) { string backtrace = cpputils::backtrace(); EXPECT_THAT(backtrace, HasSubstr("cpp-utils-test")); @@ -29,7 +35,7 @@ TEST(BacktraceTest, ContainsTopLevelLine) { EXPECT_THAT(backtrace, HasSubstr("BacktraceTest")); EXPECT_THAT(backtrace, HasSubstr("ContainsTopLevelLine")); } - +#endif namespace { std::string call_process_exiting_with_nullptr_violation() { @@ -77,6 +83,7 @@ TEST(BacktraceTest, DoesntCrashOnCaughtException) { } } +#if !(defined(_MSC_VER) && defined(NDEBUG)) TEST(BacktraceTest, ShowBacktraceOnNullptrAccess) { auto output = call_process_exiting_with_nullptr_violation(); EXPECT_THAT(output, HasSubstr("cpp-utils-test_exit_signal")); @@ -96,6 +103,27 @@ TEST(BacktraceTest, ShowBacktraceOnSigIll) { auto output = call_process_exiting_with_sigill(); EXPECT_THAT(output, HasSubstr("cpp-utils-test_exit_signal")); } +#else +TEST(BacktraceTest, ShowBacktraceOnNullptrAccess) { + auto output = call_process_exiting_with_nullptr_violation(); + EXPECT_THAT(output, HasSubstr("#1")); +} + +TEST(BacktraceTest, ShowBacktraceOnSigSegv) { + auto output = call_process_exiting_with_sigsegv(); + EXPECT_THAT(output, HasSubstr("#1")); +} + +TEST(BacktraceTest, ShowBacktraceOnUnhandledException) { + auto output = call_process_exiting_with_exception("my_exception_message"); + EXPECT_THAT(output, HasSubstr("#1")); +} + +TEST(BacktraceTest, ShowBacktraceOnSigIll) { + auto output = call_process_exiting_with_sigill(); + EXPECT_THAT(output, HasSubstr("#1")); +} +#endif #if !defined(_MSC_VER) TEST(BacktraceTest, ShowBacktraceOnSigAbrt) { From ad6125a7e4e2d624feb397bf6fc5e25d2b386f66 Mon Sep 17 00:00:00 2001 From: Sebastian Messmer Date: Thu, 24 Jan 2019 00:42:58 -0800 Subject: [PATCH 15/15] Remove AnyCPU build from AppVeyor --- appveyor.yml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 6cfcdd52..9f2857ae 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,13 +1,11 @@ image: -#- Visual Studio 2013 -#- Visual Studio 2015 - Visual Studio 2017 #- Visual Studio 2017 Preview platform: - x64 - x86 -- Any CPU +#- Any CPU configuration: - Debug