[Lldb-commits] [lldb] r364355 - Revert "Add ReadCStringFromMemory for faster string reads"
Antonio Afonso via lldb-commits
lldb-commits at lists.llvm.org
Tue Jun 25 15:22:13 PDT 2019
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
More information about the lldb-commits
mailing list