aprantl created this revision.
aprantl added reviewers: jingham, davide.
aprantl added a parent revision: D71231: Replace redundant code in
FormatManager and FormatCache with templates (NFC).
The cache in FormatCache uses only a type name as key. The hardcoded formats,
synthetic children, etc inspect an entire ValueObject to determine their
eligibility, which isn't modelled in the cache. This leads to bugs such as the
one in this patch (where two similarly named types in different files have
different hardcoded summary providers). The problem is exaggerated in the Swift
language plugin due to the language's dynamic nature.
rdar://problem/57756763
https://reviews.llvm.org/D71233
Files:
lldb/include/lldb/DataFormatters/FormatManager.h
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/Makefile
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/TestDataFormatterCaching.py
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/a.c
lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/b.c
lldb/source/DataFormatters/FormatManager.cpp
Index: lldb/source/DataFormatters/FormatManager.cpp
===================================================================
--- lldb/source/DataFormatters/FormatManager.cpp
+++ lldb/source/DataFormatters/FormatManager.cpp
@@ -626,9 +626,19 @@
}
template <typename ImplSP> void
-FormatManager::GetCached(ValueObject &valobj,
+FormatManager::Get(ValueObject &valobj,
lldb::DynamicValueType use_dynamic, ImplSP &retval) {
FormattersMatchData match_data(valobj, use_dynamic);
+ GetCached(match_data, retval);
+ if (!retval) {
+ Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_DATAFORMATTERS));
+ LLDB_LOGF(log, "[%s] Search failed. Giving hardcoded a chance.", __FUNCTION__);
+ GetHardcoded(match_data, retval);
+ }
+}
+
+template <typename ImplSP> void
+FormatManager::GetCached(FormattersMatchData &match_data, ImplSP &retval) {
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_DATAFORMATTERS));
if (match_data.GetTypeForCache()) {
LLDB_LOGF(log, "\n\n[%s] Looking into cache for type %s", __FUNCTION__,
@@ -661,11 +671,6 @@
return;
}
}
- if (!retval) {
- LLDB_LOGF(log, "[%s] Search failed. Giving hardcoded a chance.", __FUNCTION__);
- GetHardcoded(match_data, retval);
- }
-
if (match_data.GetTypeForCache() && (!retval || !retval->NonCacheable())) {
LLDB_LOGF(log, "[%s] Caching %p for type %s", __FUNCTION__,
static_cast<void *>(retval.get()),
@@ -680,7 +685,7 @@
FormatManager::GetFormat(ValueObject &valobj,
lldb::DynamicValueType use_dynamic) {
TypeFormatImplSP retval;
- GetCached(valobj, use_dynamic, retval);
+ Get(valobj, use_dynamic, retval);
return retval;
}
@@ -688,7 +693,7 @@
FormatManager::GetSummaryFormat(ValueObject &valobj,
lldb::DynamicValueType use_dynamic) {
TypeSummaryImplSP retval;
- GetCached(valobj, use_dynamic, retval);
+ Get(valobj, use_dynamic, retval);
return retval;
}
@@ -696,7 +701,7 @@
FormatManager::GetSyntheticChildren(ValueObject &valobj,
lldb::DynamicValueType use_dynamic) {
SyntheticChildrenSP retval;
- GetCached(valobj, use_dynamic, retval);
+ Get(valobj, use_dynamic, retval);
return retval;
}
@@ -704,7 +709,7 @@
FormatManager::GetValidator(ValueObject &valobj,
lldb::DynamicValueType use_dynamic) {
TypeValidatorImplSP retval;
- GetCached(valobj, use_dynamic, retval);
+ Get(valobj, use_dynamic, retval);
return retval;
}
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/b.c
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/b.c
@@ -0,0 +1,8 @@
+typedef float float4 __attribute__((ext_vector_type(4)));
+void stop() {}
+int a() {
+ float4 f4 = {1, 2, 3, 4};
+ // break here
+ stop();
+ return 0;
+}
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/a.c
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/a.c
@@ -0,0 +1,7 @@
+typedef float float4;
+
+int main() {
+ float4 f = 4.0f;
+ // break here
+ return a();
+}
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/TestDataFormatterCaching.py
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/TestDataFormatterCaching.py
@@ -0,0 +1,27 @@
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class TestDataFormatterCaching(TestBase):
+
+ mydir = TestBase.compute_mydir(__file__)
+
+ def setUp(self):
+ TestBase.setUp(self)
+
+ def test_with_run_command(self):
+ """
+ Test that hardcoded summary formatter matches aren't improperly cached.
+ """
+ self.build()
+ target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(
+ self, 'break here', lldb.SBFileSpec('a.c'))
+ valobj = self.frame().FindVariable('f')
+ self.assertEqual(valobj.GetValue(), '4')
+ bkpt_b = target.BreakpointCreateBySourceRegex('break here',
+ lldb.SBFileSpec('b.c'))
+ lldbutil.continue_to_breakpoint(process, bkpt_b)
+ valobj = self.frame().FindVariable('f4')
+ self.assertEqual(valobj.GetSummary(), '(1, 2, 3, 4)')
Index: lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/Makefile
===================================================================
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/Makefile
@@ -0,0 +1,3 @@
+C_SOURCES := a.c b.c
+
+include Makefile.rules
Index: lldb/include/lldb/DataFormatters/FormatManager.h
===================================================================
--- lldb/include/lldb/DataFormatters/FormatManager.h
+++ lldb/include/lldb/DataFormatters/FormatManager.h
@@ -212,8 +212,10 @@
ConstString m_vectortypes_category_name;
template <typename ImplSP>
- void GetCached(ValueObject &valobj, lldb::DynamicValueType use_dynamic,
- ImplSP &retval);
+ void Get(ValueObject &valobj, lldb::DynamicValueType use_dynamic,
+ ImplSP &retval);
+ template <typename ImplSP>
+ void GetCached(FormattersMatchData &match_data, ImplSP &retval);
template <typename ImplSP> void GetHardcoded(FormattersMatchData &, ImplSP &);
TypeCategoryMap &GetCategories() { return m_categories_map; }
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
