[LLDB] Add formatters for MSVC STL std::(forward_)list #148285
Conversation
|
@llvm/pr-subscribers-lldb Author: nerix (Nerixyz) ChangesAdds synthetic providers for MSVC's Towards #24834. Patch is 32.66 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/148285.diff 9 Files Affected:
diff --git a/lldb/examples/synthetic/gnu_libstdcpp.py b/lldb/examples/synthetic/gnu_libstdcpp.py
index 20b9488af5597..e59eb5fa18c02 100644
--- a/lldb/examples/synthetic/gnu_libstdcpp.py
+++ b/lldb/examples/synthetic/gnu_libstdcpp.py
@@ -6,15 +6,6 @@
# thing for your setup
-def ForwardListSummaryProvider(valobj, dict):
- list_capping_size = valobj.GetTarget().GetMaximumNumberOfChildrenToDisplay()
- text = "size=" + str(valobj.GetNumChildren())
- if valobj.GetNumChildren() > list_capping_size:
- return "(capped) " + text
- else:
- return text
-
-
def StdOptionalSummaryProvider(valobj, dict):
has_value = valobj.GetNumChildren() > 0
# We add wrapping spaces for consistency with the libcxx formatter
diff --git a/lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt b/lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
index 296159ea28407..c7f12c1255ece 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
+++ b/lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
@@ -14,11 +14,11 @@ add_lldb_library(lldbPluginCPlusPlusLanguage PLUGIN
CxxStringTypes.cpp
Generic.cpp
GenericBitset.cpp
+ GenericList.cpp
GenericOptional.cpp
LibCxx.cpp
LibCxxAtomic.cpp
LibCxxInitializerList.cpp
- LibCxxList.cpp
LibCxxMap.cpp
LibCxxQueue.cpp
LibCxxRangesRefView.cpp
diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index 2db3e6f0ca315..8ac023f3bb9cd 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -1440,14 +1440,12 @@ static void LoadLibStdcppFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
stl_deref_flags,
"lldb.formatters.cpp.gnu_libstdcpp.StdUnorderedMapSynthProvider")));
cpp_category_sp->AddTypeSynthetic(
- "^std::((__debug::)?|(__cxx11::)?)list<.+>(( )?&)?$",
- eFormatterMatchRegex,
+ "^std::__(debug|cxx11)::list<.+>(( )?&)?$", eFormatterMatchRegex,
SyntheticChildrenSP(new ScriptedSyntheticChildren(
stl_deref_flags,
"lldb.formatters.cpp.gnu_libstdcpp.StdListSynthProvider")));
cpp_category_sp->AddTypeSynthetic(
- "^std::((__debug::)?|(__cxx11::)?)forward_list<.+>(( )?&)?$",
- eFormatterMatchRegex,
+ "^std::__(debug|cxx11)::forward_list<.+>(( )?&)?$", eFormatterMatchRegex,
SyntheticChildrenSP(new ScriptedSyntheticChildren(
stl_synth_flags,
"lldb.formatters.cpp.gnu_libstdcpp.StdForwardListSynthProvider")));
@@ -1501,22 +1499,19 @@ static void LoadLibStdcppFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
"^std::(__debug::)?unordered_(multi)?(map|set)<.+> >$",
stl_summary_flags, true);
- AddCXXSummary(cpp_category_sp,
- lldb_private::formatters::ContainerSizeSummaryProvider,
- "libstdc++ std::list summary provider",
- "^std::((__debug::)?|(__cxx11::)?)list<.+>(( )?&)?$",
- stl_summary_flags, true);
+ AddCXXSummary(
+ cpp_category_sp, lldb_private::formatters::ContainerSizeSummaryProvider,
+ "libstdc++ std::list summary provider",
+ "^std::__(debug|cxx11)::list<.+>(( )?&)?$", stl_summary_flags, true);
AddCXXSummary(cpp_category_sp, ContainerSizeSummaryProvider,
"libstdc++ std::tuple summary provider",
"^std::tuple<.*>(( )?&)?$", stl_summary_flags, true);
- cpp_category_sp->AddTypeSummary(
- "^std::((__debug::)?|(__cxx11::)?)forward_list<.+>(( )?&)?$",
- eFormatterMatchRegex,
- TypeSummaryImplSP(new ScriptSummaryFormat(
- stl_summary_flags,
- "lldb.formatters.cpp.gnu_libstdcpp.ForwardListSummaryProvider")));
+ AddCXXSummary(cpp_category_sp, ContainerSizeSummaryProvider,
+ "libstdc++ std::forward_list summary provider",
+ "^std::__(debug|cxx11)::forward_list<.+>(( )?&)?$",
+ stl_summary_flags, true);
cpp_category_sp->AddTypeSummary(
"^std::variant<.+>$", eFormatterMatchRegex,
TypeSummaryImplSP(new ScriptSummaryFormat(
@@ -1599,6 +1594,31 @@ GenericSmartPointerSummaryProvider(ValueObject &valobj, Stream &stream,
return LibStdcppSmartPointerSummaryProvider(valobj, stream, options);
}
+static lldb_private::SyntheticChildrenFrontEnd *
+GenericListSyntheticFrontEndCreator(CXXSyntheticChildren *children,
+ lldb::ValueObjectSP valobj_sp) {
+ if (!valobj_sp)
+ return nullptr;
+
+ if (IsMsvcStlList(*valobj_sp))
+ return MsvcStlListSyntheticFrontEndCreator(children, valobj_sp);
+ return new ScriptedSyntheticChildren::FrontEnd(
+ "lldb.formatters.cpp.gnu_libstdcpp.StdListSynthProvider", *valobj_sp);
+}
+
+static lldb_private::SyntheticChildrenFrontEnd *
+GenericForwardListSyntheticFrontEndCreator(CXXSyntheticChildren *children,
+ lldb::ValueObjectSP valobj_sp) {
+ if (!valobj_sp)
+ return nullptr;
+
+ if (IsMsvcStlList(*valobj_sp))
+ return MsvcStlForwardListSyntheticFrontEndCreator(children, valobj_sp);
+ return new ScriptedSyntheticChildren::FrontEnd(
+ "lldb.formatters.cpp.gnu_libstdcpp.StdForwardListSynthProvider",
+ *valobj_sp);
+}
+
/// Load formatters that are formatting types from more than one STL
static void LoadCommonStlFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
if (!cpp_category_sp)
@@ -1642,12 +1662,21 @@ static void LoadCommonStlFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
},
"MSVC STL/libstdc++ std::wstring summary provider"));
+ stl_summary_flags.SetDontShowChildren(false);
+ stl_summary_flags.SetSkipPointers(false);
+
AddCXXSynthetic(cpp_category_sp, GenericSmartPointerSyntheticFrontEndCreator,
"std::shared_ptr synthetic children",
"^std::shared_ptr<.+>(( )?&)?$", stl_synth_flags, true);
AddCXXSynthetic(cpp_category_sp, GenericSmartPointerSyntheticFrontEndCreator,
"std::weak_ptr synthetic children",
"^std::weak_ptr<.+>(( )?&)?$", stl_synth_flags, true);
+ AddCXXSynthetic(cpp_category_sp, GenericListSyntheticFrontEndCreator,
+ "std::list synthetic children", "^std::list<.+>(( )?&)?$",
+ stl_synth_flags, true);
+ AddCXXSynthetic(cpp_category_sp, GenericForwardListSyntheticFrontEndCreator,
+ "std::forward_list synthetic children",
+ "^std::forward_list<.+>(( )?&)?$", stl_synth_flags, true);
AddCXXSummary(cpp_category_sp, GenericSmartPointerSummaryProvider,
"MSVC STL/libstdc++ std::shared_ptr summary provider",
@@ -1655,6 +1684,12 @@ static void LoadCommonStlFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
AddCXXSummary(cpp_category_sp, GenericSmartPointerSummaryProvider,
"MSVC STL/libstdc++ std::weak_ptr summary provider",
"^std::weak_ptr<.+>(( )?&)?$", stl_summary_flags, true);
+ AddCXXSummary(cpp_category_sp, ContainerSizeSummaryProvider,
+ "MSVC STL/libstdc++ std::list summary provider",
+ "^std::list<.+>(( )?&)?$", stl_summary_flags, true);
+ AddCXXSummary(cpp_category_sp, ContainerSizeSummaryProvider,
+ "MSVC STL/libstdc++ std::forward_list summary provider",
+ "^std::forward_list<.+>(( )?&)?$", stl_summary_flags, true);
}
static void LoadMsvcStlFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp b/lldb/source/Plugins/Language/CPlusPlus/GenericList.cpp
similarity index 58%
rename from lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
rename to lldb/source/Plugins/Language/CPlusPlus/GenericList.cpp
index 826e6ab090e10..ea1edbfd3ac9b 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxxList.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/GenericList.cpp
@@ -1,4 +1,4 @@
-//===-- LibCxxList.cpp ----------------------------------------------------===//
+//===-- GenericList.cpp ---------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -7,14 +7,11 @@
//===----------------------------------------------------------------------===//
#include "LibCxx.h"
+#include "MsvcStl.h"
-#include "Plugins/TypeSystem/Clang/TypeSystemClang.h"
#include "lldb/DataFormatters/FormattersHelpers.h"
#include "lldb/Target/Target.h"
-#include "lldb/Utility/DataBufferHeap.h"
-#include "lldb/Utility/Endian.h"
#include "lldb/Utility/Status.h"
-#include "lldb/Utility/Stream.h"
#include "lldb/ValueObject/ValueObject.h"
#include "lldb/ValueObject/ValueObjectConstResult.h"
#include "lldb/lldb-enumerations.h"
@@ -25,31 +22,27 @@ using namespace lldb_private::formatters;
namespace {
-class ListEntry {
+enum class StlType {
+ LibCxx,
+ MsvcStl,
+};
+
+template <StlType Stl> class ListEntry {
public:
ListEntry() = default;
ListEntry(ValueObjectSP entry_sp) : m_entry_sp(std::move(entry_sp)) {}
ListEntry(ValueObject *entry)
: m_entry_sp(entry ? entry->GetSP() : ValueObjectSP()) {}
- ListEntry next() {
- if (!m_entry_sp)
- return ListEntry();
- return ListEntry(m_entry_sp->GetChildMemberWithName("__next_"));
- }
-
- ListEntry prev() {
- if (!m_entry_sp)
- return ListEntry();
- return ListEntry(m_entry_sp->GetChildMemberWithName("__prev_"));
- }
-
uint64_t value() const {
if (!m_entry_sp)
return 0;
return m_entry_sp->GetValueAsUnsigned(0);
}
+ ListEntry next();
+ ListEntry prev();
+
bool null() { return (value() == 0); }
explicit operator bool() { return GetEntry() && !null(); }
@@ -66,10 +59,34 @@ class ListEntry {
ValueObjectSP m_entry_sp;
};
-class ListIterator {
+template <> ListEntry<StlType::LibCxx> ListEntry<StlType::LibCxx>::next() {
+ if (!m_entry_sp)
+ return ListEntry();
+ return ListEntry(m_entry_sp->GetChildMemberWithName("__next_"));
+}
+
+template <> ListEntry<StlType::LibCxx> ListEntry<StlType::LibCxx>::prev() {
+ if (!m_entry_sp)
+ return ListEntry();
+ return ListEntry(m_entry_sp->GetChildMemberWithName("__prev_"));
+}
+
+template <> ListEntry<StlType::MsvcStl> ListEntry<StlType::MsvcStl>::next() {
+ if (!m_entry_sp)
+ return ListEntry();
+ return ListEntry(m_entry_sp->GetChildMemberWithName("_Next"));
+}
+
+template <> ListEntry<StlType::MsvcStl> ListEntry<StlType::MsvcStl>::prev() {
+ if (!m_entry_sp)
+ return ListEntry();
+ return ListEntry(m_entry_sp->GetChildMemberWithName("_Prev"));
+}
+
+template <StlType Stl> class ListIterator {
public:
ListIterator() = default;
- ListIterator(ListEntry entry) : m_entry(std::move(entry)) {}
+ ListIterator(ListEntry<Stl> entry) : m_entry(std::move(entry)) {}
ListIterator(ValueObjectSP entry) : m_entry(std::move(entry)) {}
ListIterator(ValueObject *entry) : m_entry(entry) {}
@@ -101,9 +118,10 @@ class ListIterator {
void prev() { m_entry = m_entry.prev(); }
private:
- ListEntry m_entry;
+ ListEntry<Stl> m_entry;
};
+template <StlType Stl>
class AbstractListFrontEnd : public SyntheticChildrenFrontEnd {
public:
llvm::Expected<size_t> GetIndexOfChildWithName(ConstString name) override {
@@ -124,33 +142,31 @@ class AbstractListFrontEnd : public SyntheticChildrenFrontEnd {
ValueObject *m_head = nullptr;
static constexpr bool g_use_loop_detect = true;
- size_t m_loop_detected = 0; // The number of elements that have had loop
- // detection run over them.
- ListEntry m_slow_runner; // Used for loop detection
- ListEntry m_fast_runner; // Used for loop detection
+ size_t m_loop_detected = 0; // The number of elements that have had loop
+ // detection run over them.
+ ListEntry<Stl> m_slow_runner; // Used for loop detection
+ ListEntry<Stl> m_fast_runner; // Used for loop detection
size_t m_list_capping_size = 0;
CompilerType m_element_type;
- std::map<size_t, ListIterator> m_iterators;
+ std::map<size_t, ListIterator<Stl>> m_iterators;
bool HasLoop(size_t count);
ValueObjectSP GetItem(size_t idx);
};
-class ForwardListFrontEnd : public AbstractListFrontEnd {
+class LibCxxForwardListFrontEnd : public AbstractListFrontEnd<StlType::LibCxx> {
public:
- ForwardListFrontEnd(ValueObject &valobj);
+ LibCxxForwardListFrontEnd(ValueObject &valobj);
llvm::Expected<uint32_t> CalculateNumChildren() override;
ValueObjectSP GetChildAtIndex(uint32_t idx) override;
lldb::ChildCacheState Update() override;
};
-class ListFrontEnd : public AbstractListFrontEnd {
+class LibCxxListFrontEnd : public AbstractListFrontEnd<StlType::LibCxx> {
public:
- ListFrontEnd(lldb::ValueObjectSP valobj_sp);
-
- ~ListFrontEnd() override = default;
+ LibCxxListFrontEnd(lldb::ValueObjectSP valobj_sp);
llvm::Expected<uint32_t> CalculateNumChildren() override;
@@ -163,9 +179,34 @@ class ListFrontEnd : public AbstractListFrontEnd {
ValueObject *m_tail = nullptr;
};
+class MsvcStlForwardListFrontEnd
+ : public AbstractListFrontEnd<StlType::MsvcStl> {
+public:
+ MsvcStlForwardListFrontEnd(ValueObject &valobj);
+
+ llvm::Expected<uint32_t> CalculateNumChildren() override;
+ ValueObjectSP GetChildAtIndex(uint32_t idx) override;
+ lldb::ChildCacheState Update() override;
+};
+
+class MsvcStlListFrontEnd : public AbstractListFrontEnd<StlType::MsvcStl> {
+public:
+ MsvcStlListFrontEnd(lldb::ValueObjectSP valobj_sp);
+
+ llvm::Expected<uint32_t> CalculateNumChildren() override;
+
+ lldb::ValueObjectSP GetChildAtIndex(uint32_t idx) override;
+
+ lldb::ChildCacheState Update() override;
+
+private:
+ ValueObject *m_tail = nullptr;
+};
+
} // end anonymous namespace
-lldb::ChildCacheState AbstractListFrontEnd::Update() {
+template <StlType Stl>
+lldb::ChildCacheState AbstractListFrontEnd<Stl>::Update() {
m_loop_detected = 0;
m_count = UINT32_MAX;
m_head = nullptr;
@@ -191,7 +232,7 @@ lldb::ChildCacheState AbstractListFrontEnd::Update() {
return lldb::ChildCacheState::eRefetch;
}
-bool AbstractListFrontEnd::HasLoop(size_t count) {
+template <StlType Stl> bool AbstractListFrontEnd<Stl>::HasLoop(size_t count) {
if (!g_use_loop_detect)
return false;
// don't bother checking for a loop if we won't actually need to jump nodes
@@ -201,7 +242,7 @@ bool AbstractListFrontEnd::HasLoop(size_t count) {
if (m_loop_detected == 0) {
// This is the first time we are being run (after the last update). Set up
// the loop invariant for the first element.
- m_slow_runner = ListEntry(m_head).next();
+ m_slow_runner = ListEntry<Stl>(m_head).next();
m_fast_runner = m_slow_runner.next();
m_loop_detected = 1;
}
@@ -225,9 +266,10 @@ bool AbstractListFrontEnd::HasLoop(size_t count) {
return m_slow_runner == m_fast_runner;
}
-ValueObjectSP AbstractListFrontEnd::GetItem(size_t idx) {
+template <StlType Stl>
+ValueObjectSP AbstractListFrontEnd<Stl>::GetItem(size_t idx) {
size_t advance = idx;
- ListIterator current(m_head);
+ ListIterator<Stl> current(m_head);
if (idx > 0) {
auto cached_iterator = m_iterators.find(idx - 1);
if (cached_iterator != m_iterators.end()) {
@@ -240,16 +282,16 @@ ValueObjectSP AbstractListFrontEnd::GetItem(size_t idx) {
return value_sp;
}
-ForwardListFrontEnd::ForwardListFrontEnd(ValueObject &valobj)
+LibCxxForwardListFrontEnd::LibCxxForwardListFrontEnd(ValueObject &valobj)
: AbstractListFrontEnd(valobj) {
Update();
}
-llvm::Expected<uint32_t> ForwardListFrontEnd::CalculateNumChildren() {
+llvm::Expected<uint32_t> LibCxxForwardListFrontEnd::CalculateNumChildren() {
if (m_count != UINT32_MAX)
return m_count;
- ListEntry current(m_head);
+ ListEntry<StlType::LibCxx> current(m_head);
m_count = 0;
while (current && m_count < m_list_capping_size) {
++m_count;
@@ -258,7 +300,7 @@ llvm::Expected<uint32_t> ForwardListFrontEnd::CalculateNumChildren() {
return m_count;
}
-ValueObjectSP ForwardListFrontEnd::GetChildAtIndex(uint32_t idx) {
+ValueObjectSP LibCxxForwardListFrontEnd::GetChildAtIndex(uint32_t idx) {
if (idx >= CalculateNumChildrenIgnoringErrors())
return nullptr;
@@ -289,7 +331,7 @@ ValueObjectSP ForwardListFrontEnd::GetChildAtIndex(uint32_t idx) {
m_element_type);
}
-lldb::ChildCacheState ForwardListFrontEnd::Update() {
+lldb::ChildCacheState LibCxxForwardListFrontEnd::Update() {
AbstractListFrontEnd::Update();
Status err;
@@ -312,13 +354,13 @@ lldb::ChildCacheState ForwardListFrontEnd::Update() {
return ChildCacheState::eRefetch;
}
-ListFrontEnd::ListFrontEnd(lldb::ValueObjectSP valobj_sp)
+LibCxxListFrontEnd::LibCxxListFrontEnd(lldb::ValueObjectSP valobj_sp)
: AbstractListFrontEnd(*valobj_sp) {
if (valobj_sp)
Update();
}
-llvm::Expected<uint32_t> ListFrontEnd::CalculateNumChildren() {
+llvm::Expected<uint32_t> LibCxxListFrontEnd::CalculateNumChildren() {
if (m_count != UINT32_MAX)
return m_count;
if (!m_head || !m_tail || m_node_address == 0)
@@ -351,7 +393,7 @@ llvm::Expected<uint32_t> ListFrontEnd::CalculateNumChildren() {
if (next_val == prev_val)
return 1;
uint64_t size = 2;
- ListEntry current(m_head);
+ ListEntry<StlType::LibCxx> current(m_head);
while (current.next() && current.next().value() != m_node_address) {
size++;
current = current.next();
@@ -361,7 +403,7 @@ llvm::Expected<uint32_t> ListFrontEnd::CalculateNumChildren() {
return m_count = (size - 1);
}
-lldb::ValueObjectSP ListFrontEnd::GetChildAtIndex(uint32_t idx) {
+lldb::ValueObjectSP LibCxxListFrontEnd::GetChildAtIndex(uint32_t idx) {
static ConstString g_value("__value_");
static ConstString g_next("__next_");
@@ -412,7 +454,7 @@ lldb::ValueObjectSP ListFrontEnd::GetChildAtIndex(uint32_t idx) {
m_element_type);
}
-lldb::ChildCacheState ListFrontEnd::Update() {
+lldb::ChildCacheState LibCxxListFrontEnd::Update() {
AbstractListFrontEnd::Update();
m_tail = nullptr;
m_node_address = 0;
@@ -432,13 +474,167 @@ lldb::ChildCacheState ListFrontEnd::Update() {
return lldb::ChildCacheState::eRefetch;
}
+MsvcStlForwardListFrontEnd::MsvcStlForwardListFrontEnd(ValueObject &valobj)
+ : AbstractListFrontEnd(valobj) {
+ Update();
+}
+
+llvm::Expected<uint32_t> MsvcStlForwardListFrontEnd::CalculateNumChildren() {
+ if (m_count != UINT32_MAX)
+ return m_count;
+
+ ListEntry<StlType::MsvcStl> current(m_head);
+ m_count = 0;
+ while (current && m_count < m_list_capping_size) {
+ ++m_count;
+ current = current.next();
+ }
+ return m_count;
+}
+
+ValueObjectSP MsvcStlForwardListFrontEnd::GetChildAtIndex(uint32_t idx) {
+ if (idx >= CalculateNumChildrenIgnoringErrors())
+ return nullptr;
+
+ if (!m_head)
+ return nullptr;
+
+ if (HasLoop(idx + 1))
+ return nullptr;
+
+ ValueObjectSP current_sp = GetItem(idx);
+ if (!current_sp)
+ return nullptr;
+
+ current_sp = current_sp->GetChildAtIndex(1); // get the _Myval child
+ if (!current_sp)
+ return nullptr;
+
+ // we need to copy current_sp into a new object otherwise we will end up with
+ // all items named _Myval
+ DataExtractor data;
+ Status error;
+ current_sp->GetData(data, error);
+ if (error.Fail())
+ return nullptr;
+
+ return CreateValueObjectFromData(llvm::formatv("[{0}]", idx).str(), data,
+ m_backend.GetExecutionContextRef(),
+ m_element_type);
+}
+
+lldb::ChildCacheState MsvcStlForwardListFrontEnd::Update() {
+ AbstractListFrontEnd::Update();
+
+ if (auto head_sp =
+ m_backend.GetChildAtNamePath({"_Mypair", "_Myval2", "_Myhead"}))
+ m_head = head_sp.get();
+
+ return ChildCacheState::eRefetch;
+}
+
+MsvcStlListFrontEnd::MsvcStlListFrontEnd(lldb::ValueObjectSP valobj_sp)
+ : AbstractListFrontEnd(*valobj_sp) {
+ if (valobj_sp)
+ Update();
+}
+
+llvm::Expected<uint32_t> MsvcStlListFrontEnd::CalculateNumChildren() {
+ if (m_count != UINT32_MAX)
+ return m_count;
+ if (!m_head || !m_tail)
+ return 0;
+
+ auto size_sp =
+ m_backend.GetChildAtNamePath({"_Mypair", "_Myval2", "_Mysize"});
+ if (!size_sp...
[truncated]
|
There was a problem hiding this comment.
I'm in two minds about generic formatters. It does make the implementation harder to follow. And is fragile to refactorings of the corresponding STL.
In this case it might be OK since you already did all the work to do so.
Side-note, I'm also a bit wary of all the cycle-detection stuff. Might be a remnant of protection against old libcxx bugs? We don't really do this stuff for other libcxx containers. Should we keep it? (CC @labath)
There was a problem hiding this comment.
Side-note, I'm also a bit wary of all the cycle-detection stuff. Might be a remnant of protection against old libcxx bugs? We don't really do this stuff for other libcxx containers.
We don't do that for other containers, but it's not really as acute there. For example, std::map has an explicit "size" field, so even if we spin in circles, we'd stop when reach that number. Of course, if the size field is corrupted and the binary tree has a loop, then we're doomed, but ...
It's also not so much about the libcxx bugs as it is about bugs in the program using it. If the user program has a bug, then there's no way of telling what state its data structures will be in -- just because the data structure is defined in the standard library, it doesn't mean the user can't corrupt it. A looping list might just as well be the cause of the crash, so ideally we don't it want to bring down the debugger as well.
Should we keep it?
That's a complicated question.
Because std::forward_list does not have an explicit size member, the only way to compute its size is to traverse it. As you can imagine, this can be quite slow, particularly if you need to go over a network to fetch each element. This makes it a very bad match for lldb's SB API, which really wants you to call GetNumChildren on each value, which means that doing just about anything with this type can be very slow. And of course, if the list loops, then you get a broken debugger if you simply have this somewhere in your stack frame.
We currently have two mechanisms to prevent this. Both of them were added before my time, and the history is unclear as to their order. One is the loop detection thingy. The other is m_list_capping_size, which prevents you from going over target.max-children-count children.
I don't think we need both of these. However, I also don't think the currents state of things is particularly reasonable. My beef with m_list_capping_size is that it gives you no indication that the list actually contains more elements. It will lie to your face about the size of the list. It's particularly ridiculous now that the setting value is 24 by default. I guess the only reason this didn't come up yet is because people don't use this container very often.
So, while removing the loop detector is one option, if this was up to me (which it isn't), I would remove m_list_capping_size instead. The reason is that we now have better mechanisms to deal with the children count issue. Some 10 years ago we added the GetNumChildren(max) overload, which lets you cap the number of children on a per-call basis. So like, if all you want to know is whether the list has more then 100 elements (because your UI doesn't show more by default), you can call GetNumChildren(100) and it will not attempt to go over that. However, this requires discipline to not call the "exact" overload willy-nilly (which, for synthetic values basically means -- never). I think the current lldb code is in a pretty good state (we've made sure of that as we have data formatters where enumerating all children is fairly expensive, even though they're not linked lists), but there's no way to tell what other SB users are doing.
That leaves us with the summary string, which wants to print the size=%d thingy, but there I think the solution is to just not do that. There's no law that states that a container summary has to include its (exact) size, so I'd just change the summary to display an approximate value as well. E.g. we could say that if the size is smaller some value (maybe the value of target.max-children-count -- I think this is the only legitimate use of that setting in a formatter), we print the exact size. Otherwise, we just say "larger than N".
| @@ -1,8 +1,3 @@ | |||
| // Evil hack: To simulate memory corruption, we want to fiddle with some internals of std::list. | |||
There was a problem hiding this comment.
It looks like this test doesn't actually circularize the list anymore, so you might as well delete it completely. Alternatively, we could try reviving it. Instead of the #define hack, maybe it would be more reliable to corrupt the list from inside the test itself (modify the list using the SBValue API).
There was a problem hiding this comment.
Would be nice to fix the test to cover cycle detection. Happy to do that in a separate PR though. Since this is already how this test runs
Yea I agree, it is definitely a nice-to-have. I haven't actually tried on a real program what the cycle-detection looks like. We don't actually point out the cycle right? We just avoid printing in a loop? I think it would be cool if we somehow told the user "this is a corrupted std::list". Maybe we are already doing that, I haven't read the implementation too carefully. If not, it would be a nice enhancement in my opinion.
Yikes, yea that's not great.
Happy with just keeping the cycle detection!
I vaguely remember a thread somewhere else about this where you recommended this too. I think that's a good idea --- ah found it: #139805 (comment) |
|
All that being said, i think we can defer the questions about capping list size, etc. until after this lands. Since this just mimicks what libc++ already does |
Adds synthetic providers for MSVC's
std::forward_listandstd::list. It refactorsLibCxxListto be generic over the STL type (currently libc++ or MSVC STL). The libstdc++ synthetic providers use something similar in Python here. Eventually, this could be ported to C++ as well.Towards #24834.