date:20180716

[Lldb-commits] [PATCH] D49018: Convert a location information from PDB to a DWARF expression

2018-07-16 Thread Aleksandr Urakov via Phabricator via lldb-commits

aleksandr.urakov added a comment.

This doesn't look like the cause, the test fails for me without this patch... 
Here is my tests output for PDB folder:

  -- Testing: 10 tests, 8 threads --
  FAIL: lldb :: SymbolFile/PDB/enums-layout.test (1 of 10)
  FAIL: lldb :: SymbolFile/PDB/typedefs.test (2 of 10)
  FAIL: lldb :: SymbolFile/PDB/type-quals.test (3 of 10)
  PASS: lldb :: SymbolFile/PDB/function-nested-block.test (4 of 10)
  PASS: lldb :: SymbolFile/PDB/function-level-linking.test (5 of 10)
  FAIL: lldb :: SymbolFile/PDB/func-symbols.test (6 of 10)
  FAIL: lldb :: SymbolFile/PDB/variables.test (7 of 10)
  FAIL: lldb :: SymbolFile/PDB/variables-locations.test (8 of 10)
  FAIL: lldb :: SymbolFile/PDB/udt-completion.test (9 of 10)
  PASS: lldb :: SymbolFile/PDB/compilands.test (10 of 10)
  Testing Time: 23.51s
  
  Failing Tests (7):
  lldb :: SymbolFile/PDB/enums-layout.test
  lldb :: SymbolFile/PDB/func-symbols.test
  lldb :: SymbolFile/PDB/type-quals.test
  lldb :: SymbolFile/PDB/typedefs.test
  lldb :: SymbolFile/PDB/udt-completion.test
  lldb :: SymbolFile/PDB/variables-locations.test
  lldb :: SymbolFile/PDB/variables.test
  
Expected Passes: 3
Unexpected Failures: 7


Repository:
  rL LLVM

https://reviews.llvm.org/D49018



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

2018-07-16 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added a comment.

Looks good to me, modulo the inline test (or the current comments addressed). 
Thanks Shafik!


https://reviews.llvm.org/D49271



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

2018-07-16 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

Could you also add a test case for this?
I think it should be possible to test this via the gdb-client 
(`test/testcases/functionalities/gdb_remote_client/`) test suite. If I 
understood the previous comments correctly, you'll need to mock a server that 
sends a `thread-pcs` field, but does not implement a `jThreadsInfo` packet.


Repository:
  rL LLVM

https://reviews.llvm.org/D48868



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49368: Complete user-defined types from PDB symbol files

2018-07-16 Thread Aleksandr Urakov via Phabricator via lldb-commits

aleksandr.urakov created this revision.
aleksandr.urakov added reviewers: asmith, zturner, labath, clayborg.
Herald added a subscriber: JDevlieghere.

This patch adds the implementation of an UDT completion from PDB symbol files. 
For now it supports different UDT kinds (union, struct and class), static and 
non-static members, different member and base access, bit fields, virtual and 
non-virtual inheritance.

There is a problem with the virtual inheritance from packed types, but it 
refers to Clang's MicrosoftRecordLayoutBuilder. I am preparing a separate patch 
for that.


https://reviews.llvm.org/D49368

Files:
  include/lldb/Symbol/ClangASTContext.h
  lit/SymbolFile/PDB/Inputs/UdtCompletionTest.cpp
  lit/SymbolFile/PDB/Inputs/UdtCompletionTest.script
  lit/SymbolFile/PDB/udt-completion.test
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  source/Plugins/SymbolFile/PDB/PDBASTParser.h
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  source/Symbol/ClangASTContext.cpp

Index: source/Symbol/ClangASTContext.cpp
===
--- source/Symbol/ClangASTContext.cpp
+++ source/Symbol/ClangASTContext.cpp
@@ -124,6 +124,84 @@
  // Use Clang for D until there is a proper language plugin for it
  language == eLanguageTypeD;
 }
+
+// Checks whether m1 is an overload of m2 (as opposed to an override). This is
+// called by addOverridesForMethod to distinguish overrides (which share a
+// vtable entry) from overloads (which require distinct entries).
+bool isOverload(clang::CXXMethodDecl *m1, clang::CXXMethodDecl *m2) {
+  // FIXME: This should detect covariant return types, but currently doesn't.
+  lldbassert(&m1->getASTContext() == &m2->getASTContext() &&
+ "Methods should have the same AST context");
+  clang::ASTContext &context = m1->getASTContext();
+
+  const auto *m1Type = llvm::cast(
+  context.getCanonicalType(m1->getType()));
+
+  const auto *m2Type = llvm::cast(
+  context.getCanonicalType(m2->getType()));
+
+  auto compareArgTypes = [&context](const clang::QualType &m1p,
+const clang::QualType &m2p) {
+return context.hasSameType(m1p.getUnqualifiedType(),
+   m2p.getUnqualifiedType());
+  };
+
+  // FIXME: In C++14 and later, we can just pass m2Type->param_type_end()
+  //as a fourth parameter to std::equal().
+  return (m1->getNumParams() != m2->getNumParams()) ||
+ !std::equal(m1Type->param_type_begin(), m1Type->param_type_end(),
+ m2Type->param_type_begin(), compareArgTypes);
+}
+
+// If decl is a virtual method, walk the base classes looking for methods that
+// decl overrides. This table of overridden methods is used by IRGen to
+// determine the vtable layout for decl's parent class.
+void addOverridesForMethod(clang::CXXMethodDecl *decl) {
+  if (!decl->isVirtual())
+return;
+
+  clang::CXXBasePaths paths;
+
+  auto find_overridden_methods =
+  [decl](const clang::CXXBaseSpecifier *specifier,
+ clang::CXXBasePath &path) {
+if (auto *base_record = llvm::dyn_cast(
+specifier->getType()->getAs()->getDecl())) {
+
+  clang::DeclarationName name = decl->getDeclName();
+
+  // If this is a destructor, check whether the base class destructor is
+  // virtual.
+  if (name.getNameKind() == clang::DeclarationName::CXXDestructorName)
+if (auto *baseDtorDecl = base_record->getDestructor()) {
+  if (baseDtorDecl->isVirtual()) {
+path.Decls = baseDtorDecl;
+return true;
+  } else
+return false;
+}
+
+  // Otherwise, search for name in the base class.
+  for (path.Decls = base_record->lookup(name); !path.Decls.empty();
+   path.Decls = path.Decls.slice(1)) {
+if (auto *method_decl =
+llvm::dyn_cast(path.Decls.front()))
+  if (method_decl->isVirtual() && !isOverload(decl, method_decl)) {
+path.Decls = method_decl;
+return true;
+  }
+  }
+}
+
+return false;
+  };
+
+  if (decl->getParent()->lookupInBases(find_overridden_methods, paths)) {
+for (auto *overridden_decl : paths.found_decls())
+  decl->addOverriddenMethod(
+  llvm::cast(overridden_decl));
+  }
+}
 }
 
 typedef lldb_private::ThreadSafeDenseMap
@@ -8125,6 +8203,13 @@
   return cxx_method_decl;
 }
 
+void ClangASTContext::AddMethodOverridesForCXXRecordType(
+lldb::opaque_compiler_type_t type) {
+  if (auto *record = GetAsCXXRecordDecl(type))
+for (auto *method : record->methods())
+  addOverridesForMethod(method);
+}
+
 #pragma mark C++ Base Classes
 
 clang::CXXBaseSpecifier *
@@ -9664,11 +9749,16 @@
 llvm::DenseMap
 &vbase_offsets) {
   ClangASTContext

[Lldb-commits] [PATCH] D49307: Fix some crashes and deadlocks in FormatAnsiTerminalCodes

2018-07-16 Thread Pavel Labath via Phabricator via lldb-commits

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

This looks straight-forward enough.




Comment at: unittests/Utility/AnsiTerminalTest.cpp:18
+  std::string format = ansi::FormatAnsiTerminalCodes("");
+  EXPECT_STREQ("", format.c_str());
+}

I would suggest getting rid of the temporary `format` variable. That way, if 
this assertion fails, the error message will immediately tell you what the 
failing command was.
(you can also use `EXPECT_EQ` here as `operator==` will do the right thing when 
one of the arguments is a std::string)


https://reviews.llvm.org/D49307



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] r336991 - Add abbreviated name for Debugger::EventHandlerThread.

2018-07-16 Thread Pavel Labath via lldb-commits

On Fri, 13 Jul 2018 at 18:36, Jim Ingham via lldb-commits
 wrote:
>
> There's code in the ThreadHandler to handle systems with short thread names.  
> If that isn't producing readable names, we should fix it there.  A better 
> algorithm might be to drop the leading "lldb" and then instead of truncating 
> drop vowels (maybe leaving the first vowel after a .)  So you'd get 
> "dbggr.evnt-hndlr" which isn't too bad.  Could drop duplicated consonants 
> too.  It seems a shame for every caller to have to worry about this.

+1 for this idea.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49334: [LLDB} Added syntax highlighting support

2018-07-16 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

We're also trying to avoid adding new clang-specific code to the debugger core. 
I think it would make more sense if the (clang-based) c++ highlighter was 
provided by some plugin. I see a couple of options:

- the c++ language plugin: I think this is the most low-level plugin that is 
still language specific. However, it is specific to c(++), whereas here you 
format other languages too.
- the clang expression parser plugin: it's a bit of a stretch, but syntax 
higlighting is a kind of expression parsing
- a completely new plugin


Repository:
  rL LLVM

https://reviews.llvm.org/D49334



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49377: Move pretty stack trace printer into driver.

2018-07-16 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere created this revision.
JDevlieghere added reviewers: jingham, labath, zturner.
Herald added a reviewer: jfb.

We used to have a pretty stack trace printer in SystemInitializerCommon. This 
was disabled on Apple because we didn't want the library to be setting signal 
handlers, as this was causing issues when loaded into Xcode. However, I think 
it's useful to have this for the LLDB driver, so I moved it up to use the 
PrettyStackTraceProgram in the driver's main.


https://reviews.llvm.org/D49377

Files:
  source/Initialization/SystemInitializerCommon.cpp
  tools/driver/Driver.cpp


Index: tools/driver/Driver.cpp
===
--- tools/driver/Driver.cpp
+++ tools/driver/Driver.cpp
@@ -41,7 +41,10 @@
 #include "lldb/API/SBStringList.h"
 #include "lldb/API/SBTarget.h"
 #include "lldb/API/SBThread.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Signals.h"
 #include 
 
 #if !defined(__APPLE__)
@@ -1225,6 +1228,10 @@
   const char **argv = argvPointers.data();
 #endif
 
+  llvm::StringRef ToolName = argv[0];
+  llvm::sys::PrintStackTraceOnErrorSignal(ToolName);
+  llvm::PrettyStackTraceProgram X(argc, argv);
+
   SBDebugger::Initialize();
 
   SBHostOS::ThreadCreated("");
Index: source/Initialization/SystemInitializerCommon.cpp
===
--- source/Initialization/SystemInitializerCommon.cpp
+++ source/Initialization/SystemInitializerCommon.cpp
@@ -29,7 +29,6 @@
 #include "lldb/Host/windows/windows.h"
 #endif
 
-#include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/TargetSelect.h"
 
 #include 
@@ -63,9 +62,6 @@
   }
 #endif
 
-#if not defined(__APPLE__)
-  llvm::EnablePrettyStackTrace();
-#endif
   Log::Initialize();
   HostInfo::Initialize();
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);


Index: tools/driver/Driver.cpp
===
--- tools/driver/Driver.cpp
+++ tools/driver/Driver.cpp
@@ -41,7 +41,10 @@
 #include "lldb/API/SBStringList.h"
 #include "lldb/API/SBTarget.h"
 #include "lldb/API/SBThread.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Signals.h"
 #include 
 
 #if !defined(__APPLE__)
@@ -1225,6 +1228,10 @@
   const char **argv = argvPointers.data();
 #endif
 
+  llvm::StringRef ToolName = argv[0];
+  llvm::sys::PrintStackTraceOnErrorSignal(ToolName);
+  llvm::PrettyStackTraceProgram X(argc, argv);
+
   SBDebugger::Initialize();
 
   SBHostOS::ThreadCreated("");
Index: source/Initialization/SystemInitializerCommon.cpp
===
--- source/Initialization/SystemInitializerCommon.cpp
+++ source/Initialization/SystemInitializerCommon.cpp
@@ -29,7 +29,6 @@
 #include "lldb/Host/windows/windows.h"
 #endif
 
-#include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/TargetSelect.h"
 
 #include 
@@ -63,9 +62,6 @@
   }
 #endif
 
-#if not defined(__APPLE__)
-  llvm::EnablePrettyStackTrace();
-#endif
   Log::Initialize();
   HostInfo::Initialize();
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r337173 - Fix TestDataFormatterUnordered for older libc++ versions

2018-07-16 Thread Pavel Labath via lldb-commits

Author: labath
Date: Mon Jul 16 07:37:58 2018
New Revision: 337173

URL: http://llvm.org/viewvc/llvm-project?rev=337173&view=rev
Log:
Fix TestDataFormatterUnordered for older libc++ versions

clang recently started diagnosing "exception specification in
declaration does not match previous declaration" errors. Unfortunately
old libc++ versions had a bug, where they violated this rule, which
means that tests using this library version now fail due to build
errors.

Since it was easy to work around the bug by compiling this test with
-fno-exceptions, I do that here. If supporting old libc++ versions
becomes a burden, we'll have to revisit this.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/unordered/Makefile

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/unordered/Makefile
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/unordered/Makefile?rev=337173&r1=337172&r2=337173&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/unordered/Makefile
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/unordered/Makefile
 Mon Jul 16 07:37:58 2018
@@ -2,6 +2,11 @@ LEVEL = ../../../../../make
 
 CXX_SOURCES := main.cpp
 
+# Work around "exception specification in declaration does not match previous
+# declaration" errors present in older libc++ releases. This error was fixed in
+# the 3.8 release.
+CFLAGS_EXTRAS += -fno-exceptions
+
 USE_LIBCPP := 1
 include $(LEVEL)/Makefile.rules
 CXXFLAGS += -O0


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49282: [cmake] Add option to skip building lldb-server

2018-07-16 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

I think this is fine (though the meaning of SKIP_LLDB_SERVER is subtly 
different than SKIP_DEBUGSERVER), but while looking at this I got an idea for a 
possible improvement.

How do you currently convince lldb to use ds2 instead of lldb-server? Do you 
just set the LLDB_DEBUGSERVER_PATH env var or do you do something more fancy?

I was thinking we could make the debugserver to use configurable at build time. 
That way you could build with LLDB_EXTERNAL_DEBUGSERVER=path/to/ds2.exe, which 
would make lldb automagically know how to launch it, and we would skip building 
lldb-server as a side effect.


https://reviews.llvm.org/D49282



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49377: Move pretty stack trace printer into driver.

2018-07-16 Thread Pavel Labath via Phabricator via lldb-commits

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

I think this makes perfect sense. Could you also add the same thing to the 
other binaries in the tools subfolder?


https://reviews.llvm.org/D49377



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49377: Move pretty stack trace printer into driver.

2018-07-16 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere updated this revision to Diff 155684.
JDevlieghere added a comment.
Herald added a subscriber: ki.stfu.

I've added it to the tools that made sense to me. Let me know if I missed 
something obvious.


https://reviews.llvm.org/D49377

Files:
  source/Initialization/SystemInitializerCommon.cpp
  tools/driver/Driver.cpp
  tools/lldb-mi/MIDriverMain.cpp
  tools/lldb-server/lldb-server.cpp


Index: tools/lldb-server/lldb-server.cpp
===
--- tools/lldb-server/lldb-server.cpp
+++ tools/lldb-server/lldb-server.cpp
@@ -12,7 +12,10 @@
 #include "lldb/lldb-private.h"
 
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Signals.h"
 
 #include 
 #include 
@@ -45,6 +48,10 @@
 // main
 //--
 int main(int argc, char *argv[]) {
+  llvm::StringRef ToolName = argv[0];
+  llvm::sys::PrintStackTraceOnErrorSignal(ToolName);
+  llvm::PrettyStackTraceProgram X(argc, argv);
+
   int option_error = 0;
   const char *progname = argv[0];
   if (argc < 2) {
Index: tools/lldb-mi/MIDriverMain.cpp
===
--- tools/lldb-mi/MIDriverMain.cpp
+++ tools/lldb-mi/MIDriverMain.cpp
@@ -33,6 +33,9 @@
 
 // Third party headers:
 #include "lldb/API/SBHostOS.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Signals.h"
 #include 
 #include 
 #include 
@@ -174,6 +177,10 @@
 #endif //  _WIN32
 #endif // MICONFIG_DEBUG_SHOW_ATTACH_DBG_DLG
 
+  llvm::StringRef ToolName = argv[0];
+  llvm::sys::PrintStackTraceOnErrorSignal(ToolName);
+  llvm::PrettyStackTraceProgram X(argc, argv);
+
   // *** Order is important here ***
   bool bOk = DriverSystemInit();
   if (!bOk) {
Index: tools/driver/Driver.cpp
===
--- tools/driver/Driver.cpp
+++ tools/driver/Driver.cpp
@@ -41,7 +41,10 @@
 #include "lldb/API/SBStringList.h"
 #include "lldb/API/SBTarget.h"
 #include "lldb/API/SBThread.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ConvertUTF.h"
+#include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Signals.h"
 #include 
 
 #if !defined(__APPLE__)
@@ -1225,6 +1228,10 @@
   const char **argv = argvPointers.data();
 #endif
 
+  llvm::StringRef ToolName = argv[0];
+  llvm::sys::PrintStackTraceOnErrorSignal(ToolName);
+  llvm::PrettyStackTraceProgram X(argc, argv);
+
   SBDebugger::Initialize();
 
   SBHostOS::ThreadCreated("");
Index: source/Initialization/SystemInitializerCommon.cpp
===
--- source/Initialization/SystemInitializerCommon.cpp
+++ source/Initialization/SystemInitializerCommon.cpp
@@ -29,7 +29,6 @@
 #include "lldb/Host/windows/windows.h"
 #endif
 
-#include "llvm/Support/PrettyStackTrace.h"
 #include "llvm/Support/TargetSelect.h"
 
 #include 
@@ -63,9 +62,6 @@
   }
 #endif
 
-#if not defined(__APPLE__)
-  llvm::EnablePrettyStackTrace();
-#endif
   Log::Initialize();
   HostInfo::Initialize();
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);


Index: tools/lldb-server/lldb-server.cpp
===
--- tools/lldb-server/lldb-server.cpp
+++ tools/lldb-server/lldb-server.cpp
@@ -12,7 +12,10 @@
 #include "lldb/lldb-private.h"
 
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ManagedStatic.h"
+#include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Signals.h"
 
 #include 
 #include 
@@ -45,6 +48,10 @@
 // main
 //--
 int main(int argc, char *argv[]) {
+  llvm::StringRef ToolName = argv[0];
+  llvm::sys::PrintStackTraceOnErrorSignal(ToolName);
+  llvm::PrettyStackTraceProgram X(argc, argv);
+
   int option_error = 0;
   const char *progname = argv[0];
   if (argc < 2) {
Index: tools/lldb-mi/MIDriverMain.cpp
===
--- tools/lldb-mi/MIDriverMain.cpp
+++ tools/lldb-mi/MIDriverMain.cpp
@@ -33,6 +33,9 @@
 
 // Third party headers:
 #include "lldb/API/SBHostOS.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/PrettyStackTrace.h"
+#include "llvm/Support/Signals.h"
 #include 
 #include 
 #include 
@@ -174,6 +177,10 @@
 #endif //  _WIN32
 #endif // MICONFIG_DEBUG_SHOW_ATTACH_DBG_DLG
 
+  llvm::StringRef ToolName = argv[0];
+  llvm::sys::PrintStackTraceOnErrorSignal(ToolName);
+  llvm::PrettyStackTraceProgram X(argc, argv);
+
   // *** Order is important here ***
   bool bOk = DriverSystemInit();
   if (!bOk) {
Index: tools/driver/Driver.cpp
===
--- tools/driver/Driver.cpp
+++ tools/driver/Driver.cpp
@@ -41,7 +41,10 @@
 #include "lldb/AP

[Lldb-commits] [PATCH] D48865: [LLDB] CommandObjectThreadUntil::DoExecute() sets the wrong selected thread ID

2018-07-16 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

Could you please add a test for the new behavior as well?


https://reviews.llvm.org/D48865



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49038: [CMake] Give lldb tools functional install targets when building LLDB.framework

2018-07-16 Thread Pavel Labath via Phabricator via lldb-commits

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Makes sense to me.


https://reviews.llvm.org/D49038



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks

2018-07-16 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

I don't agree with the two-stage initialization of the MinidumpParser class 
being introduced here. We deliberately introduced the `Create` static function 
to avoid this. If this `Initialize` function in checking invariants which are 
assumed to be hold by other parser methods, then it should be done by the 
`Create` function. Ideally this would be done before even constructing the 
parser object, but if this is impractical for some reason then you can make the 
`Initialize` function private and call it directly from `Create`. This way a 
user will never be able to see an malformed parser object. To make sure you 
propagate the error, you can change the return type of `Create` to 
`llvm::Expectedhttps://reviews.llvm.org/D49202



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r337188 - Fix typo in find-basic-function test

2018-07-16 Thread Pavel Labath via lldb-commits

Author: labath
Date: Mon Jul 16 09:18:52 2018
New Revision: 337188

URL: http://llvm.org/viewvc/llvm-project?rev=337188&view=rev
Log:
Fix typo in find-basic-function test

Wrong FileCheck header meant that we were not matching what we should.

This allows us to get rid of the -allow-deprecated-dag-overlap flag in
the test.

Modified:
lldb/trunk/lit/SymbolFile/DWARF/find-basic-function.cpp

Modified: lldb/trunk/lit/SymbolFile/DWARF/find-basic-function.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/DWARF/find-basic-function.cpp?rev=337188&r1=337187&r2=337188&view=diff
==
--- lldb/trunk/lit/SymbolFile/DWARF/find-basic-function.cpp (original)
+++ lldb/trunk/lit/SymbolFile/DWARF/find-basic-function.cpp Mon Jul 16 09:18:52 
2018
@@ -7,7 +7,7 @@
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=method 
%t | \
 // RUN:   FileCheck --check-prefix=METHOD %s
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t 
| \
-// RUN:   FileCheck -allow-deprecated-dag-overlap --check-prefix=FULL %s
+// RUN:   FileCheck --check-prefix=FULL %s
 // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full 
%t | \
 // RUN:   FileCheck --check-prefix=FULL-MANGLED %s
 // RUN: lldb-test symbols --name=foo --context=context --find=function 
--function-flags=base %t | \
@@ -21,7 +21,7 @@
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=method 
%t | \
 // RUN:   FileCheck --check-prefix=METHOD %s
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t 
| \
-// RUN:   FileCheck -allow-deprecated-dag-overlap --check-prefix=FULL %s
+// RUN:   FileCheck --check-prefix=FULL %s
 // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full 
%t | \
 // RUN:   FileCheck --check-prefix=FULL-MANGLED %s
 // RUN: lldb-test symbols --name=foo --context=context --find=function 
--function-flags=base %t | \
@@ -36,7 +36,7 @@
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=method 
%t | \
 // RUN:   FileCheck --check-prefix=METHOD %s
 // RUN: lldb-test symbols --name=foo --find=function --function-flags=full %t 
| \
-// RUN:   FileCheck -allow-deprecated-dag-overlap --check-prefix=FULL %s
+// RUN:   FileCheck --check-prefix=FULL %s
 // RUN: lldb-test symbols --name=_Z3fooi --find=function --function-flags=full 
%t | \
 // RUN:   FileCheck --check-prefix=FULL-MANGLED %s
 // RUN: lldb-test symbols --name=foo --context=context --find=function 
--function-flags=base %t | \
@@ -65,7 +65,7 @@
 // FULL-DAG: name = "ffbar()::sbaz::foo()", mangled = "_ZZ5ffbarvEN4sbaz3fooEv"
 
 // FULL-MANGLED: Found 1 functions:
-// FULL-DAG: name = "foo(int)", mangled = "_Z3fooi"
+// FULL-MANGLED-DAG: name = "foo(int)", mangled = "_Z3fooi"
 
 // CONTEXT: Found 1 functions:
 // CONTEXT-DAG: name = "bar::foo()", mangled = "_ZN3bar3fooEv"


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49307: Fix some crashes and deadlocks in FormatAnsiTerminalCodes

2018-07-16 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor updated this revision to Diff 155706.
teemperor added a comment.

- Removed temp string variables.


https://reviews.llvm.org/D49307

Files:
  include/lldb/Utility/AnsiTerminal.h
  unittests/Utility/AnsiTerminalTest.cpp
  unittests/Utility/CMakeLists.txt


Index: unittests/Utility/CMakeLists.txt
===
--- unittests/Utility/CMakeLists.txt
+++ unittests/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(UtilityTests
+  AnsiTerminalTest.cpp
   ArgsTest.cpp
   OptionsWithRawTest.cpp
   ArchSpecTest.cpp
Index: unittests/Utility/AnsiTerminalTest.cpp
===
--- /dev/null
+++ unittests/Utility/AnsiTerminalTest.cpp
@@ -0,0 +1,55 @@
+//===-- AnsiTerminalTest.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/Utility/AnsiTerminal.h"
+
+using namespace lldb_utility;
+
+TEST(AnsiTerminal, Empty) { EXPECT_EQ("", ansi::FormatAnsiTerminalCodes("")); }
+
+TEST(AnsiTerminal, WhiteSpace) {
+  EXPECT_EQ(" ", ansi::FormatAnsiTerminalCodes(" "));
+}
+
+TEST(AnsiTerminal, AtEnd) {
+  EXPECT_EQ("abc\x1B[30m",
+ansi::FormatAnsiTerminalCodes("abc${ansi.fg.black}"));
+}
+
+TEST(AnsiTerminal, AtStart) {
+  EXPECT_EQ("\x1B[30mabc",
+ansi::FormatAnsiTerminalCodes("${ansi.fg.black}abc"));
+}
+
+TEST(AnsiTerminal, KnownPrefix) {
+  EXPECT_EQ("${ansi.fg.redish}abc",
+ansi::FormatAnsiTerminalCodes("${ansi.fg.redish}abc"));
+}
+
+TEST(AnsiTerminal, Unknown) {
+  EXPECT_EQ("${ansi.fg.foo}abc",
+ansi::FormatAnsiTerminalCodes("${ansi.fg.foo}abc"));
+}
+
+TEST(AnsiTerminal, Incomplete) {
+  EXPECT_EQ("abc${ansi.", ansi::FormatAnsiTerminalCodes("abc${ansi."));
+}
+
+TEST(AnsiTerminal, Twice) {
+  EXPECT_EQ("\x1B[30m\x1B[31mabc",
+
ansi::FormatAnsiTerminalCodes("${ansi.fg.black}${ansi.fg.red}abc"));
+}
+
+TEST(AnsiTerminal, Basic) {
+  EXPECT_EQ(
+  "abc\x1B[31mabc\x1B[0mabc",
+  ansi::FormatAnsiTerminalCodes("abc${ansi.fg.red}abc${ansi.normal}abc"));
+}
Index: include/lldb/Utility/AnsiTerminal.h
===
--- include/lldb/Utility/AnsiTerminal.h
+++ include/lldb/Utility/AnsiTerminal.h
@@ -119,17 +119,21 @@
   break;
 }
 
+bool found_code = false;
 for (const auto &code : codes) {
   if (!right.consume_front(code.name))
 continue;
 
   if (do_color)
 fmt.append(code.value);
-  format = right;
+  found_code = true;
   break;
 }
-
-format = format.drop_front();
+format = right;
+// If we haven't found a valid replacement value, we just copy the string
+// to the result without any modifications.
+if (!found_code)
+  fmt.append(tok_hdr);
   }
   return fmt;
 }


Index: unittests/Utility/CMakeLists.txt
===
--- unittests/Utility/CMakeLists.txt
+++ unittests/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(UtilityTests
+  AnsiTerminalTest.cpp
   ArgsTest.cpp
   OptionsWithRawTest.cpp
   ArchSpecTest.cpp
Index: unittests/Utility/AnsiTerminalTest.cpp
===
--- /dev/null
+++ unittests/Utility/AnsiTerminalTest.cpp
@@ -0,0 +1,55 @@
+//===-- AnsiTerminalTest.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/Utility/AnsiTerminal.h"
+
+using namespace lldb_utility;
+
+TEST(AnsiTerminal, Empty) { EXPECT_EQ("", ansi::FormatAnsiTerminalCodes("")); }
+
+TEST(AnsiTerminal, WhiteSpace) {
+  EXPECT_EQ(" ", ansi::FormatAnsiTerminalCodes(" "));
+}
+
+TEST(AnsiTerminal, AtEnd) {
+  EXPECT_EQ("abc\x1B[30m",
+ansi::FormatAnsiTerminalCodes("abc${ansi.fg.black}"));
+}
+
+TEST(AnsiTerminal, AtStart) {
+  EXPECT_EQ("\x1B[30mabc",
+ansi::FormatAnsiTerminalCodes("${ansi.fg.black}abc"));
+}
+
+TEST(AnsiTerminal, KnownPrefix) {
+  EXPECT_EQ("${ansi.fg.redish}abc",
+ansi::FormatAnsiTerminalCodes("${ansi.fg.redish}abc"));
+}
+
+TEST(AnsiTerminal, Unknown) {
+  EXPECT_EQ("${ansi.fg.foo}abc",
+ansi::FormatAnsiTerminalCodes("${ansi.fg.foo}abc"));
+}
+
+TEST(AnsiTerminal, Incomplete) {
+  EXPECT_EQ("abc${ansi.", ansi::FormatAnsiTerminalCodes("abc${ansi."));
+}
+
+TEST(AnsiTerminal, Twice) {
+  EXPECT_EQ("\x1B[30m\x1B[31mabc",
+ans

[Lldb-commits] [lldb] r337189 - Fix some crashes and deadlocks in FormatAnsiTerminalCodes

2018-07-16 Thread Raphael Isemann via lldb-commits

Author: teemperor
Date: Mon Jul 16 09:38:30 2018
New Revision: 337189

URL: http://llvm.org/viewvc/llvm-project?rev=337189&view=rev
Log:
Fix some crashes and deadlocks in FormatAnsiTerminalCodes

Summary:
This patch fixes a few problems with the FormatAnsiTerminalCodes function:

* It does an infinite loop on an unknown color value.
* It crashes when the color value is at the end of the string.
* It deletes the first character behind the color token.

Also added a few tests that reproduce those problems (and test some other 
corner cases).

Reviewers: davide, labath

Reviewed By: labath

Subscribers: labath, lldb-commits, mgorny

Differential Revision: https://reviews.llvm.org/D49307

Added:
lldb/trunk/unittests/Utility/AnsiTerminalTest.cpp
Modified:
lldb/trunk/include/lldb/Utility/AnsiTerminal.h
lldb/trunk/unittests/Utility/CMakeLists.txt

Modified: lldb/trunk/include/lldb/Utility/AnsiTerminal.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/AnsiTerminal.h?rev=337189&r1=337188&r2=337189&view=diff
==
--- lldb/trunk/include/lldb/Utility/AnsiTerminal.h (original)
+++ lldb/trunk/include/lldb/Utility/AnsiTerminal.h Mon Jul 16 09:38:30 2018
@@ -119,17 +119,21 @@ inline std::string FormatAnsiTerminalCod
   break;
 }
 
+bool found_code = false;
 for (const auto &code : codes) {
   if (!right.consume_front(code.name))
 continue;
 
   if (do_color)
 fmt.append(code.value);
-  format = right;
+  found_code = true;
   break;
 }
-
-format = format.drop_front();
+format = right;
+// If we haven't found a valid replacement value, we just copy the string
+// to the result without any modifications.
+if (!found_code)
+  fmt.append(tok_hdr);
   }
   return fmt;
 }

Added: lldb/trunk/unittests/Utility/AnsiTerminalTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/AnsiTerminalTest.cpp?rev=337189&view=auto
==
--- lldb/trunk/unittests/Utility/AnsiTerminalTest.cpp (added)
+++ lldb/trunk/unittests/Utility/AnsiTerminalTest.cpp Mon Jul 16 09:38:30 2018
@@ -0,0 +1,55 @@
+//===-- AnsiTerminalTest.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/Utility/AnsiTerminal.h"
+
+using namespace lldb_utility;
+
+TEST(AnsiTerminal, Empty) { EXPECT_EQ("", ansi::FormatAnsiTerminalCodes("")); }
+
+TEST(AnsiTerminal, WhiteSpace) {
+  EXPECT_EQ(" ", ansi::FormatAnsiTerminalCodes(" "));
+}
+
+TEST(AnsiTerminal, AtEnd) {
+  EXPECT_EQ("abc\x1B[30m",
+ansi::FormatAnsiTerminalCodes("abc${ansi.fg.black}"));
+}
+
+TEST(AnsiTerminal, AtStart) {
+  EXPECT_EQ("\x1B[30mabc",
+ansi::FormatAnsiTerminalCodes("${ansi.fg.black}abc"));
+}
+
+TEST(AnsiTerminal, KnownPrefix) {
+  EXPECT_EQ("${ansi.fg.redish}abc",
+ansi::FormatAnsiTerminalCodes("${ansi.fg.redish}abc"));
+}
+
+TEST(AnsiTerminal, Unknown) {
+  EXPECT_EQ("${ansi.fg.foo}abc",
+ansi::FormatAnsiTerminalCodes("${ansi.fg.foo}abc"));
+}
+
+TEST(AnsiTerminal, Incomplete) {
+  EXPECT_EQ("abc${ansi.", ansi::FormatAnsiTerminalCodes("abc${ansi."));
+}
+
+TEST(AnsiTerminal, Twice) {
+  EXPECT_EQ("\x1B[30m\x1B[31mabc",
+
ansi::FormatAnsiTerminalCodes("${ansi.fg.black}${ansi.fg.red}abc"));
+}
+
+TEST(AnsiTerminal, Basic) {
+  EXPECT_EQ(
+  "abc\x1B[31mabc\x1B[0mabc",
+  ansi::FormatAnsiTerminalCodes("abc${ansi.fg.red}abc${ansi.normal}abc"));
+}

Modified: lldb/trunk/unittests/Utility/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/CMakeLists.txt?rev=337189&r1=337188&r2=337189&view=diff
==
--- lldb/trunk/unittests/Utility/CMakeLists.txt (original)
+++ lldb/trunk/unittests/Utility/CMakeLists.txt Mon Jul 16 09:38:30 2018
@@ -1,4 +1,5 @@
 add_lldb_unittest(UtilityTests
+  AnsiTerminalTest.cpp
   ArgsTest.cpp
   OptionsWithRawTest.cpp
   ArchSpecTest.cpp


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49307: Fix some crashes and deadlocks in FormatAnsiTerminalCodes

2018-07-16 Thread Raphael Isemann via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL337189: Fix some crashes and deadlocks in 
FormatAnsiTerminalCodes (authored by teemperor, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49307?vs=155706&id=155707#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D49307

Files:
  lldb/trunk/include/lldb/Utility/AnsiTerminal.h
  lldb/trunk/unittests/Utility/AnsiTerminalTest.cpp
  lldb/trunk/unittests/Utility/CMakeLists.txt


Index: lldb/trunk/unittests/Utility/CMakeLists.txt
===
--- lldb/trunk/unittests/Utility/CMakeLists.txt
+++ lldb/trunk/unittests/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(UtilityTests
+  AnsiTerminalTest.cpp
   ArgsTest.cpp
   OptionsWithRawTest.cpp
   ArchSpecTest.cpp
Index: lldb/trunk/unittests/Utility/AnsiTerminalTest.cpp
===
--- lldb/trunk/unittests/Utility/AnsiTerminalTest.cpp
+++ lldb/trunk/unittests/Utility/AnsiTerminalTest.cpp
@@ -0,0 +1,55 @@
+//===-- AnsiTerminalTest.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/Utility/AnsiTerminal.h"
+
+using namespace lldb_utility;
+
+TEST(AnsiTerminal, Empty) { EXPECT_EQ("", ansi::FormatAnsiTerminalCodes("")); }
+
+TEST(AnsiTerminal, WhiteSpace) {
+  EXPECT_EQ(" ", ansi::FormatAnsiTerminalCodes(" "));
+}
+
+TEST(AnsiTerminal, AtEnd) {
+  EXPECT_EQ("abc\x1B[30m",
+ansi::FormatAnsiTerminalCodes("abc${ansi.fg.black}"));
+}
+
+TEST(AnsiTerminal, AtStart) {
+  EXPECT_EQ("\x1B[30mabc",
+ansi::FormatAnsiTerminalCodes("${ansi.fg.black}abc"));
+}
+
+TEST(AnsiTerminal, KnownPrefix) {
+  EXPECT_EQ("${ansi.fg.redish}abc",
+ansi::FormatAnsiTerminalCodes("${ansi.fg.redish}abc"));
+}
+
+TEST(AnsiTerminal, Unknown) {
+  EXPECT_EQ("${ansi.fg.foo}abc",
+ansi::FormatAnsiTerminalCodes("${ansi.fg.foo}abc"));
+}
+
+TEST(AnsiTerminal, Incomplete) {
+  EXPECT_EQ("abc${ansi.", ansi::FormatAnsiTerminalCodes("abc${ansi."));
+}
+
+TEST(AnsiTerminal, Twice) {
+  EXPECT_EQ("\x1B[30m\x1B[31mabc",
+
ansi::FormatAnsiTerminalCodes("${ansi.fg.black}${ansi.fg.red}abc"));
+}
+
+TEST(AnsiTerminal, Basic) {
+  EXPECT_EQ(
+  "abc\x1B[31mabc\x1B[0mabc",
+  ansi::FormatAnsiTerminalCodes("abc${ansi.fg.red}abc${ansi.normal}abc"));
+}
Index: lldb/trunk/include/lldb/Utility/AnsiTerminal.h
===
--- lldb/trunk/include/lldb/Utility/AnsiTerminal.h
+++ lldb/trunk/include/lldb/Utility/AnsiTerminal.h
@@ -119,17 +119,21 @@
   break;
 }
 
+bool found_code = false;
 for (const auto &code : codes) {
   if (!right.consume_front(code.name))
 continue;
 
   if (do_color)
 fmt.append(code.value);
-  format = right;
+  found_code = true;
   break;
 }
-
-format = format.drop_front();
+format = right;
+// If we haven't found a valid replacement value, we just copy the string
+// to the result without any modifications.
+if (!found_code)
+  fmt.append(tok_hdr);
   }
   return fmt;
 }


Index: lldb/trunk/unittests/Utility/CMakeLists.txt
===
--- lldb/trunk/unittests/Utility/CMakeLists.txt
+++ lldb/trunk/unittests/Utility/CMakeLists.txt
@@ -1,4 +1,5 @@
 add_lldb_unittest(UtilityTests
+  AnsiTerminalTest.cpp
   ArgsTest.cpp
   OptionsWithRawTest.cpp
   ArchSpecTest.cpp
Index: lldb/trunk/unittests/Utility/AnsiTerminalTest.cpp
===
--- lldb/trunk/unittests/Utility/AnsiTerminalTest.cpp
+++ lldb/trunk/unittests/Utility/AnsiTerminalTest.cpp
@@ -0,0 +1,55 @@
+//===-- AnsiTerminalTest.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/Utility/AnsiTerminal.h"
+
+using namespace lldb_utility;
+
+TEST(AnsiTerminal, Empty) { EXPECT_EQ("", ansi::FormatAnsiTerminalCodes("")); }
+
+TEST(AnsiTerminal, WhiteSpace) {
+  EXPECT_EQ(" ", ansi::FormatAnsiTerminalCodes(" "));
+}
+
+TEST(AnsiTerminal, AtEnd) {
+  EXPECT_EQ("abc\x1B[30m",
+ansi::FormatAnsiTerminalCodes("abc${ansi.fg.black}"));
+}
+
+TEST(AnsiTerminal, AtStart) {
+  EXPECT_EQ("\x1B[30mabc",
+ansi::FormatAnsiTerminalCodes("${ansi.fg.

[Lldb-commits] [PATCH] D49334: [LLDB} Added syntax highlighting support

2018-07-16 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added a comment.

@zturner We can migrate the existing AnsiTerminal.h to reuse the LLVM coloring 
backend. This way we fix also the code that already uses this convenient 
interface.

@labath I think we can add to the Language class the option to add its related 
syntax highlighting support. I'll check if/how that would work.

(Thanks for the reviews!)


Repository:
  rL LLVM

https://reviews.llvm.org/D49334



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49018: Convert a location information from PDB to a DWARF expression

2018-07-16 Thread Stella Stamenova via Phabricator via lldb-commits

stella.stamenova added a comment.

I'll spend some time looking into this today, but with commit 
0fa537f42f1af238c74bf41998dc1af31195839a variables.test passes. Then with 
commit d9899ad86e0a9b05781015cacced1438fcf70343, the test fails. There are 
clearly a couple of other commits in that range, but they are a lot less likely 
to have caused the failure.


Repository:
  rL LLVM

https://reviews.llvm.org/D49018



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks

2018-07-16 Thread Leonard Mosescu via lldb-commits

The problem is not returning an error from Minidump::Create() - if that was
the case this could easily be improved indeed. The two-phase initialization
is a consequence of the LLDB plugin lookup:

1. Target::CreateProcess() calls Process::FindPlugin()
2. ProcessMinidump::CreateInstance() then has to inspect the core file to
see if it's a minidump
2b. ... if it is a minidump, we need to create a ProcessMinidump (which
calls MinidumpParser::Create())
3. once the plugin is selected, Process::LoadCore() is finally called and
this the earliest we can do minidump-specific error checking

Note that at step 2b. we don't have a way to propagate the error since
we're just doing the plugin lookup (the most we can pass on the lookup to
the rest of the plugins). We can't easily defer the
MinidumpParser::Create() as step 2b either since that only morphs into a
different kind of two-stage initialization (having a ProcessMinidump w/o a
parser).

I agree that it would be nicer with a one step initialization but overall
changing the LLDB plugin lookup is too intrusive for the relatively small
benefit. If you have any suggestions I'd love to hear them.

On Mon, Jul 16, 2018 at 9:04 AM, Pavel Labath via Phabricator <
revi...@reviews.llvm.org> wrote:

> labath added a comment.
>
> I don't agree with the two-stage initialization of the MinidumpParser
> class being introduced here. We deliberately introduced the `Create` static
> function to avoid this. If this `Initialize` function in checking
> invariants which are assumed to be hold by other parser methods, then it
> should be done by the `Create` function. Ideally this would be done before
> even constructing the parser object, but if this is impractical for some
> reason then you can make the `Initialize` function private and call it
> directly from `Create`. This way a user will never be able to see an
> malformed parser object. To make sure you propagate the error, you can
> change the return type of `Create` to `llvm::Expected (the only reason we did not do this back then was that there was no
> precedent for using `Expected` in LLDB, but this is no longer the case).
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D49202
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks

2018-07-16 Thread Leonard Mosescu via Phabricator via lldb-commits

lemo added subscribers: amccarth, bgianfo, labath, penryu.
lemo added a comment.

The problem is not returning an error from Minidump::Create() - if that was
the case this could easily be improved indeed. The two-phase initialization
is a consequence of the LLDB plugin lookup:

1. Target::CreateProcess() calls Process::FindPlugin()
2. ProcessMinidump::CreateInstance() then has to inspect the core file to

see if it's a minidump
2b. ... if it is a minidump, we need to create a ProcessMinidump (which
calls MinidumpParser::Create())

3. once the plugin is selected, Process::LoadCore() is finally called and

this the earliest we can do minidump-specific error checking

Note that at step 2b. we don't have a way to propagate the error since
we're just doing the plugin lookup (the most we can pass on the lookup to
the rest of the plugins). We can't easily defer the
MinidumpParser::Create() as step 2b either since that only morphs into a
different kind of two-stage initialization (having a ProcessMinidump w/o a
parser).

I agree that it would be nicer with a one step initialization but overall
changing the LLDB plugin lookup is too intrusive for the relatively small
benefit. If you have any suggestions I'd love to hear them.


Repository:
  rL LLVM

https://reviews.llvm.org/D49202



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49309: No longer pass a StringRef to the Python API

2018-07-16 Thread David Blaikie via Phabricator via lldb-commits

dblaikie added subscribers: teemperor, dblaikie.
dblaikie added a comment.

If  the implementation of a function requires a string - it should probably
accept string (not a StringRef) as a parameter - to avoid back-and-forth
conversions in cases where the caller already has a string to use.


Repository:
  rL LLVM

https://reviews.llvm.org/D49309



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D49309: No longer pass a StringRef to the Python API

2018-07-16 Thread David Blaikie via lldb-commits

If  the implementation of a function requires a string - it should probably
accept string (not a StringRef) as a parameter - to avoid back-and-forth
conversions in cases where the caller already has a string to use.

On Fri, Jul 13, 2018 at 12:43 PM Stella Stamenova via Phabricator via
llvm-commits  wrote:

> stella.stamenova added a comment.
>
> All better now! Tests are passing.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D49309
>
>
>
> ___
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks

2018-07-16 Thread Pavel Labath via lldb-commits

Ok, I see what you mean now.

Looking at the other core file plugins (elf, mach-o), it looks like
they do only very basic verification before the accept the file. The
pretty much just check the magic numbers, which would be more-or-less
equivalent to our `MinidumpHeader::Parse` call. As this does not
require creating a parser object, maybe we could delay the parser
creation until `LoadCore` gets called (at which point you can easily
report errors).

This will leave us with a nice MinidumpParser interface.
ProcessMinidump will still use two-stage initialization, but this is
nothing new, and this change will make it easier for us to change the
initialization method of the Process objects in the future.

WDYT?

On Mon, 16 Jul 2018 at 18:16, Leonard Mosescu  wrote:
>
> The problem is not returning an error from Minidump::Create() - if that was 
> the case this could easily be improved indeed. The two-phase initialization 
> is a consequence of the LLDB plugin lookup:
>
> 1. Target::CreateProcess() calls Process::FindPlugin()
> 2. ProcessMinidump::CreateInstance() then has to inspect the core file to see 
> if it's a minidump
> 2b. ... if it is a minidump, we need to create a ProcessMinidump (which calls 
> MinidumpParser::Create())
> 3. once the plugin is selected, Process::LoadCore() is finally called and 
> this the earliest we can do minidump-specific error checking
>
> Note that at step 2b. we don't have a way to propagate the error since we're 
> just doing the plugin lookup (the most we can pass on the lookup to the rest 
> of the plugins). We can't easily defer the MinidumpParser::Create() as step 
> 2b either since that only morphs into a different kind of two-stage 
> initialization (having a ProcessMinidump w/o a parser).
>
> I agree that it would be nicer with a one step initialization but overall 
> changing the LLDB plugin lookup is too intrusive for the relatively small 
> benefit. If you have any suggestions I'd love to hear them.
>
>
> On Mon, Jul 16, 2018 at 9:04 AM, Pavel Labath via Phabricator 
>  wrote:
>>
>> labath added a comment.
>>
>> I don't agree with the two-stage initialization of the MinidumpParser class 
>> being introduced here. We deliberately introduced the `Create` static 
>> function to avoid this. If this `Initialize` function in checking invariants 
>> which are assumed to be hold by other parser methods, then it should be done 
>> by the `Create` function. Ideally this would be done before even 
>> constructing the parser object, but if this is impractical for some reason 
>> then you can make the `Initialize` function private and call it directly 
>> from `Create`. This way a user will never be able to see an malformed parser 
>> object. To make sure you propagate the error, you can change the return type 
>> of `Create` to `llvm::Expected> do this back then was that there was no precedent for using `Expected` in 
>> LLDB, but this is no longer the case).
>>
>>
>> Repository:
>>   rL LLVM
>>
>> https://reviews.llvm.org/D49202
>>
>>
>>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D49018: Convert a location information from PDB to a DWARF expression

2018-07-16 Thread Zachary Turner via lldb-commits

Fwiw I’ve seen cases where tests have passed even though they shouldn’t
have — the functionality being tested was broken. The one that comes to
mind was where we were doing a backtrace and then checking that it matched
the regex “main\(argc=3” to make sure the local variable argc had the
correct value. But the actual backtrace was more like mian(argc=3751589203,
...). I.e. a garbage value.

This test passed for months this way until an unrelated change caused argc
to change to a different junk value in the backtrace

This isn’t necessarily the case here, but something to keep in mind.
On Mon, Jul 16, 2018 at 10:10 AM Stella Stamenova via Phabricator <
revi...@reviews.llvm.org> wrote:

> stella.stamenova added a comment.
>
> I'll spend some time looking into this today, but with commit
> 0fa537f42f1af238c74bf41998dc1af31195839a variables.test passes. Then with
> commit d9899ad86e0a9b05781015cacced1438fcf70343, the test fails. There are
> clearly a couple of other commits in that range, but they are a lot less
> likely to have caused the failure.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D49018
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49018: Convert a location information from PDB to a DWARF expression

2018-07-16 Thread Zachary Turner via Phabricator via lldb-commits

zturner added a comment.

Fwiw I’ve seen cases where tests have passed even though they shouldn’t
have — the functionality being tested was broken. The one that comes to
mind was where we were doing a backtrace and then checking that it matched
the regex “main\(argc=3” to make sure the local variable argc had the
correct value. But the actual backtrace was more like mian(argc=3751589203,
...). I.e. a garbage value.

This test passed for months this way until an unrelated change caused argc
to change to a different junk value in the backtrace

This isn’t necessarily the case here, but something to keep in mind.


Repository:
  rL LLVM

https://reviews.llvm.org/D49018



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49282: [cmake] Add option to skip building lldb-server

2018-07-16 Thread Alex Langford via Phabricator via lldb-commits

xiaobai added a comment.

In https://reviews.llvm.org/D49282#1163517, @labath wrote:

> I think this is fine (though the meaning of SKIP_LLDB_SERVER is subtly 
> different than SKIP_DEBUGSERVER), but while looking at this I got an idea for 
> a possible improvement.

How is it subtly different? Admittedly I haven't looked in excruciating detail, 
but I didn't notice any large differences.

> How do you currently convince lldb to use ds2 instead of lldb-server? Do you 
> just set the LLDB_DEBUGSERVER_PATH env var or do you do something more fancy?

That's one way we do that. In some situations we launch ds2 and then tell lldb 
to connect to it. It just depends on what you're debugging.

> I was thinking we could make the debugserver to use configurable at build 
> time. That way you could build with 
> LLDB_EXTERNAL_DEBUGSERVER=path/to/ds2.exe, which would make lldb 
> automagically know how to launch it, and we would skip building lldb-server 
> as a side effect.

I do think this would be nice, but you might have some problems later on with 
installation+distribution. For example, if for any reason ds2 is in a separate 
location on a build machine than an end-user's machine, lldb will not have a 
good time. This could happen if the path to ds2's build tree is used for 
LLDB_EXTERNAL_DEBUGSERVER instead of the path to its install location. In some 
cases, the path to its install location isn't necessarily known ahead of time.

The option would be fine for lldb-server and debugserver though, since they (in 
general) will have well-known locations if already present on your system (e.g. 
an xcode related directory).

https://reviews.llvm.org/D49282

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r337202 - [CMake] Give lldb tools functional install targets when building LLDB.framework

2018-07-16 Thread Alex Langford via lldb-commits

Author: xiaobai
Date: Mon Jul 16 12:19:56 2018
New Revision: 337202

URL: http://llvm.org/viewvc/llvm-project?rev=337202&view=rev
Log:
[CMake] Give lldb tools functional install targets when building LLDB.framework

Summary:
This change makes the install targets for lldb tools functional when
building for the framework.

I am currently working on the install rules for lldb-framework and this will
let me make `install-lldb-framework` rely on `install-lldb-argdumper` for
instance. This is especially important for `install-lldb-framework-stripped`. It
is much better for `install-lldb-framework-stripped` to rely on
`install-lldb-argdumper-stripped` than to copy and strip lldb-argdumper
manually.

Differential Revision: https://reviews.llvm.org/D49038

Modified:
lldb/trunk/cmake/modules/AddLLDB.cmake

Modified: lldb/trunk/cmake/modules/AddLLDB.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/AddLLDB.cmake?rev=337202&r1=337201&r2=337202&view=diff
==
--- lldb/trunk/cmake/modules/AddLLDB.cmake (original)
+++ lldb/trunk/cmake/modules/AddLLDB.cmake Mon Jul 16 12:19:56 2018
@@ -111,18 +111,6 @@ function(add_lldb_executable name)
 RUNTIME_OUTPUT_DIRECTORY $${resource_dir}
 BUILD_WITH_INSTALL_RPATH On
 INSTALL_RPATH 
"@loader_path/../../../${resource_dots}${_dots}/${LLDB_FRAMEWORK_INSTALL_DIR}")
-  # For things inside the framework we don't need functional install 
targets
-  # because CMake copies the resources and headers from the build 
directory.
-  # But we still need this target to exist in order to use the
-  # LLVM_DISTRIBUTION_COMPONENTS build option. We also need the
-  # install-liblldb target to depend on this tool, so that it gets put into
-  # the Resources directory before the framework is installed.
-  if(ARG_GENERATE_INSTALL)
-add_custom_target(install-${name} DEPENDS ${name})
-add_dependencies(install-liblldb ${name})
-add_custom_target(install-${name}-stripped DEPENDS ${name})
-add_dependencies(install-liblldb-stripped ${name})
-  endif()
 endif()
   endif()
 
@@ -132,10 +120,14 @@ function(add_lldb_executable name)
   INSTALL_RPATH "@loader_path/../${LLDB_FRAMEWORK_INSTALL_DIR}")
   endif()
 
-  if(ARG_GENERATE_INSTALL AND NOT (ARG_INCLUDE_IN_SUITE AND 
LLDB_BUILD_FRAMEWORK ))
+  if(ARG_GENERATE_INSTALL)
+set(out_dir "bin")
+if (LLDB_BUILD_FRAMEWORK AND ARG_INCLUDE_IN_SUITE)
+  set(out_dir ${LLDB_FRAMEWORK_INSTALL_DIR}/${LLDB_FRAMEWORK_RESOURCE_DIR})
+endif()
 install(TARGETS ${name}
   COMPONENT ${name}
-  RUNTIME DESTINATION bin)
+  RUNTIME DESTINATION ${out_dir})
 if (NOT CMAKE_CONFIGURATION_TYPES)
   add_llvm_install_targets(install-${name}
DEPENDS ${name}


___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49038: [CMake] Give lldb tools functional install targets when building LLDB.framework

2018-07-16 Thread Alex Langford via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL337202: [CMake] Give lldb tools functional install targets 
when building LLDB.framework (authored by xiaobai, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D49038

Files:
  lldb/trunk/cmake/modules/AddLLDB.cmake


Index: lldb/trunk/cmake/modules/AddLLDB.cmake
===
--- lldb/trunk/cmake/modules/AddLLDB.cmake
+++ lldb/trunk/cmake/modules/AddLLDB.cmake
@@ -111,18 +111,6 @@
 RUNTIME_OUTPUT_DIRECTORY $${resource_dir}
 BUILD_WITH_INSTALL_RPATH On
 INSTALL_RPATH 
"@loader_path/../../../${resource_dots}${_dots}/${LLDB_FRAMEWORK_INSTALL_DIR}")
-  # For things inside the framework we don't need functional install 
targets
-  # because CMake copies the resources and headers from the build 
directory.
-  # But we still need this target to exist in order to use the
-  # LLVM_DISTRIBUTION_COMPONENTS build option. We also need the
-  # install-liblldb target to depend on this tool, so that it gets put into
-  # the Resources directory before the framework is installed.
-  if(ARG_GENERATE_INSTALL)
-add_custom_target(install-${name} DEPENDS ${name})
-add_dependencies(install-liblldb ${name})
-add_custom_target(install-${name}-stripped DEPENDS ${name})
-add_dependencies(install-liblldb-stripped ${name})
-  endif()
 endif()
   endif()
 
@@ -132,10 +120,14 @@
   INSTALL_RPATH "@loader_path/../${LLDB_FRAMEWORK_INSTALL_DIR}")
   endif()
 
-  if(ARG_GENERATE_INSTALL AND NOT (ARG_INCLUDE_IN_SUITE AND 
LLDB_BUILD_FRAMEWORK ))
+  if(ARG_GENERATE_INSTALL)
+set(out_dir "bin")
+if (LLDB_BUILD_FRAMEWORK AND ARG_INCLUDE_IN_SUITE)
+  set(out_dir ${LLDB_FRAMEWORK_INSTALL_DIR}/${LLDB_FRAMEWORK_RESOURCE_DIR})
+endif()
 install(TARGETS ${name}
   COMPONENT ${name}
-  RUNTIME DESTINATION bin)
+  RUNTIME DESTINATION ${out_dir})
 if (NOT CMAKE_CONFIGURATION_TYPES)
   add_llvm_install_targets(install-${name}
DEPENDS ${name}


Index: lldb/trunk/cmake/modules/AddLLDB.cmake
===
--- lldb/trunk/cmake/modules/AddLLDB.cmake
+++ lldb/trunk/cmake/modules/AddLLDB.cmake
@@ -111,18 +111,6 @@
 RUNTIME_OUTPUT_DIRECTORY $${resource_dir}
 BUILD_WITH_INSTALL_RPATH On
 INSTALL_RPATH "@loader_path/../../../${resource_dots}${_dots}/${LLDB_FRAMEWORK_INSTALL_DIR}")
-  # For things inside the framework we don't need functional install targets
-  # because CMake copies the resources and headers from the build directory.
-  # But we still need this target to exist in order to use the
-  # LLVM_DISTRIBUTION_COMPONENTS build option. We also need the
-  # install-liblldb target to depend on this tool, so that it gets put into
-  # the Resources directory before the framework is installed.
-  if(ARG_GENERATE_INSTALL)
-add_custom_target(install-${name} DEPENDS ${name})
-add_dependencies(install-liblldb ${name})
-add_custom_target(install-${name}-stripped DEPENDS ${name})
-add_dependencies(install-liblldb-stripped ${name})
-  endif()
 endif()
   endif()
 
@@ -132,10 +120,14 @@
   INSTALL_RPATH "@loader_path/../${LLDB_FRAMEWORK_INSTALL_DIR}")
   endif()
 
-  if(ARG_GENERATE_INSTALL AND NOT (ARG_INCLUDE_IN_SUITE AND LLDB_BUILD_FRAMEWORK ))
+  if(ARG_GENERATE_INSTALL)
+set(out_dir "bin")
+if (LLDB_BUILD_FRAMEWORK AND ARG_INCLUDE_IN_SUITE)
+  set(out_dir ${LLDB_FRAMEWORK_INSTALL_DIR}/${LLDB_FRAMEWORK_RESOURCE_DIR})
+endif()
 install(TARGETS ${name}
   COMPONENT ${name}
-  RUNTIME DESTINATION bin)
+  RUNTIME DESTINATION ${out_dir})
 if (NOT CMAKE_CONFIGURATION_TYPES)
   add_llvm_install_targets(install-${name}
DEPENDS ${name}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49282: [cmake] Add option to skip building lldb-server

2018-07-16 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In https://reviews.llvm.org/D49282#1163853, @xiaobai wrote:

> In https://reviews.llvm.org/D49282#1163517, @labath wrote:
>
> > I think this is fine (though the meaning of SKIP_LLDB_SERVER is subtly 
> > different than SKIP_DEBUGSERVER), but while looking at this I got an idea 
> > for a possible improvement.
>
>
> How is it subtly different? Admittedly I haven't looked in excruciating 
> detail, but I didn't notice any large differences.

The main difference is that for case of SKIP_DEBUGSERVER, we take the system 
debugserver (which is assumed to be always present), and copy it into the build 
folder (I doubt we create install rules for it though). So, the result is that 
you always end up with a functional lldb. However, that is not the case for 
SKIP_LLDB_SERVER.

> 
> 
>> How do you currently convince lldb to use ds2 instead of lldb-server? Do you 
>> just set the LLDB_DEBUGSERVER_PATH env var or do you do something more fancy?
> 
> That's one way we do that. In some situations we launch ds2 and then tell 
> lldb to connect to it. It just depends on what you're debugging.
> 
>> I was thinking we could make the debugserver to use configurable at build 
>> time. That way you could build with 
>> LLDB_EXTERNAL_DEBUGSERVER=path/to/ds2.exe, which would make lldb 
>> automagically know how to launch it, and we would skip building lldb-server 
>> as a side effect.
> 
> I do think this would be nice, but you might have some problems later on with 
> installation+distribution. For example, if for any reason ds2 is in a 
> separate location on a build machine than an end-user's machine, lldb will 
> not have a good time. This could happen if the path to ds2's build tree is 
> used for LLDB_EXTERNAL_DEBUGSERVER instead of the path to its install 
> location. In some cases, the path to its install location isn't necessarily 
> known ahead of time.

This variable was just an example. An absolute path probably wouldn't be 
entirely useful. We would probably need to make it relative to some other 
directory, or install it alongside lldb a'la SKIP_DEBUGSERVER. There are a lot 
of possibilities, so I'm sure we could find something that works. That is, if 
you're interested in such a thing in the first place.

https://reviews.llvm.org/D49282

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49018: Convert a location information from PDB to a DWARF expression

2018-07-16 Thread Stella Stamenova via Phabricator via lldb-commits

stella.stamenova added a comment.

The CHECK-SAME expression on line 10 can no longer find the expected string in 
the output. This is due to an extra `location = DW_OP_addr(000140004114) ,` 
in the output between the two expected strings `CHECK-SAME: scope = global, 
external`, so it looks like it is this change that is causing the failure. This 
can be fixed by updating the CHECK-SAME expression, but I will leave it up to 
you to decide if that is the correct fix. I also noticed that `location = 
DW_OP_addr(000140004114) ,` has an extra space before the comma which other 
values do not. Here is the log:

`location = DW_OP_addr(000140004114) ,`

   # command stderr:
  ##[error]llvm\tools\lldb\lit\SymbolFile\PDB\variables.test(10,13): Error 
GBE9F3850: CHECK-SAME: expected string not found in input
   
3>C:\agent1\_work\15\s\llvm\tools\lldb\lit\SymbolFile\PDB\variables.test(10,13):
 error GBE9F3850: CHECK-SAME: expected string not found in input 
[C:\agent1\_work\15\b\LLVMBuild\tools\lldb\lit\check-lldb-lit.vcxproj]
   
   CHECK-SAME: scope = global, external
   
   ^
   
   :117:58: note: scanning from here
   
   022CA2072050: Variable{0x0002}, name = "g_IntVar", type = 
{0004} 0x022ca20c2390 (int), scope = global, location = 
DW_OP_addr(000140004114) , external
   
^
   
   :117:112: note: possible intended match here
   
   022CA2072050: Variable{0x0002}, name = "g_IntVar", type = 
{0004} 0x022ca20c2390 (int), scope = global, location = 
DW_OP_addr(000140004114) , external




Repository:
  rL LLVM

https://reviews.llvm.org/D49018



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D49202: Restructure the minidump loading path and add early & explicit consistency checks

2018-07-16 Thread Leonard Mosescu via lldb-commits

That sounds reasonable to me. I'll make a note to revisit this (I don't the
the cycles to do it right away but I'm planning a few more changes in the
area soon).

On Mon, Jul 16, 2018 at 10:36 AM, Pavel Labath  wrote:

> Ok, I see what you mean now.
>
> Looking at the other core file plugins (elf, mach-o), it looks like
> they do only very basic verification before the accept the file. The
> pretty much just check the magic numbers, which would be more-or-less
> equivalent to our `MinidumpHeader::Parse` call. As this does not
> require creating a parser object, maybe we could delay the parser
> creation until `LoadCore` gets called (at which point you can easily
> report errors).
>
> This will leave us with a nice MinidumpParser interface.
> ProcessMinidump will still use two-stage initialization, but this is
> nothing new, and this change will make it easier for us to change the
> initialization method of the Process objects in the future.
>
> WDYT?
>
> On Mon, 16 Jul 2018 at 18:16, Leonard Mosescu  wrote:
> >
> > The problem is not returning an error from Minidump::Create() - if that
> was the case this could easily be improved indeed. The two-phase
> initialization is a consequence of the LLDB plugin lookup:
> >
> > 1. Target::CreateProcess() calls Process::FindPlugin()
> > 2. ProcessMinidump::CreateInstance() then has to inspect the core file
> to see if it's a minidump
> > 2b. ... if it is a minidump, we need to create a ProcessMinidump (which
> calls MinidumpParser::Create())
> > 3. once the plugin is selected, Process::LoadCore() is finally called
> and this the earliest we can do minidump-specific error checking
> >
> > Note that at step 2b. we don't have a way to propagate the error since
> we're just doing the plugin lookup (the most we can pass on the lookup to
> the rest of the plugins). We can't easily defer the
> MinidumpParser::Create() as step 2b either since that only morphs into a
> different kind of two-stage initialization (having a ProcessMinidump w/o a
> parser).
> >
> > I agree that it would be nicer with a one step initialization but
> overall changing the LLDB plugin lookup is too intrusive for the relatively
> small benefit. If you have any suggestions I'd love to hear them.
> >
> >
> > On Mon, Jul 16, 2018 at 9:04 AM, Pavel Labath via Phabricator <
> revi...@reviews.llvm.org> wrote:
> >>
> >> labath added a comment.
> >>
> >> I don't agree with the two-stage initialization of the MinidumpParser
> class being introduced here. We deliberately introduced the `Create` static
> function to avoid this. If this `Initialize` function in checking
> invariants which are assumed to be hold by other parser methods, then it
> should be done by the `Create` function. Ideally this would be done before
> even constructing the parser object, but if this is impractical for some
> reason then you can make the `Initialize` function private and call it
> directly from `Create`. This way a user will never be able to see an
> malformed parser object. To make sure you propagate the error, you can
> change the return type of `Create` to `llvm::Expected (the only reason we did not do this back then was that there was no
> precedent for using `Expected` in LLDB, but this is no longer the case).
> >>
> >>
> >> Repository:
> >>   rL LLVM
> >>
> >> https://reviews.llvm.org/D49202
> >>
> >>
> >>
> >
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49282: [cmake] Add option to skip building lldb-server

2018-07-16 Thread Alex Langford via Phabricator via lldb-commits

xiaobai added a comment.

In https://reviews.llvm.org/D49282#1164050, @labath wrote:

> In https://reviews.llvm.org/D49282#1163853, @xiaobai wrote:
>
> > In https://reviews.llvm.org/D49282#1163517, @labath wrote:
> >
> > > I think this is fine (though the meaning of SKIP_LLDB_SERVER is subtly 
> > > different than SKIP_DEBUGSERVER), but while looking at this I got an idea 
> > > for a possible improvement.
> >
> >
> > How is it subtly different? Admittedly I haven't looked in excruciating 
> > detail, but I didn't notice any large differences.
>
>
> The main difference is that for case of SKIP_DEBUGSERVER, we take the system 
> debugserver (which is assumed to be always present), and copy it into the 
> build folder (I doubt we create install rules for it though). So, the result 
> is that you always end up with a functional lldb. However, that is not the 
> case for SKIP_LLDB_SERVER.


I see. fwiw I'm pretty sure you don't need to copy the system's debugserver, 
lldb should be able to detect it. I'm not sure how it will handle a system 
lldb-server though. What we really want is a way to avoid building lldb-server 
since we use our own debug server.

>> 
>> 
>>> How do you currently convince lldb to use ds2 instead of lldb-server? Do 
>>> you just set the LLDB_DEBUGSERVER_PATH env var or do you do something more 
>>> fancy?
>> 
>> That's one way we do that. In some situations we launch ds2 and then tell 
>> lldb to connect to it. It just depends on what you're debugging.
>> 
>>> I was thinking we could make the debugserver to use configurable at build 
>>> time. That way you could build with 
>>> LLDB_EXTERNAL_DEBUGSERVER=path/to/ds2.exe, which would make lldb 
>>> automagically know how to launch it, and we would skip building lldb-server 
>>> as a side effect.
>> 
>> I do think this would be nice, but you might have some problems later on 
>> with installation+distribution. For example, if for any reason ds2 is in a 
>> separate location on a build machine than an end-user's machine, lldb will 
>> not have a good time. This could happen if the path to ds2's build tree is 
>> used for LLDB_EXTERNAL_DEBUGSERVER instead of the path to its install 
>> location. In some cases, the path to its install location isn't necessarily 
>> known ahead of time.
> 
> This variable was just an example. An absolute path probably wouldn't be 
> entirely useful. We would probably need to make it relative to some other 
> directory, or install it alongside lldb a'la SKIP_DEBUGSERVER. There are a 
> lot of possibilities, so I'm sure we could find something that works. That 
> is, if you're interested in such a thing in the first place.

It would be nice if we could make ds2 work in-tree or have LLDB use it nicely 
with CMake flags, but that's not really something interested in working on 
right now.


https://reviews.llvm.org/D49282



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

2018-07-16 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik updated this revision to Diff 155758.
shafik added a comment.

Refactoring test to use lldbinline


https://reviews.llvm.org/D49271

Files:
  lldb.xcodeproj/project.pbxproj
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/Makefile
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/TestDataFormatterLibcxxOptional.py
  
packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/optional/main.cpp
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.cpp
  source/Plugins/Language/CPlusPlus/LibCxx.h
  source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp

Index: source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
===
--- /dev/null
+++ source/Plugins/Language/CPlusPlus/LibCxxOptional.cpp
@@ -0,0 +1,78 @@
+//===-- LibCxxOptional.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "LibCxx.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+class OptionalFrontEnd : public SyntheticChildrenFrontEnd {
+public:
+  OptionalFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) {
+Update();
+  }
+
+  size_t GetIndexOfChildWithName(const ConstString &name) override {
+return formatters::ExtractIndexFromString(name.GetCString());
+  }
+
+  bool MightHaveChildren() override { return true; }
+  bool Update() override;
+  size_t CalculateNumChildren() override { return m_size; }
+  ValueObjectSP GetChildAtIndex(size_t idx) override;
+
+private:
+  size_t m_size = 0;
+  ValueObjectSP m_base_sp;
+};
+} // namespace
+
+bool OptionalFrontEnd::Update() {
+  ValueObjectSP engaged_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true));
+
+  if (!engaged_sp)
+return false;
+
+  m_size = engaged_sp->GetValueAsSigned(0);
+
+  return false;
+}
+
+ValueObjectSP OptionalFrontEnd::GetChildAtIndex(size_t idx) {
+  if (idx >= m_size)
+return ValueObjectSP();
+
+  ValueObjectSP val_sp(
+  m_backend.GetChildMemberWithName(ConstString("__engaged_"), true)
+  ->GetParent()
+  ->GetChildAtIndex(0, true)
+  ->GetChildMemberWithName(ConstString("__val_"), true));
+
+  if (!val_sp)
+return ValueObjectSP();
+
+  CompilerType holder_type = val_sp->GetCompilerType();
+
+  if (!holder_type)
+return ValueObjectSP();
+
+  return val_sp->Clone(ConstString(llvm::formatv("Value").str()));
+}
+
+SyntheticChildrenFrontEnd *
+formatters::LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp) {
+  if (valobj_sp)
+return new OptionalFrontEnd(*valobj_sp);
+  return nullptr;
+}
Index: source/Plugins/Language/CPlusPlus/LibCxx.h
===
--- source/Plugins/Language/CPlusPlus/LibCxx.h
+++ source/Plugins/Language/CPlusPlus/LibCxx.h
@@ -27,6 +27,10 @@
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions &options); // libc++ std::wstring
 
+bool LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream,
+const TypeSummaryOptions &options); // libc++ std::optional<>
+
 bool LibcxxSmartPointerSummaryProvider(
 ValueObject &valobj, Stream &stream,
 const TypeSummaryOptions
@@ -133,6 +137,10 @@
 SyntheticChildrenFrontEnd *LibcxxTupleFrontEndCreator(CXXSyntheticChildren *,
   lldb::ValueObjectSP);
 
+SyntheticChildrenFrontEnd *
+LibcxxOptionalFrontEndCreator(CXXSyntheticChildren *,
+  lldb::ValueObjectSP valobj_sp);
+
 } // namespace formatters
 } // namespace lldb_private
 
Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp
===
--- source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -33,6 +33,26 @@
 using namespace lldb_private;
 using namespace lldb_private::formatters;
 
+bool lldb_private::formatters::LibcxxOptionalSummaryProvider(
+ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
+  ValueObjectSP valobj_sp(valobj.GetNonSyntheticValue());
+  if (!valobj_sp)
+return false;
+
+  ValueObjectSP engaged_sp(
+  valobj_sp->GetChildMemberWithName(ConstString("__engaged_"), true));
+
+  if (!engaged_sp)
+return false;
+
+  llvm::StringRef engaged_as_cstring(
+  engaged_sp->GetValueAsUnsigned(0) == 1 ? "true" : "false");
+
+  stream.Printf("

[Lldb-commits] [PATCH] D49271: Adding libc++ formattors for std::optional

2018-07-16 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik marked 5 inline comments as done and 3 inline comments as done.
shafik added a comment.

@jingham @davide I believe I have addressed all your comments


https://reviews.llvm.org/D49271



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

2018-07-16 Thread Jason Molenda via Phabricator via lldb-commits

jasonmolenda added a subscriber: ramana-nvr.
jasonmolenda added a comment.

That's a good point Pavel.  I tried to write one (below) but I never saw what 
the original failure mode was.  Venkata, can you help to make a test case that 
fails before the patch and works after?  Or explain what bug was being fixed 
exactly?  I could see that the code was wrong from reading it, but I never 
understood how you got to this.

Index: 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestStopPCs.py
==

- 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestStopPCs.py 
  (nonexistent)

+++ 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestStopPCs.py 
(working copy)
@@ -0,0 +1,45 @@
+from __future__ import print_function
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestThreadSelectionBug(GDBRemoteTestBase):
+def test(self):
+class MyResponder(MockGDBServerResponder):
+def haltReason(self):
+return 
"T02thread:1ff0d;threads:1ff0d,2ff0d;thread-pcs:10001bc00,10002bc00;"
+
+def threadStopInfo(self, threadnum):
+if threadnum == 0x1ff0d:
+return 
"T02thread:1ff0d;threads:1ff0d,2ff0d;thread-pcs:10001bc00,10002bc00;0:0,1:00bc010001;"
+if threadnum == 0x2ff0d:
+return 
"T00thread:2ff0d;threads:1ff0d,2ff0d;thread-pcs:10001bc00,10002bc00;0:0,1:00bc020001;"
+
+def qXferRead(self, obj, annex, offset, length):
+if annex == "target.xml":
+return """
+
+  i386:x86-64
+  
+
+
+  
+""", False
+else:
+return None, False
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+if self.TraceOn():
+  self.runCmd("log enable gdb-remote packets")
+process = self.connect(target)
+
+self.assertEqual(process.GetNumThreads(), 2)
+th0 = process.GetThreadAtIndex(0)
+th1 = process.GetThreadAtIndex(1)
+self.assertEqual(th0.GetThreadID(), 0x1ff0d)
+self.assertEqual(th1.GetThreadID(), 0x2ff0d)
+self.assertEqual(th0.GetFrameAtIndex(0).GetPC(), 0x10001bc00)
+self.assertEqual(th1.GetFrameAtIndex(0).GetPC(), 0x10002bc00)

Index: 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
=

- 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
(revision 337215)

+++ 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
  (working copy)
@@ -130,6 +130,8 @@

  return self.QEnableErrorStrings()
  if packet == "?":
  return self.haltReason()

+if packet == "s":
+return self.haltReason()

  if packet[0] == "H":
  return self.selectThread(packet[1], int(packet[2:], 16))
  if packet[0:6] == "qXfer:":

@@ -144,6 +146,9 @@

  return self.vAttach(int(pid, 16))
  if packet[0] == "Z":
  return self.setBreakpoint(packet)

+if packet.startswith("qThreadStopInfo"):
+threadnum = int (packet[15:], 16)
+return self.threadStopInfo(threadnum)

  return self.other(packet)
   
  def interrupt(self):

@@ -204,6 +209,9 @@

  def setBreakpoint(self, packet):
  raise self.UnexpectedPacketException()
   

+def threadStopInfo(self, threadnum):
+return ""
+

  def other(self, packet):
  # empty string means unsupported
  return ""


Repository:
  rL LLVM

https://reviews.llvm.org/D48868



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D48868: [LLDB] In ProcessGDBRemote::UpdateThreadIDList(), the thread PCs should not be cleared after they are updated from the stop reply packet

2018-07-16 Thread Jason Molenda via lldb-commits

That's a good point Pavel.  I tried to write one (below) but I never saw what 
the original failure mode was.  Venkata, can you help to make a test case that 
fails before the patch and works after?  Or explain what bug was being fixed 
exactly?  I could see that the code was wrong from reading it, but I never 
understood how you got to this.


Index: 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestStopPCs.py
===
--- 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestStopPCs.py 
(nonexistent)
+++ 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/TestStopPCs.py 
(working copy)
@@ -0,0 +1,45 @@
+from __future__ import print_function
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+from gdbclientutils import *
+
+
+class TestThreadSelectionBug(GDBRemoteTestBase):
+def test(self):
+class MyResponder(MockGDBServerResponder):
+def haltReason(self):
+return 
"T02thread:1ff0d;threads:1ff0d,2ff0d;thread-pcs:10001bc00,10002bc00;"
+
+def threadStopInfo(self, threadnum):
+if threadnum == 0x1ff0d:
+return 
"T02thread:1ff0d;threads:1ff0d,2ff0d;thread-pcs:10001bc00,10002bc00;0:0,1:00bc010001;"
+if threadnum == 0x2ff0d:
+return 
"T00thread:2ff0d;threads:1ff0d,2ff0d;thread-pcs:10001bc00,10002bc00;0:0,1:00bc020001;"
+
+def qXferRead(self, obj, annex, offset, length):
+if annex == "target.xml":
+return """
+
+  i386:x86-64
+  
+
+
+  
+""", False
+else:
+return None, False
+
+self.server.responder = MyResponder()
+target = self.dbg.CreateTarget('')
+if self.TraceOn():
+  self.runCmd("log enable gdb-remote packets")
+process = self.connect(target)
+
+self.assertEqual(process.GetNumThreads(), 2)
+th0 = process.GetThreadAtIndex(0)
+th1 = process.GetThreadAtIndex(1)
+self.assertEqual(th0.GetThreadID(), 0x1ff0d)
+self.assertEqual(th1.GetThreadID(), 0x2ff0d)
+self.assertEqual(th0.GetFrameAtIndex(0).GetPC(), 0x10001bc00)
+self.assertEqual(th1.GetFrameAtIndex(0).GetPC(), 0x10002bc00)
Index: 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
===
--- 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
  (revision 337215)
+++ 
packages/Python/lldbsuite/test/functionalities/gdb_remote_client/gdbclientutils.py
  (working copy)
@@ -130,6 +130,8 @@
 return self.QEnableErrorStrings()
 if packet == "?":
 return self.haltReason()
+if packet == "s":
+return self.haltReason()
 if packet[0] == "H":
 return self.selectThread(packet[1], int(packet[2:], 16))
 if packet[0:6] == "qXfer:":
@@ -144,6 +146,9 @@
 return self.vAttach(int(pid, 16))
 if packet[0] == "Z":
 return self.setBreakpoint(packet)
+if packet.startswith("qThreadStopInfo"):
+threadnum = int (packet[15:], 16)
+return self.threadStopInfo(threadnum)
 return self.other(packet)
 
 def interrupt(self):
@@ -204,6 +209,9 @@
 def setBreakpoint(self, packet):
 raise self.UnexpectedPacketException()
 
+def threadStopInfo(self, threadnum):
+return ""
+
 def other(self, packet):
 # empty string means unsupported
 return ""


> On Jul 16, 2018, at 3:15 AM, Pavel Labath via Phabricator 
>  wrote:
> 
> labath added a comment.
> 
> Could you also add a test case for this?
> I think it should be possible to test this via the gdb-client 
> (`test/testcases/functionalities/gdb_remote_client/`) test suite. If I 
> understood the previous comments correctly, you'll need to mock a server that 
> sends a `thread-pcs` field, but does not implement a `jThreadsInfo` packet.
> 
> 
> Repository:
>  rL LLVM
> 
> https://reviews.llvm.org/D48868
> 
> 
> 

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49406: Invert dependency between lldb-framework and lldb-suite

2018-07-16 Thread Alex Langford via Phabricator via lldb-commits

xiaobai created this revision.
xiaobai added reviewers: sas, labath.
Herald added a subscriber: mgorny.

Currently, if you build lldb-framework the entire framework doesn't
actually build. In order to build the entire framework, you need to actually
build lldb-suite. This abstraction doesn't feel quite right because
lldb-framework truly does depend on lldb-suite (liblldb + related tools).

In this change I want to invert their dependency. This will mean that lldb and
finish_swig will depend on lldb-framework in a framework build, and lldb-suite
otherwise. Instead of adding conditional logic everywhere to handle this, I
introduce LLDB_SUITE_TARGET to handle it.

I tested this by building lldb with:

  cmake .. -G Ninja -DCMAKE_BUILD_TYPE=Debug -DLLDB_CODESIGN_IDENTITY=""
  -DLLDB_BUILD_FRAMEWORK=1 -DLLDB_USE_SYSTEM_SIX=1 -DCMAKE_INSTALL_PREFIX=""

and

  ninja lldb


https://reviews.llvm.org/D49406

Files:
  CMakeLists.txt
  cmake/modules/LLDBFramework.cmake
  source/API/CMakeLists.txt
  tools/driver/CMakeLists.txt

Index: tools/driver/CMakeLists.txt
===
--- tools/driver/CMakeLists.txt
+++ tools/driver/CMakeLists.txt
@@ -24,4 +24,4 @@
   add_definitions( -DIMPORT_LIBLLDB )
 endif()
 
-add_dependencies(lldb lldb-suite)
+add_dependencies(lldb ${LLDB_SUITE_TARGET})
Index: source/API/CMakeLists.txt
===
--- source/API/CMakeLists.txt
+++ source/API/CMakeLists.txt
@@ -91,6 +91,8 @@
 Support
   )
 
+add_dependencies(lldb-suite liblldb)
+
 if (MSVC)
   set_property(SOURCE ${LLDB_WRAP_PYTHON} APPEND_STRING PROPERTY COMPILE_FLAGS " /W0")
 else()
@@ -108,10 +110,20 @@
 PROPERTY COMPILE_FLAGS " -Wno-sequence-point -Wno-cast-qual")
 endif ()
 
-set_target_properties(liblldb
-  PROPERTIES
-  VERSION ${LLDB_VERSION}
-  )
+if (LLDB_BUILD_FRAMEWORK)
+  set_target_properties(liblldb
+PROPERTIES
+OUTPUT_NAME LLDB
+FRAMEWORK ON
+FRAMEWORK_VERSION ${LLDB_FRAMEWORK_VERSION}
+MACOSX_FRAMEWORK_INFO_PLIST ${LLDB_SOURCE_DIR}/resources/LLDB-Info.plist
+LIBRARY_OUTPUT_DIRECTORY ${LLDB_FRAMEWORK_DIR})
+else()
+  set_target_properties(liblldb
+PROPERTIES
+VERSION ${LLDB_VERSION}
+OUTPUT_NAME lldb)
+endif()
 
 if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
   if (NOT LLDB_EXPORT_ALL_SYMBOLS)
@@ -134,11 +146,6 @@
   if (MSVC AND NOT LLDB_DISABLE_PYTHON)
 target_link_libraries(liblldb PRIVATE ${PYTHON_LIBRARY})
   endif()
-else()
-  set_target_properties(liblldb
-PROPERTIES
-OUTPUT_NAME lldb
-)
 endif()
 
 if (LLDB_WRAP_PYTHON)
Index: cmake/modules/LLDBFramework.cmake
===
--- cmake/modules/LLDBFramework.cmake
+++ cmake/modules/LLDBFramework.cmake
@@ -33,13 +33,8 @@
 endif()
 
 set_target_properties(liblldb PROPERTIES
-  OUTPUT_NAME LLDB
-  FRAMEWORK On
-  FRAMEWORK_VERSION ${LLDB_FRAMEWORK_VERSION}
-  MACOSX_FRAMEWORK_INFO_PLIST ${LLDB_SOURCE_DIR}/resources/LLDB-Info.plist
-  LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${LLDB_FRAMEWORK_INSTALL_DIR}
   PUBLIC_HEADER "${framework_headers}")
 
 add_dependencies(lldb-framework
-  liblldb
-  lldb-framework-headers)
+  lldb-framework-headers
+  lldb-suite)
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -40,6 +40,7 @@
 # lldb-suite is a dummy target that encompasses all the necessary tools and
 # libraries for building a fully-functioning liblldb.
 add_custom_target(lldb-suite)
+set(LLDB_SUITE_TARGET lldb-suite)
 
 option(LLDB_BUILD_FRAMEWORK "Build the Darwin LLDB.framework" Off)
 if(LLDB_BUILD_FRAMEWORK)
@@ -55,6 +56,7 @@
   set(PRODUCT_NAME "LLDB")
   set(EXECUTABLE_NAME "LLDB")
   set(CURRENT_PROJECT_VERSION "360.99.0")
+  set(LLDB_SUITE_TARGET lldb-framework)
 
   set(LLDB_FRAMEWORK_DIR
 ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${LLDB_FRAMEWORK_INSTALL_DIR})
@@ -163,9 +165,7 @@
 if (LLDB_BUILD_FRAMEWORK)
   add_custom_target(lldb-framework)
   include(LLDBFramework)
-  add_dependencies(lldb-suite lldb-framework)
 endif()
-add_dependencies(lldb-suite liblldb)
 
 if (NOT LLDB_DISABLE_PYTHON)
 # Add a Post-Build Event to copy over Python files and create the symlink
@@ -187,7 +187,7 @@
 COMMENT "Python script sym-linking LLDB Python API")
 
 # We depend on liblldb and lldb-argdumper being built before we can do this step.
-add_dependencies(finish_swig lldb-suite)
+add_dependencies(finish_swig ${LLDB_SUITE_TARGET})
 
 # If we build the readline module, we depend on that happening
 # first.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D49406: Invert dependency between lldb-framework and lldb-suite

2018-07-16 Thread Alex Langford via Phabricator via lldb-commits

xiaobai updated this revision to Diff 155790.
xiaobai added a comment.

Accidentally merged the contents of two commits into one. Removing the contents 
of one of the commits from this one.


https://reviews.llvm.org/D49406

Files:
  CMakeLists.txt
  cmake/modules/LLDBFramework.cmake
  source/API/CMakeLists.txt
  tools/driver/CMakeLists.txt


Index: tools/driver/CMakeLists.txt
===
--- tools/driver/CMakeLists.txt
+++ tools/driver/CMakeLists.txt
@@ -24,4 +24,4 @@
   add_definitions( -DIMPORT_LIBLLDB )
 endif()
 
-add_dependencies(lldb lldb-suite)
+add_dependencies(lldb ${LLDB_SUITE_TARGET})
Index: source/API/CMakeLists.txt
===
--- source/API/CMakeLists.txt
+++ source/API/CMakeLists.txt
@@ -91,6 +91,8 @@
 Support
   )
 
+add_dependencies(lldb-suite liblldb)
+
 if (MSVC)
   set_property(SOURCE ${LLDB_WRAP_PYTHON} APPEND_STRING PROPERTY COMPILE_FLAGS 
" /W0")
 else()
@@ -111,7 +113,7 @@
 set_target_properties(liblldb
   PROPERTIES
   VERSION ${LLDB_VERSION}
-  )
+)
 
 if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
   if (NOT LLDB_EXPORT_ALL_SYMBOLS)
@@ -138,7 +140,7 @@
   set_target_properties(liblldb
 PROPERTIES
 OUTPUT_NAME lldb
-)
+  )
 endif()
 
 if (LLDB_WRAP_PYTHON)
Index: cmake/modules/LLDBFramework.cmake
===
--- cmake/modules/LLDBFramework.cmake
+++ cmake/modules/LLDBFramework.cmake
@@ -41,5 +41,5 @@
   PUBLIC_HEADER "${framework_headers}")
 
 add_dependencies(lldb-framework
-  liblldb
-  lldb-framework-headers)
+  lldb-framework-headers
+  lldb-suite)
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -40,6 +40,7 @@
 # lldb-suite is a dummy target that encompasses all the necessary tools and
 # libraries for building a fully-functioning liblldb.
 add_custom_target(lldb-suite)
+set(LLDB_SUITE_TARGET lldb-suite)
 
 option(LLDB_BUILD_FRAMEWORK "Build the Darwin LLDB.framework" Off)
 if(LLDB_BUILD_FRAMEWORK)
@@ -55,6 +56,7 @@
   set(PRODUCT_NAME "LLDB")
   set(EXECUTABLE_NAME "LLDB")
   set(CURRENT_PROJECT_VERSION "360.99.0")
+  set(LLDB_SUITE_TARGET lldb-framework)
 
   set(LLDB_FRAMEWORK_DIR
 ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${LLDB_FRAMEWORK_INSTALL_DIR})
@@ -163,9 +165,7 @@
 if (LLDB_BUILD_FRAMEWORK)
   add_custom_target(lldb-framework)
   include(LLDBFramework)
-  add_dependencies(lldb-suite lldb-framework)
 endif()
-add_dependencies(lldb-suite liblldb)
 
 if (NOT LLDB_DISABLE_PYTHON)
 # Add a Post-Build Event to copy over Python files and create the symlink
@@ -187,7 +187,7 @@
 COMMENT "Python script sym-linking LLDB Python API")
 
 # We depend on liblldb and lldb-argdumper being built before we can do 
this step.
-add_dependencies(finish_swig lldb-suite)
+add_dependencies(finish_swig ${LLDB_SUITE_TARGET})
 
 # If we build the readline module, we depend on that happening
 # first.


Index: tools/driver/CMakeLists.txt
===
--- tools/driver/CMakeLists.txt
+++ tools/driver/CMakeLists.txt
@@ -24,4 +24,4 @@
   add_definitions( -DIMPORT_LIBLLDB )
 endif()
 
-add_dependencies(lldb lldb-suite)
+add_dependencies(lldb ${LLDB_SUITE_TARGET})
Index: source/API/CMakeLists.txt
===
--- source/API/CMakeLists.txt
+++ source/API/CMakeLists.txt
@@ -91,6 +91,8 @@
 Support
   )
 
+add_dependencies(lldb-suite liblldb)
+
 if (MSVC)
   set_property(SOURCE ${LLDB_WRAP_PYTHON} APPEND_STRING PROPERTY COMPILE_FLAGS " /W0")
 else()
@@ -111,7 +113,7 @@
 set_target_properties(liblldb
   PROPERTIES
   VERSION ${LLDB_VERSION}
-  )
+)
 
 if (NOT CMAKE_SYSTEM_NAME MATCHES "Windows")
   if (NOT LLDB_EXPORT_ALL_SYMBOLS)
@@ -138,7 +140,7 @@
   set_target_properties(liblldb
 PROPERTIES
 OUTPUT_NAME lldb
-)
+  )
 endif()
 
 if (LLDB_WRAP_PYTHON)
Index: cmake/modules/LLDBFramework.cmake
===
--- cmake/modules/LLDBFramework.cmake
+++ cmake/modules/LLDBFramework.cmake
@@ -41,5 +41,5 @@
   PUBLIC_HEADER "${framework_headers}")
 
 add_dependencies(lldb-framework
-  liblldb
-  lldb-framework-headers)
+  lldb-framework-headers
+  lldb-suite)
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -40,6 +40,7 @@
 # lldb-suite is a dummy target that encompasses all the necessary tools and
 # libraries for building a fully-functioning liblldb.
 add_custom_target(lldb-suite)
+set(LLDB_SUITE_TARGET lldb-suite)
 
 option(LLDB_BUILD_FRAMEWORK "Build the Darwin LLDB.framework" Off)
 if(LLDB_BUILD_FRAMEWORK)
@@ -55,6 +56,7 @@
   set(PRODUCT_NAME "LLDB")
   set(EXECUTABLE_NAME "LLDB")
   set(CURRENT_PROJECT_VERSION "360.99.0")
+  set(LLDB_SUITE_TARGET lldb-fra

[Lldb-commits] [PATCH] D49410: [PDB] Parse UDT symbols and pointers to members

2018-07-16 Thread Aaron Smith via Phabricator via lldb-commits

asmith created this revision.
asmith added reviewers: lldb-commits, aleksandr.urakov, rnk, zturner.
Herald added a subscriber: llvm-commits.

This is an alternative to https://reviews.llvm.org/D49368


Repository:
  rL LLVM

https://reviews.llvm.org/D49410

Files:
  lit/SymbolFile/PDB/class-layout.test
  lit/SymbolFile/PDB/pointers.test
  source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
  source/Plugins/SymbolFile/PDB/PDBASTParser.h
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -45,7 +45,7 @@
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeTypedef.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeUDT.h"
 
-#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h" // For IsCPPMangledName
 #include "Plugins/SymbolFile/PDB/PDBASTParser.h"
 #include "Plugins/SymbolFile/PDB/PDBLocationToDWARFExpression.h"
 
@@ -557,9 +557,37 @@
   if (pdb_type == nullptr)
 return nullptr;
 
+  // Parse base classes and nested UDTs first.
+  switch (pdb_type->getSymTag()) {
+  case PDB_SymType::UDT:
+  case PDB_SymType::BaseClass: {
+if (auto base_classes =
+pdb_type->findAllChildren()) {
+  while (auto base_class = base_classes->getNext())
+ResolveTypeUID(base_class->getSymIndexId());
+}
+if (pdb_type->getRawSymbol().hasNestedTypes()) {
+  if (auto nested_udts = pdb_type->findAllChildren()) {
+while (auto nested = nested_udts->getNext())
+  ResolveTypeUID(nested->getSymIndexId());
+  }
+}
+  } break;
+  default:
+break;
+  }
+
   lldb::TypeSP result = pdb->CreateLLDBTypeFromPDBType(*pdb_type);
   if (result) {
 m_types.insert(std::make_pair(type_uid, result));
+
+// Complete the type for UDT & BaseClass symbol immediately. BaseClass is
+// parsed by its type which might have been completed before. Make sure we
+// only do this once.
+if (result->GetID() == type_uid) {
+  pdb->CompleteRecordTypeForPDBSymbol(*pdb_type, result);
+}
+
 auto type_list = GetTypeList();
 if (type_list)
   type_list->Insert(result);
Index: source/Plugins/SymbolFile/PDB/PDBASTParser.h
===
--- source/Plugins/SymbolFile/PDB/PDBASTParser.h
+++ source/Plugins/SymbolFile/PDB/PDBASTParser.h
@@ -31,6 +31,7 @@
 class PDBSymbol;
 class PDBSymbolData;
 class PDBSymbolTypeBuiltin;
+class PDBSymbolTypeUDT;
 } // namespace pdb
 } // namespace llvm
 
@@ -41,10 +42,17 @@
 
   lldb::TypeSP CreateLLDBTypeFromPDBType(const llvm::pdb::PDBSymbol &type);
 
+  bool CompleteRecordTypeForPDBSymbol(const llvm::pdb::PDBSymbol &pdb_symbol,
+  lldb::TypeSP type_sp);
+
 private:
   bool AddEnumValue(lldb_private::CompilerType enum_type,
 const llvm::pdb::PDBSymbolData &data) const;
 
+  bool
+  AddMembersToRecordType(const lldb_private::CompilerType &udt_compiler_type,
+ const llvm::pdb::PDBSymbolTypeUDT &pdb_udt);
+
   lldb_private::ClangASTContext &m_ast;
   lldb_private::ClangASTImporter m_ast_importer;
 };
Index: source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
===
--- source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
+++ source/Plugins/SymbolFile/PDB/PDBASTParser.cpp
@@ -14,6 +14,7 @@
 #include "clang/AST/DeclCXX.h"
 
 #include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Symbol/ClangExternalASTSourceCommon.h" // For ClangASTMetadata
 #include "lldb/Symbol/ClangUtil.h"
 #include "lldb/Symbol/Declaration.h"
 #include "lldb/Symbol/SymbolFile.h"
@@ -175,6 +176,28 @@
   return compiler_type.GetTypeName();
 }
 
+AccessType TranslateMemberAccess(PDB_MemberAccess access) {
+  if (access == PDB_MemberAccess::Public)
+return lldb::eAccessPublic;
+  if (access == PDB_MemberAccess::Private)
+return lldb::eAccessPrivate;
+  if (access == PDB_MemberAccess::Protected)
+return lldb::eAccessProtected;
+  return lldb::eAccessNone;
+}
+
+AccessType GetDefaultAccessibilityForUdtKind(PDB_UdtType udt_kind) {
+  if (udt_kind == PDB_UdtType::Class)
+return lldb::eAccessPrivate;
+  if (udt_kind == PDB_UdtType::Struct)
+return lldb::eAccessPublic;
+  if (udt_kind == PDB_UdtType::Union)
+return lldb::eAccessPublic;
+  if (udt_kind == PDB_UdtType::Interface)
+return lldb::eAccessPrivate;
+  return lldb::eAccessNone;
+}
+
 bool GetDeclarationForSymbol(const PDBSymbol &symbol, Declaration &decl) {
   auto &raw_sym = symbol.getRawSymbol();
   auto first_line_up = raw_sym.getSrcLineOnTypeDefn();
@@ -216,29 +239,84 @@
   Declaration decl;
 
   switch (type.getSymTag()) {
+  case PDB_SymType::BaseClass: {
+auto ty =
+m_ast.GetSymbolFile()->ResolveTypeUID(type.getRawSymbol().getTyp

[Lldb-commits] [PATCH] D49411: Move from StringRef to std::string in the ScriptInterpreter API

2018-07-16 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor created this revision.
teemperor added a reviewer: dblaikie.

After https://reviews.llvm.org/D49309 it became clear that we always need a 
null-terminated string (for
the Python API), so we might as well change the API to accept an std::string&
instead of taking a StringRef and then always allocating a new std::string.


https://reviews.llvm.org/D49411

Files:
  include/lldb/Interpreter/ScriptInterpreter.h
  source/Plugins/ScriptInterpreter/None/ScriptInterpreterNone.cpp
  source/Plugins/ScriptInterpreter/None/ScriptInterpreterNone.h
  source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h

Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
===
--- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
+++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.h
@@ -151,13 +151,13 @@
   bool Interrupt() override;
 
   bool ExecuteOneLine(
-  llvm::StringRef command, CommandReturnObject *result,
+  const std::string &command, CommandReturnObject *result,
   const ExecuteScriptOptions &options = ExecuteScriptOptions()) override;
 
   void ExecuteInterpreterLoop() override;
 
   bool ExecuteOneLineWithReturn(
-  llvm::StringRef in_string,
+  const std::string &in_string,
   ScriptInterpreter::ScriptReturnType return_type, void *ret_value,
   const ExecuteScriptOptions &options = ExecuteScriptOptions()) override;
 
Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -751,9 +751,8 @@
 }
 
 bool ScriptInterpreterPython::ExecuteOneLine(
-llvm::StringRef command, CommandReturnObject *result,
+const std::string &command, CommandReturnObject *result,
 const ExecuteScriptOptions &options) {
-  std::string command_str = command.str();
 
   if (!m_valid_session)
 return false;
@@ -857,7 +856,7 @@
   if (PyCallable_Check(m_run_one_line_function.get())) {
 PythonObject pargs(
 PyRefType::Owned,
-Py_BuildValue("(Os)", session_dict.get(), command_str.c_str()));
+Py_BuildValue("(Os)", session_dict.get(), command.c_str()));
 if (pargs.IsValid()) {
   PythonObject return_value(
   PyRefType::Owned,
@@ -898,7 +897,7 @@
 // The one-liner failed.  Append the error message.
 if (result) {
   result->AppendErrorWithFormat(
-  "python failed attempting to evaluate '%s'\n", command_str.c_str());
+  "python failed attempting to evaluate '%s'\n", command.c_str());
 }
 return false;
   }
@@ -1024,8 +1023,9 @@
   return false;
 }
 bool ScriptInterpreterPython::ExecuteOneLineWithReturn(
-llvm::StringRef in_string, ScriptInterpreter::ScriptReturnType return_type,
-void *ret_value, const ExecuteScriptOptions &options) {
+const std::string &in_string,
+ScriptInterpreter::ScriptReturnType return_type, void *ret_value,
+const ExecuteScriptOptions &options) {
 
   Locker locker(this, ScriptInterpreterPython::Locker::AcquireLock |
   ScriptInterpreterPython::Locker::InitSession |
@@ -1059,19 +1059,18 @@
   if (py_error.IsValid())
 PyErr_Clear();
 
-  std::string as_string = in_string.str();
   { // scope for PythonInputReaderManager
 // PythonInputReaderManager py_input(options.GetEnableIO() ? this : NULL);
 py_return.Reset(PyRefType::Owned,
-PyRun_String(as_string.c_str(), Py_eval_input,
+PyRun_String(in_string.c_str(), Py_eval_input,
  globals.get(), locals.get()));
 if (!py_return.IsValid()) {
   py_error.Reset(PyRefType::Borrowed, PyErr_Occurred());
   if (py_error.IsValid())
 PyErr_Clear();
 
   py_return.Reset(PyRefType::Owned,
-  PyRun_String(as_string.c_str(), Py_single_input,
+  PyRun_String(in_string.c_str(), Py_single_input,
globals.get(), locals.get()));
 }
   }
@@ -2892,7 +2891,7 @@
   // ExecuteOneLineWithReturn returns successfully
 
   if (ExecuteOneLineWithReturn(
-  command.c_str(), ScriptInterpreter::eScriptReturnTypeCharStrOrNone,
+  command, ScriptInterpreter::eScriptReturnTypeCharStrOrNone,
   &result_ptr,
   ScriptInterpreter::ExecuteScriptOptions().SetEnableIO(false))) {
 if (result_ptr)
Index: source/Plugins/ScriptInterpreter/None/ScriptInterpreterNone.h
===
--- source/Plugins/ScriptInterpreter/None/ScriptInterpreterNone.h
+++ source/Plugins/ScriptInterpre

[Lldb-commits] [PATCH] D49411: Move from StringRef to std::string in the ScriptInterpreter API

2018-07-16 Thread David Blaikie via Phabricator via lldb-commits

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

Seems good to me - though I haven't looked too closely/don't know this code 
terribly well (so you're welcome to wait if you know someone else more 
knowledgable might take a look too - or if you're fairly confident, commit & 
they can always follow up in post-commit)


https://reviews.llvm.org/D49411



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

44 matches

Mail list logo