[Lldb-commits] [PATCH] D30702: Fix remaining threading issues in Log.h

2017-03-09 Thread Pavel Labath via Phabricator via lldb-commits
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

2017-03-09 Thread Pavel Labath via lldb-commits
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

2017-03-09 Thread Pavel Labath via Phabricator via lldb-commits
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

2017-03-09 Thread Pavel Labath via lldb-commits
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

2017-03-09 Thread Pavel Labath via Phabricator via lldb-commits
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.

2017-03-09 Thread Zachary Turner via lldb-commits
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.

2017-03-09 Thread Zachary Turner via Phabricator via lldb-commits
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.

2017-03-09 Thread Jim Ingham via Phabricator via lldb-commits
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.

2017-03-09 Thread Zachary Turner via Phabricator via lldb-commits
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.

2017-03-09 Thread Adrian McCarthy via Phabricator via lldb-commits
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

2017-03-09 Thread Zachary Turner via Phabricator via lldb-commits
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

2017-03-09 Thread Zachary Turner via Phabricator via lldb-commits
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

2017-03-09 Thread Jason Molenda via lldb-commits
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

2017-03-09 Thread Jason Molenda via lldb-commits
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