Skip to content

Commit fe8933b

Browse files
authored
[lldb][DataFormatter] Simplify std::map formatter (#97579)
Depends on: * #97544 * #97549 * #97551 This patch tries to simplify the way in which the `std::map` formatter goes from the root `__tree` pointer to a specific key/value pair. Previously we would: 1. synthesize a structure that mimicked what `__iter_pointer` looked like in memory 2. call `GetChildCompilerTypeAtIndex` on it to find the byte offset at which the pair was located in the synthesized structure 3. finally, use that offset through a call to `GetSyntheticChildAtOffset` to retrieve the key/value pair Not only was this logic hard to follow, and encoded the libc++ layout in non-obvious ways, it was also fragile to alignment miscalculations (#97443); this would break once the new layout of std::map landed as part of #93069. Instead, this patch simply casts the `__iter_pointer` to the `__node_pointer` and uses a straightforward `GetChildMemberWithName("__value_")` to get to the key/value we care about. This allows us to get rid of some support infrastructure/class state. Ideally we would fix the underlying alignment issue, but this unblocks the libc++ refactor in the interim, while also benefitting the formatter in terms of readability (in my opinion).
1 parent 7776fba commit fe8933b

File tree

1 file changed

+41
-109
lines changed

1 file changed

+41
-109
lines changed

lldb/source/Plugins/Language/CPlusPlus/LibCxxMap.cpp

Lines changed: 41 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,31 @@
1919
#include "lldb/Utility/Stream.h"
2020
#include "lldb/lldb-enumerations.h"
2121
#include "lldb/lldb-forward.h"
22+
#include <cstdint>
23+
#include <locale>
24+
#include <optional>
2225

2326
using namespace lldb;
2427
using namespace lldb_private;
2528
using namespace lldb_private::formatters;
2629

30+
// The flattened layout of the std::__tree_iterator::__ptr_ looks
31+
// as follows:
32+
//
33+
// The following shows the contiguous block of memory:
34+
//
35+
// +-----------------------------+ class __tree_end_node
36+
// __ptr_ | pointer __left_; |
37+
// +-----------------------------+ class __tree_node_base
38+
// | pointer __right_; |
39+
// | __parent_pointer __parent_; |
40+
// | bool __is_black_; |
41+
// +-----------------------------+ class __tree_node
42+
// | __node_value_type __value_; | <<< our key/value pair
43+
// +-----------------------------+
44+
//
45+
// where __ptr_ has type __iter_pointer.
46+
2747
class MapEntry {
2848
public:
2949
MapEntry() = default;
@@ -182,10 +202,6 @@ class LibcxxStdMapSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
182202
size_t GetIndexOfChildWithName(ConstString name) override;
183203

184204
private:
185-
bool GetDataType();
186-
187-
void GetValueOffset(const lldb::ValueObjectSP &node);
188-
189205
/// Returns the ValueObject for the __tree_node type that
190206
/// holds the key/value pair of the node at index \ref idx.
191207
///
@@ -204,8 +220,7 @@ class LibcxxStdMapSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
204220

205221
ValueObject *m_tree = nullptr;
206222
ValueObject *m_root_node = nullptr;
207-
CompilerType m_element_type;
208-
uint32_t m_skip_size = UINT32_MAX;
223+
CompilerType m_node_ptr_type;
209224
size_t m_count = UINT32_MAX;
210225
std::map<size_t, MapIterator> m_iterators;
211226
};
@@ -234,7 +249,7 @@ class LibCxxMapIteratorSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
234249

235250
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::
236251
LibcxxStdMapSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
237-
: SyntheticChildrenFrontEnd(*valobj_sp), m_element_type(), m_iterators() {
252+
: SyntheticChildrenFrontEnd(*valobj_sp) {
238253
if (valobj_sp)
239254
Update();
240255
}
@@ -260,130 +275,44 @@ llvm::Expected<uint32_t> lldb_private::formatters::
260275
return m_count;
261276
}
262277

263-
bool lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetDataType() {
264-
if (m_element_type.IsValid())
265-
return true;
266-
m_element_type.Clear();
267-
ValueObjectSP deref;
268-
Status error;
269-
deref = m_root_node->Dereference(error);
270-
if (!deref || error.Fail())
271-
return false;
272-
deref = m_backend.GetChildAtNamePath({"__tree_", "__pair3_"});
273-
if (!deref)
274-
return false;
275-
m_element_type = deref->GetCompilerType()
276-
.GetTypeTemplateArgument(1)
277-
.GetTypeTemplateArgument(1);
278-
if (m_element_type) {
279-
std::string name;
280-
uint64_t bit_offset_ptr;
281-
uint32_t bitfield_bit_size_ptr;
282-
bool is_bitfield_ptr;
283-
m_element_type = m_element_type.GetFieldAtIndex(
284-
0, name, &bit_offset_ptr, &bitfield_bit_size_ptr, &is_bitfield_ptr);
285-
m_element_type = m_element_type.GetTypedefedType();
286-
return m_element_type.IsValid();
287-
} else {
288-
m_element_type = m_backend.GetCompilerType().GetTypeTemplateArgument(0);
289-
return m_element_type.IsValid();
290-
}
291-
}
292-
293-
void lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
294-
const lldb::ValueObjectSP &node) {
295-
if (m_skip_size != UINT32_MAX)
296-
return;
297-
if (!node)
298-
return;
299-
300-
CompilerType node_type(node->GetCompilerType());
301-
auto ast_ctx = node_type.GetTypeSystem().dyn_cast_or_null<TypeSystemClang>();
302-
if (!ast_ctx)
303-
return;
304-
305-
CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
306-
llvm::StringRef(),
307-
{{"ptr0", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
308-
{"ptr1", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
309-
{"ptr2", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
310-
{"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
311-
{"payload", (m_element_type.GetCompleteType(), m_element_type)}});
312-
std::string child_name;
313-
uint32_t child_byte_size;
314-
int32_t child_byte_offset = 0;
315-
uint32_t child_bitfield_bit_size;
316-
uint32_t child_bitfield_bit_offset;
317-
bool child_is_base_class;
318-
bool child_is_deref_of_parent;
319-
uint64_t language_flags;
320-
auto child_type =
321-
llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
322-
nullptr, 4, true, true, true, child_name, child_byte_size,
323-
child_byte_offset, child_bitfield_bit_size, child_bitfield_bit_offset,
324-
child_is_base_class, child_is_deref_of_parent, nullptr,
325-
language_flags));
326-
if (child_type && child_type->IsValid())
327-
m_skip_size = (uint32_t)child_byte_offset;
328-
}
329-
330278
ValueObjectSP
331279
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair(
332280
size_t idx, size_t max_depth) {
333281
MapIterator iterator(m_root_node, max_depth);
334282

335-
const bool need_to_skip = (idx > 0);
336-
size_t actual_advance = idx;
337-
if (need_to_skip) {
283+
size_t advance_by = idx;
284+
if (idx > 0) {
338285
// If we have already created the iterator for the previous
339286
// index, we can start from there and advance by 1.
340287
auto cached_iterator = m_iterators.find(idx - 1);
341288
if (cached_iterator != m_iterators.end()) {
342289
iterator = cached_iterator->second;
343-
actual_advance = 1;
290+
advance_by = 1;
344291
}
345292
}
346293

347-
ValueObjectSP iterated_sp(iterator.advance(actual_advance));
294+
ValueObjectSP iterated_sp(iterator.advance(advance_by));
348295
if (!iterated_sp)
349296
// this tree is garbage - stop
350297
return nullptr;
351298

352-
if (!GetDataType())
299+
if (!m_node_ptr_type.IsValid())
353300
return nullptr;
354301

355-
if (!need_to_skip) {
356-
Status error;
357-
iterated_sp = iterated_sp->Dereference(error);
358-
if (!iterated_sp || error.Fail())
359-
return nullptr;
360-
361-
GetValueOffset(iterated_sp);
362-
iterated_sp = iterated_sp->GetSyntheticChildAtOffset(m_skip_size,
363-
m_element_type, true);
364-
365-
if (!iterated_sp)
366-
return nullptr;
367-
} else {
368-
// because of the way our debug info is made, we need to read item 0
369-
// first so that we can cache information used to generate other elements
370-
if (m_skip_size == UINT32_MAX)
371-
GetChildAtIndex(0);
372-
373-
if (m_skip_size == UINT32_MAX)
374-
return nullptr;
375-
376-
iterated_sp = iterated_sp->GetSyntheticChildAtOffset(m_skip_size,
377-
m_element_type, true);
378-
if (!iterated_sp)
379-
return nullptr;
380-
}
302+
// iterated_sp is a __iter_pointer at this point.
303+
// We can cast it to a __node_pointer (which is what libc++ does).
304+
auto value_type_sp = iterated_sp->Cast(m_node_ptr_type);
305+
if (!value_type_sp)
306+
return nullptr;
307+
308+
// Finally, get the key/value pair.
309+
value_type_sp = value_type_sp->GetChildMemberWithName("__value_");
310+
if (!value_type_sp)
311+
return nullptr;
381312

382313
m_iterators[idx] = iterator;
383-
assert(iterated_sp != nullptr &&
384-
"Cached MapIterator for invalid ValueObject");
385314

386-
return iterated_sp;
315+
return value_type_sp;
387316
}
388317

389318
lldb::ValueObjectSP
@@ -443,6 +372,9 @@ lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::Update() {
443372
if (!m_tree)
444373
return lldb::ChildCacheState::eRefetch;
445374
m_root_node = m_tree->GetChildMemberWithName("__begin_node_").get();
375+
m_node_ptr_type =
376+
m_tree->GetCompilerType().GetDirectNestedTypeWithName("__node_pointer");
377+
446378
return lldb::ChildCacheState::eRefetch;
447379
}
448380

0 commit comments

Comments
 (0)