Skip to content

Commit 1107b88

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 9dabd3a commit 1107b88

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
};
@@ -213,7 +228,7 @@ class LibcxxStdMapSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
213228

214229
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::
215230
LibcxxStdMapSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
216-
: SyntheticChildrenFrontEnd(*valobj_sp), m_element_type(), m_iterators() {
231+
: SyntheticChildrenFrontEnd(*valobj_sp) {
217232
if (valobj_sp)
218233
Update();
219234
}
@@ -239,146 +254,52 @@ llvm::Expected<uint32_t> lldb_private::formatters::
239254
return m_count;
240255
}
241256

242-
bool lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetDataType() {
243-
if (m_element_type.IsValid())
244-
return true;
245-
m_element_type.Clear();
246-
ValueObjectSP deref;
247-
Status error;
248-
deref = m_root_node->Dereference(error);
249-
if (!deref || error.Fail())
250-
return false;
251-
deref = deref->GetChildMemberWithName("__value_");
252-
if (deref) {
253-
m_element_type = deref->GetCompilerType();
254-
return true;
255-
}
256-
deref = m_backend.GetChildAtNamePath({"__tree_", "__pair3_"});
257-
if (!deref)
258-
return false;
259-
m_element_type = deref->GetCompilerType()
260-
.GetTypeTemplateArgument(1)
261-
.GetTypeTemplateArgument(1);
262-
if (m_element_type) {
263-
std::string name;
264-
uint64_t bit_offset_ptr;
265-
uint32_t bitfield_bit_size_ptr;
266-
bool is_bitfield_ptr;
267-
m_element_type = m_element_type.GetFieldAtIndex(
268-
0, name, &bit_offset_ptr, &bitfield_bit_size_ptr, &is_bitfield_ptr);
269-
m_element_type = m_element_type.GetTypedefedType();
270-
return m_element_type.IsValid();
271-
} else {
272-
m_element_type = m_backend.GetCompilerType().GetTypeTemplateArgument(0);
273-
return m_element_type.IsValid();
274-
}
275-
}
276-
277-
void lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetValueOffset(
278-
const lldb::ValueObjectSP &node) {
279-
if (m_skip_size != UINT32_MAX)
280-
return;
281-
if (!node)
282-
return;
283-
CompilerType node_type(node->GetCompilerType());
284-
uint64_t bit_offset;
285-
if (node_type.GetIndexOfFieldWithName("__value_", nullptr, &bit_offset) !=
286-
UINT32_MAX) {
287-
// Old layout (pre 089a7cc5dea)
288-
m_skip_size = bit_offset / 8u;
289-
} else {
290-
auto ast_ctx = node_type.GetTypeSystem().dyn_cast_or_null<TypeSystemClang>();
291-
if (!ast_ctx)
292-
return;
293-
CompilerType tree_node_type = ast_ctx->CreateStructForIdentifier(
294-
llvm::StringRef(),
295-
{{"ptr0", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
296-
{"ptr1", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
297-
{"ptr2", ast_ctx->GetBasicType(lldb::eBasicTypeVoid).GetPointerType()},
298-
{"cw", ast_ctx->GetBasicType(lldb::eBasicTypeBool)},
299-
{"payload", (m_element_type.GetCompleteType(), m_element_type)}});
300-
std::string child_name;
301-
uint32_t child_byte_size;
302-
int32_t child_byte_offset = 0;
303-
uint32_t child_bitfield_bit_size;
304-
uint32_t child_bitfield_bit_offset;
305-
bool child_is_base_class;
306-
bool child_is_deref_of_parent;
307-
uint64_t language_flags;
308-
auto child_type =
309-
llvm::expectedToStdOptional(tree_node_type.GetChildCompilerTypeAtIndex(
310-
nullptr, 4, true, true, true, child_name, child_byte_size,
311-
child_byte_offset, child_bitfield_bit_size,
312-
child_bitfield_bit_offset, child_is_base_class,
313-
child_is_deref_of_parent, nullptr, language_flags));
314-
if (child_type && child_type->IsValid())
315-
m_skip_size = (uint32_t)child_byte_offset;
316-
}
317-
}
318-
319257
ValueObjectSP
320258
lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::GetKeyValuePair(
321259
size_t idx, size_t max_depth) {
322260
MapIterator iterator(m_root_node, max_depth);
323261

324-
const bool need_to_skip = (idx > 0);
325-
size_t actual_advance = idx;
326-
if (need_to_skip) {
262+
size_t advance_by = idx;
263+
if (idx > 0) {
327264
// If we have already created the iterator for the previous
328265
// index, we can start from there and advance by 1.
329266
auto cached_iterator = m_iterators.find(idx - 1);
330267
if (cached_iterator != m_iterators.end()) {
331268
iterator = cached_iterator->second;
332-
actual_advance = 1;
269+
advance_by = 1;
333270
}
334271
}
335272

336-
ValueObjectSP iterated_sp(iterator.advance(actual_advance));
273+
ValueObjectSP iterated_sp(iterator.advance(advance_by));
337274
if (!iterated_sp)
338275
// this tree is garbage - stop
339276
return nullptr;
340277

341-
if (!GetDataType())
278+
if (!m_node_ptr_type.IsValid())
342279
return nullptr;
343280

344-
if (!need_to_skip) {
345-
Status error;
346-
iterated_sp = iterated_sp->Dereference(error);
347-
if (!iterated_sp || error.Fail())
348-
return nullptr;
349-
350-
GetValueOffset(iterated_sp);
351-
auto child_sp = iterated_sp->GetChildMemberWithName("__value_");
352-
if (child_sp) {
353-
// Old layout (pre 089a7cc5dea)
354-
iterated_sp = child_sp;
355-
} else {
356-
iterated_sp = iterated_sp->GetSyntheticChildAtOffset(
357-
m_skip_size, m_element_type, true);
358-
}
281+
// iterated_sp is a __iter_pointer at this point.
282+
// We can cast it to a __node_pointer (which is what libc++ does).
283+
auto value_type_sp = iterated_sp->Cast(m_node_ptr_type);
284+
if (!value_type_sp)
285+
return nullptr;
359286

360-
if (!iterated_sp)
361-
return nullptr;
362-
} else {
363-
// because of the way our debug info is made, we need to read item 0
364-
// first so that we can cache information used to generate other elements
365-
if (m_skip_size == UINT32_MAX)
366-
GetChildAtIndex(0);
367-
368-
if (m_skip_size == UINT32_MAX)
369-
return nullptr;
370-
371-
iterated_sp = iterated_sp->GetSyntheticChildAtOffset(m_skip_size,
372-
m_element_type, true);
373-
if (!iterated_sp)
374-
return nullptr;
375-
}
287+
// After dereferencing the __node_pointer, we will have a handle to
288+
// a std::__1::__tree_node<void *>, which has the __value_ member
289+
// we are looking for.
290+
Status s;
291+
value_type_sp = value_type_sp->Dereference(s);
292+
if (!value_type_sp || s.Fail())
293+
return nullptr;
294+
295+
// Finally, get the key/value pair.
296+
value_type_sp = value_type_sp->GetChildMemberWithName("__value_");
297+
if (!value_type_sp)
298+
return nullptr;
376299

377300
m_iterators[idx] = iterator;
378-
assert(iterated_sp != nullptr &&
379-
"Cached MapIterator for invalid ValueObject");
380301

381-
return iterated_sp;
302+
return value_type_sp;
382303
}
383304

384305
lldb::ValueObjectSP
@@ -438,6 +359,9 @@ lldb_private::formatters::LibcxxStdMapSyntheticFrontEnd::Update() {
438359
if (!m_tree)
439360
return lldb::ChildCacheState::eRefetch;
440361
m_root_node = m_tree->GetChildMemberWithName("__begin_node_").get();
362+
m_node_ptr_type =
363+
m_tree->GetCompilerType().GetDirectNestedTypeWithName("__node_pointer");
364+
441365
return lldb::ChildCacheState::eRefetch;
442366
}
443367

0 commit comments

Comments
 (0)