[Lldb-commits] [PATCH] D47742: Change SWIG output directory when building LLDB.framework with CMake
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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()
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()
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()
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
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
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
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
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
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
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
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
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
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
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
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
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)
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)
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.
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.
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
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
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.
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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