From ff48e4bea09afe020e1c4373f32b65a8be29503e Mon Sep 17 00:00:00 2001 From: Greg Clayton Date: Tue, 3 Feb 2015 02:05:44 +0000 Subject: [PATCH] Fixed bugs in the multi-threaded access in HostInfoBase. Prior to this fix, static bool variables were used but this is not sufficient. We now use std::call_once in all places where the previous static bool code was used to try to implement thread safety. This was causing code that opened multiple targets to try and get a path to debugserver from the GDB remote communication class, and it would get the LLDB path and some instances would return empty strings and it would cause debugserver to not be found. llvm-svn: 227935 --- lldb/source/Core/ConstString.cpp | 2 +- lldb/source/Expression/ClangModulesDeclVendor.cpp | 6 +- lldb/source/Host/common/HostInfoBase.cpp | 267 +++++++++++++--------- lldb/source/Host/linux/HostInfoLinux.cpp | 46 ++-- lldb/source/Host/windows/HostInfoWindows.cpp | 15 +- lldb/source/Symbol/ClangASTContext.cpp | 2 +- 6 files changed, 193 insertions(+), 145 deletions(-) diff --git a/lldb/source/Core/ConstString.cpp b/lldb/source/Core/ConstString.cpp index 37d24e0..85f8d3c 100644 --- a/lldb/source/Core/ConstString.cpp +++ b/lldb/source/Core/ConstString.cpp @@ -11,7 +11,7 @@ #include "lldb/Host/Mutex.h" #include "llvm/ADT/StringMap.h" -#include +#include // std::once using namespace lldb_private; diff --git a/lldb/source/Expression/ClangModulesDeclVendor.cpp b/lldb/source/Expression/ClangModulesDeclVendor.cpp index b1ce44a..0800b52 100644 --- a/lldb/source/Expression/ClangModulesDeclVendor.cpp +++ b/lldb/source/Expression/ClangModulesDeclVendor.cpp @@ -7,8 +7,11 @@ // //===----------------------------------------------------------------------===// -#include "lldb/Core/StreamString.h" +#include // std::once + #include "lldb/Expression/ClangModulesDeclVendor.h" + +#include "lldb/Core/StreamString.h" #include "lldb/Host/FileSpec.h" #include "lldb/Host/Host.h" #include "lldb/Host/HostInfo.h" @@ -22,7 +25,6 @@ #include "clang/Sema/Lookup.h" #include "clang/Serialization/ASTReader.h" -#include using namespace lldb_private; diff --git a/lldb/source/Host/common/HostInfoBase.cpp b/lldb/source/Host/common/HostInfoBase.cpp index d65b796..630c3a2 100644 --- a/lldb/source/Host/common/HostInfoBase.cpp +++ b/lldb/source/Host/common/HostInfoBase.cpp @@ -23,60 +23,56 @@ #include "llvm/Support/raw_ostream.h" #include +#include // std::once using namespace lldb; using namespace lldb_private; namespace { -void -CleanupProcessSpecificLLDBTempDir() -{ - // Get the process specific LLDB temporary directory and delete it. - FileSpec tmpdir_file_spec; - if (!HostInfo::GetLLDBPath(ePathTypeLLDBTempSystemDir, tmpdir_file_spec)) - return; - - // Remove the LLDB temporary directory if we have one. Set "recurse" to - // true to all files that were created for the LLDB process can be cleaned up. - FileSystem::DeleteDirectory(tmpdir_file_spec.GetDirectory().GetCString(), true); -} + void + CleanupProcessSpecificLLDBTempDir() + { + // Get the process specific LLDB temporary directory and delete it. + FileSpec tmpdir_file_spec; + if (!HostInfo::GetLLDBPath(ePathTypeLLDBTempSystemDir, tmpdir_file_spec)) + return; + + // Remove the LLDB temporary directory if we have one. Set "recurse" to + // true to all files that were created for the LLDB process can be cleaned up. + FileSystem::DeleteDirectory(tmpdir_file_spec.GetDirectory().GetCString(), true); + } -struct HostInfoBaseFields -{ - uint32_t m_number_cpus; - std::string m_vendor_string; - std::string m_os_string; - std::string m_host_triple; - - ArchSpec m_host_arch_32; - ArchSpec m_host_arch_64; - - FileSpec m_lldb_so_dir; - FileSpec m_lldb_support_exe_dir; - FileSpec m_lldb_headers_dir; - FileSpec m_lldb_python_dir; - FileSpec m_lldb_clang_resource_dir; - FileSpec m_lldb_system_plugin_dir; - FileSpec m_lldb_user_plugin_dir; - FileSpec m_lldb_tmp_dir; -}; - -HostInfoBaseFields *g_fields = nullptr; -} + //---------------------------------------------------------------------- + // The HostInfoBaseFields is a work around for windows not supporting + // static variables correctly in a thread safe way. Really each of the + // variables in HostInfoBaseFields should live in the functions in which + // they are used and each one should be static, but the work around is + // in place to avoid this restriction. Ick. + //---------------------------------------------------------------------- -#define COMPUTE_LLDB_PATH(compute_function, member_var) \ - { \ - static bool is_initialized = false; \ - static bool success = false; \ - if (!is_initialized) \ - { \ - is_initialized = true; \ - success = HostInfo::compute_function(member_var); \ - } \ - if (success) \ - result = &member_var; \ - } + struct HostInfoBaseFields + { + uint32_t m_number_cpus; + std::string m_vendor_string; + std::string m_os_string; + std::string m_host_triple; + + ArchSpec m_host_arch_32; + ArchSpec m_host_arch_64; + + FileSpec m_lldb_so_dir; + FileSpec m_lldb_support_exe_dir; + FileSpec m_lldb_headers_dir; + FileSpec m_lldb_python_dir; + FileSpec m_lldb_clang_resource_dir; + FileSpec m_lldb_system_plugin_dir; + FileSpec m_lldb_user_plugin_dir; + FileSpec m_lldb_tmp_dir; + }; + + HostInfoBaseFields *g_fields = nullptr; +} void HostInfoBase::Initialize() @@ -87,13 +83,10 @@ HostInfoBase::Initialize() uint32_t HostInfoBase::GetNumberCPUS() { - static bool is_initialized = false; - if (!is_initialized) - { + static std::once_flag g_once_flag; + std::call_once(g_once_flag, []() { g_fields->m_number_cpus = std::thread::hardware_concurrency(); - is_initialized = true; - } - + }); return g_fields->m_number_cpus; } @@ -106,53 +99,40 @@ HostInfoBase::GetMaxThreadNameLength() llvm::StringRef HostInfoBase::GetVendorString() { - static bool is_initialized = false; - if (!is_initialized) - { - const ArchSpec &host_arch = HostInfo::GetArchitecture(); - const llvm::StringRef &str_ref = host_arch.GetTriple().getVendorName(); - g_fields->m_vendor_string.assign(str_ref.begin(), str_ref.end()); - is_initialized = true; - } + static std::once_flag g_once_flag; + std::call_once(g_once_flag, []() { + g_fields->m_vendor_string = std::move(HostInfo::GetArchitecture().GetTriple().getVendorName().str()); + }); return g_fields->m_vendor_string; } llvm::StringRef HostInfoBase::GetOSString() { - static bool is_initialized = false; - if (!is_initialized) - { - const ArchSpec &host_arch = HostInfo::GetArchitecture(); - const llvm::StringRef &str_ref = host_arch.GetTriple().getOSName(); - g_fields->m_os_string.assign(str_ref.begin(), str_ref.end()); - is_initialized = true; - } + static std::once_flag g_once_flag; + std::call_once(g_once_flag, []() { + g_fields->m_os_string = std::move(HostInfo::GetArchitecture().GetTriple().getOSName()); + }); return g_fields->m_os_string; } llvm::StringRef HostInfoBase::GetTargetTriple() { - static bool is_initialized = false; - if (!is_initialized) - { - const ArchSpec &host_arch = HostInfo::GetArchitecture(); - g_fields->m_host_triple = host_arch.GetTriple().getTriple(); - is_initialized = true; - } + static std::once_flag g_once_flag; + std::call_once(g_once_flag, []() { + g_fields->m_host_triple = HostInfo::GetArchitecture().GetTriple().getTriple(); + }); return g_fields->m_host_triple; } const ArchSpec & HostInfoBase::GetArchitecture(ArchitectureKind arch_kind) { - static bool is_initialized = false; - if (!is_initialized) - { + static std::once_flag g_once_flag; + std::call_once(g_once_flag, []() { HostInfo::ComputeHostArchitectureSupport(g_fields->m_host_arch_32, g_fields->m_host_arch_64); - is_initialized = true; - } + }); // If an explicit 32 or 64-bit architecture was requested, return that. if (arch_kind == eArchKind32) @@ -174,52 +154,123 @@ HostInfoBase::GetLLDBPath(lldb::PathType type, FileSpec &file_spec) return false; #endif - Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST); FileSpec *result = nullptr; switch (type) { case lldb::ePathTypeLLDBShlibDir: - COMPUTE_LLDB_PATH(ComputeSharedLibraryDirectory, g_fields->m_lldb_so_dir) - if (log) - log->Printf("HostInfoBase::GetLLDBPath(ePathTypeLLDBShlibDir) => '%s'", g_fields->m_lldb_so_dir.GetPath().c_str()); + { + static std::once_flag g_once_flag; + static bool success = false; + std::call_once(g_once_flag, []() { + success = HostInfo::ComputeSharedLibraryDirectory (g_fields->m_lldb_so_dir); + Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST); + if (log) + log->Printf("HostInfoBase::GetLLDBPath(ePathTypeLLDBShlibDir) => '%s'", g_fields->m_lldb_so_dir.GetPath().c_str()); + }); + if (success) + result = &g_fields->m_lldb_so_dir; + } break; case lldb::ePathTypeSupportExecutableDir: - COMPUTE_LLDB_PATH(ComputeSupportExeDirectory, g_fields->m_lldb_support_exe_dir) - if (log) - log->Printf("HostInfoBase::GetLLDBPath(ePathTypeSupportExecutableDir) => '%s'", - g_fields->m_lldb_support_exe_dir.GetPath().c_str()); + { + static std::once_flag g_once_flag; + static bool success = false; + std::call_once(g_once_flag, []() { + success = HostInfo::ComputeSupportExeDirectory (g_fields->m_lldb_support_exe_dir); + Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST); + if (log) + log->Printf("HostInfoBase::GetLLDBPath(ePathTypeSupportExecutableDir) => '%s'", + g_fields->m_lldb_support_exe_dir.GetPath().c_str()); + }); + if (success) + result = &g_fields->m_lldb_support_exe_dir; + } break; case lldb::ePathTypeHeaderDir: - COMPUTE_LLDB_PATH(ComputeHeaderDirectory, g_fields->m_lldb_headers_dir) - if (log) - log->Printf("HostInfoBase::GetLLDBPath(ePathTypeHeaderDir) => '%s'", g_fields->m_lldb_headers_dir.GetPath().c_str()); + { + static std::once_flag g_once_flag; + static bool success = false; + std::call_once(g_once_flag, []() { + success = HostInfo::ComputeHeaderDirectory (g_fields->m_lldb_headers_dir); + Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST); + if (log) + log->Printf("HostInfoBase::GetLLDBPath(ePathTypeHeaderDir) => '%s'", g_fields->m_lldb_headers_dir.GetPath().c_str()); + }); + if (success) + result = &g_fields->m_lldb_headers_dir; + } break; case lldb::ePathTypePythonDir: - COMPUTE_LLDB_PATH(ComputePythonDirectory, g_fields->m_lldb_python_dir) - if (log) - log->Printf("HostInfoBase::GetLLDBPath(ePathTypePythonDir) => '%s'", g_fields->m_lldb_python_dir.GetPath().c_str()); + { + static std::once_flag g_once_flag; + static bool success = false; + std::call_once(g_once_flag, []() { + success = HostInfo::ComputePythonDirectory (g_fields->m_lldb_python_dir); + Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST); + if (log) + log->Printf("HostInfoBase::GetLLDBPath(ePathTypePythonDir) => '%s'", g_fields->m_lldb_python_dir.GetPath().c_str()); + }); + if (success) + result = &g_fields->m_lldb_python_dir; + } break; case lldb::ePathTypeClangDir: - COMPUTE_LLDB_PATH(ComputeClangDirectory, g_fields->m_lldb_clang_resource_dir) - if (log) - log->Printf("HostInfoBase::GetLLDBPath(ePathTypeClangResourceDir) => '%s'", g_fields->m_lldb_clang_resource_dir.GetPath().c_str()); + { + static std::once_flag g_once_flag; + static bool success = false; + std::call_once(g_once_flag, []() { + success = HostInfo::ComputeClangDirectory (g_fields->m_lldb_clang_resource_dir); + Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST); + if (log) + log->Printf("HostInfoBase::GetLLDBPath(ePathTypeClangResourceDir) => '%s'", g_fields->m_lldb_clang_resource_dir.GetPath().c_str()); + }); + if (success) + result = &g_fields->m_lldb_clang_resource_dir; + } break; case lldb::ePathTypeLLDBSystemPlugins: - COMPUTE_LLDB_PATH(ComputeSystemPluginsDirectory, g_fields->m_lldb_system_plugin_dir) - if (log) - log->Printf("HostInfoBase::GetLLDBPath(ePathTypeLLDBSystemPlugins) => '%s'", - g_fields->m_lldb_system_plugin_dir.GetPath().c_str()); + { + static std::once_flag g_once_flag; + static bool success = false; + std::call_once(g_once_flag, []() { + success = HostInfo::ComputeSystemPluginsDirectory (g_fields->m_lldb_system_plugin_dir); + Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST); + if (log) + log->Printf("HostInfoBase::GetLLDBPath(ePathTypeLLDBSystemPlugins) => '%s'", + g_fields->m_lldb_system_plugin_dir.GetPath().c_str()); + }); + if (success) + result = &g_fields->m_lldb_system_plugin_dir; + } break; case lldb::ePathTypeLLDBUserPlugins: - COMPUTE_LLDB_PATH(ComputeUserPluginsDirectory, g_fields->m_lldb_user_plugin_dir) - if (log) - log->Printf("HostInfoBase::GetLLDBPath(ePathTypeLLDBUserPlugins) => '%s'", - g_fields->m_lldb_user_plugin_dir.GetPath().c_str()); + { + static std::once_flag g_once_flag; + static bool success = false; + std::call_once(g_once_flag, []() { + success = HostInfo::ComputeUserPluginsDirectory (g_fields->m_lldb_user_plugin_dir); + Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST); + if (log) + log->Printf("HostInfoBase::GetLLDBPath(ePathTypeLLDBUserPlugins) => '%s'", + g_fields->m_lldb_user_plugin_dir.GetPath().c_str()); + }); + if (success) + result = &g_fields->m_lldb_user_plugin_dir; + } break; case lldb::ePathTypeLLDBTempSystemDir: - COMPUTE_LLDB_PATH(ComputeTempFileDirectory, g_fields->m_lldb_tmp_dir) - if (log) - log->Printf("HostInfoBase::GetLLDBPath(ePathTypeLLDBTempSystemDir) => '%s'", g_fields->m_lldb_tmp_dir.GetPath().c_str()); + { + static std::once_flag g_once_flag; + static bool success = false; + std::call_once(g_once_flag, []() { + success = HostInfo::ComputeTempFileDirectory (g_fields->m_lldb_tmp_dir); + Log *log = lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST); + if (log) + log->Printf("HostInfoBase::GetLLDBPath(ePathTypeLLDBTempSystemDir) => '%s'", g_fields->m_lldb_tmp_dir.GetPath().c_str()); + }); + if (success) + result = &g_fields->m_lldb_tmp_dir; + } break; } diff --git a/lldb/source/Host/linux/HostInfoLinux.cpp b/lldb/source/Host/linux/HostInfoLinux.cpp index bace258..bca92ec 100644 --- a/lldb/source/Host/linux/HostInfoLinux.cpp +++ b/lldb/source/Host/linux/HostInfoLinux.cpp @@ -16,6 +16,7 @@ #include #include +#include // std::once using namespace lldb_private; @@ -56,32 +57,29 @@ HostInfoLinux::GetMaxThreadNameLength() bool HostInfoLinux::GetOSVersion(uint32_t &major, uint32_t &minor, uint32_t &update) { - static bool is_initialized = false; static bool success = false; + static std::once_flag g_once_flag; + std::call_once(g_once_flag, []() { - if (!is_initialized) - { - is_initialized = true; struct utsname un; - - if (uname(&un)) - goto finished; - - int status = sscanf(un.release, "%u.%u.%u", &g_fields->m_os_major, &g_fields->m_os_minor, &g_fields->m_os_update); - if (status == 3) + if (uname(&un) == 0) { - success = true; - goto finished; + int status = sscanf(un.release, "%u.%u.%u", &g_fields->m_os_major, &g_fields->m_os_minor, &g_fields->m_os_update); + if (status == 3) + success = true; + else + { + // Some kernels omit the update version, so try looking for just "X.Y" and + // set update to 0. + g_fields->m_os_update = 0; + status = sscanf(un.release, "%u.%u", &g_fields->m_os_major, &g_fields->m_os_minor); + if (status == 2) + success = true; + } } + }); - // Some kernels omit the update version, so try looking for just "X.Y" and - // set update to 0. - g_fields->m_os_update = 0; - status = sscanf(un.release, "%u.%u", &g_fields->m_os_major, &g_fields->m_os_minor); - success = !!(status == 2); - } -finished: major = g_fields->m_os_major; minor = g_fields->m_os_minor; update = g_fields->m_os_update; @@ -120,13 +118,11 @@ HostInfoLinux::GetOSKernelDescription(std::string &s) llvm::StringRef HostInfoLinux::GetDistributionId() { - static bool is_initialized = false; // Try to run 'lbs_release -i', and use that response // for the distribution id. - - if (!is_initialized) - { - is_initialized = true; + static bool success = false; + static std::once_flag g_once_flag; + std::call_once(g_once_flag, []() { Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_HOST)); if (log) @@ -202,7 +198,7 @@ HostInfoLinux::GetDistributionId() // clean up the file pclose(file); } - } + }); return g_fields->m_distribution_id.c_str(); } diff --git a/lldb/source/Host/windows/HostInfoWindows.cpp b/lldb/source/Host/windows/HostInfoWindows.cpp index c0366dd..a2b5994 100644 --- a/lldb/source/Host/windows/HostInfoWindows.cpp +++ b/lldb/source/Host/windows/HostInfoWindows.cpp @@ -9,6 +9,8 @@ #include "lldb/Host/windows/windows.h" +#include // std::once + #include "lldb/Host/windows/HostInfoWindows.h" #include "llvm/ADT/SmallString.h" #include "llvm/Support/raw_ostream.h" @@ -84,14 +86,11 @@ HostInfoWindows::GetHostname(std::string &s) FileSpec HostInfoWindows::GetProgramFileSpec() { - static bool is_initialized = false; - if (!is_initialized) - { - is_initialized = true; - - std::vector buffer(PATH_MAX); - ::GetModuleFileName(NULL, &buffer[0], buffer.size()); - m_program_filespec.SetFile(&buffer[0], false); + static std::once_flag g_once_flag; + std::call_once(g_once_flag, []() { + char buffer[PATH_MAX]; + ::GetModuleFileName(NULL, buffer, sizeof(buffer)); + m_program_filespec.SetFile(buffer, false); } return m_program_filespec; } diff --git a/lldb/source/Symbol/ClangASTContext.cpp b/lldb/source/Symbol/ClangASTContext.cpp index 73f6920..da25eb8 100644 --- a/lldb/source/Symbol/ClangASTContext.cpp +++ b/lldb/source/Symbol/ClangASTContext.cpp @@ -11,7 +11,7 @@ // C Includes // C++ Includes -#include +#include // std::once #include // Other libraries and framework includes -- 2.7.4