From 6c781b14283ba3318daffb0380f6fe7c78419ff9 Mon Sep 17 00:00:00 2001 From: Andy Heninger Date: Mon, 12 Nov 2018 10:20:49 -0800 Subject: [PATCH] ICU-10183 Use std::mutex by default; clean up related dependency check issues. --- icu4c/source/common/putilimp.h | 2 +- icu4c/source/common/umutex.cpp | 5 ++++ icu4c/source/common/umutex.h | 31 ++++++++++++++++++--- icu4c/source/test/depstest/dependencies.txt | 11 ++++++++ icu4c/source/test/depstest/depstest.py | 22 +++++++++++++-- 5 files changed, 63 insertions(+), 8 deletions(-) mode change 100644 => 100755 icu4c/source/common/umutex.h diff --git a/icu4c/source/common/putilimp.h b/icu4c/source/common/putilimp.h index e4821e420bf..73defd9d072 100644 --- a/icu4c/source/common/putilimp.h +++ b/icu4c/source/common/putilimp.h @@ -245,7 +245,7 @@ typedef size_t uintptr_t; #ifdef U_HAVE_STD_MUTEX /* Use the predefined value. */ #else -# define U_HAVE_STD_MUTEX 0 +# define U_HAVE_STD_MUTEX 1 #endif /*===========================================================================*/ diff --git a/icu4c/source/common/umutex.cpp b/icu4c/source/common/umutex.cpp index b4671e3b556..8044430120e 100644 --- a/icu4c/source/common/umutex.cpp +++ b/icu4c/source/common/umutex.cpp @@ -64,6 +64,11 @@ umtx_unlock(UMutex* mutex) mutex->fMutex.unlock(); } +UConditionVar::UConditionVar() : fCV() { +} + +UConditionVar::~UConditionVar() { +} U_CAPI void U_EXPORT2 umtx_condWait(UConditionVar *cond, UMutex *mutex) { diff --git a/icu4c/source/common/umutex.h b/icu4c/source/common/umutex.h old mode 100644 new mode 100755 index 94166630ce4..92539e411d9 --- a/icu4c/source/common/umutex.h +++ b/icu4c/source/common/umutex.h @@ -341,19 +341,42 @@ U_NAMESPACE_END #include #include +#include "unicode/uobject.h" -struct UMutex { - std::mutex fMutex; +struct UMutex : public icu::UMemory { + UMutex() = default; + ~UMutex() = default; + UMutex(const UMutex &other) = delete; + UMutex &operator =(const UMutex &other) = delete; + + std::mutex fMutex = {}; // Note: struct - pubic members - because most access is from + // // plain C style functions (umtx_lock(), etc.) }; -struct UConditionVar { + +struct UConditionVar : public icu::UMemory { + U_COMMON_API UConditionVar(); + U_COMMON_API ~UConditionVar(); + UConditionVar(const UConditionVar &other) = delete; + UConditionVar &operator =(const UConditionVar &other) = delete; + std::condition_variable_any fCV; }; #define U_MUTEX_INITIALIZER {} #define U_CONDITION_INITIALIZER {} - +// Implementation notes for UConditionVar: +// +// Use an out-of-line constructor to reduce problems with the ICU dependency checker. +// On Linux, the default constructor of std::condition_variable_any +// produces an in-line reference to global operator new(), which the +// dependency checker flags for any file that declares a UConditionVar. With +// an out-of-line constructor, the dependency is constrained to umutex.o +// +// Do not export (U_COMMON_API) the entire class, but only the constructor +// and destructor, to avoid Windows build problems with attempting to export the +// std::condition_variable_any. #elif U_PLATFORM_USES_ONLY_WIN32_API diff --git a/icu4c/source/test/depstest/dependencies.txt b/icu4c/source/test/depstest/dependencies.txt index 7e447afb54e..00cc5ff2975 100644 --- a/icu4c/source/test/depstest/dependencies.txt +++ b/icu4c/source/test/depstest/dependencies.txt @@ -24,6 +24,7 @@ system_symbols: stdio_input stdio_output file_io readlink_function dir_io mmap_functions dlfcn # C++ cplusplus iostream + std_mutex group: PIC # Position-Independent Code (-fPIC) requires a Global Offset Table. @@ -38,6 +39,15 @@ group: system_debug group: malloc_functions free malloc realloc +group: std_mutex + std::condition_variable::notify_one() + std::condition_variable::wait(std::unique_lock&) + std::condition_variable::notify_all() + std::condition_variable::condition_variable() + std::condition_variable::~condition_variable() + std::condition_variable_any::condition_variable_any() + std::condition_variable_any::~condition_variable_any() + group: ubsan # UBSan=UndefinedBehaviorSanitizer, clang -fsanitize=bounds __ubsan_handle_out_of_bounds @@ -819,6 +829,7 @@ group: platform stdio_input readlink_function dir_io dlfcn # Move related code into icuplug.c? cplusplus + std_mutex # ICU i18n library ----------------------------------------------------------- # diff --git a/icu4c/source/test/depstest/depstest.py b/icu4c/source/test/depstest/depstest.py index 231c6819c6e..dcdbca8af0b 100755 --- a/icu4c/source/test/depstest/depstest.py +++ b/icu4c/source/test/depstest/depstest.py @@ -16,8 +16,11 @@ This probably works only on Linux. The exit code is 0 if everything is fine, 1 for errors, 2 for only warnings. -Sample invocation: - ~/svn.icu/trunk/src/source/test/depstest$ ./depstest.py ~/svn.icu/trunk/dbg +Sample invocation with an in-source build: + ~/icu/icu4c/source/test/depstest$ ./depstest.py ../../ + +Sample invocation with an out-of-source build: + ~/icu/icu4c/source/test/depstest$ ./depstest.py ~/build/ """ __author__ = "Markus W. Scherer" @@ -87,6 +90,16 @@ def _ReadLibrary(root_path, library_name): for path in obj_paths: _ReadObjFile(root_path, library_name, os.path.basename(path)) +# Dependencies that would otherwise be errors, but that are to be allowed +# in a limited (not transitive) context. List of (file_name, symbol) +# TODO: Move this data to dependencies.txt? +allowed_errors = ( + ("common/umutex.o", "operator new(unsigned long)"), + ("common/umutex.o", "std::__throw_bad_alloc()"), + ("common/umutex.o", "std::__throw_system_error(int)"), + ("common/umutex.o", "std::uncaught_exception()"), +) + def _Resolve(name, parents): global _ignored_symbols, _obj_files, _symbols_to_files, _return_value item = dependencies.items[name] @@ -130,6 +143,9 @@ def _Resolve(name, parents): imports -= exports | system_symbols for symbol in imports: for file_name in files: + if (file_name, symbol) in allowed_errors: + sys.stderr.write("Info: ignoring %s imports %s\n\n" % (file_name, symbol)) + continue if symbol in _obj_files[file_name]["imports"]: neededFile = _symbols_to_files.get(symbol) if neededFile in dependencies.file_to_item: @@ -138,7 +154,7 @@ def _Resolve(name, parents): neededItem = "- is this a new system symbol?" sys.stderr.write("Error: in %s %s: %s imports %s %s\n" % (item_type, name, file_name, symbol, neededItem)) - _return_value = 1 + _return_value = 1 del parents[-1] return item -- 2.40.0