Index: lldb/trunk/include/lldb/Utility/CleanUp.h =================================================================== --- lldb/trunk/include/lldb/Utility/CleanUp.h +++ lldb/trunk/include/lldb/Utility/CleanUp.h @@ -13,249 +13,31 @@ #include "lldb/lldb-public.h" #include -namespace lldb_utility { +namespace lldb_private { -//---------------------------------------------------------------------- -// Templated class that guarantees that a cleanup callback function will -// be called. The cleanup function will be called once under the -// following conditions: -// - when the object goes out of scope -// - when the user explicitly calls clean. -// - the current value will be cleaned up when a new value is set using -// set(T value) as long as the current value hasn't already been cleaned. -// -// This class is designed to be used with simple types for type T (like -// file descriptors, opaque handles, pointers, etc). If more complex -// type T objects are desired, we need to probably specialize this class -// to take "const T&" for all input T parameters. Yet if a type T is -// complex already it might be better to build the cleanup functionality -// into T. -// -// The cleanup function must take one argument that is of type T. -// The calback function return type is R. The return value is currently -// needed for "CallbackType". If there is an easy way to get around the -// need for the return value we can change this class. -// -// The two template parameters are: -// T - The variable type of value that will be stored and used as the -// sole argument for the cleanup callback. -// R - The return type for the cleanup function. -// -// EXAMPLES -// // Use with file handles that get opened where you want to close -// // them. Below we use "int open(const char *path, int oflag, ...)" -// // which returns an integer file descriptor. -1 is the invalid file -// // descriptor so to make an object that will call "int close(int fd)" -// // automatically we can use: -// -// CleanUp fd(open("/tmp/a.txt", O_RDONLY, 0), -1, close); -// -// // malloc/free example -// CleanUp malloced_bytes(malloc(32), NULL, free); -//---------------------------------------------------------------------- -template class CleanUp { -public: - typedef T value_type; - typedef std::function CallbackType; - - //---------------------------------------------------------------------- - // Constructor that sets the current value only. No values are - // considered to be invalid and the cleanup function will be called - // regardless of the value of m_current_value. - //---------------------------------------------------------------------- - CleanUp(value_type value, CallbackType callback) - : m_current_value(value), m_invalid_value(), m_callback(callback), - m_callback_called(false), m_invalid_value_is_valid(false) {} - - //---------------------------------------------------------------------- - // Constructor that sets the current value and also the invalid value. - // The cleanup function will be called on "m_value" as long as it isn't - // equal to "m_invalid_value". - //---------------------------------------------------------------------- - CleanUp(value_type value, value_type invalid, CallbackType callback) - : m_current_value(value), m_invalid_value(invalid), m_callback(callback), - m_callback_called(false), m_invalid_value_is_valid(true) {} - - //---------------------------------------------------------------------- - // Automatically cleanup when this object goes out of scope. - //---------------------------------------------------------------------- - ~CleanUp() { clean(); } - - //---------------------------------------------------------------------- - // Access the value stored in this class - //---------------------------------------------------------------------- - value_type get() { return m_current_value; } - - //---------------------------------------------------------------------- - // Access the value stored in this class - //---------------------------------------------------------------------- - const value_type get() const { return m_current_value; } - - //---------------------------------------------------------------------- - // Reset the owned value to "value". If a current value is valid and - // the cleanup callback hasn't been called, the previous value will - // be cleaned up (see void CleanUp::clean()). - //---------------------------------------------------------------------- - void set(const value_type value) { - // Cleanup the current value if needed - clean(); - // Now set the new value and mark our callback as not called - m_callback_called = false; - m_current_value = value; - } - - //---------------------------------------------------------------------- - // Checks is "m_current_value" is valid. The value is considered valid - // no invalid value was supplied during construction of this object or - // if an invalid value was supplied and "m_current_value" is not equal - // to "m_invalid_value". - // - // Returns true if "m_current_value" is valid, false otherwise. - //---------------------------------------------------------------------- - bool is_valid() const { - if (m_invalid_value_is_valid) - return m_current_value != m_invalid_value; - return true; - } - - //---------------------------------------------------------------------- - // This function will call the cleanup callback provided in the - // constructor one time if the value is considered valid (See is_valid()). - // This function sets m_callback_called to true so we don't call the - // cleanup callback multiple times on the same value. - //---------------------------------------------------------------------- - void clean() { - if (m_callback && !m_callback_called) { - m_callback_called = true; - if (is_valid()) - m_callback(m_current_value); - } - } +/// Run a cleanup function on scope exit unless it's explicitly disabled. +class CleanUp { + std::function Clean; - //---------------------------------------------------------------------- - // Cancels the cleanup that would have been called on "m_current_value" - // if it was valid. This function can be used to release the value - // contained in this object so ownership can be transferred to the caller. - //---------------------------------------------------------------------- - value_type release() { - m_callback_called = true; - return m_current_value; - } - -private: - value_type m_current_value; - const value_type m_invalid_value; - CallbackType m_callback; - bool m_callback_called; - bool m_invalid_value_is_valid; - - // Outlaw default constructor, copy constructor and the assignment operator - DISALLOW_COPY_AND_ASSIGN(CleanUp); -}; - -template class CleanUp2 { public: - typedef T value_type; - typedef std::function CallbackType; - - //---------------------------------------------------------------------- - // Constructor that sets the current value only. No values are - // considered to be invalid and the cleanup function will be called - // regardless of the value of m_current_value. - //---------------------------------------------------------------------- - CleanUp2(value_type value, CallbackType callback, A0 arg) - : m_current_value(value), m_invalid_value(), m_callback(callback), - m_callback_called(false), m_invalid_value_is_valid(false), - m_argument(arg) {} - - //---------------------------------------------------------------------- - // Constructor that sets the current value and also the invalid value. - // The cleanup function will be called on "m_value" as long as it isn't - // equal to "m_invalid_value". - //---------------------------------------------------------------------- - CleanUp2(value_type value, value_type invalid, CallbackType callback, A0 arg) - : m_current_value(value), m_invalid_value(invalid), m_callback(callback), - m_callback_called(false), m_invalid_value_is_valid(true), - m_argument(arg) {} - - //---------------------------------------------------------------------- - // Automatically cleanup when this object goes out of scope. - //---------------------------------------------------------------------- - ~CleanUp2() { clean(); } + /// Register a cleanup function which applies \p Func to a list of arguments. + /// Use caution with arguments which are references: they will be copied. + template + CleanUp(F &&Func, Args &&... args) + : Clean(std::bind(std::forward(Func), std::forward(args)...)) {} - //---------------------------------------------------------------------- - // Access the value stored in this class - //---------------------------------------------------------------------- - value_type get() { return m_current_value; } - - //---------------------------------------------------------------------- - // Access the value stored in this class - //---------------------------------------------------------------------- - const value_type get() const { return m_current_value; } - - //---------------------------------------------------------------------- - // Reset the owned value to "value". If a current value is valid and - // the cleanup callback hasn't been called, the previous value will - // be cleaned up (see void CleanUp::clean()). - //---------------------------------------------------------------------- - void set(const value_type value) { - // Cleanup the current value if needed - clean(); - // Now set the new value and mark our callback as not called - m_callback_called = false; - m_current_value = value; + ~CleanUp() { + if (Clean) + Clean(); } - //---------------------------------------------------------------------- - // Checks is "m_current_value" is valid. The value is considered valid - // no invalid value was supplied during construction of this object or - // if an invalid value was supplied and "m_current_value" is not equal - // to "m_invalid_value". - // - // Returns true if "m_current_value" is valid, false otherwise. - //---------------------------------------------------------------------- - bool is_valid() const { - if (m_invalid_value_is_valid) - return m_current_value != m_invalid_value; - return true; - } - - //---------------------------------------------------------------------- - // This function will call the cleanup callback provided in the - // constructor one time if the value is considered valid (See is_valid()). - // This function sets m_callback_called to true so we don't call the - // cleanup callback multiple times on the same value. - //---------------------------------------------------------------------- - void clean() { - if (m_callback && !m_callback_called) { - m_callback_called = true; - if (is_valid()) - m_callback(m_current_value, m_argument); - } - } + /// Disable the cleanup. + void disable() { Clean = nullptr; } - //---------------------------------------------------------------------- - // Cancels the cleanup that would have been called on "m_current_value" - // if it was valid. This function can be used to release the value - // contained in this object so ownership can be transferred to the caller. - //---------------------------------------------------------------------- - value_type release() { - m_callback_called = true; - return m_current_value; - } - -private: - value_type m_current_value; - const value_type m_invalid_value; - CallbackType m_callback; - bool m_callback_called; - bool m_invalid_value_is_valid; - A0 m_argument; - - // Outlaw default constructor, copy constructor and the assignment operator - DISALLOW_COPY_AND_ASSIGN(CleanUp2); + // Prevent cleanups from being run more than once. + DISALLOW_COPY_AND_ASSIGN(CleanUp); }; -} // namespace lldb_utility +} // namespace lldb_private #endif // #ifndef liblldb_CleanUp_h_ Index: lldb/trunk/lldb.xcodeproj/project.pbxproj =================================================================== --- lldb/trunk/lldb.xcodeproj/project.pbxproj +++ lldb/trunk/lldb.xcodeproj/project.pbxproj @@ -770,6 +770,7 @@ 6D99A3631BBC2F3200979793 /* ArmUnwindInfo.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 6D99A3621BBC2F3200979793 /* ArmUnwindInfo.cpp */; }; 6D9AB3DD1BB2B74E003F2289 /* TypeMap.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 6D9AB3DC1BB2B74E003F2289 /* TypeMap.cpp */; }; 6DEC6F391BD66D750091ABA6 /* TaskPool.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 6DEC6F381BD66D750091ABA6 /* TaskPool.cpp */; }; + 7F94D7182040A13A006EE3EA /* CleanUpTest.cpp in CopyFiles */ = {isa = PBXBuildFile; fileRef = 7F94D7172040A13A006EE3EA /* CleanUpTest.cpp */; }; 8C26C4261C3EA5F90031DF7C /* TSanRuntime.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 8C26C4241C3EA4340031DF7C /* TSanRuntime.cpp */; }; 8C2D6A53197A1EAF006989C9 /* MemoryHistory.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 8C2D6A52197A1EAF006989C9 /* MemoryHistory.cpp */; }; 8C2D6A5E197A250F006989C9 /* MemoryHistoryASan.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 8C2D6A5A197A1FDC006989C9 /* MemoryHistoryASan.cpp */; }; @@ -1242,6 +1243,7 @@ dstPath = "$(DEVELOPER_INSTALL_DIR)/usr/share/man/man1"; dstSubfolderSpec = 0; files = ( + 7F94D7182040A13A006EE3EA /* CleanUpTest.cpp in CopyFiles */, AF90106515AB7D3600FF120D /* lldb.1 in CopyFiles */, ); runOnlyForDeploymentPostprocessing = 1; @@ -2672,6 +2674,7 @@ 6D9AB3DE1BB2B76B003F2289 /* TypeMap.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = TypeMap.h; path = include/lldb/Symbol/TypeMap.h; sourceTree = ""; }; 6DEC6F381BD66D750091ABA6 /* TaskPool.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = TaskPool.cpp; path = source/Host/common/TaskPool.cpp; sourceTree = ""; }; 6DEC6F3A1BD66D950091ABA6 /* TaskPool.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = TaskPool.h; path = include/lldb/Host/TaskPool.h; sourceTree = ""; }; + 7F94D7172040A13A006EE3EA /* CleanUpTest.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = CleanUpTest.cpp; sourceTree = ""; }; 8C26C4241C3EA4340031DF7C /* TSanRuntime.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; name = TSanRuntime.cpp; path = TSan/TSanRuntime.cpp; sourceTree = ""; }; 8C26C4251C3EA4340031DF7C /* TSanRuntime.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = TSanRuntime.h; path = TSan/TSanRuntime.h; sourceTree = ""; }; 8C2D6A52197A1EAF006989C9 /* MemoryHistory.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = MemoryHistory.cpp; path = source/Target/MemoryHistory.cpp; sourceTree = ""; }; @@ -3419,6 +3422,7 @@ 2321F9421BDD343A00BA9A93 /* Utility */ = { isa = PBXGroup; children = ( + 7F94D7172040A13A006EE3EA /* CleanUpTest.cpp */, 23E2E5161D903689006F38BB /* ArchSpecTest.cpp */, 9A3D43C81F3150D200EB767C /* ConstStringTest.cpp */, 9A3D43C71F3150D200EB767C /* LogTest.cpp */, Index: lldb/trunk/source/Host/macosx/Host.mm =================================================================== --- lldb/trunk/source/Host/macosx/Host.mm +++ lldb/trunk/source/Host/macosx/Host.mm @@ -1277,10 +1277,8 @@ return error; } - // Make a quick class that will cleanup the posix spawn attributes in case - // we return in the middle of this function. - lldb_utility::CleanUp posix_spawnattr_cleanup( - &attr, posix_spawnattr_destroy); + // Make sure we clean up the posix spawn attributes before exiting this scope. + CleanUp cleanup_attr(posix_spawnattr_destroy, &attr); sigset_t no_signals; sigset_t all_signals; @@ -1382,11 +1380,8 @@ return error; } - // Make a quick class that will cleanup the posix spawn attributes in case - // we return in the middle of this function. - lldb_utility::CleanUp - posix_spawn_file_actions_cleanup(&file_actions, - posix_spawn_file_actions_destroy); + // Make sure we clean up the posix file actions before exiting this scope. + CleanUp cleanup_fileact(posix_spawn_file_actions_destroy, &file_actions); for (size_t i = 0; i < num_file_actions; ++i) { const FileAction *launch_file_action = Index: lldb/trunk/source/Host/macosx/Symbols.cpp =================================================================== --- lldb/trunk/source/Host/macosx/Symbols.cpp +++ lldb/trunk/source/Host/macosx/Symbols.cpp @@ -240,52 +240,53 @@ const lldb_private::UUID *uuid, const ArchSpec *arch) { char path[PATH_MAX]; + if (dsym_bundle_fspec.GetPath(path, sizeof(path)) == 0) + return {}; - FileSpec dsym_fspec; + ::strncat(path, "/Contents/Resources/DWARF", sizeof(path) - strlen(path) - 1); - if (dsym_bundle_fspec.GetPath(path, sizeof(path))) { - ::strncat(path, "/Contents/Resources/DWARF", - sizeof(path) - strlen(path) - 1); - - lldb_utility::CleanUp dirp(opendir(path), NULL, closedir); - if (dirp.is_valid()) { - dsym_fspec.GetDirectory().SetCString(path); - struct dirent *dp; - while ((dp = readdir(dirp.get())) != NULL) { - // Only search directories - if (dp->d_type == DT_DIR || dp->d_type == DT_UNKNOWN) { - if (dp->d_namlen == 1 && dp->d_name[0] == '.') - continue; - - if (dp->d_namlen == 2 && dp->d_name[0] == '.' && dp->d_name[1] == '.') - continue; - } - - if (dp->d_type == DT_REG || dp->d_type == DT_UNKNOWN) { - dsym_fspec.GetFilename().SetCString(dp->d_name); - ModuleSpecList module_specs; - if (ObjectFile::GetModuleSpecifications(dsym_fspec, 0, 0, - module_specs)) { - ModuleSpec spec; - for (size_t i = 0; i < module_specs.GetSize(); ++i) { - bool got_spec = module_specs.GetModuleSpecAtIndex(i, spec); - UNUSED_IF_ASSERT_DISABLED(got_spec); - assert(got_spec); - if ((uuid == NULL || - (spec.GetUUIDPtr() && spec.GetUUID() == *uuid)) && - (arch == NULL || - (spec.GetArchitecturePtr() && - spec.GetArchitecture().IsCompatibleMatch(*arch)))) { - return dsym_fspec; - } - } + DIR *dirp = opendir(path); + if (!dirp) + return {}; + + // Make sure we close the directory before exiting this scope. + CleanUp cleanup_dir(closedir, dirp); + + FileSpec dsym_fspec; + dsym_fspec.GetDirectory().SetCString(path); + struct dirent *dp; + while ((dp = readdir(dirp)) != NULL) { + // Only search directories + if (dp->d_type == DT_DIR || dp->d_type == DT_UNKNOWN) { + if (dp->d_namlen == 1 && dp->d_name[0] == '.') + continue; + + if (dp->d_namlen == 2 && dp->d_name[0] == '.' && dp->d_name[1] == '.') + continue; + } + + if (dp->d_type == DT_REG || dp->d_type == DT_UNKNOWN) { + dsym_fspec.GetFilename().SetCString(dp->d_name); + ModuleSpecList module_specs; + if (ObjectFile::GetModuleSpecifications(dsym_fspec, 0, 0, module_specs)) { + ModuleSpec spec; + for (size_t i = 0; i < module_specs.GetSize(); ++i) { + bool got_spec = module_specs.GetModuleSpecAtIndex(i, spec); + UNUSED_IF_ASSERT_DISABLED(got_spec); + assert(got_spec); + if ((uuid == NULL || + (spec.GetUUIDPtr() && spec.GetUUID() == *uuid)) && + (arch == NULL || + (spec.GetArchitecturePtr() && + spec.GetArchitecture().IsCompatibleMatch(*arch)))) { + return dsym_fspec; } } } } } - dsym_fspec.Clear(); - return dsym_fspec; + + return {}; } static bool GetModuleSpecInfoFromUUIDDictionary(CFDictionaryRef uuid_dict, Index: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp =================================================================== --- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -3316,27 +3316,22 @@ int communication_fd = -1; #ifdef USE_SOCKETPAIR_FOR_LOCAL_CONNECTION - // Auto close the sockets we might open up unless everything goes OK. This - // helps us not leak file descriptors when things go wrong. - lldb_utility::CleanUp our_socket(-1, -1, close); - lldb_utility::CleanUp gdb_socket(-1, -1, close); - // Use a socketpair on non-Windows systems for security and performance // reasons. - { - int sockets[2]; /* the pair of socket descriptors */ - if (socketpair(AF_UNIX, SOCK_STREAM, 0, sockets) == -1) { - error.SetErrorToErrno(); - return error; - } - - our_socket.set(sockets[0]); - gdb_socket.set(sockets[1]); + int sockets[2]; /* the pair of socket descriptors */ + if (socketpair(AF_UNIX, SOCK_STREAM, 0, sockets) == -1) { + error.SetErrorToErrno(); + return error; } + int our_socket = sockets[0]; + int gdb_socket = sockets[1]; + CleanUp cleanup_our(close, our_socket); + CleanUp cleanup_gdb(close, gdb_socket); + // Don't let any child processes inherit our communication socket - SetCloexecFlag(our_socket.get()); - communication_fd = gdb_socket.get(); + SetCloexecFlag(our_socket); + communication_fd = gdb_socket; #endif error = m_gdb_comm.StartDebugserverProcess( @@ -3352,8 +3347,8 @@ #ifdef USE_SOCKETPAIR_FOR_LOCAL_CONNECTION // Our process spawned correctly, we can now set our connection to use our // end of the socket pair - m_gdb_comm.SetConnection( - new ConnectionFileDescriptor(our_socket.release(), true)); + cleanup_our.disable(); + m_gdb_comm.SetConnection(new ConnectionFileDescriptor(our_socket, true)); #endif StartAsyncThread(); } Index: lldb/trunk/unittests/Utility/CMakeLists.txt =================================================================== --- lldb/trunk/unittests/Utility/CMakeLists.txt +++ lldb/trunk/unittests/Utility/CMakeLists.txt @@ -1,5 +1,6 @@ add_lldb_unittest(UtilityTests ArchSpecTest.cpp + CleanUpTest.cpp ConstStringTest.cpp EnvironmentTest.cpp JSONTest.cpp Index: lldb/trunk/unittests/Utility/CleanUpTest.cpp =================================================================== --- lldb/trunk/unittests/Utility/CleanUpTest.cpp +++ lldb/trunk/unittests/Utility/CleanUpTest.cpp @@ -0,0 +1,47 @@ +//===-- CleanUpTest.cpp -----------------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "lldb/Utility/CleanUp.h" +#include "gtest/gtest.h" + +using namespace lldb_private; + +TEST(CleanUpTest, no_args) { + bool f = false; + { + CleanUp cleanup([&] { f = true; }); + } + ASSERT_TRUE(f); +} + +TEST(CleanUpTest, multiple_args) { + bool f1 = false; + bool f2 = false; + bool f3 = false; + { + CleanUp cleanup( + [](bool arg1, bool *arg2, bool &arg3) { + ASSERT_FALSE(arg1); + *arg2 = true; + arg3 = true; + }, + f1, &f2, f3); + } + ASSERT_TRUE(f2); + ASSERT_FALSE(f3); +} + +TEST(CleanUpTest, disable) { + bool f = false; + { + CleanUp cleanup([&] { f = true; }); + cleanup.disable(); + } + ASSERT_FALSE(f); +}