[Lldb-commits] [PATCH] D47742: Change SWIG output directory when building LLDB.framework with CMake

2018-06-04 Thread Stephane Sezer via Phabricator via lldb-commits
sas accepted this revision.
sas added inline comments.
This revision is now accepted and ready to land.



Comment at: CMakeLists.txt:149
 DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/scripts/finishSwigWrapperClasses.py
-DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/scripts/lldb.py
 COMMENT "Python script sym-linking LLDB Python API")

This path...



Comment at: scripts/CMakeLists.txt:38
   OUTPUT ${LLDB_WRAP_PYTHON}
-  OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/lldb.py
   DEPENDS ${SWIG_SOURCES}

...and this path were not the same before your change. Was that a bug that 
you're fixing?


https://reviews.llvm.org/D47742



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


[Lldb-commits] [PATCH] D47742: Change SWIG output directory when building LLDB.framework with CMake

2018-06-04 Thread Stephane Sezer via Phabricator via lldb-commits
sas added inline comments.



Comment at: scripts/CMakeLists.txt:38
   OUTPUT ${LLDB_WRAP_PYTHON}
-  OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/lldb.py
   DEPENDS ${SWIG_SOURCES}

xiaobai wrote:
> sas wrote:
> > ...and this path were not the same before your change. Was that a bug that 
> > you're fixing?
> Yes. Those paths were actually the same if you were not building the 
> framework, which is why this bug never surfaced. Specifically, 
> `${CMAKE_CURRENT_BINARY_DIR}` in scripts/CMakeLists.txt resolves to  the same 
> path as `${CMAKE_CURRENT_BINARY_DIR}/scripts` in the top level CMakeLists.txt.
> 
> The pitfall is the assumption that this path is always the same as 
> `${LLDB_PYTHON_TARGET_DIR}`, which it is not if you are trying to build the 
> framework.
Makes sense. This should be good to go then.


https://reviews.llvm.org/D47742



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


[Lldb-commits] [PATCH] D47801: Make lldb tools dependent on liblldb target when building LLDB.framework with CMake

2018-06-05 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a comment.

Don't we risk creating circular dependencies with this? What happens when a 
tool depends on liblldb? you'll have theTool depend on liblldb and liblldb 
depend on theTool.


https://reviews.llvm.org/D47801



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


[Lldb-commits] [PATCH] D47792: Fix up Info.plist when building LLDB.framework with CMake

2018-06-05 Thread Stephane Sezer via Phabricator via lldb-commits
sas added inline comments.



Comment at: source/API/CMakeLists.txt:184
+  set(EXECUTABLE_NAME "LLDB")
+  set(CURRENT_PROJECT_VERSION "360.99.0")
   set_target_properties(liblldb PROPERTIES

Hardcoding this here isn't ideal. Where did you get this number from? Is there 
a way to fetch it from a single location?


https://reviews.llvm.org/D47792



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


[Lldb-commits] [PATCH] D47801: Make lldb tools dependent on liblldb target when building LLDB.framework with CMake

2018-06-06 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a comment.

> One idea I had was to introduce another target for the framwork itself, e.g. 
> lldbFramework, which gets built if LLDB_BUILD_FRAMEWORK is set. It would 
> depend on liblldb and all the necessary tools, headers, etc, that the 
> framework would need. That way liblldb can depend only on what it needs to 
> build instead of treating it as both the library and the entire framework. 
> How do you feel about this?

I think this is the cleanest way of handling this. If @labath agrees and if 
it's not too much of a pain to implement in cmake, we should do this.


https://reviews.llvm.org/D47801



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


[Lldb-commits] [PATCH] D47792: Fix up Info.plist when building LLDB.framework with CMake

2018-06-18 Thread Stephane Sezer via Phabricator via lldb-commits
sas accepted this revision.
sas added a comment.
This revision is now accepted and ready to land.

LGTM for now.


https://reviews.llvm.org/D47792



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


[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

2017-11-06 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a comment.

@davide: I agree, the extra dependency is annoying.

FWIW, I was trying to run tests on Windows and nothing worked because lld 
wasn't built. Maybe the behavior is different on Windows and on other hosts? I 
just assumed this was the new default because I saw a couple of changes to how 
tests are run recently.


https://reviews.llvm.org/D39689



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


[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

2017-11-06 Thread Stephane Sezer via Phabricator via lldb-commits
sas updated this revision to Diff 121750.
sas added a comment.

Check only on Windows.


https://reviews.llvm.org/D39689

Files:
  test/CMakeLists.txt


Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -113,6 +113,13 @@
 # to run tests.
 if (TARGET clang)
   add_dependencies(check-lldb clang)
+  if (CMAKE_SYSTEM_NAME MATCHES "Windows")
+if (TARGET lld)
+  add_dependencies(check-lldb lld)
+else ()
+  message(WARNING "lld required to test LLDB on Windows")
+endif ()
+  endif ()
 endif()
 
 add_custom_target(lldb-test-depends DEPENDS ${LLDB_TEST_DEPENDS})


Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -113,6 +113,13 @@
 # to run tests.
 if (TARGET clang)
   add_dependencies(check-lldb clang)
+  if (CMAKE_SYSTEM_NAME MATCHES "Windows")
+if (TARGET lld)
+  add_dependencies(check-lldb lld)
+else ()
+  message(WARNING "lld required to test LLDB on Windows")
+endif ()
+  endif ()
 endif()
 
 add_custom_target(lldb-test-depends DEPENDS ${LLDB_TEST_DEPENDS})
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

2017-11-06 Thread Stephane Sezer via Phabricator via lldb-commits
sas updated this revision to Diff 121760.
sas added a comment.

Move check out of if (TARGET clang) block.


https://reviews.llvm.org/D39689

Files:
  test/CMakeLists.txt


Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -115,6 +115,15 @@
   add_dependencies(check-lldb clang)
 endif()
 
+# LLD is required to link test executables on Windows.
+if (CMAKE_SYSTEM_NAME MATCHES "Windows")
+  if (TARGET lld)
+add_dependencies(check-lldb lld)
+  else ()
+message(WARNING "lld required to test LLDB on Windows")
+  endif ()
+endif ()
+
 add_custom_target(lldb-test-depends DEPENDS ${LLDB_TEST_DEPENDS})
 # This will add LLDB's test dependencies to the depenednecies for check-all and
 # include them in the test-depends target.


Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -115,6 +115,15 @@
   add_dependencies(check-lldb clang)
 endif()
 
+# LLD is required to link test executables on Windows.
+if (CMAKE_SYSTEM_NAME MATCHES "Windows")
+  if (TARGET lld)
+add_dependencies(check-lldb lld)
+  else ()
+message(WARNING "lld required to test LLDB on Windows")
+  endif ()
+endif ()
+
 add_custom_target(lldb-test-depends DEPENDS ${LLDB_TEST_DEPENDS})
 # This will add LLDB's test dependencies to the depenednecies for check-all and
 # include them in the test-depends target.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D39689: Add a dependency from check-lldb on lld

2017-11-06 Thread Stephane Sezer via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL317501: Add a dependency from check-lldb on lld (authored by 
sas).

Repository:
  rL LLVM

https://reviews.llvm.org/D39689

Files:
  lldb/trunk/test/CMakeLists.txt


Index: lldb/trunk/test/CMakeLists.txt
===
--- lldb/trunk/test/CMakeLists.txt
+++ lldb/trunk/test/CMakeLists.txt
@@ -115,6 +115,15 @@
   add_dependencies(check-lldb clang)
 endif()
 
+# LLD is required to link test executables on Windows.
+if (CMAKE_SYSTEM_NAME MATCHES "Windows")
+  if (TARGET lld)
+add_dependencies(check-lldb lld)
+  else ()
+message(WARNING "lld required to test LLDB on Windows")
+  endif ()
+endif ()
+
 add_custom_target(lldb-test-depends DEPENDS ${LLDB_TEST_DEPENDS})
 # This will add LLDB's test dependencies to the depenednecies for check-all and
 # include them in the test-depends target.


Index: lldb/trunk/test/CMakeLists.txt
===
--- lldb/trunk/test/CMakeLists.txt
+++ lldb/trunk/test/CMakeLists.txt
@@ -115,6 +115,15 @@
   add_dependencies(check-lldb clang)
 endif()
 
+# LLD is required to link test executables on Windows.
+if (CMAKE_SYSTEM_NAME MATCHES "Windows")
+  if (TARGET lld)
+add_dependencies(check-lldb lld)
+  else ()
+message(WARNING "lld required to test LLDB on Windows")
+  endif ()
+endif ()
+
 add_custom_target(lldb-test-depends DEPENDS ${LLDB_TEST_DEPENDS})
 # This will add LLDB's test dependencies to the depenednecies for check-all and
 # include them in the test-depends target.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D39692: Disable tests in lang/c/shared_lib on Windows

2017-11-06 Thread Stephane Sezer via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL317529: Disable tests in lang/c/shared_lib on Windows 
(authored by sas).

Repository:
  rL LLVM

https://reviews.llvm.org/D39692

Files:
  lldb/trunk/packages/Python/lldbsuite/test/lang/c/shared_lib/TestSharedLib.py


Index: 
lldb/trunk/packages/Python/lldbsuite/test/lang/c/shared_lib/TestSharedLib.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/shared_lib/TestSharedLib.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/shared_lib/TestSharedLib.py
@@ -5,6 +5,7 @@
 
 import unittest2
 import lldb
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 import lldbsuite.test.lldbutil as lldbutil
 
@@ -34,10 +35,12 @@
 "expression GetMeASubFoo(my_foo_ptr)",
 startstr="(sub_foo *) $")
 
+@expectedFailureAll(oslist=["windows"])
 def test_expr(self):
 """Test that types work when defined in a shared library and 
forward-declared in the main executable"""
 self.common_test_expr(True)
 
+@expectedFailureAll(oslist=["windows"])
 def test_expr_no_preload(self):
 """Test that types work when defined in a shared library and 
forward-declared in the main executable, but with preloading disabled"""
 self.common_test_expr(False)


Index: lldb/trunk/packages/Python/lldbsuite/test/lang/c/shared_lib/TestSharedLib.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/c/shared_lib/TestSharedLib.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/c/shared_lib/TestSharedLib.py
@@ -5,6 +5,7 @@
 
 import unittest2
 import lldb
+from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 import lldbsuite.test.lldbutil as lldbutil
 
@@ -34,10 +35,12 @@
 "expression GetMeASubFoo(my_foo_ptr)",
 startstr="(sub_foo *) $")
 
+@expectedFailureAll(oslist=["windows"])
 def test_expr(self):
 """Test that types work when defined in a shared library and forward-declared in the main executable"""
 self.common_test_expr(True)
 
+@expectedFailureAll(oslist=["windows"])
 def test_expr_no_preload(self):
 """Test that types work when defined in a shared library and forward-declared in the main executable, but with preloading disabled"""
 self.common_test_expr(False)
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

2017-11-06 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a comment.

Friendly ping.


https://reviews.llvm.org/D39315



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


[Lldb-commits] [PATCH] D40517: Mark UUID::GetByteSize() const

2017-11-27 Thread Stephane Sezer via Phabricator via lldb-commits
sas created this revision.

This method doesn't modify anything in the object it's called on so we
can mark it const to make it usable in a const context.


https://reviews.llvm.org/D40517

Files:
  include/lldb/Utility/UUID.h
  source/Utility/UUID.cpp


Index: source/Utility/UUID.cpp
===
--- source/Utility/UUID.cpp
+++ source/Utility/UUID.cpp
@@ -109,7 +109,7 @@
   return false;
 }
 
-size_t UUID::GetByteSize() { return m_num_uuid_bytes; }
+size_t UUID::GetByteSize() const { return m_num_uuid_bytes; }
 
 bool UUID::IsValid() const {
   return m_uuid[0] || m_uuid[1] || m_uuid[2] || m_uuid[3] || m_uuid[4] ||
Index: include/lldb/Utility/UUID.h
===
--- include/lldb/Utility/UUID.h
+++ include/lldb/Utility/UUID.h
@@ -46,7 +46,7 @@
 
   const void *GetBytes() const;
 
-  size_t GetByteSize();
+  size_t GetByteSize() const;
 
   bool IsValid() const;
 


Index: source/Utility/UUID.cpp
===
--- source/Utility/UUID.cpp
+++ source/Utility/UUID.cpp
@@ -109,7 +109,7 @@
   return false;
 }
 
-size_t UUID::GetByteSize() { return m_num_uuid_bytes; }
+size_t UUID::GetByteSize() const { return m_num_uuid_bytes; }
 
 bool UUID::IsValid() const {
   return m_uuid[0] || m_uuid[1] || m_uuid[2] || m_uuid[3] || m_uuid[4] ||
Index: include/lldb/Utility/UUID.h
===
--- include/lldb/Utility/UUID.h
+++ include/lldb/Utility/UUID.h
@@ -46,7 +46,7 @@
 
   const void *GetBytes() const;
 
-  size_t GetByteSize();
+  size_t GetByteSize() const;
 
   bool IsValid() const;
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D40517: Mark UUID::GetByteSize() const

2017-11-27 Thread Stephane Sezer via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL319095: Mark UUID::GetByteSize() const (authored by sas).

Repository:
  rL LLVM

https://reviews.llvm.org/D40517

Files:
  lldb/trunk/include/lldb/Utility/UUID.h
  lldb/trunk/source/Utility/UUID.cpp


Index: lldb/trunk/include/lldb/Utility/UUID.h
===
--- lldb/trunk/include/lldb/Utility/UUID.h
+++ lldb/trunk/include/lldb/Utility/UUID.h
@@ -46,7 +46,7 @@
 
   const void *GetBytes() const;
 
-  size_t GetByteSize();
+  size_t GetByteSize() const;
 
   bool IsValid() const;
 
Index: lldb/trunk/source/Utility/UUID.cpp
===
--- lldb/trunk/source/Utility/UUID.cpp
+++ lldb/trunk/source/Utility/UUID.cpp
@@ -109,7 +109,7 @@
   return false;
 }
 
-size_t UUID::GetByteSize() { return m_num_uuid_bytes; }
+size_t UUID::GetByteSize() const { return m_num_uuid_bytes; }
 
 bool UUID::IsValid() const {
   return m_uuid[0] || m_uuid[1] || m_uuid[2] || m_uuid[3] || m_uuid[4] ||


Index: lldb/trunk/include/lldb/Utility/UUID.h
===
--- lldb/trunk/include/lldb/Utility/UUID.h
+++ lldb/trunk/include/lldb/Utility/UUID.h
@@ -46,7 +46,7 @@
 
   const void *GetBytes() const;
 
-  size_t GetByteSize();
+  size_t GetByteSize() const;
 
   bool IsValid() const;
 
Index: lldb/trunk/source/Utility/UUID.cpp
===
--- lldb/trunk/source/Utility/UUID.cpp
+++ lldb/trunk/source/Utility/UUID.cpp
@@ -109,7 +109,7 @@
   return false;
 }
 
-size_t UUID::GetByteSize() { return m_num_uuid_bytes; }
+size_t UUID::GetByteSize() const { return m_num_uuid_bytes; }
 
 bool UUID::IsValid() const {
   return m_uuid[0] || m_uuid[1] || m_uuid[2] || m_uuid[3] || m_uuid[4] ||
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D40519: Remove some duplicated code in UUID.cpp

2017-11-27 Thread Stephane Sezer via Phabricator via lldb-commits
sas created this revision.

Formatting needs to be done only once. Ran check-lldb and nothing changes.


https://reviews.llvm.org/D40519

Files:
  source/Utility/UUID.cpp


Index: source/Utility/UUID.cpp
===
--- source/Utility/UUID.cpp
+++ source/Utility/UUID.cpp
@@ -74,14 +74,8 @@
 }
 
 void UUID::Dump(Stream *s) const {
-  const uint8_t *u = (const uint8_t *)GetBytes();
-  s->Printf("%2.2X%2.2X%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X%"
-"2.2X%2.2X%2.2X%2.2X",
-u[0], u[1], u[2], u[3], u[4], u[5], u[6], u[7], u[8], u[9], u[10],
-u[11], u[12], u[13], u[14], u[15]);
-  if (m_num_uuid_bytes == 20) {
-s->Printf("-%2.2X%2.2X%2.2X%2.2X", u[16], u[17], u[18], u[19]);
-  }
+  auto str = GetAsString();
+  s->Printf("%s", str.c_str());
 }
 
 bool UUID::SetBytes(const void *uuid_bytes, uint32_t num_uuid_bytes) {


Index: source/Utility/UUID.cpp
===
--- source/Utility/UUID.cpp
+++ source/Utility/UUID.cpp
@@ -74,14 +74,8 @@
 }
 
 void UUID::Dump(Stream *s) const {
-  const uint8_t *u = (const uint8_t *)GetBytes();
-  s->Printf("%2.2X%2.2X%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X%"
-"2.2X%2.2X%2.2X%2.2X",
-u[0], u[1], u[2], u[3], u[4], u[5], u[6], u[7], u[8], u[9], u[10],
-u[11], u[12], u[13], u[14], u[15]);
-  if (m_num_uuid_bytes == 20) {
-s->Printf("-%2.2X%2.2X%2.2X%2.2X", u[16], u[17], u[18], u[19]);
-  }
+  auto str = GetAsString();
+  s->Printf("%s", str.c_str());
 }
 
 bool UUID::SetBytes(const void *uuid_bytes, uint32_t num_uuid_bytes) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D40519: Remove some duplicated code in UUID.cpp

2017-11-27 Thread Stephane Sezer via Phabricator via lldb-commits
sas updated this revision to Diff 124499.
sas added a comment.

Address @zturner's comment and remove a useless temporary variable.


https://reviews.llvm.org/D40519

Files:
  source/Utility/UUID.cpp


Index: source/Utility/UUID.cpp
===
--- source/Utility/UUID.cpp
+++ source/Utility/UUID.cpp
@@ -74,14 +74,7 @@
 }
 
 void UUID::Dump(Stream *s) const {
-  const uint8_t *u = (const uint8_t *)GetBytes();
-  s->Printf("%2.2X%2.2X%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X%"
-"2.2X%2.2X%2.2X%2.2X",
-u[0], u[1], u[2], u[3], u[4], u[5], u[6], u[7], u[8], u[9], u[10],
-u[11], u[12], u[13], u[14], u[15]);
-  if (m_num_uuid_bytes == 20) {
-s->Printf("-%2.2X%2.2X%2.2X%2.2X", u[16], u[17], u[18], u[19]);
-  }
+  s->PutCString(GetAsString().c_str());
 }
 
 bool UUID::SetBytes(const void *uuid_bytes, uint32_t num_uuid_bytes) {


Index: source/Utility/UUID.cpp
===
--- source/Utility/UUID.cpp
+++ source/Utility/UUID.cpp
@@ -74,14 +74,7 @@
 }
 
 void UUID::Dump(Stream *s) const {
-  const uint8_t *u = (const uint8_t *)GetBytes();
-  s->Printf("%2.2X%2.2X%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X%"
-"2.2X%2.2X%2.2X%2.2X",
-u[0], u[1], u[2], u[3], u[4], u[5], u[6], u[7], u[8], u[9], u[10],
-u[11], u[12], u[13], u[14], u[15]);
-  if (m_num_uuid_bytes == 20) {
-s->Printf("-%2.2X%2.2X%2.2X%2.2X", u[16], u[17], u[18], u[19]);
-  }
+  s->PutCString(GetAsString().c_str());
 }
 
 bool UUID::SetBytes(const void *uuid_bytes, uint32_t num_uuid_bytes) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D40519: Remove some duplicated code in UUID.cpp

2017-11-27 Thread Stephane Sezer via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL319132: Remove some duplicated code in UUID.cpp (authored by 
sas).

Repository:
  rL LLVM

https://reviews.llvm.org/D40519

Files:
  lldb/trunk/source/Utility/UUID.cpp


Index: lldb/trunk/source/Utility/UUID.cpp
===
--- lldb/trunk/source/Utility/UUID.cpp
+++ lldb/trunk/source/Utility/UUID.cpp
@@ -74,14 +74,7 @@
 }
 
 void UUID::Dump(Stream *s) const {
-  const uint8_t *u = (const uint8_t *)GetBytes();
-  s->Printf("%2.2X%2.2X%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X%"
-"2.2X%2.2X%2.2X%2.2X",
-u[0], u[1], u[2], u[3], u[4], u[5], u[6], u[7], u[8], u[9], u[10],
-u[11], u[12], u[13], u[14], u[15]);
-  if (m_num_uuid_bytes == 20) {
-s->Printf("-%2.2X%2.2X%2.2X%2.2X", u[16], u[17], u[18], u[19]);
-  }
+  s->PutCString(GetAsString().c_str());
 }
 
 bool UUID::SetBytes(const void *uuid_bytes, uint32_t num_uuid_bytes) {


Index: lldb/trunk/source/Utility/UUID.cpp
===
--- lldb/trunk/source/Utility/UUID.cpp
+++ lldb/trunk/source/Utility/UUID.cpp
@@ -74,14 +74,7 @@
 }
 
 void UUID::Dump(Stream *s) const {
-  const uint8_t *u = (const uint8_t *)GetBytes();
-  s->Printf("%2.2X%2.2X%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X-%2.2X%2.2X%"
-"2.2X%2.2X%2.2X%2.2X",
-u[0], u[1], u[2], u[3], u[4], u[5], u[6], u[7], u[8], u[9], u[10],
-u[11], u[12], u[13], u[14], u[15]);
-  if (m_num_uuid_bytes == 20) {
-s->Printf("-%2.2X%2.2X%2.2X%2.2X", u[16], u[17], u[18], u[19]);
-  }
+  s->PutCString(GetAsString().c_str());
 }
 
 bool UUID::SetBytes(const void *uuid_bytes, uint32_t num_uuid_bytes) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D40536: Simplify UUID constructors

2017-11-27 Thread Stephane Sezer via Phabricator via lldb-commits
sas created this revision.

This remove a small amount of duplicated code.


https://reviews.llvm.org/D40536

Files:
  source/Utility/UUID.cpp


Index: source/Utility/UUID.cpp
===
--- source/Utility/UUID.cpp
+++ source/Utility/UUID.cpp
@@ -21,11 +21,10 @@
 
 namespace lldb_private {
 
-UUID::UUID() : m_num_uuid_bytes(16) { ::memset(m_uuid, 0, sizeof(m_uuid)); }
+UUID::UUID() { Clear(); }
 
 UUID::UUID(const UUID &rhs) {
-  m_num_uuid_bytes = rhs.m_num_uuid_bytes;
-  ::memcpy(m_uuid, rhs.m_uuid, sizeof(m_uuid));
+  SetBytes(rhs.m_uuid, rhs.m_num_uuid_bytes);
 }
 
 UUID::UUID(const void *uuid_bytes, uint32_t num_uuid_bytes) {


Index: source/Utility/UUID.cpp
===
--- source/Utility/UUID.cpp
+++ source/Utility/UUID.cpp
@@ -21,11 +21,10 @@
 
 namespace lldb_private {
 
-UUID::UUID() : m_num_uuid_bytes(16) { ::memset(m_uuid, 0, sizeof(m_uuid)); }
+UUID::UUID() { Clear(); }
 
 UUID::UUID(const UUID &rhs) {
-  m_num_uuid_bytes = rhs.m_num_uuid_bytes;
-  ::memcpy(m_uuid, rhs.m_uuid, sizeof(m_uuid));
+  SetBytes(rhs.m_uuid, rhs.m_num_uuid_bytes);
 }
 
 UUID::UUID(const void *uuid_bytes, uint32_t num_uuid_bytes) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D40537: Simplify UUID::IsValid()

2017-11-27 Thread Stephane Sezer via Phabricator via lldb-commits
sas created this revision.

This change also prevents looking further than the size of the current
UUID.


https://reviews.llvm.org/D40537

Files:
  source/Utility/UUID.cpp


Index: source/Utility/UUID.cpp
===
--- source/Utility/UUID.cpp
+++ source/Utility/UUID.cpp
@@ -15,9 +15,12 @@
 #include "llvm/ADT/StringRef.h"
 
 // C Includes
+// C++ Includes
 #include 
 #include 
 #include 
+#include 
+#include 
 
 namespace lldb_private {
 
@@ -104,10 +107,8 @@
 size_t UUID::GetByteSize() const { return m_num_uuid_bytes; }
 
 bool UUID::IsValid() const {
-  return m_uuid[0] || m_uuid[1] || m_uuid[2] || m_uuid[3] || m_uuid[4] ||
- m_uuid[5] || m_uuid[6] || m_uuid[7] || m_uuid[8] || m_uuid[9] ||
- m_uuid[10] || m_uuid[11] || m_uuid[12] || m_uuid[13] || m_uuid[14] ||
- m_uuid[15] || m_uuid[16] || m_uuid[17] || m_uuid[18] || m_uuid[19];
+  return std::any_of(std::begin(m_uuid), std::begin(m_uuid) + m_num_uuid_bytes,
+ [](uint32_t val) { return val != 0; });
 }
 
 static inline int xdigit_to_int(char ch) {


Index: source/Utility/UUID.cpp
===
--- source/Utility/UUID.cpp
+++ source/Utility/UUID.cpp
@@ -15,9 +15,12 @@
 #include "llvm/ADT/StringRef.h"
 
 // C Includes
+// C++ Includes
 #include 
 #include 
 #include 
+#include 
+#include 
 
 namespace lldb_private {
 
@@ -104,10 +107,8 @@
 size_t UUID::GetByteSize() const { return m_num_uuid_bytes; }
 
 bool UUID::IsValid() const {
-  return m_uuid[0] || m_uuid[1] || m_uuid[2] || m_uuid[3] || m_uuid[4] ||
- m_uuid[5] || m_uuid[6] || m_uuid[7] || m_uuid[8] || m_uuid[9] ||
- m_uuid[10] || m_uuid[11] || m_uuid[12] || m_uuid[13] || m_uuid[14] ||
- m_uuid[15] || m_uuid[16] || m_uuid[17] || m_uuid[18] || m_uuid[19];
+  return std::any_of(std::begin(m_uuid), std::begin(m_uuid) + m_num_uuid_bytes,
+ [](uint32_t val) { return val != 0; });
 }
 
 static inline int xdigit_to_int(char ch) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D40539: Allow using the object file name for debug symbol resolution

2017-11-27 Thread Stephane Sezer via Phabricator via lldb-commits
sas created this revision.
Herald added subscribers: JDevlieghere, emaste.

In ObjectFileELF we try to read the `.gnu_debuglink` section of the ELF
file to determine the name of the debug symbols file. If this section
does not exist, we stop the search. Instead, what we should do is locate
a file with the same name as the ELF binary, which is the default
behavior when there isn't any `.gnu_debuglink` section present.

Ran check-lldb to make sure this doesn't break anything.


https://reviews.llvm.org/D40539

Files:
  source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp


Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1000,7 +1000,11 @@
   if (!m_gnu_debuglink_file.empty()) {
 FileSpec file_spec(m_gnu_debuglink_file, false);
 file_spec_list.Append(file_spec);
+  } else {
+FileSpec file_spec(GetFileSpec().GetFilename().AsCString(), false);
+file_spec_list.Append(file_spec);
   }
+
   return file_spec_list;
 }
 


Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -1000,7 +1000,11 @@
   if (!m_gnu_debuglink_file.empty()) {
 FileSpec file_spec(m_gnu_debuglink_file, false);
 file_spec_list.Append(file_spec);
+  } else {
+FileSpec file_spec(GetFileSpec().GetFilename().AsCString(), false);
+file_spec_list.Append(file_spec);
   }
+
   return file_spec_list;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D40536: Simplify UUID constructors

2017-11-28 Thread Stephane Sezer via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL319191: Simplify UUID constructors (authored by sas).

Repository:
  rL LLVM

https://reviews.llvm.org/D40536

Files:
  lldb/trunk/source/Utility/UUID.cpp


Index: lldb/trunk/source/Utility/UUID.cpp
===
--- lldb/trunk/source/Utility/UUID.cpp
+++ lldb/trunk/source/Utility/UUID.cpp
@@ -21,11 +21,10 @@
 
 namespace lldb_private {
 
-UUID::UUID() : m_num_uuid_bytes(16) { ::memset(m_uuid, 0, sizeof(m_uuid)); }
+UUID::UUID() { Clear(); }
 
 UUID::UUID(const UUID &rhs) {
-  m_num_uuid_bytes = rhs.m_num_uuid_bytes;
-  ::memcpy(m_uuid, rhs.m_uuid, sizeof(m_uuid));
+  SetBytes(rhs.m_uuid, rhs.m_num_uuid_bytes);
 }
 
 UUID::UUID(const void *uuid_bytes, uint32_t num_uuid_bytes) {


Index: lldb/trunk/source/Utility/UUID.cpp
===
--- lldb/trunk/source/Utility/UUID.cpp
+++ lldb/trunk/source/Utility/UUID.cpp
@@ -21,11 +21,10 @@
 
 namespace lldb_private {
 
-UUID::UUID() : m_num_uuid_bytes(16) { ::memset(m_uuid, 0, sizeof(m_uuid)); }
+UUID::UUID() { Clear(); }
 
 UUID::UUID(const UUID &rhs) {
-  m_num_uuid_bytes = rhs.m_num_uuid_bytes;
-  ::memcpy(m_uuid, rhs.m_uuid, sizeof(m_uuid));
+  SetBytes(rhs.m_uuid, rhs.m_num_uuid_bytes);
 }
 
 UUID::UUID(const void *uuid_bytes, uint32_t num_uuid_bytes) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D40539: Allow using the object file name for debug symbol resolution

2017-11-28 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a comment.

Basically, if you have a `.debug` directory in the same directory where the 
original object file is, you can have debug symbols there. For instance, you 
can have:
/my/project/myElf.exe
/my/project/.debug/myElf.exe

with the first file being a standard stripped elf file, and the second one 
being the associated debug symbols.

One other place where these names can be used is with the 
`target.debug-file-search-paths` setting.


https://reviews.llvm.org/D40539



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


[Lldb-commits] [PATCH] D40537: Simplify UUID::IsValid()

2017-11-28 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a comment.

In https://reviews.llvm.org/D40537#937196, @zturner wrote:

> You could use llvm's range adapters to make this slightly better.
>
>   auto Bytes = makeArrayRef(m_uuid, m_num_uuid_bytes);
>   return llvm::find(Bytes, 0) != Bytes.end();
>   
>
> Another option would just be `return *this != UUID(m_num_uuid_bytes);`


We want at least one **non-zero** element, we don't want a zero-element, so the 
proposed code wouldn't work. I'm not sure if there's an llvm utility that 
allows doing that directly without having to pass a lambda.


https://reviews.llvm.org/D40537



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


[Lldb-commits] [PATCH] D40537: Simplify UUID::IsValid()

2017-11-28 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a comment.

In https://reviews.llvm.org/D40537#937862, @clayborg wrote:

> A better solution would be to initialize UUID::m_num_uuid_bytes with zero and 
> only set it to a valid value if we set bytes into it. Then UUID::IsValid() 
> becomes easy:
>
>   bool UUID::IsValid() const { return m_num_uuid_bytes > 0; }
>
>
> This would allows us to actually have a UUID value that is valid and all 
> zeroes. A few comments would need to be fixed as it currently assumes length 
> is 16 or 20.


Yes but the current default constructor of the `UUID` class creates a 16-bytes 
all-zeroes UUID. I'm not sure I want to be changing the default behavior that 
the rest of lldb might be depending on currently.


https://reviews.llvm.org/D40537



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


[Lldb-commits] [PATCH] D40537: Simplify UUID::IsValid()

2017-11-28 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a comment.

In https://reviews.llvm.org/D40537#937880, @zturner wrote:

> In https://reviews.llvm.org/D40537#937866, @sas wrote:
>
> > In https://reviews.llvm.org/D40537#937196, @zturner wrote:
> >
> > > You could use llvm's range adapters to make this slightly better.
> > >
> > >   auto Bytes = makeArrayRef(m_uuid, m_num_uuid_bytes);
> > >   return llvm::find(Bytes, 0) != Bytes.end();
> > >   
> > >
> > > Another option would just be `return *this != UUID(m_num_uuid_bytes);`
> >
> >
> > We want at least one **non-zero** element, we don't want a zero-element, so 
> > the proposed code wouldn't work. I'm not sure if there's an llvm utility 
> > that allows doing that directly without having to pass a lambda.
>
>
> Wouldn't the other alternative work, where you just use `operator==` against 
> a default constructed instance?


The other alternative seems a bit less explicit to me but I don't mind it too 
much. What's the issue with using `std::any_of` exactly?


https://reviews.llvm.org/D40537



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


[Lldb-commits] [PATCH] D40539: Allow using the object file name for debug symbol resolution

2017-11-28 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a comment.

In https://reviews.llvm.org/D40539#937900, @clayborg wrote:

> In https://reviews.llvm.org/D40539#937854, @sas wrote:
>
> > Basically, if you have a `.debug` directory in the same directory where the 
> > original object file is, you can have debug symbols there. For instance, 
> > you can have:
> >
> >   /my/project/myElf.exe
> >   /my/project/.debug/myElf.exe
> >
> >
> > with the first file being a standard stripped elf file, and the second one 
> > being the associated debug symbols.
>
>
> Do adding a FileSpec that contains just the basename will cause us to look 
> for this file in the same directory as the original ELF file in the .debug 
> folder?


Yes.

> Does this occur elsewhere? Why not just construct the correct path in 
> FileSpec to begin with?

It occurs in some other class where we try to locate the debug symbols for a 
given object file. There, we construct the list of paths to look for because we 
use multiple sources to get the paths. We use the setting I mentioned earlier, 
the `.debug` folder, and some order path scheme with `.build-id` folders.


https://reviews.llvm.org/D40539



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


[Lldb-commits] [PATCH] D31108: Delete LLDB code for MD5'ing a file. Use LLVM instead

2017-03-17 Thread Stephane Sezer via Phabricator via lldb-commits
sas accepted this revision.
sas added a comment.
This revision is now accepted and ready to land.

So nice.


https://reviews.llvm.org/D31108



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


[Lldb-commits] [PATCH] D31086: Remove FileSystem::MakeDirectory

2017-03-17 Thread Stephane Sezer via Phabricator via lldb-commits
sas accepted this revision.
sas added a comment.
This revision is now accepted and ready to land.

The second behavioral change seems good but the first thing you described is a 
bit odd. Creating folders with 770 does not seem like a common behavior, and 
700 or 755 is usually more standard.

Either way, I think this change is good.


https://reviews.llvm.org/D31086



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


[Lldb-commits] [PATCH] D31086: Remove FileSystem::MakeDirectory

2017-03-20 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a comment.

In https://reviews.llvm.org/D31086#705159, @labath wrote:

> In https://reviews.llvm.org/D31086#704518, @sas wrote:
>
> > The second behavioral change seems good but the first thing you described 
> > is a bit odd. Creating folders with 770 does not seem like a common 
> > behavior, and 700 or 755 is usually more standard.
>
>
> Don't forget that the real permissions will also reflect the user's 
> `umask(2)`. The completely standard way would be to use 777 or 666 for the 
> permissions (except perhaps for some temporary/sensitive files), and then let 
> the user choose what are the real permissions he wants by setting umask in 
> his bashrc. I guess we are being a bit paranoid here and disallowing the 
> "others" permissions just in case...


Didn't know that. If these respect the user's `umask(2)` then this is good.


Repository:
  rL LLVM

https://reviews.llvm.org/D31086



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


[Lldb-commits] [PATCH] D31485: Verify memory address range validity in GDBRemoteCommunicationClient

2017-03-29 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a reviewer: clayborg.
sas added a subscriber: clayborg.
sas added a comment.

Doing additional checking on the packets returned over the wire seems decent to 
me. CC'ing @clayborg to see what he thinks about it also.


https://reviews.llvm.org/D31485



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


[Lldb-commits] [PATCH] D31485: Verify memory address range validity in GDBRemoteCommunicationClient

2017-03-31 Thread Stephane Sezer via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL299239: Verify memory address range validity in 
GDBRemoteCommunicationClient (authored by sas).

Changed prior to commit:
  https://reviews.llvm.org/D31485?vs=93674&id=93685#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D31485

Files:
  lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp


Index: 
lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -13,6 +13,7 @@
 #include "Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/StructuredData.h"
+#include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Utility/DataBuffer.h"
 
 #include "llvm/ADT/ArrayRef.h"
@@ -331,3 +332,41 @@
   HandlePacket(server, "QPassSignals:", "OK");
   EXPECT_TRUE(result.get().Success());
 }
+
+TEST_F(GDBRemoteCommunicationClientTest, GetMemoryRegionInfo) {
+  TestClient client;
+  MockServer server;
+  Connect(client, server);
+  if (HasFailure())
+return;
+
+  const lldb::addr_t addr = 0xa000;
+  MemoryRegionInfo region_info;
+  std::future result = std::async(std::launch::async, [&] {
+return client.GetMemoryRegionInfo(addr, region_info);
+  });
+
+  // name is: /foo/bar.so
+  HandlePacket(server,
+  "qMemoryRegionInfo:a000",
+  "start:a000;size:2000;permissions:rx;name:2f666f6f2f6261722e736f;");
+  EXPECT_TRUE(result.get().Success());
+
+}
+
+TEST_F(GDBRemoteCommunicationClientTest, GetMemoryRegionInfoInvalidResponse) {
+  TestClient client;
+  MockServer server;
+  Connect(client, server);
+  if (HasFailure())
+return;
+
+  const lldb::addr_t addr = 0x4000;
+  MemoryRegionInfo region_info;
+  std::future result = std::async(std::launch::async, [&] {
+return client.GetMemoryRegionInfo(addr, region_info);
+  });
+
+  HandlePacket(server, "qMemoryRegionInfo:4000", "start:4000;size:;");
+  EXPECT_FALSE(result.get().Success());
+}
Index: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1503,13 +1503,18 @@
 }
   }
 
-  // We got a valid address range back but no permissions -- which means
-  // this is an unmapped page
-  if (region_info.GetRange().IsValid() && saw_permissions == false) {
-region_info.SetReadable(MemoryRegionInfo::eNo);
-region_info.SetWritable(MemoryRegionInfo::eNo);
-region_info.SetExecutable(MemoryRegionInfo::eNo);
-region_info.SetMapped(MemoryRegionInfo::eNo);
+  if (region_info.GetRange().IsValid()) {
+// We got a valid address range back but no permissions -- which means
+// this is an unmapped page
+if (!saw_permissions) {
+  region_info.SetReadable(MemoryRegionInfo::eNo);
+  region_info.SetWritable(MemoryRegionInfo::eNo);
+  region_info.SetExecutable(MemoryRegionInfo::eNo);
+  region_info.SetMapped(MemoryRegionInfo::eNo);
+}
+  } else {
+// We got an invalid address range back
+error.SetErrorString("Server returned invalid range");
   }
 } else {
   m_supports_memory_region_info = eLazyBoolNo;


Index: lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
===
--- lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
+++ lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
@@ -13,6 +13,7 @@
 #include "Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/StructuredData.h"
+#include "lldb/Target/MemoryRegionInfo.h"
 #include "lldb/Utility/DataBuffer.h"
 
 #include "llvm/ADT/ArrayRef.h"
@@ -331,3 +332,41 @@
   HandlePacket(server, "QPassSignals:", "OK");
   EXPECT_TRUE(result.get().Success());
 }
+
+TEST_F(GDBRemoteCommunicationClientTest, GetMemoryRegionInfo) {
+  TestClient client;
+  MockServer server;
+  Connect(client, server);
+  if (HasFailure())
+return;
+
+  const lldb::addr_t addr = 0xa000;
+  MemoryRegionInfo region_info;
+  std::future result = std::async(std::launch::async, [&] {
+return client.GetMemoryRegionInfo(addr, region_info);
+  });
+
+  // name is: /foo/bar.so
+  HandlePacket(server,
+  "qMemoryRegionInfo:a000",
+  "start:a000;size:2000;permissions:rx;name:2f666f6f2f6261722e736f;");
+  EXPECT_TRUE(result.get().Success());
+

[Lldb-commits] [PATCH] D32137: [Utility] Placate another GCC warning

2017-04-19 Thread Stephane Sezer via Phabricator via lldb-commits
sas accepted this revision.
sas added inline comments.
This revision is now accepted and ready to land.



Comment at: source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp:59
+  assert(m_num_registers ==
+ static_cast(m_registers_count[gpr_registers_count] +
+   m_registers_count[fpr_registers_count] +

If you're gonna go with casting, I think using the same type as 
`m_num_registers` makes more sense.

assert(m_num_registers == static_cast(...));




https://reviews.llvm.org/D32137



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


[Lldb-commits] [PATCH] D33853: Avoid invalid string access in ObjCLanguage::MethodName::SetName

2017-06-02 Thread Stephane Sezer via Phabricator via lldb-commits
sas created this revision.

Don't access `name[1]  if the string is only of length 1.  Avoids a
crash/assertion failure when parsing the string `-`.

Test Plan:
Debug a swift binary, set a breakpoint, watch lldb not crash

Original change by Paul Menage 


https://reviews.llvm.org/D33853

Files:
  source/Plugins/Language/ObjC/ObjCLanguage.cpp


Index: source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -95,7 +95,7 @@
   // or '-' can be omitted
   bool valid_prefix = false;
 
-  if (name[0] == '+' || name[0] == '-') {
+  if (name.size() > 1 && (name[0] == '+' || name[0] == '-')) {
 valid_prefix = name[1] == '[';
 if (name[0] == '+')
   m_type = eTypeClassMethod;


Index: source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -95,7 +95,7 @@
   // or '-' can be omitted
   bool valid_prefix = false;
 
-  if (name[0] == '+' || name[0] == '-') {
+  if (name.size() > 1 && (name[0] == '+' || name[0] == '-')) {
 valid_prefix = name[1] == '[';
 if (name[0] == '+')
   m_type = eTypeClassMethod;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D33853: Avoid invalid string access in ObjCLanguage::MethodName::SetName

2017-06-05 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a comment.

This seems like a fairly straightforward fix so I'll merge it.


https://reviews.llvm.org/D33853



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


[Lldb-commits] [PATCH] D33853: Avoid invalid string access in ObjCLanguage::MethodName::SetName

2017-06-05 Thread Stephane Sezer via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL304725: Avoid invalid string access in 
ObjCLanguage::MethodName::SetName (authored by sas).

Changed prior to commit:
  https://reviews.llvm.org/D33853?vs=101284&id=101429#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D33853

Files:
  lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp


Index: lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -95,7 +95,7 @@
   // or '-' can be omitted
   bool valid_prefix = false;
 
-  if (name[0] == '+' || name[0] == '-') {
+  if (name.size() > 1 && (name[0] == '+' || name[0] == '-')) {
 valid_prefix = name[1] == '[';
 if (name[0] == '+')
   m_type = eTypeClassMethod;


Index: lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp
===
--- lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ lldb/trunk/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -95,7 +95,7 @@
   // or '-' can be omitted
   bool valid_prefix = false;
 
-  if (name[0] == '+' || name[0] == '-') {
+  if (name.size() > 1 && (name[0] == '+' || name[0] == '-')) {
 valid_prefix = name[1] == '[';
 if (name[0] == '+')
   m_type = eTypeClassMethod;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D35036: switch on enum should be exhaustive and warning-free

2017-07-05 Thread Stephane Sezer via Phabricator via lldb-commits
sas accepted this revision.
sas added a comment.
This revision is now accepted and ready to land.

Looks like the numeric values of `TypeCode` are contiguous. It would probably 
be cleaner to a check after the cast to validate the value instead of having to 
add a line for each label in the `switch` statement. Something like this:

  if (type_code < TypeCodes::sint8 || type_code > TypeCodes::sint128)
  return false;

You could also add `TypeCodesBegin = -1` and `TypeCodesEnd` as the first and 
last enumerations of `TypeCodes` so the above check would not have to be 
modified when you change the definition of `TypeCodes`.

Other than that, looks good.


https://reviews.llvm.org/D35036



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


[Lldb-commits] [PATCH] D35036: switch on enum should be exhaustive and warning-free

2017-07-05 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a comment.

Putting that check in an inline function instead of doing ad-hoc validation in 
the call-site seems better indeed.


https://reviews.llvm.org/D35036



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


[Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

2017-09-08 Thread Stephane Sezer via Phabricator via lldb-commits
sas requested changes to this revision.
sas added a comment.
This revision now requires changes to proceed.

I think it would be cleaner/smaller if you didn't use pure-virtual functions, 
but instead you had `CanReseume()` return `false` in `Process.h`, and had a 
default implementation of `DoResume()`  that called 
`llvm::report_fatal_error()`.

Instead, here, you're defaulting `CanResume()` to `true` and forcing every 
child class to redefine as `false` and provide an error'ing implementation of 
`DoResume()`.


https://reviews.llvm.org/D37651



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


[Lldb-commits] [PATCH] D37651: Fix for bug 34532 - A few rough corners related to post-mortem debugging (core/minidump)

2017-09-13 Thread Stephane Sezer via Phabricator via lldb-commits
sas accepted this revision.
sas added a comment.
This revision is now accepted and ready to land.

Seems fine.


https://reviews.llvm.org/D37651



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


[Lldb-commits] [PATCH] D22231: Implement trampoline step-through for Windows-x86.

2017-09-25 Thread Stephane Sezer via Phabricator via lldb-commits
sas updated this revision to Diff 116440.
sas added a comment.

Rebase + clang-format.


https://reviews.llvm.org/D22231

Files:
  source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp


Index: source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
===
--- source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
+++ source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
@@ -11,8 +11,11 @@
 #include "DynamicLoaderWindowsDYLD.h"
 
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
+#include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/ThreadPlanStepInstruction.h"
 
 #include "llvm/ADT/Triple.h"
 
@@ -72,5 +75,44 @@
 ThreadPlanSP
 DynamicLoaderWindowsDYLD::GetStepThroughTrampolinePlan(Thread &thread,
bool stop) {
-  return ThreadPlanSP();
+  auto arch = m_process->GetTarget().GetArchitecture();
+  if (arch.GetMachine() != llvm::Triple::x86) {
+return ThreadPlanSP();
+  }
+
+  uint64_t pc = thread.GetRegisterContext()->GetPC();
+  // Max size of an instruction in x86 is 15 bytes.
+  AddressRange range(pc, 2 * 15);
+
+  ExecutionContext exe_ctx(m_process->GetTarget());
+  DisassemblerSP disassembler_sp = Disassembler::DisassembleRange(
+  arch, nullptr, nullptr, exe_ctx, range, true);
+  if (!disassembler_sp) {
+return ThreadPlanSP();
+  }
+
+  InstructionList *insn_list = &disassembler_sp->GetInstructionList();
+  if (insn_list == nullptr) {
+return ThreadPlanSP();
+  }
+
+  // First instruction in a x86 Windows trampoline is going to be an indirect
+  // jump through the IAT and the next one will be a nop (usually there for
+  // alignment purposes). e.g.:
+  // 0x70ff4cfc <+956>: jmpl   *0x7100c2a8
+  // 0x70ff4d02 <+962>: nop
+
+  auto first_insn = insn_list->GetInstructionAtIndex(0);
+  auto second_insn = insn_list->GetInstructionAtIndex(1);
+
+  if (first_insn == nullptr || second_insn == nullptr ||
+  strcmp(first_insn->GetMnemonic(&exe_ctx), "jmpl") != 0 ||
+  strcmp(second_insn->GetMnemonic(&exe_ctx), "nop") != 0) {
+return ThreadPlanSP();
+  }
+
+  assert(first_insn->DoesBranch() && !second_insn->DoesBranch());
+
+  return ThreadPlanSP(new ThreadPlanStepInstruction(
+  thread, false, false, eVoteNoOpinion, eVoteNoOpinion));
 }


Index: source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
===
--- source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
+++ source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
@@ -11,8 +11,11 @@
 #include "DynamicLoaderWindowsDYLD.h"
 
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
+#include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/ThreadPlanStepInstruction.h"
 
 #include "llvm/ADT/Triple.h"
 
@@ -72,5 +75,44 @@
 ThreadPlanSP
 DynamicLoaderWindowsDYLD::GetStepThroughTrampolinePlan(Thread &thread,
bool stop) {
-  return ThreadPlanSP();
+  auto arch = m_process->GetTarget().GetArchitecture();
+  if (arch.GetMachine() != llvm::Triple::x86) {
+return ThreadPlanSP();
+  }
+
+  uint64_t pc = thread.GetRegisterContext()->GetPC();
+  // Max size of an instruction in x86 is 15 bytes.
+  AddressRange range(pc, 2 * 15);
+
+  ExecutionContext exe_ctx(m_process->GetTarget());
+  DisassemblerSP disassembler_sp = Disassembler::DisassembleRange(
+  arch, nullptr, nullptr, exe_ctx, range, true);
+  if (!disassembler_sp) {
+return ThreadPlanSP();
+  }
+
+  InstructionList *insn_list = &disassembler_sp->GetInstructionList();
+  if (insn_list == nullptr) {
+return ThreadPlanSP();
+  }
+
+  // First instruction in a x86 Windows trampoline is going to be an indirect
+  // jump through the IAT and the next one will be a nop (usually there for
+  // alignment purposes). e.g.:
+  // 0x70ff4cfc <+956>: jmpl   *0x7100c2a8
+  // 0x70ff4d02 <+962>: nop
+
+  auto first_insn = insn_list->GetInstructionAtIndex(0);
+  auto second_insn = insn_list->GetInstructionAtIndex(1);
+
+  if (first_insn == nullptr || second_insn == nullptr ||
+  strcmp(first_insn->GetMnemonic(&exe_ctx), "jmpl") != 0 ||
+  strcmp(second_insn->GetMnemonic(&exe_ctx), "nop") != 0) {
+return ThreadPlanSP();
+  }
+
+  assert(first_insn->DoesBranch() && !second_insn->DoesBranch());
+
+  return ThreadPlanSP(new ThreadPlanStepInstruction(
+  thread, false, false, eVoteNoOpinion, eVoteNoOpinion));
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D22231: Implement trampoline step-through for Windows-x86.

2017-09-25 Thread Stephane Sezer via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL314045: Implement trampoline step-through for Windows-x86. 
(authored by sas).

Repository:
  rL LLVM

https://reviews.llvm.org/D22231

Files:
  
lldb/trunk/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp


Index: 
lldb/trunk/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
===
--- 
lldb/trunk/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
+++ 
lldb/trunk/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
@@ -11,8 +11,11 @@
 #include "DynamicLoaderWindowsDYLD.h"
 
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
+#include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/ThreadPlanStepInstruction.h"
 
 #include "llvm/ADT/Triple.h"
 
@@ -72,5 +75,44 @@
 ThreadPlanSP
 DynamicLoaderWindowsDYLD::GetStepThroughTrampolinePlan(Thread &thread,
bool stop) {
-  return ThreadPlanSP();
+  auto arch = m_process->GetTarget().GetArchitecture();
+  if (arch.GetMachine() != llvm::Triple::x86) {
+return ThreadPlanSP();
+  }
+
+  uint64_t pc = thread.GetRegisterContext()->GetPC();
+  // Max size of an instruction in x86 is 15 bytes.
+  AddressRange range(pc, 2 * 15);
+
+  ExecutionContext exe_ctx(m_process->GetTarget());
+  DisassemblerSP disassembler_sp = Disassembler::DisassembleRange(
+  arch, nullptr, nullptr, exe_ctx, range, true);
+  if (!disassembler_sp) {
+return ThreadPlanSP();
+  }
+
+  InstructionList *insn_list = &disassembler_sp->GetInstructionList();
+  if (insn_list == nullptr) {
+return ThreadPlanSP();
+  }
+
+  // First instruction in a x86 Windows trampoline is going to be an indirect
+  // jump through the IAT and the next one will be a nop (usually there for
+  // alignment purposes). e.g.:
+  // 0x70ff4cfc <+956>: jmpl   *0x7100c2a8
+  // 0x70ff4d02 <+962>: nop
+
+  auto first_insn = insn_list->GetInstructionAtIndex(0);
+  auto second_insn = insn_list->GetInstructionAtIndex(1);
+
+  if (first_insn == nullptr || second_insn == nullptr ||
+  strcmp(first_insn->GetMnemonic(&exe_ctx), "jmpl") != 0 ||
+  strcmp(second_insn->GetMnemonic(&exe_ctx), "nop") != 0) {
+return ThreadPlanSP();
+  }
+
+  assert(first_insn->DoesBranch() && !second_insn->DoesBranch());
+
+  return ThreadPlanSP(new ThreadPlanStepInstruction(
+  thread, false, false, eVoteNoOpinion, eVoteNoOpinion));
 }


Index: lldb/trunk/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
===
--- lldb/trunk/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
+++ lldb/trunk/source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
@@ -11,8 +11,11 @@
 #include "DynamicLoaderWindowsDYLD.h"
 
 #include "lldb/Core/PluginManager.h"
+#include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
+#include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/Target.h"
+#include "lldb/Target/ThreadPlanStepInstruction.h"
 
 #include "llvm/ADT/Triple.h"
 
@@ -72,5 +75,44 @@
 ThreadPlanSP
 DynamicLoaderWindowsDYLD::GetStepThroughTrampolinePlan(Thread &thread,
bool stop) {
-  return ThreadPlanSP();
+  auto arch = m_process->GetTarget().GetArchitecture();
+  if (arch.GetMachine() != llvm::Triple::x86) {
+return ThreadPlanSP();
+  }
+
+  uint64_t pc = thread.GetRegisterContext()->GetPC();
+  // Max size of an instruction in x86 is 15 bytes.
+  AddressRange range(pc, 2 * 15);
+
+  ExecutionContext exe_ctx(m_process->GetTarget());
+  DisassemblerSP disassembler_sp = Disassembler::DisassembleRange(
+  arch, nullptr, nullptr, exe_ctx, range, true);
+  if (!disassembler_sp) {
+return ThreadPlanSP();
+  }
+
+  InstructionList *insn_list = &disassembler_sp->GetInstructionList();
+  if (insn_list == nullptr) {
+return ThreadPlanSP();
+  }
+
+  // First instruction in a x86 Windows trampoline is going to be an indirect
+  // jump through the IAT and the next one will be a nop (usually there for
+  // alignment purposes). e.g.:
+  // 0x70ff4cfc <+956>: jmpl   *0x7100c2a8
+  // 0x70ff4d02 <+962>: nop
+
+  auto first_insn = insn_list->GetInstructionAtIndex(0);
+  auto second_insn = insn_list->GetInstructionAtIndex(1);
+
+  if (first_insn == nullptr || second_insn == nullptr ||
+  strcmp(first_insn->GetMnemonic(&exe_ctx), "jmpl") != 0 ||
+  strcmp(second_insn->GetMnemonic(&exe_ctx), "nop") != 0) {
+return ThreadPlanSP();
+  }
+
+  assert(first_insn->DoesBranch() && !second_insn->DoesBranch());
+
+  return ThreadPlanSP(new ThreadPlanStepInstruction(
+  thread, false, false, eVoteNoOpinion, eVoteNoOpinion));
 }
_

[Lldb-commits] [PATCH] D38373: Add a few missing newlines in lldb-server messages

2017-09-28 Thread Stephane Sezer via Phabricator via lldb-commits
sas created this revision.

https://reviews.llvm.org/D38373

Files:
  tools/lldb-server/lldb-gdbserver.cpp


Index: tools/lldb-server/lldb-gdbserver.cpp
===
--- tools/lldb-server/lldb-gdbserver.cpp
+++ tools/lldb-server/lldb-gdbserver.cpp
@@ -251,12 +251,12 @@
 auto connection_result = connection_up->Connect(connection_url, &error);
 if (connection_result != eConnectionStatusSuccess) {
   fprintf(stderr, "error: failed to connect to client at '%s' "
-  "(connection status: %d)",
+  "(connection status: %d)\n",
   connection_url, static_cast(connection_result));
   exit(-1);
 }
 if (error.Fail()) {
-  fprintf(stderr, "error: failed to connect to client at '%s': %s",
+  fprintf(stderr, "error: failed to connect to client at '%s': %s\n",
   connection_url, error.AsCString());
   exit(-1);
 }
@@ -287,7 +287,7 @@
   // Ensure we have a port number for the connection.
   if (connection_portno == 0) {
 fprintf(stderr, "error: port number must be specified on when using "
-"reverse connect");
+"reverse connect\n");
 exit(1);
   }
 
@@ -301,20 +301,20 @@
   auto connection_result = connection_up->Connect(connection_url, &error);
   if (connection_result != eConnectionStatusSuccess) {
 fprintf(stderr, "error: failed to connect to client at '%s' "
-"(connection status: %d)",
+"(connection status: %d)\n",
 connection_url, static_cast(connection_result));
 exit(-1);
   }
   if (error.Fail()) {
-fprintf(stderr, "error: failed to connect to client at '%s': %s",
+fprintf(stderr, "error: failed to connect to client at '%s': %s\n",
 connection_url, error.AsCString());
 exit(-1);
   }
 } else {
   std::unique_ptr acceptor_up(
   Acceptor::Create(final_host_and_port, false, error));
   if (error.Fail()) {
-fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
+fprintf(stderr, "failed to create acceptor: %s\n", error.AsCString());
 exit(1);
   }
   error = acceptor_up->Listen(1);
@@ -328,15 +328,15 @@
 if (named_pipe_path && named_pipe_path[0]) {
   error = writeSocketIdToPipe(named_pipe_path, socket_id);
   if (error.Fail())
-fprintf(stderr, "failed to write to the named pipe \'%s\': %s",
+fprintf(stderr, "failed to write to the named pipe \'%s\': %s\n",
 named_pipe_path, error.AsCString());
 }
 // If we have an unnamed pipe to write the socket id back to, do that
 // now.
 else if (unnamed_pipe_fd >= 0) {
   error = writeSocketIdToPipe(unnamed_pipe_fd, socket_id);
   if (error.Fail())
-fprintf(stderr, "failed to write to the unnamed pipe: %s",
+fprintf(stderr, "failed to write to the unnamed pipe: %s\n",
 error.AsCString());
 }
   } else {


Index: tools/lldb-server/lldb-gdbserver.cpp
===
--- tools/lldb-server/lldb-gdbserver.cpp
+++ tools/lldb-server/lldb-gdbserver.cpp
@@ -251,12 +251,12 @@
 auto connection_result = connection_up->Connect(connection_url, &error);
 if (connection_result != eConnectionStatusSuccess) {
   fprintf(stderr, "error: failed to connect to client at '%s' "
-  "(connection status: %d)",
+  "(connection status: %d)\n",
   connection_url, static_cast(connection_result));
   exit(-1);
 }
 if (error.Fail()) {
-  fprintf(stderr, "error: failed to connect to client at '%s': %s",
+  fprintf(stderr, "error: failed to connect to client at '%s': %s\n",
   connection_url, error.AsCString());
   exit(-1);
 }
@@ -287,7 +287,7 @@
   // Ensure we have a port number for the connection.
   if (connection_portno == 0) {
 fprintf(stderr, "error: port number must be specified on when using "
-"reverse connect");
+"reverse connect\n");
 exit(1);
   }
 
@@ -301,20 +301,20 @@
   auto connection_result = connection_up->Connect(connection_url, &error);
   if (connection_result != eConnectionStatusSuccess) {
 fprintf(stderr, "error: failed to connect to client at '%s' "
-"(connection status: %d)",
+"(connection status: %d)\n",
 connection_url, static_cast(connection_result));
 exit(-1);
   }
   if (error.Fail()) {
-fprintf(stderr, "error: failed to connect to client at '%s': %s",
+fprintf(stderr, "error: failed to connect to client at '%s': %s\n",
 con

[Lldb-commits] [PATCH] D38373: Add a few missing newlines in lldb-server messages

2017-09-28 Thread Stephane Sezer via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL314455: Add a few missing newlines in lldb-server messages 
(authored by sas).

Repository:
  rL LLVM

https://reviews.llvm.org/D38373

Files:
  lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp


Index: lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
===
--- lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
@@ -251,12 +251,12 @@
 auto connection_result = connection_up->Connect(connection_url, &error);
 if (connection_result != eConnectionStatusSuccess) {
   fprintf(stderr, "error: failed to connect to client at '%s' "
-  "(connection status: %d)",
+  "(connection status: %d)\n",
   connection_url, static_cast(connection_result));
   exit(-1);
 }
 if (error.Fail()) {
-  fprintf(stderr, "error: failed to connect to client at '%s': %s",
+  fprintf(stderr, "error: failed to connect to client at '%s': %s\n",
   connection_url, error.AsCString());
   exit(-1);
 }
@@ -287,7 +287,7 @@
   // Ensure we have a port number for the connection.
   if (connection_portno == 0) {
 fprintf(stderr, "error: port number must be specified on when using "
-"reverse connect");
+"reverse connect\n");
 exit(1);
   }
 
@@ -301,20 +301,20 @@
   auto connection_result = connection_up->Connect(connection_url, &error);
   if (connection_result != eConnectionStatusSuccess) {
 fprintf(stderr, "error: failed to connect to client at '%s' "
-"(connection status: %d)",
+"(connection status: %d)\n",
 connection_url, static_cast(connection_result));
 exit(-1);
   }
   if (error.Fail()) {
-fprintf(stderr, "error: failed to connect to client at '%s': %s",
+fprintf(stderr, "error: failed to connect to client at '%s': %s\n",
 connection_url, error.AsCString());
 exit(-1);
   }
 } else {
   std::unique_ptr acceptor_up(
   Acceptor::Create(final_host_and_port, false, error));
   if (error.Fail()) {
-fprintf(stderr, "failed to create acceptor: %s", error.AsCString());
+fprintf(stderr, "failed to create acceptor: %s\n", error.AsCString());
 exit(1);
   }
   error = acceptor_up->Listen(1);
@@ -328,15 +328,15 @@
 if (named_pipe_path && named_pipe_path[0]) {
   error = writeSocketIdToPipe(named_pipe_path, socket_id);
   if (error.Fail())
-fprintf(stderr, "failed to write to the named pipe \'%s\': %s",
+fprintf(stderr, "failed to write to the named pipe \'%s\': %s\n",
 named_pipe_path, error.AsCString());
 }
 // If we have an unnamed pipe to write the socket id back to, do that
 // now.
 else if (unnamed_pipe_fd >= 0) {
   error = writeSocketIdToPipe(unnamed_pipe_fd, socket_id);
   if (error.Fail())
-fprintf(stderr, "failed to write to the unnamed pipe: %s",
+fprintf(stderr, "failed to write to the unnamed pipe: %s\n",
 error.AsCString());
 }
   } else {


Index: lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
===
--- lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
@@ -251,12 +251,12 @@
 auto connection_result = connection_up->Connect(connection_url, &error);
 if (connection_result != eConnectionStatusSuccess) {
   fprintf(stderr, "error: failed to connect to client at '%s' "
-  "(connection status: %d)",
+  "(connection status: %d)\n",
   connection_url, static_cast(connection_result));
   exit(-1);
 }
 if (error.Fail()) {
-  fprintf(stderr, "error: failed to connect to client at '%s': %s",
+  fprintf(stderr, "error: failed to connect to client at '%s': %s\n",
   connection_url, error.AsCString());
   exit(-1);
 }
@@ -287,7 +287,7 @@
   // Ensure we have a port number for the connection.
   if (connection_portno == 0) {
 fprintf(stderr, "error: port number must be specified on when using "
-"reverse connect");
+"reverse connect\n");
 exit(1);
   }
 
@@ -301,20 +301,20 @@
   auto connection_result = connection_up->Connect(connection_url, &error);
   if (connection_result != eConnectionStatusSuccess) {
 fprintf(stderr, "error: failed to connect to client at '%s' "
-"(connection status: %d)",
+"(connection status: %d)\n",
 connection_url, static_cast(con

[Lldb-commits] [PATCH] D19604: Allow ObjectFilePECOFF to initialize with ARM binaries.

2017-10-24 Thread Stephane Sezer via Phabricator via lldb-commits
sas updated this revision to Diff 120146.
sas added a comment.
Herald added a subscriber: kristof.beyls.

Rebase.


https://reviews.llvm.org/D19604

Files:
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp


Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -136,6 +136,11 @@
   spec.SetTriple("i686-pc-windows");
   specs.Append(ModuleSpec(file, spec));
 }
+else if (coff_header.machine == MachineArmNt)
+{
+  spec.SetTriple("arm-pc-windows");
+  specs.Append(ModuleSpec(file, spec));
+}
   }
 }
   }


Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -136,6 +136,11 @@
   spec.SetTriple("i686-pc-windows");
   specs.Append(ModuleSpec(file, spec));
 }
+else if (coff_header.machine == MachineArmNt)
+{
+  spec.SetTriple("arm-pc-windows");
+  specs.Append(ModuleSpec(file, spec));
+}
   }
 }
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D19604: Allow ObjectFilePECOFF to initialize with ARM binaries.

2017-10-24 Thread Stephane Sezer via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316532: Allow ObjectFilePECOFF to initialize with ARM 
binaries. (authored by sas).

Repository:
  rL LLVM

https://reviews.llvm.org/D19604

Files:
  lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp


Index: lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -136,6 +136,11 @@
   spec.SetTriple("i686-pc-windows");
   specs.Append(ModuleSpec(file, spec));
 }
+else if (coff_header.machine == MachineArmNt)
+{
+  spec.SetTriple("arm-pc-windows");
+  specs.Append(ModuleSpec(file, spec));
+}
   }
 }
   }


Index: lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ lldb/trunk/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -136,6 +136,11 @@
   spec.SetTriple("i686-pc-windows");
   specs.Append(ModuleSpec(file, spec));
 }
+else if (coff_header.machine == MachineArmNt)
+{
+  spec.SetTriple("arm-pc-windows");
+  specs.Append(ModuleSpec(file, spec));
+}
   }
 }
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

2017-10-25 Thread Stephane Sezer via Phabricator via lldb-commits
sas created this revision.

This commit implements basic DidAttach and DidLaunch for the windows
DynamicLoader plugin which allow us to load shared libraries from the
inferior.

This is an improved version of https://reviews.llvm.org/D12245 and should work 
much better.


https://reviews.llvm.org/D39314

Files:
  source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp


Index: source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
===
--- source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
+++ source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
@@ -10,12 +10,14 @@
 
 #include "DynamicLoaderWindowsDYLD.h"
 
+#include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/ThreadPlanStepInstruction.h"
+#include "lldb/Utility/Log.h"
 
 #include "llvm/ADT/Triple.h"
 
@@ -60,9 +62,49 @@
   return nullptr;
 }
 
-void DynamicLoaderWindowsDYLD::DidAttach() {}
+void DynamicLoaderWindowsDYLD::DidAttach() {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
+  if (log)
+log->Printf("DynamicLoaderWindowsDYLD::%s()", __FUNCTION__);
 
-void DynamicLoaderWindowsDYLD::DidLaunch() {}
+  DidLaunch();
+
+  m_process->LoadModules();
+}
+
+void DynamicLoaderWindowsDYLD::DidLaunch() {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
+  if (log)
+log->Printf("DynamicLoaderWindowsDYLD::%s()", __FUNCTION__);
+
+  ModuleSP executable = GetTargetExecutable();
+
+  if (!executable.get())
+return;
+
+  // Try to fetch the load address of the file from the process, since there
+  // could be randomization of the load address.
+
+  // It might happen that the remote has a different dir for the file, so we
+  // only send the basename of the executable in the query. I think this is 
safe
+  // because I doubt that two executables with the same basenames are loaded in
+  // memory...
+  FileSpec file_spec(
+  executable->GetPlatformFileSpec().GetFilename().GetCString(), false);
+  bool is_loaded;
+  addr_t base_addr = 0;
+  lldb::addr_t load_addr;
+  Status error = m_process->GetFileLoadAddress(file_spec, is_loaded, 
load_addr);
+  if (error.Success() && is_loaded) {
+base_addr = load_addr;
+  }
+
+  UpdateLoadedSections(executable, LLDB_INVALID_ADDRESS, base_addr, false);
+
+  ModuleList module_list;
+  module_list.Append(executable);
+  m_process->GetTarget().ModulesDidLoad(module_list);
+}
 
 Status DynamicLoaderWindowsDYLD::CanLoadImage() { return Status(); }
 


Index: source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
===
--- source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
+++ source/Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.cpp
@@ -10,12 +10,14 @@
 
 #include "DynamicLoaderWindowsDYLD.h"
 
+#include "lldb/Core/Module.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Target/ExecutionContext.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/RegisterContext.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Target/ThreadPlanStepInstruction.h"
+#include "lldb/Utility/Log.h"
 
 #include "llvm/ADT/Triple.h"
 
@@ -60,9 +62,49 @@
   return nullptr;
 }
 
-void DynamicLoaderWindowsDYLD::DidAttach() {}
+void DynamicLoaderWindowsDYLD::DidAttach() {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
+  if (log)
+log->Printf("DynamicLoaderWindowsDYLD::%s()", __FUNCTION__);
 
-void DynamicLoaderWindowsDYLD::DidLaunch() {}
+  DidLaunch();
+
+  m_process->LoadModules();
+}
+
+void DynamicLoaderWindowsDYLD::DidLaunch() {
+  Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_DYNAMIC_LOADER));
+  if (log)
+log->Printf("DynamicLoaderWindowsDYLD::%s()", __FUNCTION__);
+
+  ModuleSP executable = GetTargetExecutable();
+
+  if (!executable.get())
+return;
+
+  // Try to fetch the load address of the file from the process, since there
+  // could be randomization of the load address.
+
+  // It might happen that the remote has a different dir for the file, so we
+  // only send the basename of the executable in the query. I think this is safe
+  // because I doubt that two executables with the same basenames are loaded in
+  // memory...
+  FileSpec file_spec(
+  executable->GetPlatformFileSpec().GetFilename().GetCString(), false);
+  bool is_loaded;
+  addr_t base_addr = 0;
+  lldb::addr_t load_addr;
+  Status error = m_process->GetFileLoadAddress(file_spec, is_loaded, load_addr);
+  if (error.Success() && is_loaded) {
+base_addr = load_addr;
+  }
+
+  UpdateLoadedSections(executable, LLDB_INVALID_ADDRESS, base_addr, false);
+
+  ModuleList module_list;
+  module_list.Append(executable);
+  m_process->GetTarget().ModulesDidLoa

[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

2017-10-25 Thread Stephane Sezer via Phabricator via lldb-commits
sas created this revision.
Herald added subscribers: kristof.beyls, aemerson.

This assumes that the environment is always Thumb

Change by Walter Erquinigo 


https://reviews.llvm.org/D39315

Files:
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp


Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -648,6 +648,16 @@
sizeof(uint32_t) * name_ordinal;
   uint32_t function_rva = symtab_data.GetU32(&function_offset);
 
+  ArchSpec header_arch;
+  GetArchitecture(header_arch);
+  if (header_arch.GetMachine() == llvm::Triple::arm) {
+if (function_rva & 1) {
+  // For Thumb we need the last bit to be 0 so that the address
+  // points to the right beginning of the symbol.
+  function_rva ^= 1;
+}
+  }
+
   Address symbol_addr(m_coff_header_opt.image_base + function_rva,
   sect_list);
   symbols[i].GetMangled().SetValue(ConstString(symbol_name.c_str()));


Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -648,6 +648,16 @@
sizeof(uint32_t) * name_ordinal;
   uint32_t function_rva = symtab_data.GetU32(&function_offset);
 
+  ArchSpec header_arch;
+  GetArchitecture(header_arch);
+  if (header_arch.GetMachine() == llvm::Triple::arm) {
+if (function_rva & 1) {
+  // For Thumb we need the last bit to be 0 so that the address
+  // points to the right beginning of the symbol.
+  function_rva ^= 1;
+}
+  }
+
   Address symbol_addr(m_coff_header_opt.image_base + function_rva,
   sect_list);
   symbols[i].GetMangled().SetValue(ConstString(symbol_name.c_str()));
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D39335: Allow SysV-i386 ABI on everything other than Apple targets

2017-10-26 Thread Stephane Sezer via Phabricator via lldb-commits
sas created this revision.
Herald added subscribers: kristof.beyls, aemerson.

This matches other SysV ABIs that are different on Apple and non-Apple targets,
like `ABISysV_arm.cpp` for instance.


https://reviews.llvm.org/D39335

Files:
  source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp


Index: source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
===
--- source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
+++ source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
@@ -205,11 +205,12 @@
 ABISP
 ABISysV_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) 
{
   static ABISP g_abi_sp;
-  if ((arch.GetTriple().getArch() == llvm::Triple::x86) &&
-  (arch.GetTriple().isOSLinux() || arch.GetTriple().isOSFreeBSD())) {
-if (!g_abi_sp)
-  g_abi_sp.reset(new ABISysV_i386(process_sp));
-return g_abi_sp;
+  if (arch.GetTriple().getVendor() != llvm::Triple::Apple) {
+if (arch.GetTriple().getArch() == llvm::Triple::x86) {
+  if (!g_abi_sp)
+g_abi_sp.reset(new ABISysV_i386(process_sp));
+  return g_abi_sp;
+}
   }
   return ABISP();
 }


Index: source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
===
--- source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
+++ source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
@@ -205,11 +205,12 @@
 ABISP
 ABISysV_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) {
   static ABISP g_abi_sp;
-  if ((arch.GetTriple().getArch() == llvm::Triple::x86) &&
-  (arch.GetTriple().isOSLinux() || arch.GetTriple().isOSFreeBSD())) {
-if (!g_abi_sp)
-  g_abi_sp.reset(new ABISysV_i386(process_sp));
-return g_abi_sp;
+  if (arch.GetTriple().getVendor() != llvm::Triple::Apple) {
+if (arch.GetTriple().getArch() == llvm::Triple::x86) {
+  if (!g_abi_sp)
+g_abi_sp.reset(new ABISysV_i386(process_sp));
+  return g_abi_sp;
+}
   }
   return ABISP();
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D39314: Implement basic DidAttach and DidLaunch for DynamicLoaderWindowsDYLD

2017-10-26 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a comment.

In https://reviews.llvm.org/D39314#908065, @davide wrote:

> can you please try adding a test for this one? :)


To be fully honest, I'm not 100% sure how I'm supposed to add tests for this. I 
looked through the directories containing unit tests and didn't find anything 
specific to DYLD testing.

I'm going to try to sync with @zturner to see if he is still able to run unit 
tests on Windows. We exclusively debug Windows remotes from a macOS or Linux 
host, so I don't have any setup to run local windows tests.


https://reviews.llvm.org/D39314



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


[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

2017-10-26 Thread Stephane Sezer via Phabricator via lldb-commits
sas requested review of this revision.
sas added a comment.

I saw the `bool is_arm = ` pattern only in `ObjectFileMachO.cpp`. I'm not sure 
about the specifics for Darwin targets, but having an object file with some 
functions in ARM and some functions in Thumb is valid, so I think checking for 
each symbol is the right thing to do.

As for the `GetAddressClass` function, I have another diff that does exactly 
what you are talking about. I'll upload it when this one goes in.


https://reviews.llvm.org/D39315



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


[Lldb-commits] [PATCH] D39335: Allow SysV-i386 ABI on everything other than Apple targets

2017-10-26 Thread Stephane Sezer via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316673: Allow SysV-i386 ABI on everything other than Apple 
targets (authored by sas).

Repository:
  rL LLVM

https://reviews.llvm.org/D39335

Files:
  lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp


Index: lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
===
--- lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
+++ lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
@@ -205,11 +205,12 @@
 ABISP
 ABISysV_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) 
{
   static ABISP g_abi_sp;
-  if ((arch.GetTriple().getArch() == llvm::Triple::x86) &&
-  (arch.GetTriple().isOSLinux() || arch.GetTriple().isOSFreeBSD())) {
-if (!g_abi_sp)
-  g_abi_sp.reset(new ABISysV_i386(process_sp));
-return g_abi_sp;
+  if (arch.GetTriple().getVendor() != llvm::Triple::Apple) {
+if (arch.GetTriple().getArch() == llvm::Triple::x86) {
+  if (!g_abi_sp)
+g_abi_sp.reset(new ABISysV_i386(process_sp));
+  return g_abi_sp;
+}
   }
   return ABISP();
 }


Index: lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
===
--- lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
+++ lldb/trunk/source/Plugins/ABI/SysV-i386/ABISysV_i386.cpp
@@ -205,11 +205,12 @@
 ABISP
 ABISysV_i386::CreateInstance(lldb::ProcessSP process_sp, const ArchSpec &arch) {
   static ABISP g_abi_sp;
-  if ((arch.GetTriple().getArch() == llvm::Triple::x86) &&
-  (arch.GetTriple().isOSLinux() || arch.GetTriple().isOSFreeBSD())) {
-if (!g_abi_sp)
-  g_abi_sp.reset(new ABISysV_i386(process_sp));
-return g_abi_sp;
+  if (arch.GetTriple().getVendor() != llvm::Triple::Apple) {
+if (arch.GetTriple().getArch() == llvm::Triple::x86) {
+  if (!g_abi_sp)
+g_abi_sp.reset(new ABISysV_i386(process_sp));
+  return g_abi_sp;
+}
   }
   return ABISP();
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

2017-10-26 Thread Stephane Sezer via Phabricator via lldb-commits
sas updated this revision to Diff 120447.
sas added a comment.

Address comments by @clayborg.


https://reviews.llvm.org/D39315

Files:
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp


Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -37,6 +37,7 @@
 #define OPT_HEADER_MAGIC_PE32 0x010b
 #define OPT_HEADER_MAGIC_PE32_PLUS 0x020b
 
+#define THUMB_ADDRESS_BIT_MASK 0xfffeull
 using namespace lldb;
 using namespace lldb_private;
 
@@ -633,6 +634,10 @@
 
 std::string symbol_name;
 
+ArchSpec header_arch;
+GetArchitecture(header_arch);
+bool is_arm = header_arch.GetMachine() == llvm::Triple::arm;
+
 // Read each export table entry
 for (size_t i = 0; i < export_table.number_of_names; ++i) {
   uint32_t name_ordinal =
@@ -648,6 +653,12 @@
sizeof(uint32_t) * name_ordinal;
   uint32_t function_rva = symtab_data.GetU32(&function_offset);
 
+  if (is_arm && function_rva & 1) {
+// For Thumb we need the last bit to be 0 so that the address
+// points to the right beginning of the symbol.
+function_rva &= THUMB_ADDRESS_BIT_MASK;
+  }
+
   Address symbol_addr(m_coff_header_opt.image_base + function_rva,
   sect_list);
   symbols[i].GetMangled().SetValue(ConstString(symbol_name.c_str()));


Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -37,6 +37,7 @@
 #define OPT_HEADER_MAGIC_PE32 0x010b
 #define OPT_HEADER_MAGIC_PE32_PLUS 0x020b
 
+#define THUMB_ADDRESS_BIT_MASK 0xfffeull
 using namespace lldb;
 using namespace lldb_private;
 
@@ -633,6 +634,10 @@
 
 std::string symbol_name;
 
+ArchSpec header_arch;
+GetArchitecture(header_arch);
+bool is_arm = header_arch.GetMachine() == llvm::Triple::arm;
+
 // Read each export table entry
 for (size_t i = 0; i < export_table.number_of_names; ++i) {
   uint32_t name_ordinal =
@@ -648,6 +653,12 @@
sizeof(uint32_t) * name_ordinal;
   uint32_t function_rva = symtab_data.GetU32(&function_offset);
 
+  if (is_arm && function_rva & 1) {
+// For Thumb we need the last bit to be 0 so that the address
+// points to the right beginning of the symbol.
+function_rva &= THUMB_ADDRESS_BIT_MASK;
+  }
+
   Address symbol_addr(m_coff_header_opt.image_base + function_rva,
   sect_list);
   symbols[i].GetMangled().SetValue(ConstString(symbol_name.c_str()));
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D19603: Fix entry point lookup for ObjectFilePECOFF

2017-10-26 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a comment.

In https://reviews.llvm.org/D19603#908130, @clayborg wrote:

> Does this need some ARM support where we strip bit zero in case the entry 
> point is Thumb?


Good question. Somehow we never had any issue with this but I don't remember 
explicitly checking for difference with thumb entry points either.

That being said, Winphone expects thumb-only for user-space code, so I'd assume 
`m_coff_header_opt.entry` does **not** include the thumb bit, but the kernel 
still jumps to user code in thumb mode.


https://reviews.llvm.org/D19603



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


[Lldb-commits] [PATCH] D39337: Implement a simple GetAddressClass for PECOFF

2017-10-26 Thread Stephane Sezer via Phabricator via lldb-commits
sas created this revision.

This function apparently is called only by addresses that are valid.
We mainly need to use the ISA class whenever is needed.

Depends on https://reviews.llvm.org/D39315.

Change by Walter Erquinigo 


https://reviews.llvm.org/D39337

Files:
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
  source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h


Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -102,8 +102,7 @@
 
   uint32_t GetAddressByteSize() const override;
 
-  //virtual lldb_private::AddressClass
-  //GetAddressClass (lldb::addr_t file_addr);
+  lldb::AddressClass GetAddressClass(lldb::addr_t file_addr) override;
 
   lldb_private::Symtab *GetSymtab() override;
 
Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -678,6 +678,23 @@
   return false;
 }
 
+lldb::AddressClass ObjectFilePECOFF::GetAddressClass(lldb::addr_t file_addr) {
+  auto address_class = ObjectFile::GetAddressClass(file_addr);
+  // Some addresses (e.g. from trampolines) are marked as eAddressClassUnknown.
+  if (address_class == eAddressClassCode ||
+  address_class == eAddressClassUnknown) {
+ArchSpec header_arch;
+GetArchitecture(header_arch);
+if (header_arch.GetMachine() == llvm::Triple::arm) {
+  // The only PECOFF/ARM target we support, Windows Phone, requires all
+  // user code to be Thumb, so we can always return
+  // eAddressClassCodeAlternateISA.
+  return eAddressClassCodeAlternateISA;
+}
+  }
+  return address_class;
+}
+
 void ObjectFilePECOFF::CreateSections(SectionList &unified_section_list) {
   if (!m_sections_ap.get()) {
 m_sections_ap.reset(new SectionList());


Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h
@@ -102,8 +102,7 @@
 
   uint32_t GetAddressByteSize() const override;
 
-  //virtual lldb_private::AddressClass
-  //GetAddressClass (lldb::addr_t file_addr);
+  lldb::AddressClass GetAddressClass(lldb::addr_t file_addr) override;
 
   lldb_private::Symtab *GetSymtab() override;
 
Index: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
===
--- source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
+++ source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp
@@ -678,6 +678,23 @@
   return false;
 }
 
+lldb::AddressClass ObjectFilePECOFF::GetAddressClass(lldb::addr_t file_addr) {
+  auto address_class = ObjectFile::GetAddressClass(file_addr);
+  // Some addresses (e.g. from trampolines) are marked as eAddressClassUnknown.
+  if (address_class == eAddressClassCode ||
+  address_class == eAddressClassUnknown) {
+ArchSpec header_arch;
+GetArchitecture(header_arch);
+if (header_arch.GetMachine() == llvm::Triple::arm) {
+  // The only PECOFF/ARM target we support, Windows Phone, requires all
+  // user code to be Thumb, so we can always return
+  // eAddressClassCodeAlternateISA.
+  return eAddressClassCodeAlternateISA;
+}
+  }
+  return address_class;
+}
+
 void ObjectFilePECOFF::CreateSections(SectionList &unified_section_list) {
   if (!m_sections_ap.get()) {
 m_sections_ap.reset(new SectionList());
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

2017-10-26 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a comment.

@clayborg: the follow-up to this change is in https://reviews.llvm.org/D39337, 
where we implement `GetAddressClass()`.


https://reviews.llvm.org/D39315



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


[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

2017-10-26 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a comment.

Same thing here, I have no idea how to go about testing something specific like 
this. Given that this is Windows Phone, it's even worse than simply Windows 
Desktop because the only way to test is by doing remote debugging. I suppose 
checking in binaries to the tree so we can analyze the symbols and ensure 
correctness is the only way to do it?


https://reviews.llvm.org/D39315



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


[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

2017-10-26 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a comment.

In https://reviews.llvm.org/D39315#908251, @zturner wrote:

> In https://reviews.llvm.org/D39315#908246, @sas wrote:
>
> > Same thing here, I have no idea how to go about testing something specific 
> > like this. Given that this is Windows Phone, it's even worse than simply 
> > Windows Desktop because the only way to test is by doing remote debugging. 
> > I suppose checking in binaries to the tree so we can analyze the symbols 
> > and ensure correctness is the only way to do it?
>
>
> Do you guys not have access to Windows Desktops?  It sounds like we need to 
> get remote debugging of Windows targets working.  I don't think I'm gonna be 
> the one to do that in the near future, but if it's going to be a frequent 
> thing that you're improving windows support for remote targets like Windows 
> Phone, I imagine you're going to want the code to be tested, so hopefully you 
> can convince someone to give you cycles to make remote debugging work 
> properly, otherwise you're shipping untested code :-/


Well as far as I can tell, remote debugging for Windows **does** work, assuming 
you use ds2 on the remote, and that you have a few of our local patches (which 
I'm trying to send upstream currently). Remote Windows debugging is the 
**only** way we debug on Windows for now and the testing we have is just a 
combination of people using our tools internally as well as some end-to-end 
tests we run internally with full debug session setup + breakpoints + execution 
control, etc, with a remote session.

Given that this works for us, I'm gonna have a hard time justifying assigning 
people to bringing up windows remote debugging in lldb without ds2, and that 
leaves me in a state where testing stuff **in** lldb is hard/impossible for now.


https://reviews.llvm.org/D39315



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


[Lldb-commits] [PATCH] D39315: Correct the start address for exported windows functions in arm

2017-10-26 Thread Stephane Sezer via Phabricator via lldb-commits
sas added a subscriber: fjricci.
sas added a comment.

In https://reviews.llvm.org/D39315#908284, @zturner wrote:

> So when you're using ds2 as the remote, are you still using the normal lldb 
> test suite?  `dotest.py`?  If so, then we could have a test decorator that 
> says `@expectedFailure(not(debugserver=ds2))` or something.  Then, even 
> though you're the only one that can run it, at least YOU are sure it works.  
> Some coverage is better than nothing.  Is something like this possible?


So there are two ways we test our stuff.

First part is centered around ds2 testing, and you can see it here: 
https://travis-ci.org/facebook/ds2/builds/292216534. We build ds2 for a bunch 
of different targets, and for some of them, we run `dotest.py` with 
`LLDB_DEBUGSERVER_PATH=/path/to/ds2`. This gives us a lot of coverage for 
non-Windows targets.

For Windows targets, we can only build ds2 (see 
https://ci.appveyor.com/project/a20012251/ds2/branch/master) and we are unable 
to run tests because we need a binary of lldb for Windows, but the builds 
available on http://releases.llvm.org/ were historically broken, and when 
@fjricci tried to fix them it didn't lead anywhere IIRC.

The second way we have to test our debugging tools is to simply run our 
debugging scripts with a known target, internally. That doesn't use 
`dotest.py`. It simply launches lldb and ds2, tries to set breakpoints, control 
the execution of the process, etc. The changes I am currently sending upstream 
work with that type of internal testing, but that's of no use for you because 
you can't run it yourself, nor can you see the results of our runs. :/


https://reviews.llvm.org/D39315



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


[Lldb-commits] [PATCH] D39429: Use LLVM version string

2017-10-30 Thread Stephane Sezer via Phabricator via lldb-commits
sas created this revision.

macOS builds of LLDB use the bundle version from
`tools/driver/lldb-Info.plist`. That file hasn't been updated since the 4.0
release so the version we print provides no value. I also think that even if it
were up to date, that number has no meaning and displaying the version from the
LLVM tree is more valuable.

I know that Apple folks have some form of override for the clang version to
match the Xcode version, so it'd make sense for them to do the same for LLDB.


https://reviews.llvm.org/D39429

Files:
  source/lldb.cpp


Index: source/lldb.cpp
===
--- source/lldb.cpp
+++ source/lldb.cpp
@@ -47,13 +47,9 @@
   // as the clang tool.
   static std::string g_version_str;
   if (g_version_str.empty()) {
-
-#ifdef LLDB_VERSION_STRING
-g_version_str += EXPAND_AND_QUOTE(LLDB_VERSION_STRING);
-#else
 g_version_str += "lldb version ";
 g_version_str += CLANG_VERSION_STRING;
-#endif
+
 const char *lldb_repo = GetLLDBRepository();
 const char *lldb_rev = GetLLDBRevision();
 if (lldb_repo || lldb_rev) {


Index: source/lldb.cpp
===
--- source/lldb.cpp
+++ source/lldb.cpp
@@ -47,13 +47,9 @@
   // as the clang tool.
   static std::string g_version_str;
   if (g_version_str.empty()) {
-
-#ifdef LLDB_VERSION_STRING
-g_version_str += EXPAND_AND_QUOTE(LLDB_VERSION_STRING);
-#else
 g_version_str += "lldb version ";
 g_version_str += CLANG_VERSION_STRING;
-#endif
+
 const char *lldb_repo = GetLLDBRepository();
 const char *lldb_rev = GetLLDBRevision();
 if (lldb_repo || lldb_rev) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D39429: Use LLVM version string

2017-11-02 Thread Stephane Sezer via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL317218: Use LLVM version string (authored by sas).

Repository:
  rL LLVM

https://reviews.llvm.org/D39429

Files:
  lldb/trunk/source/lldb.cpp


Index: lldb/trunk/source/lldb.cpp
===
--- lldb/trunk/source/lldb.cpp
+++ lldb/trunk/source/lldb.cpp
@@ -47,13 +47,9 @@
   // as the clang tool.
   static std::string g_version_str;
   if (g_version_str.empty()) {
-
-#ifdef LLDB_VERSION_STRING
-g_version_str += EXPAND_AND_QUOTE(LLDB_VERSION_STRING);
-#else
 g_version_str += "lldb version ";
 g_version_str += CLANG_VERSION_STRING;
-#endif
+
 const char *lldb_repo = GetLLDBRepository();
 const char *lldb_rev = GetLLDBRevision();
 if (lldb_repo || lldb_rev) {


Index: lldb/trunk/source/lldb.cpp
===
--- lldb/trunk/source/lldb.cpp
+++ lldb/trunk/source/lldb.cpp
@@ -47,13 +47,9 @@
   // as the clang tool.
   static std::string g_version_str;
   if (g_version_str.empty()) {
-
-#ifdef LLDB_VERSION_STRING
-g_version_str += EXPAND_AND_QUOTE(LLDB_VERSION_STRING);
-#else
 g_version_str += "lldb version ";
 g_version_str += CLANG_VERSION_STRING;
-#endif
+
 const char *lldb_repo = GetLLDBRepository();
 const char *lldb_rev = GetLLDBRevision();
 if (lldb_repo || lldb_rev) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits