Index: include/lldb/Utility/CleanUp.h =================================================================== --- include/lldb/Utility/CleanUp.h +++ include/lldb/Utility/CleanUp.h @@ -15,245 +15,25 @@ namespace lldb_utility { -//---------------------------------------------------------------------- -// 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 { + bool PerformCleanup; + 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(); } + CleanUp(std::function Clean) + : PerformCleanup(true), Clean(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; - } + // Prevent cleanups from being run more than once. + DISALLOW_COPY_AND_ASSIGN(CleanUp); - //---------------------------------------------------------------------- - // 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() { PerformCleanup = false; } - //---------------------------------------------------------------------- - // 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; + ~CleanUp() { + if (PerformCleanup) + Clean(); } - -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); }; } // namespace lldb_utility Index: source/Host/macosx/Host.mm =================================================================== --- source/Host/macosx/Host.mm +++ 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. + lldb_utility::CleanUp cleanup_attr([&] { posix_spawnattr_destroy(&attr); }); sigset_t no_signals; sigset_t all_signals; @@ -1384,9 +1382,8 @@ // 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); + lldb_utility::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: source/Host/macosx/Symbols.cpp =================================================================== --- source/Host/macosx/Symbols.cpp +++ 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; - } + DIR *dirp = opendir(path); + if (!dirp) + return {}; - 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; - } - } + // Make sure we close the directory before exiting this scope. + lldb_utility::CleanUp cleanup_dir([dirp] { 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: source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp =================================================================== --- source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ 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]; + lldb_utility::CleanUp cleanup_our([our_socket] { close(our_socket); }); + lldb_utility::CleanUp cleanup_gdb([gdb_socket] { 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(); }