[Lldb-commits] [PATCH] D30702: Fix remaining threading issues in Log.h
labath added a comment. Comment at: source/Utility/Log.cpp:82 + if (mask | flags) { +m_options.store(options, std::memory_order_release); +m_stream_sp = stream_sp; zturner wrote: > Might as well use `memory_order_relaxed` here. Done. I missed that one. Comment at: unittests/Utility/LogTest.cpp:16 #include "llvm/Support/Threading.h" +#include #include zturner wrote: > Is this included needed? Not anymore. I had tried implementing the test with futures, but I couldn't get tsan to work with them properly. https://reviews.llvm.org/D30702 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r297369 - cmake: Increase LINK_INTERFACE_MULTIPLICITY of lldbCore
Author: labath Date: Thu Mar 9 04:16:15 2017 New Revision: 297369 URL: http://llvm.org/viewvc/llvm-project?rev=297369&view=rev Log: cmake: Increase LINK_INTERFACE_MULTIPLICITY of lldbCore This is necessary to get debug builds of unit tests working on linux. I think we are at a point where removing dependencies does not prevent us from depending on the whole world yet. What it does do though, is make the dependency chains longer as the dependency graph gets sparser, which means we need to repeat the libraries more times to get the thing to link. Modified: lldb/trunk/source/Core/CMakeLists.txt Modified: lldb/trunk/source/Core/CMakeLists.txt URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/CMakeLists.txt?rev=297369&r1=297368&r2=297369&view=diff == --- lldb/trunk/source/Core/CMakeLists.txt (original) +++ lldb/trunk/source/Core/CMakeLists.txt Thu Mar 9 04:16:15 2017 @@ -74,4 +74,4 @@ add_lldb_library(lldbCore # Needed to properly resolve references in a debug build. # TODO: Remove once we have better layering -set_target_properties(lldbCore PROPERTIES LINK_INTERFACE_MULTIPLICITY 3) +set_target_properties(lldbCore PROPERTIES LINK_INTERFACE_MULTIPLICITY 4) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30702: Fix remaining threading issues in Log.h
This revision was automatically updated to reflect the committed changes. Closed by commit rL297368: Fix remaining threading issues in Log.h (authored by labath). Changed prior to commit: https://reviews.llvm.org/D30702?vs=90994&id=91147#toc Repository: rL LLVM https://reviews.llvm.org/D30702 Files: lldb/trunk/include/lldb/Utility/Log.h lldb/trunk/source/Utility/Log.cpp lldb/trunk/unittests/Utility/LogTest.cpp Index: lldb/trunk/include/lldb/Utility/Log.h === --- lldb/trunk/include/lldb/Utility/Log.h +++ lldb/trunk/include/lldb/Utility/Log.h @@ -17,6 +17,7 @@ // Other libraries and framework includes #include "llvm/Support/FormatVariadic.h" +#include "llvm/Support/ManagedStatic.h" #include "llvm/Support/RWMutex.h" // C++ Includes #include @@ -55,6 +56,7 @@ // necessary to enable a log channel in an atomic manner. class Channel { std::atomic log_ptr; +friend class Log; public: const llvm::ArrayRef categories; @@ -66,29 +68,26 @@ default_flags(default_flags) {} // This function is safe to call at any time -// FIXME: Not true yet, mask access is not atomic +// If the channel is disabled after (or concurrently with) this function +// returning a non-null Log pointer, it is still safe to attempt to write to +// the Log object -- the output will be discarded. Log *GetLogIfAll(uint32_t mask) { - Log *log = log_ptr.load(std::memory_order_acquire); + Log *log = log_ptr.load(std::memory_order_relaxed); if (log && log->GetMask().AllSet(mask)) return log; return nullptr; } // This function is safe to call at any time -// FIXME: Not true yet, mask access is not atomic +// If the channel is disabled after (or concurrently with) this function +// returning a non-null Log pointer, it is still safe to attempt to write to +// the Log object -- the output will be discarded. Log *GetLogIfAny(uint32_t mask) { - Log *log = log_ptr.load(std::memory_order_acquire); + Log *log = log_ptr.load(std::memory_order_relaxed); if (log && log->GetMask().AnySet(mask)) return log; return nullptr; } - -// Calls to Enable and disable need to be serialized externally. -void Enable(Log &log, const std::shared_ptr &stream_sp, -uint32_t options, uint32_t flags); - -// Calls to Enable and disable need to be serialized externally. -void Disable(uint32_t flags); }; //-- @@ -115,12 +114,13 @@ //-- // Member functions + // + // These functions are safe to call at any time you have a Log* obtained from + // the Channel class. If logging is disabled between you obtaining the Log + // object and writing to it, the output will be silently discarded. //-- - Log(); - - Log(const std::shared_ptr &stream_sp); - - ~Log(); + Log(Channel &channel) : m_channel(channel) {} + ~Log() = default; void PutCString(const char *cstr); void PutString(llvm::StringRef str); @@ -136,56 +136,59 @@ void VAPrintf(const char *format, va_list args); - void LogIf(uint32_t mask, const char *fmt, ...) - __attribute__((format(printf, 3, 4))); - void Error(const char *fmt, ...) __attribute__((format(printf, 2, 3))); void VAError(const char *format, va_list args); void Verbose(const char *fmt, ...) __attribute__((format(printf, 2, 3))); void Warning(const char *fmt, ...) __attribute__((format(printf, 2, 3))); - Flags &GetOptions(); - - const Flags &GetOptions() const; - - Flags &GetMask(); + const Flags GetOptions() const; - const Flags &GetMask() const; + const Flags GetMask() const; bool GetVerbose() const; - void SetStream(const std::shared_ptr &stream_sp) { -llvm::sys::ScopedWriter lock(m_stream_mutex); -m_stream_sp = stream_sp; - } +private: + Channel &m_channel; - std::shared_ptr GetStream() { -llvm::sys::ScopedReader lock(m_stream_mutex); -return m_stream_sp; - } + // The mutex makes sure enable/disable operations are thread-safe. The options + // and mask variables are atomic to enable their reading in + // Channel::GetLogIfAny without taking the mutex to speed up the fast path. + // Their modification however, is still protected by this mutex. + llvm::sys::RWMutex m_mutex; -protected: - //-- - // Member variables - //-- - llvm::sys::RWMutex m_stream_mutex; // Protects m_stream_sp std::shared_ptr m_stream_sp; - - Flags m_options; - Flags m_mask_bits; - -private: - DISALLOW_COPY_AND_ASSIGN(Log); + std::atomic m_options{0}; + std::atomic m_mask{0}; void Writ
[Lldb-commits] [lldb] r297368 - Fix remaining threading issues in Log.h
Author: labath Date: Thu Mar 9 04:16:07 2017 New Revision: 297368 URL: http://llvm.org/viewvc/llvm-project?rev=297368&view=rev Log: Fix remaining threading issues in Log.h Summary: This fixes two threading issues in the logging code. The access to the mask and options flags had data races when we were trying to enable/disable logging while another thread was writing to the log. Since we can log from almost any context, and we want it to be fast, so I avoided locking primitives and used atomic variables instead. I have also removed the (unused) setters for the mask and flags to make sure that the only way to set them is through the enable/disable channel functions. I also add tests, which when run under tsan, verify that the use cases like "doing an LLDB_LOGV while another thread disables logging" are data-race-free. Reviewers: zturner, clayborg Subscribers: lldb-commits Differential Revision: https://reviews.llvm.org/D30702 Modified: lldb/trunk/include/lldb/Utility/Log.h lldb/trunk/source/Utility/Log.cpp lldb/trunk/unittests/Utility/LogTest.cpp Modified: lldb/trunk/include/lldb/Utility/Log.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/Log.h?rev=297368&r1=297367&r2=297368&view=diff == --- lldb/trunk/include/lldb/Utility/Log.h (original) +++ lldb/trunk/include/lldb/Utility/Log.h Thu Mar 9 04:16:07 2017 @@ -17,6 +17,7 @@ // Other libraries and framework includes #include "llvm/Support/FormatVariadic.h" +#include "llvm/Support/ManagedStatic.h" #include "llvm/Support/RWMutex.h" // C++ Includes #include @@ -55,6 +56,7 @@ public: // necessary to enable a log channel in an atomic manner. class Channel { std::atomic log_ptr; +friend class Log; public: const llvm::ArrayRef categories; @@ -66,29 +68,26 @@ public: default_flags(default_flags) {} // This function is safe to call at any time -// FIXME: Not true yet, mask access is not atomic +// If the channel is disabled after (or concurrently with) this function +// returning a non-null Log pointer, it is still safe to attempt to write to +// the Log object -- the output will be discarded. Log *GetLogIfAll(uint32_t mask) { - Log *log = log_ptr.load(std::memory_order_acquire); + Log *log = log_ptr.load(std::memory_order_relaxed); if (log && log->GetMask().AllSet(mask)) return log; return nullptr; } // This function is safe to call at any time -// FIXME: Not true yet, mask access is not atomic +// If the channel is disabled after (or concurrently with) this function +// returning a non-null Log pointer, it is still safe to attempt to write to +// the Log object -- the output will be discarded. Log *GetLogIfAny(uint32_t mask) { - Log *log = log_ptr.load(std::memory_order_acquire); + Log *log = log_ptr.load(std::memory_order_relaxed); if (log && log->GetMask().AnySet(mask)) return log; return nullptr; } - -// Calls to Enable and disable need to be serialized externally. -void Enable(Log &log, const std::shared_ptr &stream_sp, -uint32_t options, uint32_t flags); - -// Calls to Enable and disable need to be serialized externally. -void Disable(uint32_t flags); }; //-- @@ -115,12 +114,13 @@ public: //-- // Member functions + // + // These functions are safe to call at any time you have a Log* obtained from + // the Channel class. If logging is disabled between you obtaining the Log + // object and writing to it, the output will be silently discarded. //-- - Log(); - - Log(const std::shared_ptr &stream_sp); - - ~Log(); + Log(Channel &channel) : m_channel(channel) {} + ~Log() = default; void PutCString(const char *cstr); void PutString(llvm::StringRef str); @@ -136,9 +136,6 @@ public: void VAPrintf(const char *format, va_list args); - void LogIf(uint32_t mask, const char *fmt, ...) - __attribute__((format(printf, 3, 4))); - void Error(const char *fmt, ...) __attribute__((format(printf, 2, 3))); void VAError(const char *format, va_list args); @@ -147,38 +144,24 @@ public: void Warning(const char *fmt, ...) __attribute__((format(printf, 2, 3))); - Flags &GetOptions(); - - const Flags &GetOptions() const; - - Flags &GetMask(); + const Flags GetOptions() const; - const Flags &GetMask() const; + const Flags GetMask() const; bool GetVerbose() const; - void SetStream(const std::shared_ptr &stream_sp) { -llvm::sys::ScopedWriter lock(m_stream_mutex); -m_stream_sp = stream_sp; - } +private: + Channel &m_channel; - std::shared_ptr GetStream() { -llvm::sys::ScopedReader loc
[Lldb-commits] [PATCH] D30779: dotest.py: remove the ability to specify different architectures/compilers in a single invocation
labath created this revision. Herald added a subscriber: danalbert. This has been broken at least since the new test result framework was added, which was over a year ago. It looks like nobody has missed it since. Removing this makes the gmodules handling code saner, as it already did not know how to handle the multiple-compilers case. My motivation for this is libc++ data formatters support on android -- I am trying make a central way of determining whether libc++ tests can be run, and without this, I would have to resort to similar hacks as the gmodules code. https://reviews.llvm.org/D30779 Files: packages/Python/lldbsuite/test/configuration.py packages/Python/lldbsuite/test/dosep.py packages/Python/lldbsuite/test/dotest.py packages/Python/lldbsuite/test/dotest_args.py packages/Python/lldbsuite/test/lldbinline.py packages/Python/lldbsuite/test/lldbtest.py packages/Python/lldbsuite/test/test_categories.py Index: packages/Python/lldbsuite/test/test_categories.py === --- packages/Python/lldbsuite/test/test_categories.py +++ packages/Python/lldbsuite/test/test_categories.py @@ -46,7 +46,7 @@ return candidate -def is_supported_on_platform(category, platform, compiler_paths): +def is_supported_on_platform(category, platform, compiler_path): if category == "dwo": # -gsplit-dwarf is not implemented by clang on Windows. return platform in ["linux", "freebsd"] @@ -56,17 +56,7 @@ # First, check to see if the platform can even support gmodules. if platform not in ["linux", "freebsd", "darwin", "macosx", "ios"]: return False -# If all compilers specified support gmodules, we'll enable it. -for compiler_path in compiler_paths: -if not gmodules.is_compiler_clang_with_gmodules(compiler_path): -# Ideally in a multi-compiler scenario during a single test run, this would -# allow gmodules on compilers that support it and not on ones that don't. -# However, I didn't see an easy way for all the callers of this to know -# the compiler being used for a test invocation. As we tend to run with -# a single compiler per test run, this shouldn't be a major -# issue. -return False -return True +return gmodules.is_compiler_clang_with_gmodules(compiler_path) return True Index: packages/Python/lldbsuite/test/lldbtest.py === --- packages/Python/lldbsuite/test/lldbtest.py +++ packages/Python/lldbsuite/test/lldbtest.py @@ -1714,7 +1714,7 @@ supported_categories = [ x for x in categories if test_categories.is_supported_on_platform( -x, target_platform, configuration.compilers)] +x, target_platform, configuration.compiler)] if "dsym" in supported_categories: @decorators.add_test_categories(["dsym"]) @wraps(attrvalue) Index: packages/Python/lldbsuite/test/lldbinline.py === --- packages/Python/lldbsuite/test/lldbinline.py +++ packages/Python/lldbsuite/test/lldbinline.py @@ -226,19 +226,19 @@ target_platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2] if test_categories.is_supported_on_platform( -"dsym", target_platform, configuration.compilers): +"dsym", target_platform, configuration.compiler): test.test_with_dsym = ApplyDecoratorsToFunction( test._InlineTest__test_with_dsym, decorators) if test_categories.is_supported_on_platform( -"dwarf", target_platform, configuration.compilers): +"dwarf", target_platform, configuration.compiler): test.test_with_dwarf = ApplyDecoratorsToFunction( test._InlineTest__test_with_dwarf, decorators) if test_categories.is_supported_on_platform( -"dwo", target_platform, configuration.compilers): +"dwo", target_platform, configuration.compiler): test.test_with_dwo = ApplyDecoratorsToFunction( test._InlineTest__test_with_dwo, decorators) if test_categories.is_supported_on_platform( -"gmodules", target_platform, configuration.compilers): +"gmodules", target_platform, configuration.compiler): test.test_with_gmodules = ApplyDecoratorsToFunction( test._InlineTest__test_with_gmodules, decorators) Index: packages/Python/lldbsuite/test/dotest_args.py === --- packages/Python/lldbsuite/test/dotest_args.py +++ packages/Python/lldbsuite/test/dotest_args.py @@ -69,10 +69,9 @@ '-A', '--arch', metavar='arch', -action='append', -
[Lldb-commits] [lldb] r297405 - Make the LLDB test suite work with MSVC 2017 on Windows.
Author: zturner Date: Thu Mar 9 13:54:23 2017 New Revision: 297405 URL: http://llvm.org/viewvc/llvm-project?rev=297405&view=rev Log: Make the LLDB test suite work with MSVC 2017 on Windows. Modified: lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules Modified: lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules?rev=297405&r1=297404&r2=297405&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules (original) +++ lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules Thu Mar 9 13:54:23 2017 @@ -300,10 +300,12 @@ ifeq "$(OS)" "Windows_NT" # Clang for Windows doesn't support C++ Exceptions CXXFLAGS += -fno-exceptions CXXFLAGS += -D_HAS_EXCEPTIONS=0 - ifeq "$(VisualStudioVersion)" "14.0" - CXXFLAGS += -fms-compatibility-version=19.0 - override CXXFLAGS := $(subst -std=c++11,-std=c++14,$(CXXFLAGS)) - endif + + # MSVC 2015 or higher is required, which depends on c++14, so + # append these values unconditionally. + CXXFLAGS += -fms-compatibility-version=19.0 + override CXXFLAGS := $(subst -std=c++11,-std=c++14,$(CXXFLAGS)) + # The MSVC linker doesn't understand long section names # generated by the clang compiler. LDFLAGS += -fuse-ld=lld ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30789: Make file and directory name completion work properly on Windows.
zturner created this revision. Herald added a subscriber: mgorny. There were a couple of problems with this function on Windows. Different separators and differences in how tilde expressions are resolved for starters, but in addition there was no clear indication of what the function's inputs or outputs were supposed to be, and there were no tests to demonstrate its use. To more easily paper over the differences between Windows paths, non-Windows paths, and tilde expressions, I've ported this function to use LLVM-based directory iteration (in fact, I would like to eliminate all of LLDB's directory iteration code entirely since LLVM's is cleaner / more efficient (i.e. it invokes fewer stat calls)). and llvm's portable path manipulation library. Since file and directory completion assumes you are referring to files and directories on your local machine, it's safe to assume the path syntax properties of the host in doing so, so LLVM's APIs are perfect for this. I've also added a fairly robust set of unit tests. Since you can't really predict what users will be on your machine, or what their home directories will be, I added an interface called `TildeExpressionResolver`, and in the unit test I've mocked up a fake implementation that acts like a unix password database. This allows us to configure some fake users and home directories in the test, so we can exercise all of those hard-to-test codepaths that normally otherwise depend on the host. After this patch, file and directory completion works on Windows, and the same tests pass without my patch which suggests that I have not broken anything. Note that I think there was a bug in the original implementation regarding symlinks which I've tried to fix. Previously, if you tried to complete /foo/bar/baz, and it finds /foo/bar/bazel which is a symlink, then it treats this as a directory as long as /foo/bar is a directory, which doesn't make any sense. To fix this, if /foo/bar/bazel is a symlink, I stat it again with symlink-following on (which the previous code did not do), and check if the result is a directory. There's no good way to test this, however. https://reviews.llvm.org/D30789 Files: lldb/include/lldb/Interpreter/CommandCompletions.h lldb/include/lldb/Utility/TildeExpressionResolver.h lldb/source/Commands/CommandCompletions.cpp lldb/source/Utility/CMakeLists.txt lldb/source/Utility/TildeExpressionResolver.cpp lldb/unittests/Interpreter/CMakeLists.txt lldb/unittests/Interpreter/TestCompletion.cpp Index: lldb/unittests/Interpreter/TestCompletion.cpp === --- /dev/null +++ lldb/unittests/Interpreter/TestCompletion.cpp @@ -0,0 +1,346 @@ +//===-- TestCompletion.cpp --*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "gtest/gtest.h" + +#include "lldb/Core/StringList.h" +#include "lldb/Interpreter/CommandCompletions.h" +#include "lldb/Utility/TildeExpressionResolver.h" + +#include "llvm/ADT/SmallString.h" +#include "llvm/Support/FileSystem.h" +#include "llvm/Support/Path.h" +#include "llvm/Support/raw_ostream.h" + +namespace fs = llvm::sys::fs; +namespace path = llvm::sys::path; +using namespace llvm; +using namespace lldb_private; + +#define ASSERT_NO_ERROR(x) \ + if (std::error_code ASSERT_NO_ERROR_ec = x) {\ +SmallString<128> MessageStorage; \ +raw_svector_ostream Message(MessageStorage); \ +Message << #x ": did not return errc::success.\n" \ +<< "error number: " << ASSERT_NO_ERROR_ec.value() << "\n" \ +<< "error message: " << ASSERT_NO_ERROR_ec.message() << "\n"; \ +GTEST_FATAL_FAILURE_(MessageStorage.c_str()); \ + } else { \ + } + +namespace { + +class MockTildeExpressionResolver : public TildeExpressionResolver { + StringRef CurrentUser; + StringMap UserDirectories; + +public: + explicit MockTildeExpressionResolver(StringRef CurrentUser, StringRef HomeDir) + : CurrentUser(CurrentUser) { +UserDirectories.insert(std::make_pair(CurrentUser, HomeDir)); + } + + void AddKnownUser(StringRef User, StringRef HomeDir) { +assert(UserDirectories.find(User) == UserDirectories.end()); +UserDirectories.insert(std::make_pair(User, HomeDir)); + } + + void Clear() { +CurrentUser = StringRef(); +UserDirectories.clear(); + } + + void SetCurrentUser(StringRef User) { +assert(UserDirectories.find(User) != UserDirectories.en
[Lldb-commits] [PATCH] D30789: Make file and directory name completion work properly on Windows.
jingham added a comment. What is the motivation behind of the "2" versions of the functions you added? https://reviews.llvm.org/D30789 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30789: Make file and directory name completion work properly on Windows.
zturner added a comment. In https://reviews.llvm.org/D30789#696870, @jingham wrote: > What is the motivation behind of the "2" versions of the functions you added? Ahh yea. I can call them something else if you have a better name. But basically in a unit test I don't have an instance of a `CommandInterpreter` object, and creating one is non-trivial (it requires a `Debugger` instance, which requires calling `Debugger::CreateInstance()`, which depends on a lot of other system initialization having been done. But none of the methods I was trying to test use the `CommandInterpreter` anyway, so I just needed some kind of interface into the completion that didn't require me to pass one in. https://reviews.llvm.org/D30789 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30789: Make file and directory name completion work properly on Windows.
amccarth added inline comments. Comment at: lldb/source/Commands/CommandCompletions.cpp:108 + StringList &matches, + TildeExpressionResolver *Resolver) { + // Use the default resolver if one isn't explicitly specified. I know you're not inventing this API, but since you're changing it, can you do something about the `bool` and `bool &` parameters? It makes reading the call sites difficult without referring back here to see what those mean. Comment at: lldb/source/Commands/CommandCompletions.cpp:112 + if (!Resolver) +Resolver = &SR; + This leaves the caller with no way to say the don't want a tilde resolver. I didn't expect that from reading the API. Perhaps a comment on the API could clarify that you're going to get this default behavior if you specify nullptr. https://reviews.llvm.org/D30789 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30779: dotest.py: remove the ability to specify different architectures/compilers in a single invocation
zturner added a comment. Agree that it will be nice to see this gone. I suspect there is even more complexity than what you have here that can be removed if we aren't supporting this, but this seems like a good start. Comment at: packages/Python/lldbsuite/test/dotest.py:1204 +os.environ["ARCH"] = configuration.arch +os.environ["CC"] = configuration.compiler +configString = "arch=%s compiler=%s" % (configuration.arch, Should you be setting `CXX` here? https://reviews.llvm.org/D30779 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D30807: Use LLVM's directory enumeration code
zturner created this revision. `FileSpec::EnumerateDirectory` has a bunch of platform-specific gunk in it for posix and non-posix platforms. We can get rid of all this by using LLVM's easy-to-use directory iterators. Ideally I would like to just remove this entire `EnumerateDirectory` function. It served a useful purpose when the code for iterating directories would have to be re-invented every single time otherwise, but since we have these nice iterators, this intermediate enumerator function actually makes the code more verbose, since we have to set up this baton and return these magic values from the callback function, when usually this just means "be recursive" or "don't be recursive", something the writer of the algorithm could more cleanly express by simply choosing `recursive_directory_iterator` or `directory_iterator`. Anyway, I'm getting ahead of myself. For now just clean up this function and leave everything else as-is. https://reviews.llvm.org/D30807 Files: lldb/include/lldb/Host/FileSpec.h lldb/source/Host/common/FileSpec.cpp Index: lldb/source/Host/common/FileSpec.cpp === --- lldb/source/Host/common/FileSpec.cpp +++ lldb/source/Host/common/FileSpec.cpp @@ -7,21 +7,12 @@ // //===--===// -#ifndef _WIN32 -#include -#else -#include "lldb/Host/windows/windows.h" -#endif -#include -#ifndef _MSC_VER -#include -#endif #include #include #include -#include "lldb/Host/Config.h" // Have to include this before we test the define... -#ifdef LLDB_CONFIG_TILDE_RESOLVES_TO_USER +#include "llvm/Config/llvm-config.h" +#ifndef LLVM_ON_WIN32 #include #endif @@ -159,7 +150,7 @@ // If you want to complete "~" to the list of users, pass it to // ResolvePartialUsername. void FileSpec::ResolveUsername(llvm::SmallVectorImpl &path) { -#if LLDB_CONFIG_TILDE_RESOLVES_TO_USER +#ifndef LLVM_ON_WIN32 if (path.empty() || path[0] != '~') return; @@ -227,7 +218,7 @@ size_t FileSpec::ResolvePartialUsername(llvm::StringRef partial_name, StringList &matches) { -#ifdef LLDB_CONFIG_TILDE_RESOLVES_TO_USER +#ifndef LLVM_ON_WIN32 size_t extant_entries = matches.GetSize(); setpwent(); @@ -251,17 +242,17 @@ #else // Resolving home directories is not supported, just copy the path... return 0; -#endif // #ifdef LLDB_CONFIG_TILDE_RESOLVES_TO_USER +#endif // #ifdef LLVM_ON_WIN32 } void FileSpec::Resolve(llvm::SmallVectorImpl &path) { if (path.empty()) return; -#ifdef LLDB_CONFIG_TILDE_RESOLVES_TO_USER +#ifndef LLVM_ON_WIN32 if (path[0] == '~') ResolveUsername(path); -#endif // #ifdef LLDB_CONFIG_TILDE_RESOLVES_TO_USER +#endif // #ifdef LLVM_ON_WIN32 // Save a copy of the original path that's passed in llvm::SmallString<128> original_path(path.begin(), path.end()); @@ -776,235 +767,37 @@ return m_filename.MemorySize() + m_directory.MemorySize(); } -FileSpec::EnumerateDirectoryResult -FileSpec::ForEachItemInDirectory(llvm::StringRef dir_path, - DirectoryCallback const &callback) { - if (dir_path.empty()) -return eEnumerateDirectoryResultNext; - -#ifdef _WIN32 -std::string szDir(dir_path); -szDir += "\\*"; - -std::wstring wszDir; -if (!llvm::ConvertUTF8toWide(szDir, wszDir)) { - return eEnumerateDirectoryResultNext; -} - -WIN32_FIND_DATAW ffd; -HANDLE hFind = FindFirstFileW(wszDir.c_str(), &ffd); - -if (hFind == INVALID_HANDLE_VALUE) { - return eEnumerateDirectoryResultNext; -} - -do { - namespace fs = llvm::sys::fs; - fs::file_type ft = fs::file_type::type_unknown; - if (ffd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) { -size_t len = wcslen(ffd.cFileName); - -if (len == 1 && ffd.cFileName[0] == L'.') - continue; - -if (len == 2 && ffd.cFileName[0] == L'.' && ffd.cFileName[1] == L'.') - continue; - -ft = fs::file_type::directory_file; - } else if (ffd.dwFileAttributes & FILE_ATTRIBUTE_DEVICE) { -ft = fs::file_type::type_unknown; - } else { -ft = fs::file_type::regular_file; - } - - std::string fileName; - if (!llvm::convertWideToUTF8(ffd.cFileName, fileName)) { -continue; - } - - std::string child_path = llvm::join_items("\\", dir_path, fileName); - // Don't resolve the file type or path - FileSpec child_path_spec(child_path.data(), false); - - EnumerateDirectoryResult result = callback(ft, child_path_spec); - - switch (result) { - case eEnumerateDirectoryResultNext: -// Enumerate next entry in the current directory. We just -// exit this switch and will continue enumerating the -// current directory as we currently are... -break; - - case eEnumerateDirectoryResultEnter: // Recurse into the curre
[Lldb-commits] [lldb] r297440 - Mark this as skipped for now. There is a race condition with
Author: jmolenda Date: Thu Mar 9 23:33:27 2017 New Revision: 297440 URL: http://llvm.org/viewvc/llvm-project?rev=297440&view=rev Log: Mark this as skipped for now. There is a race condition with SectionLoadList exposed by this test. Greg tried to chase it down & got pretty far but the isn't correct so we'll disable this test for now until I can figure that out. Modified: lldb/trunk/packages/Python/lldbsuite/test/api/multiple-debuggers/TestMultipleDebuggers.py Modified: lldb/trunk/packages/Python/lldbsuite/test/api/multiple-debuggers/TestMultipleDebuggers.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/api/multiple-debuggers/TestMultipleDebuggers.py?rev=297440&r1=297439&r2=297440&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/api/multiple-debuggers/TestMultipleDebuggers.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/api/multiple-debuggers/TestMultipleDebuggers.py Thu Mar 9 23:33:27 2017 @@ -19,6 +19,7 @@ class TestMultipleSimultaneousDebuggers( mydir = TestBase.compute_mydir(__file__) @skipIfNoSBHeaders +@expectedFailureAll(bugnumber="rdar://30564102") @expectedFailureAll( archs="i[3-6]86", bugnumber="multi-process-driver.cpp creates an x64 target") ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r297441 - Add a distinction in an apple accelerator table between IsValid and
Author: jmolenda Date: Fri Mar 10 00:38:19 2017 New Revision: 297441 URL: http://llvm.org/viewvc/llvm-project?rev=297441&view=rev Log: Add a distinction in an apple accelerator table between IsValid and HasContent. If we have a valid accelerator table which has no content, we want to depend on that (empty) table as the authoritative source instead of reading through all the debug info for lookups. Modified: lldb/trunk/include/lldb/Core/MappedHash.h lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Modified: lldb/trunk/include/lldb/Core/MappedHash.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/MappedHash.h?rev=297441&r1=297440&r2=297441&view=diff == --- lldb/trunk/include/lldb/Core/MappedHash.h (original) +++ lldb/trunk/include/lldb/Core/MappedHash.h Fri Mar 10 00:38:19 2017 @@ -353,7 +353,11 @@ public: bool IsValid() const { return m_header.version == 1 && m_header.hash_function == eHashFunctionDJB && - m_header.bucket_count > 0 && m_header.hashes_count > 0; + m_header.bucket_count > 0; +} + +bool HasContent() const { +return IsValid() && m_header.hashes_count > 0; } uint32_t GetHashIndex(uint32_t bucket_idx) const { Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp?rev=297441&r1=297440&r2=297441&view=diff == --- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original) +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Fri Mar 10 00:38:19 2017 @@ -448,19 +448,19 @@ void SymbolFileDWARF::InitializeObject() if (m_data_apple_names.m_data.GetByteSize() > 0) { m_apple_names_ap.reset(new DWARFMappedHash::MemoryTable( m_data_apple_names.m_data, get_debug_str_data(), ".apple_names")); -if (m_apple_names_ap->IsValid()) - m_using_apple_tables = true; -else +if (!m_apple_names_ap->IsValid()) m_apple_names_ap.reset(); +if (m_apple_names_ap->HasContent()) + m_using_apple_tables = true; } get_apple_types_data(); if (m_data_apple_types.m_data.GetByteSize() > 0) { m_apple_types_ap.reset(new DWARFMappedHash::MemoryTable( m_data_apple_types.m_data, get_debug_str_data(), ".apple_types")); -if (m_apple_types_ap->IsValid()) - m_using_apple_tables = true; -else +if (!m_apple_types_ap->IsValid()) m_apple_types_ap.reset(); +if (m_apple_types_ap->HasContent()) + m_using_apple_tables = true; } get_apple_namespaces_data(); @@ -468,20 +468,20 @@ void SymbolFileDWARF::InitializeObject() m_apple_namespaces_ap.reset(new DWARFMappedHash::MemoryTable( m_data_apple_namespaces.m_data, get_debug_str_data(), ".apple_namespaces")); -if (m_apple_namespaces_ap->IsValid()) - m_using_apple_tables = true; -else +if (!m_apple_namespaces_ap->IsValid()) m_apple_namespaces_ap.reset(); +if (m_apple_namespaces_ap->HasContent()) + m_using_apple_tables = true; } get_apple_objc_data(); if (m_data_apple_objc.m_data.GetByteSize() > 0) { m_apple_objc_ap.reset(new DWARFMappedHash::MemoryTable( m_data_apple_objc.m_data, get_debug_str_data(), ".apple_objc")); -if (m_apple_objc_ap->IsValid()) - m_using_apple_tables = true; -else +if (!m_apple_objc_ap->IsValid()) m_apple_objc_ap.reset(); +if (m_apple_objc_ap->HasContent()) + m_using_apple_tables = true; } } ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits