Skip to content

Commit 223606f

Browse files
committed
[lldb][DataFormatter] Simplify std::map formatter
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 synthesize a structure that mimicked what `__iter_pointer` looked like in memory, then called `GetChildCompilerTypeAtIndex` on it to find the byte offset into that structure at which the pair was located at, and finally use that offset through a call to `GetSyntheticChildAtOffset` to retrieve that 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 after the new layout of std::map landed as part of inhttps://github.com//issues/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 b590e9a commit 223606f

File tree

1 file changed

+48
-124
lines changed

1 file changed

+48
-124
lines changed

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

Lines changed: 48 additions & 124 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,31 @@
1818
#include "lldb/Utility/Status.h"
1919
#include "lldb/Utility/Stream.h"
2020
#include "lldb/lldb-forward.h"
21+
#include <cstdint>
22+
#include <locale>
23+
#include <optional>
2124

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

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

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

204220
ValueObject *m_tree = nullptr;
205221
ValueObject *m_root_node = nullptr;
206-
CompilerType m_element_type;
207-
uint32_t m_skip_size = UINT32_MAX;
222+
CompilerType m_node_ptr_type;
208223
size_t m_count = UINT32_MAX;
209224
std::map<size_t, MapIterator> m_iterators;
210225
};
@@ -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,146 +275,52 @@ 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 = deref->GetChildMemberWithName("__value_");
273-
if (deref) {
274-
m_element_type = deref->GetCompilerType();
275-
return true;
276-
}
277-
deref = m_backend.GetChildAtNamePath({"__tree_", "__pair3_"});
278-
if (!deref)
279-
return false;
280-
m_element_type = deref->GetCompilerType()
281-
.GetTypeTemplateArgument(1)
282-
.GetTypeTemplateArgument(1);
283-
if (m_element_type) {
284-
std::string name;
285-
uint64_t bit_offset_ptr;
286-
uint32_t bitfield_bit_size_ptr;
287-
bool is_bitfield_ptr;
288-
m_element_type = m_element_type.GetFieldAtIndex(
289-
0, name, &bit_offset_ptr, &bitfield_bit_size_ptr, &is_bitfield_ptr);
290-
m_element_type = m_element_type.GetTypedefedType();
291-
return m_element_type.IsValid();
292-
} else {
293-
m_element_type = m_backend.GetCompilerType().GetTypeTemplateArgument(0);
294-
return m_element_type.IsValid();
295-
}
296-
}
297-
298-
void lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
299-
const lldb::ValueObjectSP &node) {
300-
if (m_skip_size != UINT32_MAX)
301-
return;
302-
if (!node)
303-
return;
304-
CompilerType node_type(node->GetCompilerType());
305-
uint64_t bit_offset;
306-
if (node_type.GetIndexOfFieldWithName("__value_", nullptr, &bit_offset) !=
307-
UINT32_MAX) {
308-
// Old layout (pre d05b10ab4fc65)
309-
m_skip_size = bit_offset / 8u;
310-
} else {
311-
auto ast_ctx = node_type.GetTypeSystem().dyn_cast_or_null<TypeSystemClang>();
312-
if (!ast_ctx)
313-
return;
314-
CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
315-
llvm::StringRef(),
316-
{{"ptr0", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
317-
{"ptr1", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
318-
{"ptr2", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
319-
{"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
320-
{"payload", (m_element_type.GetCompleteType(), m_element_type)}});
321-
std::string child_name;
322-
uint32_t child_byte_size;
323-
int32_t child_byte_offset = 0;
324-
uint32_t child_bitfield_bit_size;
325-
uint32_t child_bitfield_bit_offset;
326-
bool child_is_base_class;
327-
bool child_is_deref_of_parent;
328-
uint64_t language_flags;
329-
auto child_type =
330-
llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
331-
nullptr, 4, true, true, true, child_name, child_byte_size,
332-
child_byte_offset, child_bitfield_bit_size,
333-
child_bitfield_bit_offset, child_is_base_class,
334-
child_is_deref_of_parent, nullptr, language_flags));
335-
if (child_type && child_type->IsValid())
336-
m_skip_size = (uint32_t)child_byte_offset;
337-
}
338-
}
339-
340278
ValueObjectSP
341279
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair(
342280
size_t idx, size_t max_depth) {
343281
MapIterator iterator(m_root_node, max_depth);
344282

345-
const bool need_to_skip = (idx > 0);
346-
size_t actual_advance = idx;
347-
if (need_to_skip) {
283+
size_t advance_by = idx;
284+
if (idx > 0) {
348285
// If we have already created the iterator for the previous
349286
// index, we can start from there and advance by 1.
350287
auto cached_iterator = m_iterators.find(idx - 1);
351288
if (cached_iterator != m_iterators.end()) {
352289
iterator = cached_iterator->second;
353-
actual_advance = 1;
290+
advance_by = 1;
354291
}
355292
}
356293

357-
ValueObjectSP iterated_sp(iterator.advance(actual_advance));
294+
ValueObjectSP iterated_sp(iterator.advance(advance_by));
358295
if (!iterated_sp)
359296
// this tree is garbage - stop
360297
return nullptr;
361298

362-
if (!GetDataType())
299+
if (!m_node_ptr_type.IsValid())
363300
return nullptr;
364301

365-
if (!need_to_skip) {
366-
Status error;
367-
iterated_sp = iterated_sp->Dereference(error);
368-
if (!iterated_sp || error.Fail())
369-
return nullptr;
370-
371-
GetValueOffset(iterated_sp);
372-
auto child_sp = iterated_sp->GetChildMemberWithName("__value_");
373-
if (child_sp) {
374-
// Old layout (pre 089a7cc5dea)
375-
iterated_sp = child_sp;
376-
} else {
377-
iterated_sp = iterated_sp->GetSyntheticChildAtOffset(
378-
m_skip_size, m_element_type, true);
379-
}
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;
380307

381-
if (!iterated_sp)
382-
return nullptr;
383-
} else {
384-
// because of the way our debug info is made, we need to read item 0
385-
// first so that we can cache information used to generate other elements
386-
if (m_skip_size == UINT32_MAX)
387-
GetChildAtIndex(0);
388-
389-
if (m_skip_size == UINT32_MAX)
390-
return nullptr;
391-
392-
iterated_sp = iterated_sp->GetSyntheticChildAtOffset(m_skip_size,
393-
m_element_type, true);
394-
if (!iterated_sp)
395-
return nullptr;
396-
}
308+
// After dereferencing the __node_pointer, we will have a handle to
309+
// a std::__1::__tree_node<void *>, which has the __value_ member
310+
// we are looking for.
311+
Status s;
312+
value_type_sp = value_type_sp->Dereference(s);
313+
if (!value_type_sp || s.Fail())
314+
return nullptr;
315+
316+
// Finally, get the key/value pair.
317+
value_type_sp = value_type_sp->GetChildMemberWithName("__value_");
318+
if (!value_type_sp)
319+
return nullptr;
397320

398321
m_iterators[idx] = iterator;
399-
assert(iterated_sp != nullptr &&
400-
"Cached MapIterator for invalid ValueObject");
401322

402-
return iterated_sp;
323+
return value_type_sp;
403324
}
404325

405326
lldb::ValueObjectSP
@@ -459,6 +380,9 @@ lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::Update() {
459380
if (!m_tree)
460381
return lldb::ChildCacheState::eRefetch;
461382
m_root_node = m_tree->GetChildMemberWithName("__begin_node_").get();
383+
m_node_ptr_type =
384+
m_tree->GetCompilerType().GetDirectNestedTypeWithName("__node_pointer");
385+
462386
return lldb::ChildCacheState::eRefetch;
463387
}
464388

0 commit comments

Comments
 (0)