Skip to content

Commit bed17c0

Browse files
[lldb][NFCI] Refactor AppleObjCClassDescriptorV2 method_t parsing functions (#163291)
This rewrites ClassDescriptorV2::method_t::Read (and the loop calling that function) in an NFCI way to perform a couple of things: 1. Cleanup code with indirect style. For example, the old loop would have default-constructor a `unique_ptr<method_t>`, which was *reused* on every iteration of the loop. It called `method_t::Read` on each iteration, and never checked the return value prior to invoking the callback. In other words, if `Read` failed, the callback was called on random values. 2. Exposed memory reads that could benefit from the MultiMemoryRead packet proposed in [1]. [1]: https://discourse.llvm.org/t/rfc-a-new-vectorized-memory-read-packet/88441
1 parent 93185ea commit bed17c0

File tree

2 files changed

+62
-29
lines changed

2 files changed

+62
-29
lines changed

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp

Lines changed: 56 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "lldb/Utility/LLDBLog.h"
1515
#include "lldb/Utility/Log.h"
1616
#include "lldb/lldb-enumerations.h"
17+
#include "llvm/ADT/Sequence.h"
1718

1819
using namespace lldb;
1920
using namespace lldb_private;
@@ -266,22 +267,47 @@ bool ClassDescriptorV2::method_list_t::Read(Process *process,
266267
return true;
267268
}
268269

269-
bool ClassDescriptorV2::method_t::Read(Process *process, lldb::addr_t addr,
270-
lldb::addr_t relative_selector_base_addr,
271-
bool is_small, bool has_direct_sel) {
272-
size_t ptr_size = process->GetAddressByteSize();
273-
size_t size = GetSize(process, is_small);
270+
llvm::SmallVector<ClassDescriptorV2::method_t, 0>
271+
ClassDescriptorV2::ReadMethods(llvm::ArrayRef<lldb::addr_t> addresses,
272+
lldb::addr_t relative_selector_base_addr,
273+
bool is_small, bool has_direct_sel) const {
274+
lldb_private::Process *process = m_runtime.GetProcess();
275+
if (!process)
276+
return {};
274277

275-
DataBufferHeap buffer(size, '\0');
276-
Status error;
278+
const size_t size = method_t::GetSize(process, is_small);
279+
const size_t num_methods = addresses.size();
277280

278-
process->ReadMemory(addr, buffer.GetBytes(), size, error);
279-
if (error.Fail()) {
280-
return false;
281+
llvm::SmallVector<uint8_t, 0> buffer(num_methods * size, 0);
282+
llvm::DenseSet<uint32_t> failed_indices;
283+
284+
for (auto [idx, addr] : llvm::enumerate(addresses)) {
285+
Status error;
286+
process->ReadMemory(addr, buffer.data() + idx * size, size, error);
287+
if (error.Fail())
288+
failed_indices.insert(idx);
281289
}
282290

283-
DataExtractor extractor(buffer.GetBytes(), size, process->GetByteOrder(),
284-
ptr_size);
291+
llvm::SmallVector<method_t, 0> methods;
292+
methods.reserve(num_methods);
293+
for (auto [idx, addr] : llvm::enumerate(addresses)) {
294+
if (failed_indices.contains(idx))
295+
continue;
296+
DataExtractor extractor(buffer.data() + idx * size, size,
297+
process->GetByteOrder(),
298+
process->GetAddressByteSize());
299+
methods.push_back(method_t());
300+
methods.back().Read(extractor, process, addr, relative_selector_base_addr,
301+
is_small, has_direct_sel);
302+
}
303+
304+
return methods;
305+
}
306+
307+
bool ClassDescriptorV2::method_t::Read(DataExtractor &extractor,
308+
Process *process, lldb::addr_t addr,
309+
lldb::addr_t relative_selector_base_addr,
310+
bool is_small, bool has_direct_sel) {
285311
lldb::offset_t cursor = 0;
286312

287313
if (is_small) {
@@ -291,11 +317,11 @@ bool ClassDescriptorV2::method_t::Read(Process *process, lldb::addr_t addr,
291317

292318
m_name_ptr = addr + nameref_offset;
293319

320+
Status error;
294321
if (!has_direct_sel) {
295322
// The SEL offset points to a SELRef. We need to dereference twice.
296-
m_name_ptr = process->ReadUnsignedIntegerFromMemory(m_name_ptr, ptr_size,
297-
0, error);
298-
if (!error.Success())
323+
m_name_ptr = process->ReadPointerFromMemory(m_name_ptr, error);
324+
if (error.Fail())
299325
return false;
300326
} else if (relative_selector_base_addr != LLDB_INVALID_ADDRESS) {
301327
m_name_ptr = relative_selector_base_addr + nameref_offset;
@@ -308,13 +334,13 @@ bool ClassDescriptorV2::method_t::Read(Process *process, lldb::addr_t addr,
308334
m_imp_ptr = extractor.GetAddress_unchecked(&cursor);
309335
}
310336

337+
Status error;
311338
process->ReadCStringFromMemory(m_name_ptr, m_name, error);
312-
if (error.Fail()) {
339+
if (error.Fail())
313340
return false;
314-
}
315341

316342
process->ReadCStringFromMemory(m_types_ptr, m_types, error);
317-
return !error.Fail();
343+
return error.Success();
318344
}
319345

320346
bool ClassDescriptorV2::ivar_list_t::Read(Process *process, lldb::addr_t addr) {
@@ -447,17 +473,19 @@ ClassDescriptorV2::GetMethodList(Process *process,
447473
bool ClassDescriptorV2::ProcessMethodList(
448474
std::function<bool(const char *, const char *)> const &instance_method_func,
449475
ClassDescriptorV2::method_list_t &method_list) const {
450-
lldb_private::Process *process = m_runtime.GetProcess();
451-
auto method = std::make_unique<method_t>();
452-
lldb::addr_t relative_selector_base_addr =
453-
m_runtime.GetRelativeSelectorBaseAddr();
454-
for (uint32_t i = 0, e = method_list.m_count; i < e; ++i) {
455-
method->Read(process, method_list.m_first_ptr + (i * method_list.m_entsize),
456-
relative_selector_base_addr, method_list.m_is_small,
457-
method_list.m_has_direct_selector);
458-
if (instance_method_func(method->m_name.c_str(), method->m_types.c_str()))
476+
auto idx_to_method_addr = [&](uint32_t idx) {
477+
return method_list.m_first_ptr + (idx * method_list.m_entsize);
478+
};
479+
llvm::SmallVector<addr_t> addresses = llvm::to_vector(llvm::map_range(
480+
llvm::seq<uint32_t>(method_list.m_count), idx_to_method_addr));
481+
482+
llvm::SmallVector<method_t, 0> methods =
483+
ReadMethods(addresses, m_runtime.GetRelativeSelectorBaseAddr(),
484+
method_list.m_is_small, method_list.m_has_direct_selector);
485+
486+
for (const auto &method : methods)
487+
if (instance_method_func(method.m_name.c_str(), method.m_types.c_str()))
459488
break;
460-
}
461489
return true;
462490
}
463491

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,11 +172,16 @@ class ClassDescriptorV2 : public ObjCLanguageRuntime::ClassDescriptor {
172172
+ field_size; // IMP imp;
173173
}
174174

175-
bool Read(Process *process, lldb::addr_t addr,
175+
bool Read(DataExtractor &extractor, Process *process, lldb::addr_t addr,
176176
lldb::addr_t relative_selector_base_addr, bool is_small,
177177
bool has_direct_sel);
178178
};
179179

180+
llvm::SmallVector<method_t, 0>
181+
ReadMethods(llvm::ArrayRef<lldb::addr_t> addresses,
182+
lldb::addr_t relative_selector_base_addr, bool is_small,
183+
bool has_direct_sel) const;
184+
180185
struct ivar_list_t {
181186
uint32_t m_entsize;
182187
uint32_t m_count;

0 commit comments

Comments
 (0)