Author: aadsm Date: Tue Jun 25 15:22:13 2019 New Revision: 364355 URL: http://llvm.org/viewvc/llvm-project?rev=364355&view=rev Log: Revert "Add ReadCStringFromMemory for faster string reads"
This reverts commit a7335393f50246b59db450dc6005f7c8f29e73a6. It seems this is breaking a bunch of tests (https://reviews.llvm.org/D62503#1549874) so reverting until I find the time to repro and fix. Modified: lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h lldb/trunk/source/Host/common/NativeProcessProtocol.cpp lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp lldb/trunk/source/Plugins/Process/POSIX/NativeProcessELF.cpp lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp Modified: lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h?rev=364355&r1=364354&r2=364355&view=diff ============================================================================== --- lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h (original) +++ lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h Tue Jun 25 15:22:13 2019 @@ -84,31 +84,6 @@ public: Status ReadMemoryWithoutTrap(lldb::addr_t addr, void *buf, size_t size, size_t &bytes_read); - /// Reads a null terminated string from memory. - /// - /// Reads up to \p max_size bytes of memory until it finds a '\0'. - /// If a '\0' is not found then it reads max_size-1 bytes as a string and a - /// '\0' is added as the last character of the \p buffer. - /// - /// \param[in] addr - /// The address in memory to read from. - /// - /// \param[in] buffer - /// An allocated buffer with at least \p max_size size. - /// - /// \param[in] max_size - /// The maximum number of bytes to read from memory until it reads the - /// string. - /// - /// \param[out] total_bytes_read - /// The number of bytes read from memory into \p buffer. - /// - /// \return - /// Returns a StringRef backed up by the \p buffer passed in. - llvm::Expected<llvm::StringRef> - ReadCStringFromMemory(lldb::addr_t addr, char *buffer, size_t max_size, - size_t &total_bytes_read); - virtual Status WriteMemory(lldb::addr_t addr, const void *buf, size_t size, size_t &bytes_written) = 0; Modified: lldb/trunk/source/Host/common/NativeProcessProtocol.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/NativeProcessProtocol.cpp?rev=364355&r1=364354&r2=364355&view=diff ============================================================================== --- lldb/trunk/source/Host/common/NativeProcessProtocol.cpp (original) +++ lldb/trunk/source/Host/common/NativeProcessProtocol.cpp Tue Jun 25 15:22:13 2019 @@ -16,8 +16,6 @@ #include "lldb/Utility/State.h" #include "lldb/lldb-enumerations.h" -#include "llvm/Support/Process.h" - using namespace lldb; using namespace lldb_private; @@ -661,58 +659,6 @@ Status NativeProcessProtocol::ReadMemory return Status(); } -llvm::Expected<llvm::StringRef> -NativeProcessProtocol::ReadCStringFromMemory(lldb::addr_t addr, char *buffer, - size_t max_size, - size_t &total_bytes_read) { - static const size_t cache_line_size = - llvm::sys::Process::getPageSizeEstimate(); - size_t bytes_read = 0; - size_t bytes_left = max_size; - addr_t curr_addr = addr; - size_t string_size; - char *curr_buffer = buffer; - total_bytes_read = 0; - Status status; - - while (bytes_left > 0 && status.Success()) { - addr_t cache_line_bytes_left = - cache_line_size - (curr_addr % cache_line_size); - addr_t bytes_to_read = std::min<addr_t>(bytes_left, cache_line_bytes_left); - status = ReadMemory(curr_addr, reinterpret_cast<void *>(curr_buffer), - bytes_to_read, bytes_read); - - if (bytes_read == 0) - break; - - void *str_end = std::memchr(curr_buffer, '\0', bytes_read); - if (str_end != nullptr) { - total_bytes_read = - (size_t)(reinterpret_cast<char *>(str_end) - buffer + 1); - status.Clear(); - break; - } - - total_bytes_read += bytes_read; - curr_buffer += bytes_read; - curr_addr += bytes_read; - bytes_left -= bytes_read; - } - - string_size = total_bytes_read - 1; - - // Make sure we return a null terminated string. - if (bytes_left == 0 && max_size > 0 && buffer[max_size - 1] != '\0') { - buffer[max_size - 1] = '\0'; - total_bytes_read--; - } - - if (!status.Success()) - return status.ToError(); - - return llvm::StringRef(buffer, string_size); -} - lldb::StateType NativeProcessProtocol::GetState() const { std::lock_guard<std::recursive_mutex> guard(m_state_mutex); return m_state; Modified: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp?rev=364355&r1=364354&r2=364355&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp (original) +++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp Tue Jun 25 15:22:13 2019 @@ -2076,4 +2076,4 @@ Status NativeProcessLinux::StopProcessor m_processor_trace_monitor.erase(iter); return error; -} +} \ No newline at end of file Modified: lldb/trunk/source/Plugins/Process/POSIX/NativeProcessELF.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/POSIX/NativeProcessELF.cpp?rev=364355&r1=364354&r2=364355&view=diff ============================================================================== --- lldb/trunk/source/Plugins/Process/POSIX/NativeProcessELF.cpp (original) +++ lldb/trunk/source/Plugins/Process/POSIX/NativeProcessELF.cpp Tue Jun 25 15:22:13 2019 @@ -118,13 +118,14 @@ NativeProcessELF::ReadSVR4LibraryInfo(ll return error.ToError(); char name_buffer[PATH_MAX]; - llvm::Expected<llvm::StringRef> string_or_error = ReadCStringFromMemory( - link_map.l_name, &name_buffer[0], sizeof(name_buffer), bytes_read); - if (!string_or_error) - return string_or_error.takeError(); + error = ReadMemory(link_map.l_name, &name_buffer, sizeof(name_buffer), + bytes_read); + if (!error.Success()) + return error.ToError(); + name_buffer[PATH_MAX - 1] = '\0'; SVR4LibraryInfo info; - info.name = string_or_error->str(); + info.name = std::string(name_buffer); info.link_map = link_map_addr; info.base_addr = link_map.l_addr; info.ld_addr = link_map.l_ld; Modified: lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp?rev=364355&r1=364354&r2=364355&view=diff ============================================================================== --- lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp (original) +++ lldb/trunk/unittests/Host/NativeProcessProtocolTest.cpp Tue Jun 25 15:22:13 2019 @@ -9,7 +9,6 @@ #include "TestingSupport/Host/NativeProcessTestUtils.h" #include "lldb/Host/common/NativeProcessProtocol.h" -#include "llvm/Support/Process.h" #include "llvm/Testing/Support/Error.h" #include "gmock/gmock.h" @@ -97,53 +96,3 @@ TEST(NativeProcessProtocolTest, ReadMemo EXPECT_THAT_EXPECTED(Process.ReadMemoryWithoutTrap(4, 2), llvm::HasValue(std::vector<uint8_t>{4, 5})); } - -TEST(NativeProcessProtocolTest, ReadCStringFromMemory) { - NiceMock<MockDelegate> DummyDelegate; - MockProcess<NativeProcessProtocol> Process(DummyDelegate, - ArchSpec("aarch64-pc-linux")); - FakeMemory M({'h', 'e', 'l', 'l', 'o', 0, 'w', 'o'}); - EXPECT_CALL(Process, ReadMemory(_, _)) - .WillRepeatedly(Invoke(&M, &FakeMemory::Read)); - - char string[1024]; - size_t bytes_read; - EXPECT_THAT_EXPECTED(Process.ReadCStringFromMemory( - 0x0, &string[0], sizeof(string), bytes_read), - llvm::HasValue(llvm::StringRef("hello"))); - EXPECT_EQ(bytes_read, 6UL); -} - -TEST(NativeProcessProtocolTest, ReadCStringFromMemory_MaxSize) { - NiceMock<MockDelegate> DummyDelegate; - MockProcess<NativeProcessProtocol> Process(DummyDelegate, - ArchSpec("aarch64-pc-linux")); - FakeMemory M({'h', 'e', 'l', 'l', 'o', 0, 'w', 'o'}); - EXPECT_CALL(Process, ReadMemory(_, _)) - .WillRepeatedly(Invoke(&M, &FakeMemory::Read)); - - char string[4]; - size_t bytes_read; - EXPECT_THAT_EXPECTED(Process.ReadCStringFromMemory( - 0x0, &string[0], sizeof(string), bytes_read), - llvm::HasValue(llvm::StringRef("hel"))); - EXPECT_EQ(bytes_read, 3UL); -} - -TEST(NativeProcessProtocolTest, ReadCStringFromMemory_CrossPageBoundary) { - NiceMock<MockDelegate> DummyDelegate; - MockProcess<NativeProcessProtocol> Process(DummyDelegate, - ArchSpec("aarch64-pc-linux")); - unsigned string_start = llvm::sys::Process::getPageSizeEstimate() - 3; - FakeMemory M({'h', 'e', 'l', 'l', 'o', 0, 'w', 'o'}, string_start); - EXPECT_CALL(Process, ReadMemory(_, _)) - .WillRepeatedly(Invoke(&M, &FakeMemory::Read)); - - char string[1024]; - size_t bytes_read; - EXPECT_THAT_EXPECTED(Process.ReadCStringFromMemory(string_start, &string[0], - sizeof(string), - bytes_read), - llvm::HasValue(llvm::StringRef("hello"))); - EXPECT_EQ(bytes_read, 6UL); -} \ No newline at end of file _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
