[Lldb-commits] [lldb] 62a6d97 - Do not cache hardcoded formats in FormatManager
Adrian Prantl via lldb-commits
lldb-commits at lists.llvm.org
Tue Dec 10 15:56:49 PST 2019
Author: Adrian Prantl
Date: 2019-12-10T15:53:40-08:00
New Revision: 62a6d9770450f93a2dcdf04710a73341af2f54fa
URL: https://github.com/llvm/llvm-project/commit/62a6d9770450f93a2dcdf04710a73341af2f54fa
DIFF: https://github.com/llvm/llvm-project/commit/62a6d9770450f93a2dcdf04710a73341af2f54fa.diff
LOG: Do not cache hardcoded formats in FormatManager
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
Differential Revision: https://reviews.llvm.org/D71233
Added:
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
Modified:
lldb/include/lldb/DataFormatters/FormatManager.h
lldb/source/DataFormatters/FormatManager.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/DataFormatters/FormatManager.h b/lldb/include/lldb/DataFormatters/FormatManager.h
index dffba1f93987..c57b8c8c64a9 100644
--- a/lldb/include/lldb/DataFormatters/FormatManager.h
+++ b/lldb/include/lldb/DataFormatters/FormatManager.h
@@ -209,7 +209,8 @@ class FormatManager : public IFormatChangeListener {
ConstString m_vectortypes_category_name;
template <typename ImplSP>
- ImplSP GetCached(ValueObject &valobj, lldb::DynamicValueType use_dynamic);
+ ImplSP Get(ValueObject &valobj, lldb::DynamicValueType use_dynamic);
+ template <typename ImplSP> ImplSP GetCached(FormattersMatchData &match_data);
template <typename ImplSP> ImplSP GetHardcoded(FormattersMatchData &);
TypeCategoryMap &GetCategories() { return m_categories_map; }
diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/Makefile b/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/Makefile
new file mode 100644
index 000000000000..224ecc3c2f55
--- /dev/null
+++ b/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
diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/TestDataFormatterCaching.py b/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/TestDataFormatterCaching.py
new file mode 100644
index 000000000000..8b906506a5b6
--- /dev/null
+++ b/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)')
diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/a.c b/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/a.c
new file mode 100644
index 000000000000..ab0b6f5bd5e0
--- /dev/null
+++ b/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();
+}
diff --git a/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/b.c b/lldb/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-caching/b.c
new file mode 100644
index 000000000000..0d37c54aa339
--- /dev/null
+++ b/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;
+}
diff --git a/lldb/source/DataFormatters/FormatManager.cpp b/lldb/source/DataFormatters/FormatManager.cpp
index c8ddbd455943..db15a7f7a4cc 100644
--- a/lldb/source/DataFormatters/FormatManager.cpp
+++ b/lldb/source/DataFormatters/FormatManager.cpp
@@ -622,11 +622,21 @@ ImplSP FormatManager::GetHardcoded(FormattersMatchData &match_data) {
return retval_sp;
}
-template <typename ImplSP> ImplSP
-FormatManager::GetCached(ValueObject &valobj,
- lldb::DynamicValueType use_dynamic) {
- ImplSP retval_sp;
+template <typename ImplSP>
+ImplSP FormatManager::Get(ValueObject &valobj,
+ lldb::DynamicValueType use_dynamic) {
FormattersMatchData match_data(valobj, use_dynamic);
+ if (ImplSP retval_sp = GetCached<ImplSP>(match_data))
+ return retval_sp;
+
+ Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_DATAFORMATTERS));
+ LLDB_LOGF(log, "[%s] Search failed. Giving hardcoded a chance.", __FUNCTION__);
+ return GetHardcoded<ImplSP>(match_data);
+}
+
+template <typename ImplSP>
+ImplSP FormatManager::GetCached(FormattersMatchData &match_data) {
+ ImplSP retval_sp;
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__,
@@ -659,10 +669,6 @@ FormatManager::GetCached(ValueObject &valobj,
return retval_sp;
}
}
- if (!retval_sp) {
- LLDB_LOGF(log, "[%s] Search failed. Giving hardcoded a chance.", __FUNCTION__);
- retval_sp = GetHardcoded<ImplSP>(match_data);
- }
if (match_data.GetTypeForCache() && (!retval_sp || !retval_sp->NonCacheable())) {
LLDB_LOGF(log, "[%s] Caching %p for type %s", __FUNCTION__,
@@ -678,25 +684,25 @@ FormatManager::GetCached(ValueObject &valobj,
lldb::TypeFormatImplSP
FormatManager::GetFormat(ValueObject &valobj,
lldb::DynamicValueType use_dynamic) {
- return GetCached<lldb::TypeFormatImplSP>(valobj, use_dynamic);
+ return Get<lldb::TypeFormatImplSP>(valobj, use_dynamic);
}
lldb::TypeSummaryImplSP
FormatManager::GetSummaryFormat(ValueObject &valobj,
lldb::DynamicValueType use_dynamic) {
- return GetCached<lldb::TypeSummaryImplSP>(valobj, use_dynamic);
+ return Get<lldb::TypeSummaryImplSP>(valobj, use_dynamic);
}
lldb::SyntheticChildrenSP
FormatManager::GetSyntheticChildren(ValueObject &valobj,
lldb::DynamicValueType use_dynamic) {
- return GetCached<lldb::SyntheticChildrenSP>(valobj, use_dynamic);
+ return Get<lldb::SyntheticChildrenSP>(valobj, use_dynamic);
}
lldb::TypeValidatorImplSP
FormatManager::GetValidator(ValueObject &valobj,
lldb::DynamicValueType use_dynamic) {
- return GetCached<lldb::TypeValidatorImplSP>(valobj, use_dynamic);
+ return Get<lldb::TypeValidatorImplSP>(valobj, use_dynamic);
}
FormatManager::FormatManager()
More information about the lldb-commits
mailing list