[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange

2018-07-24 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337873: Add unit tests for VMRange (authored by teemperor, 
committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49415?vs=157172=157173#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D49415

Files:
  lldb/trunk/unittests/Utility/CMakeLists.txt
  lldb/trunk/unittests/Utility/VMRangeTest.cpp

Index: lldb/trunk/unittests/Utility/VMRangeTest.cpp
===
--- lldb/trunk/unittests/Utility/VMRangeTest.cpp
+++ lldb/trunk/unittests/Utility/VMRangeTest.cpp
@@ -0,0 +1,152 @@
+//===-- VMRangeTest.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 
+
+#include "lldb/Utility/VMRange.h"
+
+using namespace lldb_private;
+
+namespace lldb_private {
+void PrintTo(const VMRange , std::ostream *os) {
+  (*os) << "VMRange(" << v.GetBaseAddress() << ", " << v.GetEndAddress() << ")";
+}
+} // namespace lldb_private
+
+TEST(VMRange, IsValid) {
+  VMRange range;
+  EXPECT_FALSE(range.IsValid());
+
+  range.Reset(0x1, 0x100);
+  EXPECT_TRUE(range.IsValid());
+
+  range.Reset(0x1, 0x1);
+  EXPECT_FALSE(range.IsValid());
+}
+
+TEST(VMRange, Clear) {
+  VMRange range(0x100, 0x200);
+  EXPECT_NE(VMRange(), range);
+  range.Clear();
+  EXPECT_EQ(VMRange(), range);
+}
+
+TEST(VMRange, Comparison) {
+  VMRange range1(0x100, 0x200);
+  VMRange range2(0x100, 0x200);
+  EXPECT_EQ(range1, range2);
+
+  EXPECT_NE(VMRange(0x100, 0x1ff), range1);
+  EXPECT_NE(VMRange(0x100, 0x201), range1);
+  EXPECT_NE(VMRange(0x0ff, 0x200), range1);
+  EXPECT_NE(VMRange(0x101, 0x200), range1);
+
+  range2.Clear();
+  EXPECT_NE(range1, range2);
+}
+
+TEST(VMRange, Reset) {
+  VMRange range(0x100, 0x200);
+  EXPECT_FALSE(VMRange(0x200, 0x200) == range);
+  range.Reset(0x200, 0x200);
+  EXPECT_TRUE(VMRange(0x200, 0x200) == range);
+}
+
+TEST(VMRange, SetEndAddress) {
+  VMRange range(0x100, 0x200);
+
+  range.SetEndAddress(0xFF);
+  EXPECT_EQ(0U, range.GetByteSize());
+  EXPECT_FALSE(range.IsValid());
+
+  range.SetEndAddress(0x101);
+  EXPECT_EQ(1U, range.GetByteSize());
+  EXPECT_TRUE(range.IsValid());
+}
+
+TEST(VMRange, ContainsAddr) {
+  VMRange range(0x100, 0x200);
+
+  EXPECT_FALSE(range.Contains(0x00));
+  EXPECT_FALSE(range.Contains(0xFF));
+  EXPECT_TRUE(range.Contains(0x100));
+  EXPECT_TRUE(range.Contains(0x101));
+  EXPECT_TRUE(range.Contains(0x1FF));
+  EXPECT_FALSE(range.Contains(0x200));
+  EXPECT_FALSE(range.Contains(0x201));
+  EXPECT_FALSE(range.Contains(0xFFF));
+  EXPECT_FALSE(range.Contains(std::numeric_limits::max()));
+}
+
+TEST(VMRange, ContainsRange) {
+  VMRange range(0x100, 0x200);
+
+  EXPECT_FALSE(range.Contains(VMRange(0x0, 0x0)));
+
+  EXPECT_FALSE(range.Contains(VMRange(0x0, 0x100)));
+  EXPECT_FALSE(range.Contains(VMRange(0x0, 0x101)));
+  EXPECT_TRUE(range.Contains(VMRange(0x100, 0x105)));
+  EXPECT_TRUE(range.Contains(VMRange(0x101, 0x105)));
+  EXPECT_TRUE(range.Contains(VMRange(0x100, 0x1FF)));
+  EXPECT_TRUE(range.Contains(VMRange(0x105, 0x200)));
+  EXPECT_FALSE(range.Contains(VMRange(0x105, 0x201)));
+  EXPECT_FALSE(range.Contains(VMRange(0x200, 0x201)));
+  EXPECT_TRUE(range.Contains(VMRange(0x100, 0x200)));
+  EXPECT_FALSE(
+  range.Contains(VMRange(0x105, std::numeric_limits::max(;
+
+  // Empty range.
+  EXPECT_TRUE(range.Contains(VMRange(0x100, 0x100)));
+
+  range.Clear();
+  EXPECT_FALSE(range.Contains(VMRange(0x0, 0x0)));
+}
+
+TEST(VMRange, Ordering) {
+  VMRange range1(0x44, 0x200);
+  VMRange range2(0x100, 0x1FF);
+  VMRange range3(0x100, 0x200);
+
+  EXPECT_LE(range1, range1);
+  EXPECT_GE(range1, range1);
+
+  EXPECT_LT(range1, range2);
+  EXPECT_LT(range2, range3);
+
+  EXPECT_GT(range2, range1);
+  EXPECT_GT(range3, range2);
+
+  // Ensure that < and > are always false when comparing ranges with themselves.
+  EXPECT_FALSE(range1 < range1);
+  EXPECT_FALSE(range2 < range2);
+  EXPECT_FALSE(range3 < range3);
+
+  EXPECT_FALSE(range1 > range1);
+  EXPECT_FALSE(range2 > range2);
+  EXPECT_FALSE(range3 > range3);
+}
+
+TEST(VMRange, CollectionContains) {
+  VMRange::collection collection = {VMRange(0x100, 0x105),
+VMRange(0x108, 0x110)};
+
+  EXPECT_FALSE(VMRange::ContainsValue(collection, 0xFF));
+  EXPECT_TRUE(VMRange::ContainsValue(collection, 0x100));
+  EXPECT_FALSE(VMRange::ContainsValue(collection, 0x105));
+  EXPECT_TRUE(VMRange::ContainsValue(collection, 0x109));
+
+  EXPECT_TRUE(VMRange::ContainsRange(collection, VMRange(0x100, 0x104)));
+  

[Lldb-commits] [lldb] r337873 - Add unit tests for VMRange

2018-07-24 Thread Raphael Isemann via lldb-commits
Author: teemperor
Date: Tue Jul 24 16:52:39 2018
New Revision: 337873

URL: http://llvm.org/viewvc/llvm-project?rev=337873=rev
Log:
Add unit tests for VMRange

Subscribers: clayborg, labath, mgorny, lldb-commits

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

Added:
lldb/trunk/unittests/Utility/VMRangeTest.cpp
Modified:
lldb/trunk/unittests/Utility/CMakeLists.txt

Modified: lldb/trunk/unittests/Utility/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/CMakeLists.txt?rev=337873=337872=337873=diff
==
--- lldb/trunk/unittests/Utility/CMakeLists.txt (original)
+++ lldb/trunk/unittests/Utility/CMakeLists.txt Tue Jul 24 16:52:39 2018
@@ -22,6 +22,7 @@ add_lldb_unittest(UtilityTests
   UriParserTest.cpp
   UUIDTest.cpp
   VASprintfTest.cpp
+  VMRangeTest.cpp
 
   LINK_LIBS
   lldbUtility

Added: lldb/trunk/unittests/Utility/VMRangeTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/VMRangeTest.cpp?rev=337873=auto
==
--- lldb/trunk/unittests/Utility/VMRangeTest.cpp (added)
+++ lldb/trunk/unittests/Utility/VMRangeTest.cpp Tue Jul 24 16:52:39 2018
@@ -0,0 +1,152 @@
+//===-- VMRangeTest.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 
+
+#include "lldb/Utility/VMRange.h"
+
+using namespace lldb_private;
+
+namespace lldb_private {
+void PrintTo(const VMRange , std::ostream *os) {
+  (*os) << "VMRange(" << v.GetBaseAddress() << ", " << v.GetEndAddress() << 
")";
+}
+} // namespace lldb_private
+
+TEST(VMRange, IsValid) {
+  VMRange range;
+  EXPECT_FALSE(range.IsValid());
+
+  range.Reset(0x1, 0x100);
+  EXPECT_TRUE(range.IsValid());
+
+  range.Reset(0x1, 0x1);
+  EXPECT_FALSE(range.IsValid());
+}
+
+TEST(VMRange, Clear) {
+  VMRange range(0x100, 0x200);
+  EXPECT_NE(VMRange(), range);
+  range.Clear();
+  EXPECT_EQ(VMRange(), range);
+}
+
+TEST(VMRange, Comparison) {
+  VMRange range1(0x100, 0x200);
+  VMRange range2(0x100, 0x200);
+  EXPECT_EQ(range1, range2);
+
+  EXPECT_NE(VMRange(0x100, 0x1ff), range1);
+  EXPECT_NE(VMRange(0x100, 0x201), range1);
+  EXPECT_NE(VMRange(0x0ff, 0x200), range1);
+  EXPECT_NE(VMRange(0x101, 0x200), range1);
+
+  range2.Clear();
+  EXPECT_NE(range1, range2);
+}
+
+TEST(VMRange, Reset) {
+  VMRange range(0x100, 0x200);
+  EXPECT_FALSE(VMRange(0x200, 0x200) == range);
+  range.Reset(0x200, 0x200);
+  EXPECT_TRUE(VMRange(0x200, 0x200) == range);
+}
+
+TEST(VMRange, SetEndAddress) {
+  VMRange range(0x100, 0x200);
+
+  range.SetEndAddress(0xFF);
+  EXPECT_EQ(0U, range.GetByteSize());
+  EXPECT_FALSE(range.IsValid());
+
+  range.SetEndAddress(0x101);
+  EXPECT_EQ(1U, range.GetByteSize());
+  EXPECT_TRUE(range.IsValid());
+}
+
+TEST(VMRange, ContainsAddr) {
+  VMRange range(0x100, 0x200);
+
+  EXPECT_FALSE(range.Contains(0x00));
+  EXPECT_FALSE(range.Contains(0xFF));
+  EXPECT_TRUE(range.Contains(0x100));
+  EXPECT_TRUE(range.Contains(0x101));
+  EXPECT_TRUE(range.Contains(0x1FF));
+  EXPECT_FALSE(range.Contains(0x200));
+  EXPECT_FALSE(range.Contains(0x201));
+  EXPECT_FALSE(range.Contains(0xFFF));
+  EXPECT_FALSE(range.Contains(std::numeric_limits::max()));
+}
+
+TEST(VMRange, ContainsRange) {
+  VMRange range(0x100, 0x200);
+
+  EXPECT_FALSE(range.Contains(VMRange(0x0, 0x0)));
+
+  EXPECT_FALSE(range.Contains(VMRange(0x0, 0x100)));
+  EXPECT_FALSE(range.Contains(VMRange(0x0, 0x101)));
+  EXPECT_TRUE(range.Contains(VMRange(0x100, 0x105)));
+  EXPECT_TRUE(range.Contains(VMRange(0x101, 0x105)));
+  EXPECT_TRUE(range.Contains(VMRange(0x100, 0x1FF)));
+  EXPECT_TRUE(range.Contains(VMRange(0x105, 0x200)));
+  EXPECT_FALSE(range.Contains(VMRange(0x105, 0x201)));
+  EXPECT_FALSE(range.Contains(VMRange(0x200, 0x201)));
+  EXPECT_TRUE(range.Contains(VMRange(0x100, 0x200)));
+  EXPECT_FALSE(
+  range.Contains(VMRange(0x105, 
std::numeric_limits::max(;
+
+  // Empty range.
+  EXPECT_TRUE(range.Contains(VMRange(0x100, 0x100)));
+
+  range.Clear();
+  EXPECT_FALSE(range.Contains(VMRange(0x0, 0x0)));
+}
+
+TEST(VMRange, Ordering) {
+  VMRange range1(0x44, 0x200);
+  VMRange range2(0x100, 0x1FF);
+  VMRange range3(0x100, 0x200);
+
+  EXPECT_LE(range1, range1);
+  EXPECT_GE(range1, range1);
+
+  EXPECT_LT(range1, range2);
+  EXPECT_LT(range2, range3);
+
+  EXPECT_GT(range2, range1);
+  EXPECT_GT(range3, range2);
+
+  // Ensure that < and > are always false when comparing ranges with 
themselves.
+  EXPECT_FALSE(range1 < range1);
+  EXPECT_FALSE(range2 < range2);
+  EXPECT_FALSE(range3 < range3);
+
+  EXPECT_FALSE(range1 > range1);
+  

[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange

2018-07-24 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

Merging this in as Davide suggested.


https://reviews.llvm.org/D49415



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


[Lldb-commits] [PATCH] D49415: Add unit tests for VMRange

2018-07-24 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor updated this revision to Diff 157172.
teemperor added a comment.

- Applied Greg's requested changes (thank you very much).


https://reviews.llvm.org/D49415

Files:
  unittests/Utility/CMakeLists.txt
  unittests/Utility/VMRangeTest.cpp

Index: unittests/Utility/VMRangeTest.cpp
===
--- /dev/null
+++ unittests/Utility/VMRangeTest.cpp
@@ -0,0 +1,152 @@
+//===-- VMRangeTest.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 
+
+#include "lldb/Utility/VMRange.h"
+
+using namespace lldb_private;
+
+namespace lldb_private {
+void PrintTo(const VMRange , std::ostream *os) {
+  (*os) << "VMRange(" << v.GetBaseAddress() << ", " << v.GetEndAddress() << ")";
+}
+} // namespace lldb_private
+
+TEST(VMRange, IsValid) {
+  VMRange range;
+  EXPECT_FALSE(range.IsValid());
+
+  range.Reset(0x1, 0x100);
+  EXPECT_TRUE(range.IsValid());
+
+  range.Reset(0x1, 0x1);
+  EXPECT_FALSE(range.IsValid());
+}
+
+TEST(VMRange, Clear) {
+  VMRange range(0x100, 0x200);
+  EXPECT_NE(VMRange(), range);
+  range.Clear();
+  EXPECT_EQ(VMRange(), range);
+}
+
+TEST(VMRange, Comparison) {
+  VMRange range1(0x100, 0x200);
+  VMRange range2(0x100, 0x200);
+  EXPECT_EQ(range1, range2);
+
+  EXPECT_NE(VMRange(0x100, 0x1ff), range1);
+  EXPECT_NE(VMRange(0x100, 0x201), range1);
+  EXPECT_NE(VMRange(0x0ff, 0x200), range1);
+  EXPECT_NE(VMRange(0x101, 0x200), range1);
+
+  range2.Clear();
+  EXPECT_NE(range1, range2);
+}
+
+TEST(VMRange, Reset) {
+  VMRange range(0x100, 0x200);
+  EXPECT_FALSE(VMRange(0x200, 0x200) == range);
+  range.Reset(0x200, 0x200);
+  EXPECT_TRUE(VMRange(0x200, 0x200) == range);
+}
+
+TEST(VMRange, SetEndAddress) {
+  VMRange range(0x100, 0x200);
+
+  range.SetEndAddress(0xFF);
+  EXPECT_EQ(0U, range.GetByteSize());
+  EXPECT_FALSE(range.IsValid());
+
+  range.SetEndAddress(0x101);
+  EXPECT_EQ(1U, range.GetByteSize());
+  EXPECT_TRUE(range.IsValid());
+}
+
+TEST(VMRange, ContainsAddr) {
+  VMRange range(0x100, 0x200);
+
+  EXPECT_FALSE(range.Contains(0x00));
+  EXPECT_FALSE(range.Contains(0xFF));
+  EXPECT_TRUE(range.Contains(0x100));
+  EXPECT_TRUE(range.Contains(0x101));
+  EXPECT_TRUE(range.Contains(0x1FF));
+  EXPECT_FALSE(range.Contains(0x200));
+  EXPECT_FALSE(range.Contains(0x201));
+  EXPECT_FALSE(range.Contains(0xFFF));
+  EXPECT_FALSE(range.Contains(std::numeric_limits::max()));
+}
+
+TEST(VMRange, ContainsRange) {
+  VMRange range(0x100, 0x200);
+
+  EXPECT_FALSE(range.Contains(VMRange(0x0, 0x0)));
+
+  EXPECT_FALSE(range.Contains(VMRange(0x0, 0x100)));
+  EXPECT_FALSE(range.Contains(VMRange(0x0, 0x101)));
+  EXPECT_TRUE(range.Contains(VMRange(0x100, 0x105)));
+  EXPECT_TRUE(range.Contains(VMRange(0x101, 0x105)));
+  EXPECT_TRUE(range.Contains(VMRange(0x100, 0x1FF)));
+  EXPECT_TRUE(range.Contains(VMRange(0x105, 0x200)));
+  EXPECT_FALSE(range.Contains(VMRange(0x105, 0x201)));
+  EXPECT_FALSE(range.Contains(VMRange(0x200, 0x201)));
+  EXPECT_TRUE(range.Contains(VMRange(0x100, 0x200)));
+  EXPECT_FALSE(
+  range.Contains(VMRange(0x105, std::numeric_limits::max(;
+
+  // Empty range.
+  EXPECT_TRUE(range.Contains(VMRange(0x100, 0x100)));
+
+  range.Clear();
+  EXPECT_FALSE(range.Contains(VMRange(0x0, 0x0)));
+}
+
+TEST(VMRange, Ordering) {
+  VMRange range1(0x44, 0x200);
+  VMRange range2(0x100, 0x1FF);
+  VMRange range3(0x100, 0x200);
+
+  EXPECT_LE(range1, range1);
+  EXPECT_GE(range1, range1);
+
+  EXPECT_LT(range1, range2);
+  EXPECT_LT(range2, range3);
+
+  EXPECT_GT(range2, range1);
+  EXPECT_GT(range3, range2);
+
+  // Ensure that < and > are always false when comparing ranges with themselves.
+  EXPECT_FALSE(range1 < range1);
+  EXPECT_FALSE(range2 < range2);
+  EXPECT_FALSE(range3 < range3);
+
+  EXPECT_FALSE(range1 > range1);
+  EXPECT_FALSE(range2 > range2);
+  EXPECT_FALSE(range3 > range3);
+}
+
+TEST(VMRange, CollectionContains) {
+  VMRange::collection collection = {VMRange(0x100, 0x105),
+VMRange(0x108, 0x110)};
+
+  EXPECT_FALSE(VMRange::ContainsValue(collection, 0xFF));
+  EXPECT_TRUE(VMRange::ContainsValue(collection, 0x100));
+  EXPECT_FALSE(VMRange::ContainsValue(collection, 0x105));
+  EXPECT_TRUE(VMRange::ContainsValue(collection, 0x109));
+
+  EXPECT_TRUE(VMRange::ContainsRange(collection, VMRange(0x100, 0x104)));
+  EXPECT_TRUE(VMRange::ContainsRange(collection, VMRange(0x108, 0x100)));
+  EXPECT_FALSE(VMRange::ContainsRange(collection, VMRange(0xFF, 0x100)));
+
+  // TODO: Implement and test ContainsRange with values that span multiple
+  // ranges in the collection.
+}
Index: unittests/Utility/CMakeLists.txt

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

2018-07-24 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor updated this revision to Diff 157169.
teemperor added a reviewer: davide.
teemperor added a comment.

- Removed some leftover code from the refactoring.
- Correctly SetUp/TearDown the test case.


https://reviews.llvm.org/D49334

Files:
  include/lldb/Core/Debugger.h
  include/lldb/Core/Highlighter.h
  include/lldb/Target/Language.h
  packages/Python/lldbsuite/test/source-manager/TestSourceManager.py
  source/Core/CMakeLists.txt
  source/Core/Debugger.cpp
  source/Core/Highlighter.cpp
  source/Core/SourceManager.cpp
  source/Plugins/Language/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  source/Plugins/Language/ClangCommon/CMakeLists.txt
  source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
  source/Plugins/Language/ClangCommon/ClangHighlighter.h
  source/Plugins/Language/Go/GoLanguage.cpp
  source/Plugins/Language/Go/GoLanguage.h
  source/Plugins/Language/Java/JavaLanguage.cpp
  source/Plugins/Language/Java/JavaLanguage.h
  source/Plugins/Language/OCaml/OCamlLanguage.cpp
  source/Plugins/Language/OCaml/OCamlLanguage.h
  source/Plugins/Language/ObjC/CMakeLists.txt
  source/Plugins/Language/ObjC/ObjCLanguage.cpp
  source/Plugins/Language/ObjC/ObjCLanguage.h
  source/Plugins/Language/ObjCPlusPlus/CMakeLists.txt
  source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.cpp
  source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
  source/Target/Language.cpp
  unittests/Language/CMakeLists.txt
  unittests/Language/Highlighting/CMakeLists.txt
  unittests/Language/Highlighting/HighlighterTest.cpp

Index: unittests/Language/Highlighting/HighlighterTest.cpp
===
--- /dev/null
+++ unittests/Language/Highlighting/HighlighterTest.cpp
@@ -0,0 +1,233 @@
+//===-- HighlighterTest.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Core/Highlighter.h"
+
+#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "Plugins/Language/Go/GoLanguage.h"
+#include "Plugins/Language/Java/JavaLanguage.h"
+#include "Plugins/Language/OCaml/OCamlLanguage.h"
+#include "Plugins/Language/ObjC/ObjCLanguage.h"
+#include "Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h"
+
+using namespace lldb_private;
+
+namespace {
+class HighlighterTest : public testing::Test {
+public:
+  static void SetUpTestCase();
+  static void TearDownTestCase();
+};
+} // namespace
+
+void HighlighterTest::SetUpTestCase() {
+  // The HighlighterManager uses the language plugins under the hood, so we
+  // have to initialize them here for our test process.
+  CPlusPlusLanguage::Initialize();
+  GoLanguage::Initialize();
+  JavaLanguage::Initialize();
+  ObjCLanguage::Initialize();
+  ObjCPlusPlusLanguage::Initialize();
+  OCamlLanguage::Initialize();
+}
+
+void HighlighterTest::TearDownTestCase() {
+  CPlusPlusLanguage::Terminate();
+  GoLanguage::Terminate();
+  JavaLanguage::Terminate();
+  ObjCLanguage::Terminate();
+  ObjCPlusPlusLanguage::Terminate();
+  OCamlLanguage::Terminate();
+}
+
+static std::string getName(lldb::LanguageType type) {
+  HighlighterManager m;
+  return m.getHighlighterFor(type, "").GetName().str();
+}
+
+static std::string getName(llvm::StringRef path) {
+  HighlighterManager m;
+  return m.getHighlighterFor(lldb::eLanguageTypeUnknown, path).GetName().str();
+}
+
+TEST_F(HighlighterTest, HighlighterSelectionType) {
+  EXPECT_EQ(getName(lldb::eLanguageTypeC_plus_plus), "clang");
+  EXPECT_EQ(getName(lldb::eLanguageTypeC_plus_plus_03), "clang");
+  EXPECT_EQ(getName(lldb::eLanguageTypeC_plus_plus_11), "clang");
+  EXPECT_EQ(getName(lldb::eLanguageTypeC_plus_plus_14), "clang");
+  EXPECT_EQ(getName(lldb::eLanguageTypeObjC), "clang");
+  EXPECT_EQ(getName(lldb::eLanguageTypeObjC_plus_plus), "clang");
+
+  EXPECT_EQ(getName(lldb::eLanguageTypeUnknown), "none");
+  EXPECT_EQ(getName(lldb::eLanguageTypeJulia), "none");
+  EXPECT_EQ(getName(lldb::eLanguageTypeJava), "none");
+  EXPECT_EQ(getName(lldb::eLanguageTypeHaskell), "none");
+}
+
+TEST_F(HighlighterTest, HighlighterSelectionPath) {
+  EXPECT_EQ(getName("myfile.cc"), "clang");
+  EXPECT_EQ(getName("moo.cpp"), "clang");
+  EXPECT_EQ(getName("mar.cxx"), "clang");
+  EXPECT_EQ(getName("foo.C"), "clang");
+  EXPECT_EQ(getName("bar.CC"), "clang");
+  EXPECT_EQ(getName("a/dir.CC"), "clang");
+  EXPECT_EQ(getName("/a/dir.hpp"), "clang");
+  EXPECT_EQ(getName("header.h"), "clang");
+
+  EXPECT_EQ(getName(""), "none");
+  EXPECT_EQ(getName("/dev/null"), "none");
+  EXPECT_EQ(getName("Factory.java"), "none");
+  EXPECT_EQ(getName("poll.py"), "none");
+  

[Lldb-commits] [lldb] r337865 - Add DumpRegisterValue.cpp.

2018-07-24 Thread Jason Molenda via lldb-commits
Author: jmolenda
Date: Tue Jul 24 16:19:56 2018
New Revision: 337865

URL: http://llvm.org/viewvc/llvm-project?rev=337865=rev
Log:
Add DumpRegisterValue.cpp.

Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj

Modified: lldb/trunk/lldb.xcodeproj/project.pbxproj
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/project.pbxproj?rev=337865=337864=337865=diff
==
--- lldb/trunk/lldb.xcodeproj/project.pbxproj (original)
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj Tue Jul 24 16:19:56 2018
@@ -264,6 +264,7 @@
2579065F1BD0488D00178368 /* DomainSocket.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 2579065E1BD0488D00178368 /* DomainSocket.cpp */; 
};
26F5C27710F3D9E4009D5894 /* Driver.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 26F5C27310F3D9E4009D5894 /* Driver.cpp */; };
4C4EB7811E6A4DCC002035C0 /* DumpDataExtractor.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = 4C4EB77F1E6A4DB8002035C0 /* 
DumpDataExtractor.cpp */; };
+   AFA585D02107EB7400D7689A /* DumpRegisterValue.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = AFA585CF2107EB7300D7689A /* 
DumpRegisterValue.cpp */; };
9447DE431BD5963300E67212 /* DumpValueObjectOptions.cpp in 
Sources */ = {isa = PBXBuildFile; fileRef = 9447DE421BD5963300E67212 /* 
DumpValueObjectOptions.cpp */; };
268900EA13353E6F00698AC0 /* DynamicLoader.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 26BC7E7710F1B85900F91463 /* DynamicLoader.cpp 
*/; };
AF27AD551D3603EA00CF2833 /* DynamicLoaderDarwin.cpp in Sources 
*/ = {isa = PBXBuildFile; fileRef = AF27AD531D3603EA00CF2833 /* 
DynamicLoaderDarwin.cpp */; };
@@ -335,7 +336,6 @@
AE44FB301BB07EB20033EB62 /* GoUserExpression.cpp in Sources */ 
= {isa = PBXBuildFile; fileRef = AE44FB2C1BB07DD80033EB62 /* 
GoUserExpression.cpp */; };
6D95DC011B9DC057000E318A /* HashedNameToDIE.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = 6D95DBFE1B9DC057000E318A /* HashedNameToDIE.cpp 
*/; };
2666ADC81B3CB675001FAFD3 /* HexagonDYLDRendezvous.cpp in 
Sources */ = {isa = PBXBuildFile; fileRef = 2666ADC31B3CB675001FAFD3 /* 
HexagonDYLDRendezvous.cpp */; };
-   AFC2DCF31E6E30CF00283714 /* History.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = AFC2DCF21E6E30CF00283714 /* History.cpp */; };
AF1729D6182C907200E0AB97 /* HistoryThread.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = AF1729D4182C907200E0AB97 /* HistoryThread.cpp 
*/; };
AF1729D7182C907200E0AB97 /* HistoryUnwind.cpp in Sources */ = 
{isa = PBXBuildFile; fileRef = AF1729D5182C907200E0AB97 /* HistoryUnwind.cpp 
*/; };
2689007113353E1A00698AC0 /* Host.cpp in Sources */ = {isa = 
PBXBuildFile; fileRef = 69A01E1C1236C5D400C660B5 /* Host.cpp */; };
@@ -1735,6 +1735,7 @@
26F5C27410F3D9E4009D5894 /* Driver.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = 
Driver.h; path = tools/driver/Driver.h; sourceTree = ""; };
4C4EB77F1E6A4DB8002035C0 /* DumpDataExtractor.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = DumpDataExtractor.cpp; path = source/Core/DumpDataExtractor.cpp; 
sourceTree = ""; };
4C4EB7821E6A4DE7002035C0 /* DumpDataExtractor.h */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = 
DumpDataExtractor.h; path = include/lldb/Core/DumpDataExtractor.h; sourceTree = 
""; };
+   AFA585CF2107EB7300D7689A /* DumpRegisterValue.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = DumpRegisterValue.cpp; path = source/Core/DumpRegisterValue.cpp; 
sourceTree = ""; };
9447DE421BD5963300E67212 /* DumpValueObjectOptions.cpp */ = 
{isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = 
sourcecode.cpp.cpp; name = DumpValueObjectOptions.cpp; path = 
source/DataFormatters/DumpValueObjectOptions.cpp; sourceTree = ""; };
9447DE411BD5962900E67212 /* DumpValueObjectOptions.h */ = {isa 
= PBXFileReference; lastKnownFileType = sourcecode.c.h; name = 
DumpValueObjectOptions.h; path = 
include/lldb/DataFormatters/DumpValueObjectOptions.h; sourceTree = ""; };
26BC7E7710F1B85900F91463 /* DynamicLoader.cpp */ = {isa = 
PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; 
name = DynamicLoader.cpp; path = source/Core/DynamicLoader.cpp; sourceTree = 
""; };
@@ -4484,8 +4485,6 @@
AFC2DCE61E6E2ED000283714 /* FastDemangle.cpp */,
AFC2DCED1E6E2F9800283714 /* FastDemangle.h */,
4CBFF0471F579A36004AFA92 /* Flags.h */,
-   AFC2DCF21E6E30CF00283714 /* 

[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.

2018-07-24 Thread Leonard Mosescu via Phabricator via lldb-commits
lemo added inline comments.



Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:216
+  if (csd_version.find("Linux") != std::string::npos)
+triple.setOS(llvm::Triple::OSType::Linux);
+  break;

clayborg wrote:
> I have run into some minidump files that don't have linux set corrreclty as 
> the OS, but they do have "linux" in the description. So this is to handle 
> those cases. I have told the people that are generating these about the issue 
> and they will fix it, but we have minidump files out in the wild that don't 
> set linux as the OS correctly.
1. any idea where this minidumps originate from?
2. should we raise some kind of signal when we go down this path? print an 
warning or at least verbose logging?



Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:179
+auto platform_sp = target.GetPlatform();
+if (platform_sp && !platform_sp->IsCompatibleArchitecture(arch, false, 
nullptr)) {
+  ArchSpec platform_arch;

clayborg wrote:
> lemo wrote:
> > shouldn't this be a check resulting in an error? why do we need to make 
> > this implicit adjustment here?
> By default the "host" platform is selected and it was incorrectly being used 
> along with _any_ triple. So we need to adjust the platform if the host 
> platform isn't compatible. The platform being set correctly helps with many 
> things during a debug session like finding symbols and much more.
Nice catch/fix in that case!

Just curious: It still seems a bit surprising to me to have the target mutated 
by the process object - is that how it's normally done in other cases?



https://reviews.llvm.org/D49750



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


[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Eugene Birukov via Phabricator via lldb-commits
EugeneBi added inline comments.



Comment at: source/Target/Platform.cpp:252
+if (error.Success() && module_sp)
+  module_sp->SetPlatformFileSpec(spec.GetFileSpec());
+return error;

EugeneBi wrote:
> A bug here. must be resolved_spec if it succeeds.
Now I have my doubts: what "platform file spec" really means? I mean, which code
is correct: current or previous one?


https://reviews.llvm.org/D49685



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


[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Eugene Birukov via Phabricator via lldb-commits
EugeneBi updated this revision to Diff 157146.
EugeneBi added a comment.

Fix a bug with resolved_spec path.


https://reviews.llvm.org/D49685

Files:
  source/Target/Platform.cpp


Index: source/Target/Platform.cpp
===
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -228,17 +228,36 @@
 module_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
 did_create_ptr, false);
 
+  // Module resolver lambda.
+  auto resolver = [&](const ModuleSpec ) {
+Status error(eErrorTypeGeneric);
+ModuleSpec resolved_spec;
+// Check if we have sysroot set.
+if (m_sdk_sysroot) {
+  // Prepend sysroot to module spec.
+  resolved_spec = spec;
+  resolved_spec.GetFileSpec().PrependPathComponent(
+m_sdk_sysroot.GetStringRef());
+  // Try to get shared module with resolved spec.
+  error = ModuleList::GetSharedModule( 
+resolved_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr, 
+did_create_ptr, false);
+}
+// If we don't have sysroot or it didn't work then 
+// try original module spec.
+if (!error.Success()) {
+  resolved_spec = spec;
+  error = ModuleList::GetSharedModule(
+resolved_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr, 
+did_create_ptr, false);
+}
+if (error.Success() && module_sp)
+  module_sp->SetPlatformFileSpec(resolved_spec.GetFileSpec());
+return error;
+  };
+
   return GetRemoteSharedModule(module_spec, process, module_sp,
-   [&](const ModuleSpec ) {
- Status error = ModuleList::GetSharedModule(
- spec, module_sp, module_search_paths_ptr,
- old_module_sp_ptr, did_create_ptr, false);
- if (error.Success() && module_sp)
-   module_sp->SetPlatformFileSpec(
-   spec.GetFileSpec());
- return error;
-   },
-   did_create_ptr);
+   resolver, did_create_ptr);
 }
 
 bool Platform::GetModuleSpec(const FileSpec _file_spec,


Index: source/Target/Platform.cpp
===
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -228,17 +228,36 @@
 module_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
 did_create_ptr, false);
 
+  // Module resolver lambda.
+  auto resolver = [&](const ModuleSpec ) {
+Status error(eErrorTypeGeneric);
+ModuleSpec resolved_spec;
+// Check if we have sysroot set.
+if (m_sdk_sysroot) {
+  // Prepend sysroot to module spec.
+  resolved_spec = spec;
+  resolved_spec.GetFileSpec().PrependPathComponent(
+m_sdk_sysroot.GetStringRef());
+  // Try to get shared module with resolved spec.
+  error = ModuleList::GetSharedModule( 
+resolved_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr, 
+did_create_ptr, false);
+}
+// If we don't have sysroot or it didn't work then 
+// try original module spec.
+if (!error.Success()) {
+  resolved_spec = spec;
+  error = ModuleList::GetSharedModule(
+resolved_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr, 
+did_create_ptr, false);
+}
+if (error.Success() && module_sp)
+  module_sp->SetPlatformFileSpec(resolved_spec.GetFileSpec());
+return error;
+  };
+
   return GetRemoteSharedModule(module_spec, process, module_sp,
-   [&](const ModuleSpec ) {
- Status error = ModuleList::GetSharedModule(
- spec, module_sp, module_search_paths_ptr,
- old_module_sp_ptr, did_create_ptr, false);
- if (error.Success() && module_sp)
-   module_sp->SetPlatformFileSpec(
-   spec.GetFileSpec());
- return error;
-   },
-   did_create_ptr);
+   resolver, did_create_ptr);
 }
 
 bool Platform::GetModuleSpec(const FileSpec _file_spec,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.

2018-07-24 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

I will make update the patch with many of the suggested inline comments.




Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:150
+  if (m_arch.IsValid())
+return m_arch;
   const MinidumpSystemInfo *system_info = GetSystemInfo();

Can do



Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:216
+  if (csd_version.find("Linux") != std::string::npos)
+triple.setOS(llvm::Triple::OSType::Linux);
+  break;

I have run into some minidump files that don't have linux set corrreclty as the 
OS, but they do have "linux" in the description. So this is to handle those 
cases. I have told the people that are generating these about the issue and 
they will fix it, but we have minidump files out in the wild that don't set 
linux as the OS correctly.



Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:179
+auto platform_sp = target.GetPlatform();
+if (platform_sp && !platform_sp->IsCompatibleArchitecture(arch, false, 
nullptr)) {
+  ArchSpec platform_arch;

lemo wrote:
> shouldn't this be a check resulting in an error? why do we need to make this 
> implicit adjustment here?
By default the "host" platform is selected and it was incorrectly being used 
along with _any_ triple. So we need to adjust the platform if the host platform 
isn't compatible. The platform being set correctly helps with many things 
during a debug session like finding symbols and much more.


https://reviews.llvm.org/D49750



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


[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Eugene Birukov via Phabricator via lldb-commits
EugeneBi added inline comments.



Comment at: source/Target/Platform.cpp:252
+if (error.Success() && module_sp)
+  module_sp->SetPlatformFileSpec(spec.GetFileSpec());
+return error;

A bug here. must be resolved_spec if it succeeds.


https://reviews.llvm.org/D49685



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


[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.

2018-07-24 Thread Leonard Mosescu via Phabricator via lldb-commits
lemo added a comment.

Looks really good, a few comments inline.

This may not big a big deal, but the part about FP (and Apple vs. non-Apple) is 
confusing: the FP is a pretty weak convention, and in some ABIs is not actually 
"fixed" (ex. FP can be either https://reviews.llvm.org/source/openmp/ or 
https://reviews.llvm.org/source/libunwind/, which in turn can be used as GPRs). 
If Apple sticks to a strict usage it makes sense to name it but then maybe we 
should just not name any FP for non-Apple?




Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:149-150
 ArchSpec MinidumpParser::GetArchitecture() {
-  ArchSpec arch_spec;
+  if (m_arch.IsValid())
+return m_arch;
   const MinidumpSystemInfo *system_info = GetSystemInfo();

instead of useing m_arch as a cache I'd explicitly initialize it in 
MinidumpParser::Initialize() 



Comment at: source/Plugins/Process/minidump/MinidumpParser.cpp:212-216
+  std::string csd_version;
+  if (auto s = GetMinidumpString(system_info->csd_version_rva))
+csd_version = *s;
+  if (csd_version.find("Linux") != std::string::npos)
+triple.setOS(llvm::Triple::OSType::Linux);

why is this needed when we have MinidumpOSPlatform::Linux ?



Comment at: source/Plugins/Process/minidump/ProcessMinidump.cpp:179
+auto platform_sp = target.GetPlatform();
+if (platform_sp && !platform_sp->IsCompatibleArchitecture(arch, false, 
nullptr)) {
+  ArchSpec platform_arch;

shouldn't this be a check resulting in an error? why do we need to make this 
implicit adjustment here?



Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:192
+
+static size_t k_num_reg_infos = llvm::array_lengthof(g_reg_infos);
+

constexpr?



Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:195
+// ARM general purpose registers.
+const uint32_t g_gpr_regnums[] = {
+  reg_r0,  reg_r1,  reg_r2,  reg_r3,  reg_r4, reg_r5, reg_r6, reg_r7,

use std::array for these kind of static arrays? (debug bounds checks, easy 
access to the static size, ...)



Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:225
+  m_regs.context_flags = data.GetU32();
+  for (unsigned i=0; i<16; ++i)
+m_regs.r[i] = data.GetU32();

symbolic constants or use the array size?



Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:231
+m_regs.d[i] = data.GetU64();
+  assert(k_num_regs == k_num_reg_infos_apple);
+  assert(k_num_regs == k_num_reg_infos);

lldb_assert ?



Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:242
+}
+size_t RegisterContextMinidump_ARM::GetRegisterCount() {
+  return k_num_regs;

not a big deal, but this kind of accessors for constants can be constexpr 
themselves



Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:248
+RegisterContextMinidump_ARM::GetRegisterInfoAtIndex(size_t reg) {
+  if (reg < k_num_reg_infos)
+return _reg_infos[reg];

failfast if out of bounds? who'd pass an invalid index and expect a meaninful 
result?
(btw, std::array would provide the debug checks if that's all that we want)



Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp:281
+bool RegisterContextMinidump_ARM::WriteRegister(
+const RegisterInfo *reg_info, const RegisterValue _value) {
+  return false;

remove unused parameter name or [[maybe_unused]]



Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.h:82
+Context m_regs;
+RegisterInfo *m_reg_infos;
+size_t m_num_reg_infos;

 = nullptr;



Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM.h:83
+RegisterInfo *m_reg_infos;
+size_t m_num_reg_infos;
+};

= 0;



Comment at: source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.h:37
+  
+  ~RegisterContextMinidump_ARM64() override {}
+  

=default instead of {} ?


https://reviews.llvm.org/D49750



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


[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

This will help, but not fix us loading incorrect versions of the shared 
library. I wonder if there is anything in the core file that could help use get 
the build ID/UUID of each binary. I doubt it, but might be worth checking. 
Linux core files are really useless unless they can be processed with the exact 
binaries and that is a shame. The windows minidump files that breakpad can 
create are much better than standard core files since they do contain the 
information we need (path + version + UUID) and they are very similar to core 
files but just don't always save the same amount of memory (just the thread 
stacks, not all mapped memory), though that can easily be fixed.


https://reviews.llvm.org/D49685



___
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-24 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor planned changes to this revision.
teemperor added a comment.

Actually, we also have to tear down the plugins, so let me fix that first.


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] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Eugene Birukov via Phabricator via lldb-commits
EugeneBi updated this revision to Diff 157135.
EugeneBi marked 6 inline comments as done.
EugeneBi added a comment.

Code review followup:

- Restricted change to Platform.cpp
- Restricted change only to remote platforms.


https://reviews.llvm.org/D49685

Files:
  source/Target/Platform.cpp


Index: source/Target/Platform.cpp
===
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -228,17 +228,33 @@
 module_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
 did_create_ptr, false);
 
+  // Module resolver lambda.
+  auto resolver = [&](const ModuleSpec ) {
+Status error(eErrorTypeGeneric);
+// Check if we have sysroot set.
+if (m_sdk_sysroot) {
+  // Prepend sysroot to module spec.
+  auto resolved_spec(spec);
+  resolved_spec.GetFileSpec().PrependPathComponent(
+m_sdk_sysroot.GetStringRef());
+  // Try to get shared module with resolved spec.
+  error = ModuleList::GetSharedModule( 
+resolved_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr, 
+did_create_ptr, false);
+}
+// If we don't have sysroot or it didn't work then 
+// try original module spec.
+if (!error.Success())
+  error = ModuleList::GetSharedModule(
+spec, module_sp, module_search_paths_ptr, old_module_sp_ptr, 
+did_create_ptr, false);
+if (error.Success() && module_sp)
+  module_sp->SetPlatformFileSpec(spec.GetFileSpec());
+return error;
+  };
+
   return GetRemoteSharedModule(module_spec, process, module_sp,
-   [&](const ModuleSpec ) {
- Status error = ModuleList::GetSharedModule(
- spec, module_sp, module_search_paths_ptr,
- old_module_sp_ptr, did_create_ptr, false);
- if (error.Success() && module_sp)
-   module_sp->SetPlatformFileSpec(
-   spec.GetFileSpec());
- return error;
-   },
-   did_create_ptr);
+   resolver, did_create_ptr);
 }
 
 bool Platform::GetModuleSpec(const FileSpec _file_spec,


Index: source/Target/Platform.cpp
===
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -228,17 +228,33 @@
 module_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
 did_create_ptr, false);
 
+  // Module resolver lambda.
+  auto resolver = [&](const ModuleSpec ) {
+Status error(eErrorTypeGeneric);
+// Check if we have sysroot set.
+if (m_sdk_sysroot) {
+  // Prepend sysroot to module spec.
+  auto resolved_spec(spec);
+  resolved_spec.GetFileSpec().PrependPathComponent(
+m_sdk_sysroot.GetStringRef());
+  // Try to get shared module with resolved spec.
+  error = ModuleList::GetSharedModule( 
+resolved_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr, 
+did_create_ptr, false);
+}
+// If we don't have sysroot or it didn't work then 
+// try original module spec.
+if (!error.Success())
+  error = ModuleList::GetSharedModule(
+spec, module_sp, module_search_paths_ptr, old_module_sp_ptr, 
+did_create_ptr, false);
+if (error.Success() && module_sp)
+  module_sp->SetPlatformFileSpec(spec.GetFileSpec());
+return error;
+  };
+
   return GetRemoteSharedModule(module_spec, process, module_sp,
-   [&](const ModuleSpec ) {
- Status error = ModuleList::GetSharedModule(
- spec, module_sp, module_search_paths_ptr,
- old_module_sp_ptr, did_create_ptr, false);
- if (error.Success() && module_sp)
-   module_sp->SetPlatformFileSpec(
-   spec.GetFileSpec());
- return error;
-   },
-   did_create_ptr);
+   resolver, did_create_ptr);
 }
 
 bool Platform::GetModuleSpec(const FileSpec _file_spec,
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D49755: Remove unused History class

2018-07-24 Thread Raphael Isemann via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337855: Remove unused History class (authored by teemperor, 
committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D49755?vs=157075=157128#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D49755

Files:
  lldb/trunk/include/lldb/Utility/History.h
  lldb/trunk/lldb.xcodeproj/project.pbxproj
  lldb/trunk/source/Utility/CMakeLists.txt
  lldb/trunk/source/Utility/History.cpp

Index: lldb/trunk/lldb.xcodeproj/project.pbxproj
===
--- lldb/trunk/lldb.xcodeproj/project.pbxproj
+++ lldb/trunk/lldb.xcodeproj/project.pbxproj
@@ -1880,8 +1880,6 @@
 		26A0DA4D140F721D006DA411 /* HashedNameToDIE.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = HashedNameToDIE.h; sourceTree = ""; };
 		2666ADC31B3CB675001FAFD3 /* HexagonDYLDRendezvous.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = HexagonDYLDRendezvous.cpp; sourceTree = ""; };
 		2666ADC41B3CB675001FAFD3 /* HexagonDYLDRendezvous.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = HexagonDYLDRendezvous.h; sourceTree = ""; };
-		AFC2DCF21E6E30CF00283714 /* History.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = History.cpp; path = source/Utility/History.cpp; sourceTree = ""; };
-		AFC2DCF41E6E30D800283714 /* History.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = History.h; path = include/lldb/Utility/History.h; sourceTree = ""; };
 		AF1729D4182C907200E0AB97 /* HistoryThread.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = HistoryThread.cpp; path = Utility/HistoryThread.cpp; sourceTree = ""; };
 		AF061F89182C98B6A19C /* HistoryThread.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = HistoryThread.h; path = Utility/HistoryThread.h; sourceTree = ""; };
 		AF1729D5182C907200E0AB97 /* HistoryUnwind.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = HistoryUnwind.cpp; path = Utility/HistoryUnwind.cpp; sourceTree = ""; };
Index: lldb/trunk/source/Utility/CMakeLists.txt
===
--- lldb/trunk/source/Utility/CMakeLists.txt
+++ lldb/trunk/source/Utility/CMakeLists.txt
@@ -53,7 +53,6 @@
   Environment.cpp
   FastDemangle.cpp
   FileSpec.cpp
-  History.cpp
   IOObject.cpp
   JSON.cpp
   LLDBAssert.cpp
Index: lldb/trunk/source/Utility/History.cpp
===
--- lldb/trunk/source/Utility/History.cpp
+++ lldb/trunk/source/Utility/History.cpp
@@ -1,24 +0,0 @@
-//===-- History.cpp -*- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "lldb/Utility/History.h"
-
-// C Includes
-#include 
-// C++ Includes
-// Other libraries and framework includes
-// Project includes
-#include "lldb/Utility/Stream.h"
-
-using namespace lldb;
-using namespace lldb_private;
-
-void HistorySourceUInt::DumpHistoryEvent(Stream , HistoryEvent event) {
-  strm.Printf("%s %" PRIu64, m_name.c_str(), (uint64_t)((uintptr_t)event));
-}
Index: lldb/trunk/include/lldb/Utility/History.h
===
--- lldb/trunk/include/lldb/Utility/History.h
+++ lldb/trunk/include/lldb/Utility/History.h
@@ -1,136 +0,0 @@
-//===-- History.h ---*- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#ifndef lldb_History_h_
-#define lldb_History_h_
-
-#include "lldb/lldb-defines.h" // for DISALLOW_COPY_AND_ASSIGN
-
-// C++ Includes
-#include 
-#include 
-#include 
-
-#include  // for size_t
-#include 
-
-namespace lldb_private {
-class Stream;
-}
-
-namespace lldb_private {
-
-//--
-/// @class HistorySource History.h "lldb/Core/History.h"
-/// A class that defines history events.
-//--
-
-class HistorySource {
-public:
-  typedef const void *HistoryEvent;
-
-  HistorySource() : m_mutex(), m_events() {}
-
-  virtual ~HistorySource() {}
-
-  // Create a new history event. Subclasses should use any data or members in
-  // the subclass of 

[Lldb-commits] [lldb] r337855 - Remove unused History class

2018-07-24 Thread Raphael Isemann via lldb-commits
Author: teemperor
Date: Tue Jul 24 14:09:17 2018
New Revision: 337855

URL: http://llvm.org/viewvc/llvm-project?rev=337855=rev
Log:
Remove unused History class

Summary: This class doesn't seem to be used anywhere, so we might as well 
remove the code.

Reviewers: labath

Reviewed By: labath

Subscribers: labath, mgorny, lldb-commits

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

Removed:
lldb/trunk/include/lldb/Utility/History.h
lldb/trunk/source/Utility/History.cpp
Modified:
lldb/trunk/lldb.xcodeproj/project.pbxproj
lldb/trunk/source/Utility/CMakeLists.txt

Removed: lldb/trunk/include/lldb/Utility/History.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/History.h?rev=337854=auto
==
--- lldb/trunk/include/lldb/Utility/History.h (original)
+++ lldb/trunk/include/lldb/Utility/History.h (removed)
@@ -1,136 +0,0 @@
-//===-- History.h ---*- C++ 
-*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#ifndef lldb_History_h_
-#define lldb_History_h_
-
-#include "lldb/lldb-defines.h" // for DISALLOW_COPY_AND_ASSIGN
-
-// C++ Includes
-#include 
-#include 
-#include 
-
-#include  // for size_t
-#include 
-
-namespace lldb_private {
-class Stream;
-}
-
-namespace lldb_private {
-
-//--
-/// @class HistorySource History.h "lldb/Core/History.h"
-/// A class that defines history events.
-//--
-
-class HistorySource {
-public:
-  typedef const void *HistoryEvent;
-
-  HistorySource() : m_mutex(), m_events() {}
-
-  virtual ~HistorySource() {}
-
-  // Create a new history event. Subclasses should use any data or members in
-  // the subclass of this class to produce a history event and push it onto the
-  // end of the history stack.
-
-  virtual HistoryEvent CreateHistoryEvent() = 0;
-
-  virtual void DeleteHistoryEvent(HistoryEvent event) = 0;
-
-  virtual void DumpHistoryEvent(Stream , HistoryEvent event) = 0;
-
-  virtual size_t GetHistoryEventCount() = 0;
-
-  virtual HistoryEvent GetHistoryEventAtIndex(uint32_t idx) = 0;
-
-  virtual HistoryEvent GetCurrentHistoryEvent() = 0;
-
-  // Return 0 when lhs == rhs, 1 if lhs > rhs, or -1 if lhs < rhs.
-  virtual int CompareHistoryEvents(const HistoryEvent lhs,
-   const HistoryEvent rhs) = 0;
-
-  virtual bool IsCurrentHistoryEvent(const HistoryEvent event) = 0;
-
-private:
-  typedef std::stack collection;
-
-  std::recursive_mutex m_mutex;
-  collection m_events;
-
-  DISALLOW_COPY_AND_ASSIGN(HistorySource);
-};
-
-//--
-/// @class HistorySourceUInt History.h "lldb/Core/History.h"
-/// A class that defines history events that are represented by
-/// unsigned integers.
-///
-/// Any history event that is defined by a unique monotonically increasing
-/// unsigned integer
-//--
-
-class HistorySourceUInt : public HistorySource {
-  HistorySourceUInt(const char *id_name, uintptr_t start_value = 0u)
-  : HistorySource(), m_name(id_name), m_curr_id(start_value) {}
-
-  ~HistorySourceUInt() override {}
-
-  // Create a new history event. Subclasses should use any data or members in
-  // the subclass of this class to produce a history event and push it onto the
-  // end of the history stack.
-
-  HistoryEvent CreateHistoryEvent() override {
-++m_curr_id;
-return (HistoryEvent)m_curr_id;
-  }
-
-  void DeleteHistoryEvent(HistoryEvent event) override {
-// Nothing to delete, the event contains the integer
-  }
-
-  void DumpHistoryEvent(Stream , HistoryEvent event) override;
-
-  size_t GetHistoryEventCount() override { return m_curr_id; }
-
-  HistoryEvent GetHistoryEventAtIndex(uint32_t idx) override {
-return (HistoryEvent)((uintptr_t)idx);
-  }
-
-  HistoryEvent GetCurrentHistoryEvent() override {
-return (HistoryEvent)m_curr_id;
-  }
-
-  // Return 0 when lhs == rhs, 1 if lhs > rhs, or -1 if lhs < rhs.
-  int CompareHistoryEvents(const HistoryEvent lhs,
-   const HistoryEvent rhs) override {
-uintptr_t lhs_uint = (uintptr_t)lhs;
-uintptr_t rhs_uint = (uintptr_t)rhs;
-if (lhs_uint < rhs_uint)
-  return -1;
-if (lhs_uint > rhs_uint)
-  return +1;
-return 0;
-  }
-
-  bool IsCurrentHistoryEvent(const HistoryEvent event) override {
-return (uintptr_t)event == m_curr_id;
-  }
-
-protected:
-  std::string m_name;  // The name of the history unsigned integer
-  uintptr_t m_curr_id; // The current value of the 

[Lldb-commits] [PATCH] D49755: Remove unused History class

2018-07-24 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.

Yes, please.


https://reviews.llvm.org/D49755



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


[Lldb-commits] [PATCH] D49755: Remove unused History class

2018-07-24 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor created this revision.
Herald added a subscriber: mgorny.

This class doesn't seem to be used anywhere, so we might as well remove the 
code.


https://reviews.llvm.org/D49755

Files:
  include/lldb/Utility/History.h
  lldb.xcodeproj/project.pbxproj
  source/Utility/CMakeLists.txt
  source/Utility/History.cpp

Index: source/Utility/History.cpp
===
--- source/Utility/History.cpp
+++ /dev/null
@@ -1,24 +0,0 @@
-//===-- History.cpp -*- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "lldb/Utility/History.h"
-
-// C Includes
-#include 
-// C++ Includes
-// Other libraries and framework includes
-// Project includes
-#include "lldb/Utility/Stream.h"
-
-using namespace lldb;
-using namespace lldb_private;
-
-void HistorySourceUInt::DumpHistoryEvent(Stream , HistoryEvent event) {
-  strm.Printf("%s %" PRIu64, m_name.c_str(), (uint64_t)((uintptr_t)event));
-}
Index: source/Utility/CMakeLists.txt
===
--- source/Utility/CMakeLists.txt
+++ source/Utility/CMakeLists.txt
@@ -53,7 +53,6 @@
   Environment.cpp
   FastDemangle.cpp
   FileSpec.cpp
-  History.cpp
   IOObject.cpp
   JSON.cpp
   LLDBAssert.cpp
Index: lldb.xcodeproj/project.pbxproj
===
--- lldb.xcodeproj/project.pbxproj
+++ lldb.xcodeproj/project.pbxproj
@@ -1880,8 +1880,6 @@
 		26A0DA4D140F721D006DA411 /* HashedNameToDIE.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = HashedNameToDIE.h; sourceTree = ""; };
 		2666ADC31B3CB675001FAFD3 /* HexagonDYLDRendezvous.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = HexagonDYLDRendezvous.cpp; sourceTree = ""; };
 		2666ADC41B3CB675001FAFD3 /* HexagonDYLDRendezvous.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = HexagonDYLDRendezvous.h; sourceTree = ""; };
-		AFC2DCF21E6E30CF00283714 /* History.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = History.cpp; path = source/Utility/History.cpp; sourceTree = ""; };
-		AFC2DCF41E6E30D800283714 /* History.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = History.h; path = include/lldb/Utility/History.h; sourceTree = ""; };
 		AF1729D4182C907200E0AB97 /* HistoryThread.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = HistoryThread.cpp; path = Utility/HistoryThread.cpp; sourceTree = ""; };
 		AF061F89182C98B6A19C /* HistoryThread.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = HistoryThread.h; path = Utility/HistoryThread.h; sourceTree = ""; };
 		AF1729D5182C907200E0AB97 /* HistoryUnwind.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = HistoryUnwind.cpp; path = Utility/HistoryUnwind.cpp; sourceTree = ""; };
Index: include/lldb/Utility/History.h
===
--- include/lldb/Utility/History.h
+++ /dev/null
@@ -1,136 +0,0 @@
-//===-- History.h ---*- C++ -*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#ifndef lldb_History_h_
-#define lldb_History_h_
-
-#include "lldb/lldb-defines.h" // for DISALLOW_COPY_AND_ASSIGN
-
-// C++ Includes
-#include 
-#include 
-#include 
-
-#include  // for size_t
-#include 
-
-namespace lldb_private {
-class Stream;
-}
-
-namespace lldb_private {
-
-//--
-/// @class HistorySource History.h "lldb/Core/History.h"
-/// A class that defines history events.
-//--
-
-class HistorySource {
-public:
-  typedef const void *HistoryEvent;
-
-  HistorySource() : m_mutex(), m_events() {}
-
-  virtual ~HistorySource() {}
-
-  // Create a new history event. Subclasses should use any data or members in
-  // the subclass of this class to produce a history event and push it onto the
-  // end of the history stack.
-
-  virtual HistoryEvent CreateHistoryEvent() = 0;
-
-  virtual void DeleteHistoryEvent(HistoryEvent event) = 0;
-
-  virtual void DumpHistoryEvent(Stream , HistoryEvent event) = 0;
-
-  virtual size_t GetHistoryEventCount() = 0;
-
-  virtual HistoryEvent 

[Lldb-commits] [PATCH] D49750: Add support for ARM and ARM64 breakpad generated minidump files.

2018-07-24 Thread Greg Clayton via Phabricator via lldb-commits
clayborg created this revision.
clayborg added reviewers: labath, zturner, markmentovai.
Herald added a reviewer: javed.absar.
Herald added subscribers: chrib, kristof.beyls.

In this patch I add support for ARM and ARM64 break pad files. There are two 
flavors of ARM: Apple where FP is https://reviews.llvm.org/source/openmp/, and 
non Apple where FP is https://reviews.llvm.org/source/libunwind/. Added minimal 
tests that load up ARM64 and the two flavors or ARM core files with a single 
thread and known register values in each register. Each register is checked for 
the exact value.


https://reviews.llvm.org/D49750

Files:
  lldb.xcodeproj/project.pbxproj
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/TestMiniDumpNew.py
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/arm-linux.dmp
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/arm-macos.dmp
  
packages/Python/lldbsuite/test/functionalities/postmortem/minidump-new/arm64-macos.dmp
  source/Plugins/Process/minidump/MinidumpParser.cpp
  source/Plugins/Process/minidump/MinidumpParser.h
  source/Plugins/Process/minidump/ProcessMinidump.cpp
  source/Plugins/Process/minidump/RegisterContextMinidump_ARM.cpp
  source/Plugins/Process/minidump/RegisterContextMinidump_ARM.h
  source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.cpp
  source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.h
  source/Plugins/Process/minidump/ThreadMinidump.cpp

Index: source/Plugins/Process/minidump/ThreadMinidump.cpp
===
--- source/Plugins/Process/minidump/ThreadMinidump.cpp
+++ source/Plugins/Process/minidump/ThreadMinidump.cpp
@@ -13,6 +13,8 @@
 
 #include "RegisterContextMinidump_x86_32.h"
 #include "RegisterContextMinidump_x86_64.h"
+#include "RegisterContextMinidump_ARM.h"
+#include "RegisterContextMinidump_ARM64.h"
 
 // Other libraries and framework includes
 #include "Plugins/Process/Utility/RegisterContextLinux_i386.h"
@@ -88,6 +90,20 @@
   *this, reg_interface, gpregset, {}));
   break;
 }
+case llvm::Triple::aarch64: {
+  DataExtractor data(m_gpregset_data.data(), m_gpregset_data.size(),
+ lldb::eByteOrderLittle, 8);
+  m_thread_reg_ctx_sp.reset(new RegisterContextMinidump_ARM64(*this, data));
+  break;
+}
+case llvm::Triple::arm: {
+  DataExtractor data(m_gpregset_data.data(), m_gpregset_data.size(),
+ lldb::eByteOrderLittle, 8);
+  const bool apple = arch.GetTriple().getVendor() == llvm::Triple::Apple;
+  m_thread_reg_ctx_sp.reset(new RegisterContextMinidump_ARM(*this, data,
+apple));
+  break;
+}
 default:
   break;
 }
Index: source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.h
===
--- source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.h
+++ source/Plugins/Process/minidump/RegisterContextMinidump_ARM64.h
@@ -0,0 +1,86 @@
+//===-- RegisterContextMinidump_ARM64.h -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef liblldb_RegisterContextMinidump_ARM64_h_
+#define liblldb_RegisterContextMinidump_ARM64_h_
+
+// Project includes
+#include "MinidumpTypes.h"
+
+// Other libraries and framework includes
+#include "Plugins/Process/Utility/RegisterInfoInterface.h"
+#include "lldb/Target/RegisterContext.h"
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/BitmaskEnum.h"
+
+// C includes
+// C++ includes
+
+namespace lldb_private {
+
+namespace minidump {
+
+LLVM_ENABLE_BITMASK_ENUMS_IN_NAMESPACE();
+
+class RegisterContextMinidump_ARM64: public lldb_private::RegisterContext {
+public:
+  RegisterContextMinidump_ARM64(lldb_private::Thread ,
+   const DataExtractor );
+  
+  ~RegisterContextMinidump_ARM64() override {}
+  
+  void InvalidateAllRegisters() override {
+// Do nothing... registers are always valid...
+  }
+  
+  size_t GetRegisterCount() override;
+
+  const lldb_private::RegisterInfo *GetRegisterInfoAtIndex(size_t reg) override;
+
+  size_t GetRegisterSetCount() override;
+  
+  const lldb_private::RegisterSet *GetRegisterSet(size_t set) override;
+  
+  const char *GetRegisterName(unsigned reg);
+  
+  bool ReadRegister(const RegisterInfo *reg_info,
+RegisterValue _value) override;
+  
+  bool WriteRegister(const RegisterInfo *reg_info,
+ const RegisterValue _value) override;
+  
+
+  uint32_t ConvertRegisterKindToRegisterNumber(lldb::RegisterKind kind,
+   uint32_t 

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Eugene Birukov via Phabricator via lldb-commits
EugeneBi added a comment.

> You would just move:
> 
>   auto resolved_module_spec(module_spec);
> if (sysroot != nullptr)
>   resolved_module_spec.GetFileSpec().PrependPathComponent(sysroot);
> 
> 
> into the code in Platform.cpp and do it all there.

Ah, I see. Will do, thanks.


https://reviews.llvm.org/D49685



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


[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Eugene Birukov via Phabricator via lldb-commits
EugeneBi added a comment.

I need to convert char* to StringRef yet.


https://reviews.llvm.org/D49685



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


[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Eugene Birukov via Phabricator via lldb-commits
EugeneBi updated this revision to Diff 157088.
EugeneBi added a comment.

Rebased to recent master.
Included the whole file in diff.


https://reviews.llvm.org/D49685

Files:
  include/lldb/Core/ModuleList.h
  source/Core/ModuleList.cpp
  source/Target/Platform.cpp


Index: source/Target/Platform.cpp
===
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -226,13 +226,14 @@
   if (IsHost())
 return ModuleList::GetSharedModule(
 module_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
-did_create_ptr, false);
+did_create_ptr, false, m_sdk_sysroot.AsCString());
 
   return GetRemoteSharedModule(module_spec, process, module_sp,
[&](const ModuleSpec ) {
  Status error = ModuleList::GetSharedModule(
  spec, module_sp, module_search_paths_ptr,
- old_module_sp_ptr, did_create_ptr, false);
+ old_module_sp_ptr, did_create_ptr, false,
+ m_sdk_sysroot.AsCString());
  if (error.Success() && module_sp)
module_sp->SetPlatformFileSpec(
spec.GetFileSpec());
Index: source/Core/ModuleList.cpp
===
--- source/Core/ModuleList.cpp
+++ source/Core/ModuleList.cpp
@@ -770,7 +770,11 @@
ModuleSP _sp,
const FileSpecList *module_search_paths_ptr,
ModuleSP *old_module_sp_ptr,
-   bool *did_create_ptr, bool always_create) {
+   bool *did_create_ptr, bool always_create,
+   const char* sysroot) {
+  // Make sure no one else can try and get or create a module while this
+  // function is actively working on it by doing an extra lock on the
+  // global mutex list.
   ModuleList _module_list = GetSharedModuleList();
   std::lock_guard guard(
   shared_module_list.m_modules_mutex);
@@ -789,9 +793,6 @@
   const FileSpec _file_spec = module_spec.GetFileSpec();
   const ArchSpec  = module_spec.GetArchitecture();
 
-  // Make sure no one else can try and get or create a module while this
-  // function is actively working on it by doing an extra lock on the global
-  // mutex list.
   if (!always_create) {
 ModuleList matching_module_list;
 const size_t num_matching_modules =
@@ -825,7 +826,11 @@
   if (module_sp)
 return error;
 
-  module_sp.reset(new Module(module_spec));
+  auto resolved_module_spec(module_spec);
+  if (sysroot != nullptr)
+resolved_module_spec.GetFileSpec().PrependPathComponent(sysroot);
+
+  module_sp.reset(new Module(resolved_module_spec));
   // Make sure there are a module and an object file since we can specify a
   // valid file path with an architecture that might not be in that file. By
   // getting the object file we can guarantee that the architecture matches
Index: include/lldb/Core/ModuleList.h
===
--- include/lldb/Core/ModuleList.h
+++ include/lldb/Core/ModuleList.h
@@ -537,7 +537,8 @@
 const FileSpecList *module_search_paths_ptr,
 lldb::ModuleSP *old_module_sp_ptr,
 bool *did_create_ptr,
-bool always_create = false);
+bool always_create = false,
+const char* sysroot = nullptr);
 
   static bool RemoveSharedModule(lldb::ModuleSP _sp);
 


Index: source/Target/Platform.cpp
===
--- source/Target/Platform.cpp
+++ source/Target/Platform.cpp
@@ -226,13 +226,14 @@
   if (IsHost())
 return ModuleList::GetSharedModule(
 module_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
-did_create_ptr, false);
+did_create_ptr, false, m_sdk_sysroot.AsCString());
 
   return GetRemoteSharedModule(module_spec, process, module_sp,
[&](const ModuleSpec ) {
  Status error = ModuleList::GetSharedModule(
  spec, module_sp, module_search_paths_ptr,
- old_module_sp_ptr, did_create_ptr, false);
+ old_module_sp_ptr, did_create_ptr, false,
+ m_sdk_sysroot.AsCString());
  if (error.Success() && module_sp)
module_sp->SetPlatformFileSpec(
spec.GetFileSpec());
Index: 

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

In https://reviews.llvm.org/D49685#1173720, @EugeneBi wrote:

> In https://reviews.llvm.org/D49685#1173640, @clayborg wrote:
>
> > We should never be loading the wrong shared libraries. The module spec we 
> > fill out must contain the UUID of the file are looking for. If it doesn't 
> > we have no chance of every really loading the right stuff.
>
>
> Well, that's what it does on my machine. So prepending sysroot *after* trying 
> to load module without it will cause problems to me. Also, I assume that if 
> you specified sysroot for a platform, we should not try to load solibs 
> without it - these paths are just not a part of the platform.
>
> > I think doing this in the module list is not the right place. Why? Some 
> > platforms might have multiple sysroot to check. iOS for instance has a 
> > directory for each device that Xcode has connected to which can be checked. 
> > I am fine with adding this ability to>  lldb_private::Platform, but I would 
> > just do it in there. Try GetRemoteSharedModule with the spec, if it fails, 
> > try again after modifying the spec to prepend the sysroot path. Possible 
> > even just check the sysroot path + path first if m_sdk_sysroot is filled 
> > in. I don't see the need to change ModuleList itself.
>
> I do not see how this prepend sysroot could be done in the platform... 
> Essentially, my fix is doing what you suggest: the platform supplies sysroot 
> to the module list, and two different platforms (for two XCode devices, etc.) 
> would supply different sysroots. What do I miss?


You would just move:

  auto resolved_module_spec(module_spec);
if (sysroot != nullptr)
  resolved_module_spec.GetFileSpec().PrependPathComponent(sysroot);

into the code in Platform.cpp and do it all there.


Repository:
  rL LLVM

https://reviews.llvm.org/D49685



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


[Lldb-commits] [PATCH] D49713: Replace StreamTee's recursive_mutex with a normal mutex.

2018-07-24 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a reviewer: clayborg.
teemperor added a comment.

Adding Greg because he seem to be the one who added the lock. I think it's also 
worth investigating why this lock is there in the first place (as other Stream 
implementations are not thread safe, but this one is?)


https://reviews.llvm.org/D49713



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


[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Eugene Birukov via Phabricator via lldb-commits
EugeneBi added a comment.

In https://reviews.llvm.org/D49685#1173640, @clayborg wrote:

> We should never be loading the wrong shared libraries. The module spec we 
> fill out must contain the UUID of the file are looking for. If it doesn't we 
> have no chance of every really loading the right stuff.


Well, that's what it does on my machine. So prepending sysroot *after* trying 
to load module without it will cause problems to me. Also, I assume that if you 
specified sysroot for a platform, we should not try to load solibs without it - 
these paths are just not a part of the platform.

> I think doing this in the module list is not the right place. Why? Some 
> platforms might have multiple sysroot to check. iOS for instance has a 
> directory for each device that Xcode has connected to which can be checked. I 
> am fine with adding this ability to>  lldb_private::Platform, but I would 
> just do it in there. Try GetRemoteSharedModule with the spec, if it fails, 
> try again after modifying the spec to prepend the sysroot path. Possible even 
> just check the sysroot path + path first if m_sdk_sysroot is filled in. I 
> don't see the need to change ModuleList itself.

I do not see how this prepend sysroot could be done in the platform... 
Essentially, my fix is doing what you suggest: the platform supplies sysroot to 
the module list, and two different platforms (for two XCode devices, etc.) 
would supply different sysroots. What do I miss?


Repository:
  rL LLVM

https://reviews.llvm.org/D49685



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


[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

2018-07-24 Thread Stefan Gränitz via Phabricator via lldb-commits
sgraenitz marked an inline comment as done.
sgraenitz added inline comments.



Comment at: unittests/Core/CMakeLists.txt:4
   DataExtractorTest.cpp
+  MangledTest.cpp
   ListenerTest.cpp

teemperor wrote:
> This should be after ListenerTest.cpp to keep this file in alphabetical order.
Hehe good catch!


https://reviews.llvm.org/D49612



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


[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

2018-07-24 Thread Stefan Gränitz via Phabricator via lldb-commits
sgraenitz updated this revision to Diff 157062.
sgraenitz added a comment.

Keep alphabetical order of files in CMakeLists.txt


https://reviews.llvm.org/D49612

Files:
  cmake/modules/LLDBConfig.cmake
  lldb.xcodeproj/project.pbxproj
  source/Core/Mangled.cpp
  unittests/Core/CMakeLists.txt
  unittests/Core/MangledTest.cpp

Index: unittests/Core/MangledTest.cpp
===
--- /dev/null
+++ unittests/Core/MangledTest.cpp
@@ -0,0 +1,38 @@
+//===-- MangledTest.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Core/Mangled.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+TEST(MangledTest, ResultForValidName) {
+  ConstString MangledName("_ZN1a1b1cIiiiEEvm");
+  bool IsMangled = true;
+
+  Mangled TheMangled(MangledName, IsMangled);
+  const ConstString  =
+  TheMangled.GetDemangledName(eLanguageTypeC_plus_plus);
+
+  ConstString ExpectedResult("void a::b::c(unsigned long)");
+  EXPECT_STREQ(ExpectedResult.GetCString(), TheDemangled.GetCString());
+}
+
+TEST(MangledTest, EmptyForInvalidName) {
+  ConstString MangledName("_ZN1a1b1cmxktpEEvm");
+  bool IsMangled = true;
+
+  Mangled TheMangled(MangledName, IsMangled);
+  const ConstString  =
+  TheMangled.GetDemangledName(eLanguageTypeC_plus_plus);
+
+  EXPECT_STREQ("", TheDemangled.GetCString());
+}
Index: unittests/Core/CMakeLists.txt
===
--- unittests/Core/CMakeLists.txt
+++ unittests/Core/CMakeLists.txt
@@ -2,6 +2,7 @@
   BroadcasterTest.cpp
   DataExtractorTest.cpp
   ListenerTest.cpp
+  MangledTest.cpp
   ScalarTest.cpp
   StateTest.cpp
   StreamCallbackTest.cpp
Index: source/Core/Mangled.cpp
===
--- source/Core/Mangled.cpp
+++ source/Core/Mangled.cpp
@@ -16,34 +16,24 @@
 #pragma comment(lib, "dbghelp.lib")
 #endif
 
-#ifdef LLDB_USE_BUILTIN_DEMANGLER
-// Provide a fast-path demangler implemented in FastDemangle.cpp until it can
-// replace the existing C++ demangler with a complete implementation
-#include "lldb/Utility/FastDemangle.h"
-#include "llvm/Demangle/Demangle.h"
-#else
-// FreeBSD9-STABLE requires this to know about size_t in cxxabi.
-#include 
-#include 
-#endif
-
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Logging.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/Timer.h"
-#include "lldb/lldb-enumerations.h" // for LanguageType
+#include "lldb/lldb-enumerations.h"
 
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
 #include "Plugins/Language/ObjC/ObjCLanguage.h"
 
-#include "llvm/ADT/StringRef.h"// for StringRef
-#include "llvm/Support/Compiler.h" // for LLVM_PRETT...
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Demangle/Demangle.h"
+#include "llvm/Support/Compiler.h"
 
-#include// for mutex, loc...
-#include   // for string
-#include  // for pair
+#include 
+#include 
+#include 
 
 #include 
 #include 
@@ -295,18 +285,15 @@
 break;
   }
   case eManglingSchemeItanium: {
-#ifdef LLDB_USE_BUILTIN_DEMANGLER
-if (log)
-  log->Printf("demangle itanium: %s", mangled_name);
-// Try to use the fast-path demangler first for the performance win,
-// falling back to the full demangler only when necessary
-demangled_name = FastDemangle(mangled_name, m_mangled.GetLength());
-if (!demangled_name)
-  demangled_name =
-  llvm::itaniumDemangle(mangled_name, NULL, NULL, NULL);
-#else
-demangled_name = abi::__cxa_demangle(mangled_name, NULL, NULL, NULL);
-#endif
+llvm::ItaniumPartialDemangler IPD;
+bool demangle_err = IPD.partialDemangle(mangled_name);
+if (!demangle_err) {
+  // Default buffer and size (realloc is used in case it's too small).
+  size_t demangled_size = 80;
+  demangled_name = static_cast(::malloc(demangled_size));
+  demangled_name = IPD.finishDemangle(demangled_name, _size);
+}
+
 if (log) {
   if (demangled_name)
 log->Printf("demangled itanium: %s -> \"%s\"", mangled_name,
Index: lldb.xcodeproj/project.pbxproj
===
--- lldb.xcodeproj/project.pbxproj
+++ lldb.xcodeproj/project.pbxproj
@@ -483,6 +483,7 @@
 		9A20573A1F3B8E7E00F6C293 /* MainLoopTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9A2057301F3B8E7600F6C293 /* MainLoopTest.cpp */; };
 		8C3BD9961EF45DA50016C343 /* MainThreadCheckerRuntime.cpp in Sources */ = {isa = PBXBuildFile; fileRef 

[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

2018-07-24 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added inline comments.



Comment at: unittests/Core/CMakeLists.txt:4
   DataExtractorTest.cpp
+  MangledTest.cpp
   ListenerTest.cpp

This should be after ListenerTest.cpp to keep this file in alphabetical order.


https://reviews.llvm.org/D49612



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


[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

2018-07-24 Thread Stefan Gränitz via Phabricator via lldb-commits
sgraenitz updated this revision to Diff 157061.
sgraenitz added a comment.

Change EXPECTED_EQ to EXPECTED_STREQ, because ConstString::operator==() 
compares identity of pointers, not equality of strings. While "they must come 
from the same pool in order to be equal" (as stated in the description), this 
is verifying string equality not the internals of ConstString.


https://reviews.llvm.org/D49612

Files:
  cmake/modules/LLDBConfig.cmake
  lldb.xcodeproj/project.pbxproj
  source/Core/Mangled.cpp
  unittests/Core/CMakeLists.txt
  unittests/Core/MangledTest.cpp

Index: unittests/Core/MangledTest.cpp
===
--- /dev/null
+++ unittests/Core/MangledTest.cpp
@@ -0,0 +1,38 @@
+//===-- MangledTest.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Core/Mangled.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+TEST(MangledTest, ResultForValidName) {
+  ConstString MangledName("_ZN1a1b1cIiiiEEvm");
+  bool IsMangled = true;
+
+  Mangled TheMangled(MangledName, IsMangled);
+  const ConstString  =
+  TheMangled.GetDemangledName(eLanguageTypeC_plus_plus);
+
+  ConstString ExpectedResult("void a::b::c(unsigned long)");
+  EXPECT_STREQ(ExpectedResult.GetCString(), TheDemangled.GetCString());
+}
+
+TEST(MangledTest, EmptyForInvalidName) {
+  ConstString MangledName("_ZN1a1b1cmxktpEEvm");
+  bool IsMangled = true;
+
+  Mangled TheMangled(MangledName, IsMangled);
+  const ConstString  =
+  TheMangled.GetDemangledName(eLanguageTypeC_plus_plus);
+
+  EXPECT_STREQ("", TheDemangled.GetCString());
+}
Index: unittests/Core/CMakeLists.txt
===
--- unittests/Core/CMakeLists.txt
+++ unittests/Core/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_lldb_unittest(LLDBCoreTests
   BroadcasterTest.cpp
   DataExtractorTest.cpp
+  MangledTest.cpp
   ListenerTest.cpp
   ScalarTest.cpp
   StateTest.cpp
Index: source/Core/Mangled.cpp
===
--- source/Core/Mangled.cpp
+++ source/Core/Mangled.cpp
@@ -16,34 +16,24 @@
 #pragma comment(lib, "dbghelp.lib")
 #endif
 
-#ifdef LLDB_USE_BUILTIN_DEMANGLER
-// Provide a fast-path demangler implemented in FastDemangle.cpp until it can
-// replace the existing C++ demangler with a complete implementation
-#include "lldb/Utility/FastDemangle.h"
-#include "llvm/Demangle/Demangle.h"
-#else
-// FreeBSD9-STABLE requires this to know about size_t in cxxabi.
-#include 
-#include 
-#endif
-
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Logging.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/Timer.h"
-#include "lldb/lldb-enumerations.h" // for LanguageType
+#include "lldb/lldb-enumerations.h"
 
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
 #include "Plugins/Language/ObjC/ObjCLanguage.h"
 
-#include "llvm/ADT/StringRef.h"// for StringRef
-#include "llvm/Support/Compiler.h" // for LLVM_PRETT...
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Demangle/Demangle.h"
+#include "llvm/Support/Compiler.h"
 
-#include// for mutex, loc...
-#include   // for string
-#include  // for pair
+#include 
+#include 
+#include 
 
 #include 
 #include 
@@ -295,18 +285,15 @@
 break;
   }
   case eManglingSchemeItanium: {
-#ifdef LLDB_USE_BUILTIN_DEMANGLER
-if (log)
-  log->Printf("demangle itanium: %s", mangled_name);
-// Try to use the fast-path demangler first for the performance win,
-// falling back to the full demangler only when necessary
-demangled_name = FastDemangle(mangled_name, m_mangled.GetLength());
-if (!demangled_name)
-  demangled_name =
-  llvm::itaniumDemangle(mangled_name, NULL, NULL, NULL);
-#else
-demangled_name = abi::__cxa_demangle(mangled_name, NULL, NULL, NULL);
-#endif
+llvm::ItaniumPartialDemangler IPD;
+bool demangle_err = IPD.partialDemangle(mangled_name);
+if (!demangle_err) {
+  // Default buffer and size (realloc is used in case it's too small).
+  size_t demangled_size = 80;
+  demangled_name = static_cast(::malloc(demangled_size));
+  demangled_name = IPD.finishDemangle(demangled_name, _size);
+}
+
 if (log) {
   if (demangled_name)
 log->Printf("demangled itanium: %s -> \"%s\"", mangled_name,
Index: lldb.xcodeproj/project.pbxproj
===
--- lldb.xcodeproj/project.pbxproj
+++ lldb.xcodeproj/project.pbxproj
@@ -483,6 +483,7 @@
 		

[Lldb-commits] [PATCH] D49740: Move RegisterValue, Scalar, State from Core to Utility

2018-07-24 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

Did you test that with lldb's C++ modules? If not I can test it locally.


https://reviews.llvm.org/D49740



___
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-24 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor updated this revision to Diff 157054.
teemperor added a comment.

[Updated patch to address Pavel's comments, thanks!]

@zturner So I looked into the Windows support:

Windows requires us to directly flush/signal/write/flush to the console output 
stream. However lldb's output is buffered by design into a StreamString first 
to prevent overlapping with the process output. So we can't just add Windows 
support to the coloring backend as we don't have direct access to the output 
stream.

If we want to fix this in lldb, then we could write an ANSI color code 
interpreter and let that run over our final output while we print it. That 
wouldn't be too complex and would fix all existing coloring output in lldb. It 
would also fix that lldb configs that enable color support on Windows are 
broken (because people just add color codes there).

However, it seems Windows starting with https://reviews.llvm.org/W10 anyway 
supports ANSI color codes (or at least you can enable them in the settings). So 
that interpreter is only really necessary until everyone moved to a version 
that supports color codes: 
https://docs.microsoft.com/en-us/windows/console/console-virtual-terminal-sequences#span-idtextformattingspanspan-idtextformattingspanspan-idtextformattingspantext-formatting

So I would suggest we maybe just hack in the color interpreter and drop it when 
https://reviews.llvm.org/W7 reaches EoL (?). I can make another patch for that 
if it sounds good.


https://reviews.llvm.org/D49334

Files:
  include/lldb/Core/Debugger.h
  include/lldb/Core/Highlighter.h
  include/lldb/Target/Language.h
  packages/Python/lldbsuite/test/source-manager/TestSourceManager.py
  source/Core/CMakeLists.txt
  source/Core/Debugger.cpp
  source/Core/Highlighter.cpp
  source/Core/SourceManager.cpp
  source/Plugins/Language/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CMakeLists.txt
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
  source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
  source/Plugins/Language/ClangCommon/CMakeLists.txt
  source/Plugins/Language/ClangCommon/ClangHighlighter.cpp
  source/Plugins/Language/ClangCommon/ClangHighlighter.h
  source/Plugins/Language/Go/GoLanguage.cpp
  source/Plugins/Language/Go/GoLanguage.h
  source/Plugins/Language/Java/JavaLanguage.cpp
  source/Plugins/Language/Java/JavaLanguage.h
  source/Plugins/Language/OCaml/OCamlLanguage.cpp
  source/Plugins/Language/OCaml/OCamlLanguage.h
  source/Plugins/Language/ObjC/CMakeLists.txt
  source/Plugins/Language/ObjC/ObjCLanguage.cpp
  source/Plugins/Language/ObjC/ObjCLanguage.h
  source/Plugins/Language/ObjCPlusPlus/CMakeLists.txt
  source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.cpp
  source/Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h
  source/Target/Language.cpp
  unittests/Language/CMakeLists.txt
  unittests/Language/Highlighting/CMakeLists.txt
  unittests/Language/Highlighting/HighlighterTest.cpp

Index: unittests/Language/Highlighting/HighlighterTest.cpp
===
--- /dev/null
+++ unittests/Language/Highlighting/HighlighterTest.cpp
@@ -0,0 +1,228 @@
+//===-- HighlighterTest.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Core/Highlighter.h"
+
+#include "llvm/Support/Threading.h"
+
+#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+#include "Plugins/Language/Go/GoLanguage.h"
+#include "Plugins/Language/Java/JavaLanguage.h"
+#include "Plugins/Language/OCaml/OCamlLanguage.h"
+#include "Plugins/Language/ObjC/ObjCLanguage.h"
+#include "Plugins/Language/ObjCPlusPlus/ObjCPlusPlusLanguage.h"
+
+using namespace lldb_private;
+
+static void InitLanguages() {
+  static llvm::once_flag g_once_flag;
+  llvm::call_once(g_once_flag, []() {
+// The HighlighterManager uses the language plugins under the hood, so we
+// have to initialize them here for our test process.
+CPlusPlusLanguage::Initialize();
+GoLanguage::Initialize();
+JavaLanguage::Initialize();
+ObjCLanguage::Initialize();
+ObjCPlusPlusLanguage::Initialize();
+OCamlLanguage::Initialize();
+  });
+}
+
+static std::string getName(lldb::LanguageType type) {
+  HighlighterManager mgr;
+  return mgr.getHighlighterFor(type, "").GetName().str();
+}
+
+static std::string getName(llvm::StringRef path) {
+  HighlighterManager mgr;
+  return mgr.getHighlighterFor(lldb::eLanguageTypeUnknown, path)
+  .GetName()
+  .str();
+}
+
+TEST(HighlighterTest, HighlighterSelectionType) {
+  InitLanguages();
+  EXPECT_EQ(getName(lldb::eLanguageTypeC_plus_plus), "clang");
+  EXPECT_EQ(getName(lldb::eLanguageTypeC_plus_plus_03), "clang");
+  

Re: [Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Pavel Labath via lldb-commits
On Tue, 24 Jul 2018 at 17:17, Leonard Mosescu  wrote:
>>
>> The problem is that shared libraries differ on these machines and
>> LLDB either fails to load some libraries or loads wrong ones.
>
>
> Not finding the modules is not surprising but the latter (loading the wrong 
> modules) is a bit concerning. Do you know why the module build-id is not used 
> when searching for the local binary?
>
>

Core files don't contain a build-id, just a file path.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D49739: [WIP] Re-implement MI target-select command.

2018-07-24 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

I am sure that particular test is worth trying to implement in lit. That script 
of yours is full of operating system specifics, and it's going to be super 
flaky.

I'd suggest keeping this as a python test, as there you can abstract the 
platform specifics much easier, and we also have a lot of code we can (re)use 
to setup socket connections safely. (e.g. lldb-server has a mode where *it* 
selects a port to listen on  (race-free), and then you can just fetch that port 
and connect to it.)

Doing that from a shell script is going to be painful.

I suppose if you really want to make this a lit test, you should at least ditch 
the bash script and replace that bit with python.


https://reviews.llvm.org/D49739



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


[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

We should never be loading the wrong shared libraries. The module spec we fill 
out must contain the UUID of the file are looking for. If it doesn't we have no 
chance of every really loading the right stuff.


Repository:
  rL LLVM

https://reviews.llvm.org/D49685



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


[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Leonard Mosescu via Phabricator via lldb-commits
lemo added subscribers: clayborg, EugeneBi, labath, lemo.
lemo added a comment.

> The problem is that shared libraries differ on these machines and
>  LLDB either fails to load some libraries *or loads wrong ones*.

Not finding the modules is not surprising but the latter (loading the wrong
modules) is a bit concerning. Do you know why the module build-id is not
used when searching for the local binary?


Repository:
  rL LLVM

https://reviews.llvm.org/D49685



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


Re: [Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Leonard Mosescu via lldb-commits
>
> The problem is that shared libraries differ on these machines and
> LLDB either fails to load some libraries *or loads wrong ones*.
>

Not finding the modules is not surprising but the latter (loading the wrong
modules) is a bit concerning. Do you know why the module build-id is not
used when searching for the local binary?


On Tue, Jul 24, 2018 at 7:19 AM, Greg Clayton via Phabricator via
lldb-commits  wrote:

> clayborg requested changes to this revision.
> clayborg added a comment.
> This revision now requires changes to proceed.
>
> I think doing this in the module list is not the right place. Why? Some
> platforms might have multiple sysroot to check. iOS for instance has a
> directory for each device that Xcode has connected to which can be checked.
> I am fine with adding this ability to lldb_private::Platform, but I would
> just do it in there. Try GetRemoteSharedModule with the spec, if it fails,
> try again after modifying the spec to prepend the sysroot path. Possible
> even just check the sysroot path + path first if m_sdk_sysroot is filled
> in. I don't see the need to change ModuleList itself.
>
>
>
> 
> Comment at: include/lldb/Core/ModuleList.h:544-545
>  bool *did_create_ptr,
> -bool always_create = false);
> +bool always_create = false,
> +const char* sysroot = nullptr);
>static bool RemoveSharedModule(lldb::ModuleSP _sp);
> 
> Revert this? See my main comment
>
>
> 
> Comment at: source/Core/ModuleList.cpp:710-714
> +   bool *did_create_ptr, bool
> always_create,
> +   const char* sysroot) {
> +  // Make sure no one else can try and get or create a module while this
> +  // function is actively working on it by doing an extra lock on the
> +  // global mutex list.
> 
> Revert
>
>
> 
> Comment at: source/Core/ModuleList.cpp:766-770
> +  if (sysroot != nullptr)
> +resolved_module_spec.GetFileSpec().PrependPathComponent(sysroot);
> +
> +  module_sp.reset(new Module(resolved_module_spec));
>// Make sure there are a module and an object file since we can specify
> 
> Revert
>
>
> 
> Comment at: source/Target/Platform.cpp:228
>  module_spec, module_sp, module_search_paths_ptr,
> old_module_sp_ptr,
> -did_create_ptr, false);
> +did_create_ptr, false, m_sdk_sysroot.AsCString());
>return GetRemoteSharedModule(module_spec, process, module_sp,
> 
> Revert
>
>
> 
> Comment at: source/Target/Platform.cpp:230
>return GetRemoteSharedModule(module_spec, process, module_sp,
> [&](const ModuleSpec ) {
>   Status error =
> ModuleList::GetSharedModule(
> 
> Here just make a module spec that has m_sdk_sysroot prepended if
> m_sdk_sysroot is set, and check for that file first. If that succeeds,
> return that, else do the same code as before.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D49685
>
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D49739: [WIP] Re-implement MI target-select command.

2018-07-24 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added inline comments.



Comment at: include/lldb/API/SBTarget.h:275-276
 
+  void AppendImageSearchPath(const char *from, const char *to);
+
+  void AppendImageSearchPath(const char *from, const char *to,

Remove this first one? Other functions were first created and gave no error 
feedback and then another version was added that had an error. We should start 
with one that has an error and not ever add this version.



Comment at: include/lldb/Target/Target.h:1003-1005
+  void AppendImageSearchPath(const ConstString ,
+ const ConstString );
+

Remove and just use "PathMappingList ::GetImageSearchPathList();"




Comment at: lit/tools/lldb-mi/target/inputs/target-select-so-path.sh:3-11
+function get_random_unused_port {
+local port=$(shuf -i 2000-65000 -n 1)
+netstat -lat | grep $port > /dev/null
+if [[ $? == 1 ]] ; then
+echo $port
+else
+get_random_unused_port

This seems like it could be racy and cause problems on buildbots.



Comment at: source/API/SBTarget.cpp:1460-1464
+void SBTarget::AppendImageSearchPath(const char *from, const char *to) {
+  lldb::SBError error; // Ignored
+  AppendImageSearchPath(from, to, error);
+}
+

Remove this version that takes no error



Comment at: source/Target/Target.cpp:2055-2059
+void Target::AppendImageSearchPath(const ConstString ,
+   const ConstString ) {
+  m_image_search_paths.Append(from, to, true);
+}
+

You don't need this, just call "target. GetImageSearchPathList().Append(...)


https://reviews.llvm.org/D49739



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


[Lldb-commits] [PATCH] D49739: [WIP] Re-implement MI target-select command.

2018-07-24 Thread Alexander Polyakov via Phabricator via lldb-commits
apolyakov created this revision.
apolyakov added reviewers: aprantl, clayborg, labath.
Herald added a subscriber: ki.stfu.

In this patch I suggest a way to deal with the problem started in 
https://reviews.llvm.org/D47302 and use it to re-implement MI target-select 
command. You are welcome to comment this patch, especially the new methods of 
Target and SBTarget classes and the bash script(do we need to find a random 
port or should we hardcode it?).


https://reviews.llvm.org/D49739

Files:
  include/lldb/API/SBTarget.h
  include/lldb/Target/Target.h
  lit/lit.cfg
  lit/tools/lldb-mi/target/inputs/main.c
  lit/tools/lldb-mi/target/inputs/target-select-so-path.sh
  lit/tools/lldb-mi/target/lit.local.cfg
  lit/tools/lldb-mi/target/target-select-so-path.test
  source/API/SBTarget.cpp
  source/Target/Target.cpp
  tools/lldb-mi/MICmdCmdTarget.cpp

Index: tools/lldb-mi/MICmdCmdTarget.cpp
===
--- tools/lldb-mi/MICmdCmdTarget.cpp
+++ tools/lldb-mi/MICmdCmdTarget.cpp
@@ -10,9 +10,8 @@
 // Overview:CMICmdCmdTargetSelect   implementation.
 
 // Third Party Headers:
-#include "lldb/API/SBCommandInterpreter.h"
-#include "lldb/API/SBCommandReturnObject.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/API/SBError.h"
 
 // In-house headers:
 #include "MICmdArgValNumber.h"
@@ -52,7 +51,7 @@
 // Return:  None.
 // Throws:  None.
 //--
-CMICmdCmdTargetSelect::~CMICmdCmdTargetSelect() {}
+CMICmdCmdTargetSelect::~CMICmdCmdTargetSelect() = default;
 
 //++
 //
@@ -93,51 +92,44 @@
 
   CMICmnLLDBDebugSessionInfo (
   CMICmnLLDBDebugSessionInfo::Instance());
+  lldb::SBTarget target = rSessionInfo.GetTarget();
 
-  // Check we have a valid target
-  // Note: target created via 'file-exec-and-symbols' command
-  if (!rSessionInfo.GetTarget().IsValid()) {
+  // Check we have a valid target.
+  // Note: target created via 'file-exec-and-symbols' command.
+  if (!target.IsValid()) {
 SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_INVALID_TARGET_CURRENT),
m_cmdData.strMiCmd.c_str()));
 return MIstatus::failure;
   }
 
-  // Verify that we are executing remotely
+  // Verify that we are executing remotely.
   const CMIUtilString (pArgType->GetValue());
   if (rRemoteType != "remote") {
 SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_INVALID_TARGET_TYPE),
m_cmdData.strMiCmd.c_str(),
rRemoteType.c_str()));
 return MIstatus::failure;
   }
 
-  // Create a URL pointing to the remote gdb stub
+  // Create a URL pointing to the remote gdb stub.
   const CMIUtilString strUrl =
   CMIUtilString::Format("connect://%s", pArgParameters->GetValue().c_str());
 
-  // Ask LLDB to connect to the target port
-  const char *pPlugin("gdb-remote");
   lldb::SBError error;
-  lldb::SBProcess process = rSessionInfo.GetTarget().ConnectRemote(
+  // Ask LLDB to connect to the target port.
+  const char *pPlugin("gdb-remote");
+  lldb::SBProcess process = target.ConnectRemote(
   rSessionInfo.GetListener(), strUrl.c_str(), pPlugin, error);
 
-  // Verify that we have managed to connect successfully
-  lldb::SBStream errMsg;
-  error.GetDescription(errMsg);
+  // Verify that we have managed to connect successfully.
   if (!process.IsValid()) {
 SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_INVALID_TARGET_PLUGIN),
m_cmdData.strMiCmd.c_str(),
-   errMsg.GetData()));
-return MIstatus::failure;
-  }
-  if (error.Fail()) {
-SetError(CMIUtilString::Format(MIRSRC(IDS_CMD_ERR_CONNECT_TO_TARGET),
-   m_cmdData.strMiCmd.c_str(),
-   errMsg.GetData()));
+   error.GetCString()));
 return MIstatus::failure;
   }
 
-  // Set the environment path if we were given one
+  // Set the environment path if we were given one.
   CMIUtilString strWkDir;
   if (rSessionInfo.SharedDataRetrieve(
   rSessionInfo.m_constStrSharedDataKeyWkDir, strWkDir)) {
@@ -150,28 +142,13 @@
 }
   }
 
-  // Set the shared object path if we were given one
+  // Set the shared object path if we were given one.
   CMIUtilString strSolibPath;
   if (rSessionInfo.SharedDataRetrieve(
-  rSessionInfo.m_constStrSharedDataSolibPath, strSolibPath)) {
-lldb::SBDebugger  = rSessionInfo.GetDebugger();
-lldb::SBCommandInterpreter cmdIterpreter = rDbgr.GetCommandInterpreter();
-
-CMIUtilString strCmdString = CMIUtilString::Format(
-"target modules search-paths add . %s", strSolibPath.c_str());
-
-lldb::SBCommandReturnObject retObj;
-cmdIterpreter.HandleCommand(strCmdString.c_str(), retObj, false);
+  rSessionInfo.m_constStrSharedDataSolibPath, strSolibPath))
+

[Lldb-commits] [PATCH] D48351: Move dumping code out of RegisterValue class

2018-07-24 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL337832: Move dumping code out of RegisterValue class 
(authored by labath, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D48351

Files:
  lldb/trunk/include/lldb/Core/DumpRegisterValue.h
  lldb/trunk/include/lldb/Core/RegisterValue.h
  lldb/trunk/source/Commands/CommandObjectRegister.cpp
  lldb/trunk/source/Core/CMakeLists.txt
  lldb/trunk/source/Core/DumpRegisterValue.cpp
  lldb/trunk/source/Core/EmulateInstruction.cpp
  lldb/trunk/source/Core/FormatEntity.cpp
  lldb/trunk/source/Core/RegisterValue.cpp
  
lldb/trunk/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
  lldb/trunk/source/Target/ThreadPlanCallFunction.cpp
  lldb/trunk/source/Target/ThreadPlanTracer.cpp

Index: lldb/trunk/source/Core/RegisterValue.cpp
===
--- lldb/trunk/source/Core/RegisterValue.cpp
+++ lldb/trunk/source/Core/RegisterValue.cpp
@@ -9,7 +9,6 @@
 
 #include "lldb/Core/RegisterValue.h"
 
-#include "lldb/Core/DumpDataExtractor.h"
 #include "lldb/Core/Scalar.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/DataExtractor.h"
@@ -34,67 +33,6 @@
 using namespace lldb;
 using namespace lldb_private;
 
-bool RegisterValue::Dump(Stream *s, const RegisterInfo *reg_info,
- bool prefix_with_name, bool prefix_with_alt_name,
- Format format,
- uint32_t reg_name_right_align_at) const {
-  DataExtractor data;
-  if (GetData(data)) {
-bool name_printed = false;
-// For simplicity, alignment of the register name printing applies only in
-// the most common case where:
-//
-// prefix_with_name^prefix_with_alt_name is true
-//
-StreamString format_string;
-if (reg_name_right_align_at && (prefix_with_name ^ prefix_with_alt_name))
-  format_string.Printf("%%%us", reg_name_right_align_at);
-else
-  format_string.Printf("%%s");
-std::string fmt = format_string.GetString();
-if (prefix_with_name) {
-  if (reg_info->name) {
-s->Printf(fmt.c_str(), reg_info->name);
-name_printed = true;
-  } else if (reg_info->alt_name) {
-s->Printf(fmt.c_str(), reg_info->alt_name);
-prefix_with_alt_name = false;
-name_printed = true;
-  }
-}
-if (prefix_with_alt_name) {
-  if (name_printed)
-s->PutChar('/');
-  if (reg_info->alt_name) {
-s->Printf(fmt.c_str(), reg_info->alt_name);
-name_printed = true;
-  } else if (!name_printed) {
-// No alternate name but we were asked to display a name, so show the
-// main name
-s->Printf(fmt.c_str(), reg_info->name);
-name_printed = true;
-  }
-}
-if (name_printed)
-  s->PutCString(" = ");
-
-if (format == eFormatDefault)
-  format = reg_info->format;
-
-DumpDataExtractor(data, s,
-  0,// Offset in "data"
-  format,   // Format to use when dumping
-  reg_info->byte_size,  // item_byte_size
-  1,// item_count
-  UINT32_MAX,   // num_per_line
-  LLDB_INVALID_ADDRESS, // base_addr
-  0,// item_bit_size
-  0);   // item_bit_offset
-return true;
-  }
-  return false;
-}
-
 bool RegisterValue::GetData(DataExtractor ) const {
   return data.SetData(GetBytes(), GetByteSize(), GetByteOrder()) > 0;
 }
Index: lldb/trunk/source/Core/DumpRegisterValue.cpp
===
--- lldb/trunk/source/Core/DumpRegisterValue.cpp
+++ lldb/trunk/source/Core/DumpRegisterValue.cpp
@@ -0,0 +1,79 @@
+//===-- DumpRegisterValue.cpp ---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "lldb/Core/DumpRegisterValue.h"
+#include "lldb/Core/DumpDataExtractor.h"
+#include "lldb/Core/RegisterValue.h"
+#include "lldb/Utility/DataExtractor.h"
+#include "lldb/Utility/StreamString.h"
+#include "lldb/lldb-private-types.h"
+
+using namespace lldb;
+
+bool lldb_private::DumpRegisterValue(const RegisterValue _val, Stream *s,
+ const RegisterInfo *reg_info,
+ bool prefix_with_name,
+ bool prefix_with_alt_name, Format format,
+ uint32_t reg_name_right_align_at) {
+  DataExtractor data;
+  if (reg_val.GetData(data)) {
+bool 

[Lldb-commits] [lldb] r337832 - Move dumping code out of RegisterValue class

2018-07-24 Thread Pavel Labath via lldb-commits
Author: labath
Date: Tue Jul 24 08:48:13 2018
New Revision: 337832

URL: http://llvm.org/viewvc/llvm-project?rev=337832=rev
Log:
Move dumping code out of RegisterValue class

Summary:
The dump function was the only part of this class which depended on
high-level functionality. This was due to the DumpDataExtractor
function, which uses info from a running target to control dump format
(although, RegisterValue doesn't really use the high-level part of
DumpDataExtractor).

This patch follows the same approach done for the DataExtractor class,
and extracts the dumping code into a separate function/file. This file
can stay in the higher level code, while the RegisterValue class and
anything that does not depend in dumping can stay go to lower layers.

The XCode project will need to be updated after this patch.

Reviewers: zturner, jingham, clayborg

Subscribers: lldb-commits, mgorny

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

Added:
lldb/trunk/include/lldb/Core/DumpRegisterValue.h
lldb/trunk/source/Core/DumpRegisterValue.cpp
Modified:
lldb/trunk/include/lldb/Core/RegisterValue.h
lldb/trunk/source/Commands/CommandObjectRegister.cpp
lldb/trunk/source/Core/CMakeLists.txt
lldb/trunk/source/Core/EmulateInstruction.cpp
lldb/trunk/source/Core/FormatEntity.cpp
lldb/trunk/source/Core/RegisterValue.cpp

lldb/trunk/source/Plugins/UnwindAssembly/InstEmulation/UnwindAssemblyInstEmulation.cpp
lldb/trunk/source/Target/ThreadPlanCallFunction.cpp
lldb/trunk/source/Target/ThreadPlanTracer.cpp

Added: lldb/trunk/include/lldb/Core/DumpRegisterValue.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/DumpRegisterValue.h?rev=337832=auto
==
--- lldb/trunk/include/lldb/Core/DumpRegisterValue.h (added)
+++ lldb/trunk/include/lldb/Core/DumpRegisterValue.h Tue Jul 24 08:48:13 2018
@@ -0,0 +1,31 @@
+//===-- DumpRegisterValue.h -*- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLDB_CORE_DUMPREGISTERVALUE_H
+#define LLDB_CORE_DUMPREGISTERVALUE_H
+
+#include "lldb/lldb-enumerations.h"
+#include 
+
+namespace lldb_private {
+
+class RegisterValue;
+struct RegisterInfo;
+class Stream;
+
+// The default value of 0 for reg_name_right_align_at means no alignment at
+// all.
+bool DumpRegisterValue(const RegisterValue _val, Stream *s,
+   const RegisterInfo *reg_info, bool prefix_with_name,
+   bool prefix_with_alt_name, lldb::Format format,
+   uint32_t reg_name_right_align_at = 0);
+
+} // namespace lldb_private
+
+#endif // LLDB_CORE_DUMPREGISTERVALUE_H

Modified: lldb/trunk/include/lldb/Core/RegisterValue.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/RegisterValue.h?rev=337832=337831=337832=diff
==
--- lldb/trunk/include/lldb/Core/RegisterValue.h (original)
+++ lldb/trunk/include/lldb/Core/RegisterValue.h Tue Jul 24 08:48:13 2018
@@ -248,12 +248,6 @@ public:
   Status SetValueFromData(const RegisterInfo *reg_info, DataExtractor ,
   lldb::offset_t offset, bool partial_data_ok);
 
-  // The default value of 0 for reg_name_right_align_at means no alignment at
-  // all.
-  bool Dump(Stream *s, const RegisterInfo *reg_info, bool prefix_with_name,
-bool prefix_with_alt_name, lldb::Format format,
-uint32_t reg_name_right_align_at = 0) const;
-
   const void *GetBytes() const;
 
   lldb::ByteOrder GetByteOrder() const {

Modified: lldb/trunk/source/Commands/CommandObjectRegister.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectRegister.cpp?rev=337832=337831=337832=diff
==
--- lldb/trunk/source/Commands/CommandObjectRegister.cpp (original)
+++ lldb/trunk/source/Commands/CommandObjectRegister.cpp Tue Jul 24 08:48:13 
2018
@@ -9,6 +9,7 @@
 
 #include "CommandObjectRegister.h"
 #include "lldb/Core/Debugger.h"
+#include "lldb/Core/DumpRegisterValue.h"
 #include "lldb/Core/RegisterValue.h"
 #include "lldb/Core/Scalar.h"
 #include "lldb/Host/OptionParser.h"
@@ -92,8 +93,8 @@ public:
 
 bool prefix_with_altname = (bool)m_command_options.alternate_name;
 bool prefix_with_name = !prefix_with_altname;
-reg_value.Dump(, reg_info, prefix_with_name, prefix_with_altname,
-   m_format_options.GetFormat(), 8);
+DumpRegisterValue(reg_value, , reg_info, prefix_with_name,
+  prefix_with_altname, m_format_options.GetFormat(), 
8);
 if 

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Greg Clayton via Phabricator via lldb-commits
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

I think doing this in the module list is not the right place. Why? Some 
platforms might have multiple sysroot to check. iOS for instance has a 
directory for each device that Xcode has connected to which can be checked. I 
am fine with adding this ability to lldb_private::Platform, but I would just do 
it in there. Try GetRemoteSharedModule with the spec, if it fails, try again 
after modifying the spec to prepend the sysroot path. Possible even just check 
the sysroot path + path first if m_sdk_sysroot is filled in. I don't see the 
need to change ModuleList itself.




Comment at: include/lldb/Core/ModuleList.h:544-545
 bool *did_create_ptr,
-bool always_create = false);
+bool always_create = false,
+const char* sysroot = nullptr);
   static bool RemoveSharedModule(lldb::ModuleSP _sp);

Revert this? See my main comment



Comment at: source/Core/ModuleList.cpp:710-714
+   bool *did_create_ptr, bool always_create,
+   const char* sysroot) {
+  // Make sure no one else can try and get or create a module while this
+  // function is actively working on it by doing an extra lock on the
+  // global mutex list.

Revert



Comment at: source/Core/ModuleList.cpp:766-770
+  if (sysroot != nullptr)
+resolved_module_spec.GetFileSpec().PrependPathComponent(sysroot);
+
+  module_sp.reset(new Module(resolved_module_spec));
   // Make sure there are a module and an object file since we can specify

Revert



Comment at: source/Target/Platform.cpp:228
 module_spec, module_sp, module_search_paths_ptr, old_module_sp_ptr,
-did_create_ptr, false);
+did_create_ptr, false, m_sdk_sysroot.AsCString());
   return GetRemoteSharedModule(module_spec, process, module_sp,

Revert



Comment at: source/Target/Platform.cpp:230
   return GetRemoteSharedModule(module_spec, process, module_sp,
[&](const ModuleSpec ) {
  Status error = ModuleList::GetSharedModule(

Here just make a module spec that has m_sdk_sysroot prepended if m_sdk_sysroot 
is set, and check for that file first. If that succeeds, return that, else do 
the same code as before.


Repository:
  rL LLVM

https://reviews.llvm.org/D49685



___
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-24 Thread Venkata Ramanaiah via Phabricator via lldb-commits
ramana-nvr added a comment.

Sorry, I am not helpful to you in providing a unit test case for this patch. I 
am still learning about test suite framework and/or trying to get the lldb test 
suite up and running.

Regarding how I got to this, it is not that I have run into some issue due to 
the original code. For my private port, I had to create a thread ID list 
similar to m_thread_ids and during code browsing for that I happened to see the 
code in concern and observed that clearing m_thread_pcs is wrong there.

After the process gets stopped, while trying to set the thread stop info, we 
also update the thread list (ProcessGDBRemote::SetThreadStopInfo() ---> 
Process::UpdateThreadListIfNeeded() ---> ProcessGDBRemote::UpdateThreadList()) 
and here we can reach UpdateThreadIDList() if m_thread_ids.empty() is true i.e. 
if we somehow failed to populate m_thread_ids while parsing the stop info 
packet and need to populate the m_thread_ids now.


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] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

2018-07-24 Thread Stefan Gränitz via Phabricator via lldb-commits
sgraenitz marked 2 inline comments as done.
sgraenitz added inline comments.



Comment at: source/Core/Mangled.cpp:310
+#elif defined(LLDB_USE_LLVM_DEMANGLER)
+llvm::ItaniumPartialDemangler IPD;
+bool demangle_err = IPD.partialDemangle(mangled_name);

labath wrote:
> erik.pilkington wrote:
> > sgraenitz wrote:
> > > erik.pilkington wrote:
> > > > sgraenitz wrote:
> > > > > sgraenitz wrote:
> > > > > > sgraenitz wrote:
> > > > > > > erik.pilkington wrote:
> > > > > > > > I think this is going to really tank performance: 
> > > > > > > > ItaniumPartialDemangler dynamically allocates a pretty big 
> > > > > > > > buffer on construction that it uses to store the AST (and reuse 
> > > > > > > > for subsequent calls to partialDemangle). Is there somewhere 
> > > > > > > > that you can put IPD it so that it stays alive between 
> > > > > > > > demangles?
> > > > > > > > 
> > > > > > > > An alternative, if its more convenient, would be to just put 
> > > > > > > > the buffer inline into ItaniumPartialDemangler, and `= delete` 
> > > > > > > > the move operations.
> > > > > > > Thanks for the remark, I didn't dig deep enough to see what's 
> > > > > > > going on in the `BumpPointerAllocator` class. I guess there is a 
> > > > > > > reason for having `ASTAllocator` in the `Db` struct as an 
> > > > > > > instance and thus allocating upfront, instead of having a pointer 
> > > > > > > there and postponing the instantiation to `Db::reset()`?
> > > > > > > 
> > > > > > > I am not familiar enough with the code yet to know how it will 
> > > > > > > look exactly, but having a heavy demangler in every `Mangled` per 
> > > > > > > se sounds unreasonable. There's just too many of them that don't 
> > > > > > > require demangling at all. For each successfully initiated 
> > > > > > > `partialDemangle()` I will need to keep one of course.
> > > > > > > 
> > > > > > > I will have a closer look on Monday. So far thanks for mentioning 
> > > > > > > that!
> > > > > > Well, right the pointer to `BumpPointerAllocator` won't solve 
> > > > > > anything. Ok will have a look.
> > > > > > ItaniumPartialDemangler dynamically allocates a pretty big buffer 
> > > > > > on construction
> > > > > 
> > > > > I think in the end each `Mangled` instance will have a pointer to IPD 
> > > > > field for lazy access to rich demangling info. This piece of code may 
> > > > > become something like:
> > > > > ```
> > > > > m_IPD = new ItaniumPartialDemangler();
> > > > > if (bool err = m_IPD->partialDemangle(mangled_name)) {
> > > > >   delete m_IPD; m_IPD = nullptr;
> > > > > }
> > > > > ```
> > > > > 
> > > > > In order to avoid unnecessary instantiations, we can consider to 
> > > > > filter symbols upfront that we know can't be demangled. E.g. we could 
> > > > > duplicate the simple checks from `Db::parse()` before creating a 
> > > > > `ItaniumPartialDemangler` instance.
> > > > > 
> > > > > Even the simple switch with the above code in place shows performance 
> > > > > improvements. So for now I would like to leave it this way and review 
> > > > > the issue after having the bigger patch, which will actually make use 
> > > > > of the rich demangle info.
> > > > > 
> > > > > What do you think?
> > > > Sure, if this is a performance win then I can't think of any reason not 
> > > > to do it.
> > > > 
> > > > In the future though, I don't think that having copies of IPD in each 
> > > > Mangled is a good idea, even if they are lazily initialized. The 
> > > > partially demangled state held in IPD is significantly larger than the 
> > > > demangled string, so I think it would lead to a lot more memory usage. 
> > > > Do you think it is possible to have just one instance of IPD that you 
> > > > could use to demangle all the symbols to their chopped-up state, and 
> > > > just store that instead?
> > > Yes if  that will be a bit more work, but also a possibility. I did a 
> > > small experiment making the IPD in line 288 `static`, to reuse a single 
> > > instance. That didn't affect runtimes much. I tried it several times and 
> > > got the same results again, but have no explanation.
> > > 
> > > You would expect a speedup from this right? Is there maybe cleanup work 
> > > that eats up time when reusing an instance? Maybe I have to revisit that.
> > Weird, I would have expected a speedup for making it static. My only guess 
> > is that `malloc` is just quickly handing back the buffer it used for the 
> > last IPD or something. I think the cleanup work IPD does should be about 
> > the same as the cost of construction.
> If reusing the same IPD object can bring significant benefit, I think the 
> right approach would be to change/extend/add the API (not in this patch) to 
> make it possible to use it naturally. There aren't many places that do batch 
> demangling of a large amount of symbols (`Symtab::InitNameIndexes` is one I 
> recall), so it shouldn't be too hard to modify them to do 

[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

2018-07-24 Thread Stefan Gränitz via Phabricator via lldb-commits
sgraenitz updated this revision to Diff 157001.
sgraenitz added a comment.

Minor improvements on naming and comments


https://reviews.llvm.org/D49612

Files:
  cmake/modules/LLDBConfig.cmake
  lldb.xcodeproj/project.pbxproj
  source/Core/Mangled.cpp
  unittests/Core/CMakeLists.txt
  unittests/Core/MangledTest.cpp

Index: unittests/Core/MangledTest.cpp
===
--- /dev/null
+++ unittests/Core/MangledTest.cpp
@@ -0,0 +1,38 @@
+//===-- MangledTest.cpp -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "lldb/Core/Mangled.h"
+
+using namespace lldb;
+using namespace lldb_private;
+
+TEST(MangledTest, ResultForValidName) {
+  ConstString MangledName("_ZN1a1b1cIiiiEEvm");
+  bool IsMangled = true;
+
+  Mangled TheMangled(MangledName, IsMangled);
+  const ConstString  =
+  TheMangled.GetDemangledName(eLanguageTypeC_plus_plus);
+
+  ConstString ExpectedResult("void a::b::c(unsigned long)");
+  EXPECT_EQ(ExpectedResult, TheDemangled);
+}
+
+TEST(MangledTest, EmptyForInvalidName) {
+  ConstString MangledName("_ZN1a1b1cmxktpEEvm");
+  bool IsMangled = true;
+
+  Mangled TheMangled(MangledName, IsMangled);
+  const ConstString  =
+  TheMangled.GetDemangledName(eLanguageTypeC_plus_plus);
+
+  EXPECT_EQ(ConstString(""), TheDemangled);
+}
Index: unittests/Core/CMakeLists.txt
===
--- unittests/Core/CMakeLists.txt
+++ unittests/Core/CMakeLists.txt
@@ -1,6 +1,7 @@
 add_lldb_unittest(LLDBCoreTests
   BroadcasterTest.cpp
   DataExtractorTest.cpp
+  MangledTest.cpp
   ListenerTest.cpp
   ScalarTest.cpp
   StateTest.cpp
Index: source/Core/Mangled.cpp
===
--- source/Core/Mangled.cpp
+++ source/Core/Mangled.cpp
@@ -16,34 +16,24 @@
 #pragma comment(lib, "dbghelp.lib")
 #endif
 
-#ifdef LLDB_USE_BUILTIN_DEMANGLER
-// Provide a fast-path demangler implemented in FastDemangle.cpp until it can
-// replace the existing C++ demangler with a complete implementation
-#include "lldb/Utility/FastDemangle.h"
-#include "llvm/Demangle/Demangle.h"
-#else
-// FreeBSD9-STABLE requires this to know about size_t in cxxabi.
-#include 
-#include 
-#endif
-
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Log.h"
 #include "lldb/Utility/Logging.h"
 #include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/Timer.h"
-#include "lldb/lldb-enumerations.h" // for LanguageType
+#include "lldb/lldb-enumerations.h"
 
 #include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
 #include "Plugins/Language/ObjC/ObjCLanguage.h"
 
-#include "llvm/ADT/StringRef.h"// for StringRef
-#include "llvm/Support/Compiler.h" // for LLVM_PRETT...
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Demangle/Demangle.h"
+#include "llvm/Support/Compiler.h"
 
-#include// for mutex, loc...
-#include   // for string
-#include  // for pair
+#include 
+#include 
+#include 
 
 #include 
 #include 
@@ -295,18 +285,15 @@
 break;
   }
   case eManglingSchemeItanium: {
-#ifdef LLDB_USE_BUILTIN_DEMANGLER
-if (log)
-  log->Printf("demangle itanium: %s", mangled_name);
-// Try to use the fast-path demangler first for the performance win,
-// falling back to the full demangler only when necessary
-demangled_name = FastDemangle(mangled_name, m_mangled.GetLength());
-if (!demangled_name)
-  demangled_name =
-  llvm::itaniumDemangle(mangled_name, NULL, NULL, NULL);
-#else
-demangled_name = abi::__cxa_demangle(mangled_name, NULL, NULL, NULL);
-#endif
+llvm::ItaniumPartialDemangler IPD;
+bool demangle_err = IPD.partialDemangle(mangled_name);
+if (!demangle_err) {
+  // Default buffer and size (realloc is used in case it's too small).
+  size_t demangled_size = 80;
+  demangled_name = static_cast(::malloc(demangled_size));
+  demangled_name = IPD.finishDemangle(demangled_name, _size);
+}
+
 if (log) {
   if (demangled_name)
 log->Printf("demangled itanium: %s -> \"%s\"", mangled_name,
Index: lldb.xcodeproj/project.pbxproj
===
--- lldb.xcodeproj/project.pbxproj
+++ lldb.xcodeproj/project.pbxproj
@@ -483,6 +483,7 @@
 		9A20573A1F3B8E7E00F6C293 /* MainLoopTest.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 9A2057301F3B8E7600F6C293 /* MainLoopTest.cpp */; };
 		8C3BD9961EF45DA50016C343 /* MainThreadCheckerRuntime.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 8C3BD9951EF45D9B0016C343 /* 

[Lldb-commits] [lldb] r337819 - Reimplement EventDataBytes::Dump to avoid DumpDataExtractor

2018-07-24 Thread Pavel Labath via lldb-commits
Author: labath
Date: Tue Jul 24 03:49:14 2018
New Revision: 337819

URL: http://llvm.org/viewvc/llvm-project?rev=337819=rev
Log:
Reimplement EventDataBytes::Dump to avoid DumpDataExtractor

This is the only external non-trivial dependency of the Event classes.

Added:
lldb/trunk/unittests/Core/EventTest.cpp
Modified:
lldb/trunk/source/Core/Event.cpp
lldb/trunk/unittests/Core/CMakeLists.txt

Modified: lldb/trunk/source/Core/Event.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Event.cpp?rev=337819=337818=337819=diff
==
--- lldb/trunk/source/Core/Event.cpp (original)
+++ lldb/trunk/source/Core/Event.cpp Tue Jul 24 03:49:14 2018
@@ -10,7 +10,6 @@
 #include "lldb/Core/Event.h"
 
 #include "lldb/Core/Broadcaster.h"
-#include "lldb/Core/DumpDataExtractor.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/Stream.h"
@@ -134,14 +133,13 @@ const ConstString ::GetFl
 void EventDataBytes::Dump(Stream *s) const {
   size_t num_printable_chars =
   std::count_if(m_bytes.begin(), m_bytes.end(), isprint);
-  if (num_printable_chars == m_bytes.size()) {
-s->Printf("\"%s\"", m_bytes.c_str());
-  } else if (!m_bytes.empty()) {
-DataExtractor data;
-data.SetData(m_bytes.data(), m_bytes.size(), endian::InlHostByteOrder());
-DumpDataExtractor(data, s, 0, eFormatBytes, 1, m_bytes.size(), 32,
-  LLDB_INVALID_ADDRESS, 0, 0);
-  }
+  if (num_printable_chars == m_bytes.size())
+s->Format("\"{0}\"", m_bytes);
+  else
+s->Format("{0:$[ ]@[x-2]}", llvm::make_range(
+ reinterpret_cast(m_bytes.data()),
+ reinterpret_cast(m_bytes.data() +
+   m_bytes.size(;
 }
 
 const void *EventDataBytes::GetBytes() const {

Modified: lldb/trunk/unittests/Core/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Core/CMakeLists.txt?rev=337819=337818=337819=diff
==
--- lldb/trunk/unittests/Core/CMakeLists.txt (original)
+++ lldb/trunk/unittests/Core/CMakeLists.txt Tue Jul 24 03:49:14 2018
@@ -1,6 +1,7 @@
 add_lldb_unittest(LLDBCoreTests
   BroadcasterTest.cpp
   DataExtractorTest.cpp
+  EventTest.cpp
   ListenerTest.cpp
   ScalarTest.cpp
   StateTest.cpp

Added: lldb/trunk/unittests/Core/EventTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Core/EventTest.cpp?rev=337819=auto
==
--- lldb/trunk/unittests/Core/EventTest.cpp (added)
+++ lldb/trunk/unittests/Core/EventTest.cpp Tue Jul 24 03:49:14 2018
@@ -0,0 +1,25 @@
+//===-- EventTest.cpp ---*- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "lldb/Core/Event.h"
+#include "lldb/Utility/StreamString.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+static std::string to_string(const EventDataBytes ) {
+  StreamString S;
+  E.Dump();
+  return S.GetString();
+}
+
+TEST(EventTest, DumpEventDataBytes) {
+  EXPECT_EQ(R"("foo")", to_string(EventDataBytes("foo")));
+  EXPECT_EQ("01 02 03", to_string(EventDataBytes("\x01\x02\x03")));
+}


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


[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

2018-07-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
JDevlieghere added inline comments.



Comment at: source/Core/Mangled.cpp:30
 
 #include "llvm/ADT/StringRef.h"// for StringRef
+#include "llvm/Demangle/Demangle.h"

While you're here I'd remove these redundant comments so this block looks more 
consistent. 



Comment at: source/Core/Mangled.cpp:292
+  // Default buffer and size (realloc is used in case it's too small).
+  size_t default_size = 80;
+  demangled_name = static_cast(::malloc(default_size));

Nit: I'd call this `demangled_size` because finishDemangle will update the 
value.


https://reviews.llvm.org/D49612



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


[Lldb-commits] [PATCH] D49612: Use LLVM's new ItaniumPartialDemangler in LLDB

2018-07-24 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

The patch makes sense to me. It's nice to get a performance boost, while 
reducing the number of demanglers at the same time.




Comment at: source/Core/Mangled.cpp:310
+#elif defined(LLDB_USE_LLVM_DEMANGLER)
+llvm::ItaniumPartialDemangler IPD;
+bool demangle_err = IPD.partialDemangle(mangled_name);

erik.pilkington wrote:
> sgraenitz wrote:
> > erik.pilkington wrote:
> > > sgraenitz wrote:
> > > > sgraenitz wrote:
> > > > > sgraenitz wrote:
> > > > > > erik.pilkington wrote:
> > > > > > > I think this is going to really tank performance: 
> > > > > > > ItaniumPartialDemangler dynamically allocates a pretty big buffer 
> > > > > > > on construction that it uses to store the AST (and reuse for 
> > > > > > > subsequent calls to partialDemangle). Is there somewhere that you 
> > > > > > > can put IPD it so that it stays alive between demangles?
> > > > > > > 
> > > > > > > An alternative, if its more convenient, would be to just put the 
> > > > > > > buffer inline into ItaniumPartialDemangler, and `= delete` the 
> > > > > > > move operations.
> > > > > > Thanks for the remark, I didn't dig deep enough to see what's going 
> > > > > > on in the `BumpPointerAllocator` class. I guess there is a reason 
> > > > > > for having `ASTAllocator` in the `Db` struct as an instance and 
> > > > > > thus allocating upfront, instead of having a pointer there and 
> > > > > > postponing the instantiation to `Db::reset()`?
> > > > > > 
> > > > > > I am not familiar enough with the code yet to know how it will look 
> > > > > > exactly, but having a heavy demangler in every `Mangled` per se 
> > > > > > sounds unreasonable. There's just too many of them that don't 
> > > > > > require demangling at all. For each successfully initiated 
> > > > > > `partialDemangle()` I will need to keep one of course.
> > > > > > 
> > > > > > I will have a closer look on Monday. So far thanks for mentioning 
> > > > > > that!
> > > > > Well, right the pointer to `BumpPointerAllocator` won't solve 
> > > > > anything. Ok will have a look.
> > > > > ItaniumPartialDemangler dynamically allocates a pretty big buffer on 
> > > > > construction
> > > > 
> > > > I think in the end each `Mangled` instance will have a pointer to IPD 
> > > > field for lazy access to rich demangling info. This piece of code may 
> > > > become something like:
> > > > ```
> > > > m_IPD = new ItaniumPartialDemangler();
> > > > if (bool err = m_IPD->partialDemangle(mangled_name)) {
> > > >   delete m_IPD; m_IPD = nullptr;
> > > > }
> > > > ```
> > > > 
> > > > In order to avoid unnecessary instantiations, we can consider to filter 
> > > > symbols upfront that we know can't be demangled. E.g. we could 
> > > > duplicate the simple checks from `Db::parse()` before creating a 
> > > > `ItaniumPartialDemangler` instance.
> > > > 
> > > > Even the simple switch with the above code in place shows performance 
> > > > improvements. So for now I would like to leave it this way and review 
> > > > the issue after having the bigger patch, which will actually make use 
> > > > of the rich demangle info.
> > > > 
> > > > What do you think?
> > > Sure, if this is a performance win then I can't think of any reason not 
> > > to do it.
> > > 
> > > In the future though, I don't think that having copies of IPD in each 
> > > Mangled is a good idea, even if they are lazily initialized. The 
> > > partially demangled state held in IPD is significantly larger than the 
> > > demangled string, so I think it would lead to a lot more memory usage. Do 
> > > you think it is possible to have just one instance of IPD that you could 
> > > use to demangle all the symbols to their chopped-up state, and just store 
> > > that instead?
> > Yes if  that will be a bit more work, but also a possibility. I did a small 
> > experiment making the IPD in line 288 `static`, to reuse a single instance. 
> > That didn't affect runtimes much. I tried it several times and got the same 
> > results again, but have no explanation.
> > 
> > You would expect a speedup from this right? Is there maybe cleanup work 
> > that eats up time when reusing an instance? Maybe I have to revisit that.
> Weird, I would have expected a speedup for making it static. My only guess is 
> that `malloc` is just quickly handing back the buffer it used for the last 
> IPD or something. I think the cleanup work IPD does should be about the same 
> as the cost of construction.
If reusing the same IPD object can bring significant benefit, I think the right 
approach would be to change/extend/add the API (not in this patch) to make it 
possible to use it naturally. There aren't many places that do batch demangling 
of a large amount of symbols (`Symtab::InitNameIndexes` is one I recall), so it 
shouldn't be too hard to modify them to do something smarter.


https://reviews.llvm.org/D49612



___
lldb-commits mailing 

[Lldb-commits] [PATCH] D49685: LLDB does not respect platform sysroot when loading core on Linux

2018-07-24 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Which version is this patch based on? The line numbers don't seem to match what 
I see on master.

Could you rebase the patch to master, and upload the patch with a full diff 
(e.g. via `git diff -U`, see https://llvm.org/docs/Phabricator.html#id3).




Comment at: include/lldb/Core/ModuleList.h:545
+bool always_create = false,
+const char* sysroot = nullptr);
   static bool RemoveSharedModule(lldb::ModuleSP _sp);

Please make this an `llvm::StringRef` (and then change `AsCString` to 
`GetStringRef` below).


Repository:
  rL LLVM

https://reviews.llvm.org/D49685



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