-
Notifications
You must be signed in to change notification settings - Fork 15.5k
[LLDB] Run MSVC STL deque tests with PDB #172360
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@llvm/pr-subscribers-lldb Author: nerix (Nerixyz) ChangesSimilar to the other PRs, this looks up the type from a member variable. Here, we can use the type of MSVC, which isn't tested, doesn't include the value of static members in PDB for some reason. The STL works around this by adding an unnamed enum here. I have three questions about this:
Full diff: https://github.com/llvm/llvm-project/pull/172360.diff 2 Files Affected:
diff --git a/lldb/source/Plugins/Language/CPlusPlus/MsvcStlDeque.cpp b/lldb/source/Plugins/Language/CPlusPlus/MsvcStlDeque.cpp
index 873354381a6da..66fd547bda6a0 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/MsvcStlDeque.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/MsvcStlDeque.cpp
@@ -90,6 +90,38 @@ lldb_private::formatters::MsvcStlDequeSyntheticFrontEnd::GetChildAtIndex(
m_element_type);
}
+static std::optional<size_t>
+getBlockSize(const lldb_private::CompilerType &deque_type) {
+ auto block_size_decl = deque_type.GetStaticFieldWithName("_Block_size");
+ if (block_size_decl) {
+ auto block_size = block_size_decl.GetConstantValue();
+ if (block_size.IsValid())
+ return block_size.ULongLong();
+ }
+
+ // MSVC doesn't include static members like _Block_size. As a workaround, the
+ // STL has enum { _EEN_DS = _Block_size };
+ // This is named "<unnamed-enum-_EEN_DS>" in PDB.
+ auto enum_type =
+ deque_type.GetDirectNestedTypeWithName("<unnamed-enum-_EEN_DS>");
+ if (!enum_type)
+ return std::nullopt;
+
+ std::optional<size_t> value;
+ enum_type.ForEachEnumerator(
+ [&](const lldb_private::CompilerType &integer_type,
+ lldb_private::ConstString name, const llvm::APSInt &ap_value) {
+ if (name != "_EEN_DS")
+ return true; // keep iterating
+
+ int64_t signed_value = ap_value.getExtValue();
+ if (signed_value > 0)
+ value.emplace(static_cast<uint64_t>(signed_value));
+ return false;
+ });
+ return value;
+}
+
lldb::ChildCacheState
lldb_private::formatters::MsvcStlDequeSyntheticFrontEnd::Update() {
m_size = 0;
@@ -104,18 +136,8 @@ lldb_private::formatters::MsvcStlDequeSyntheticFrontEnd::Update() {
if (!deque_type)
return lldb::eRefetch;
- auto block_size_decl = deque_type.GetStaticFieldWithName("_Block_size");
- if (!block_size_decl)
- return lldb::eRefetch;
- auto block_size = block_size_decl.GetConstantValue();
- if (!block_size.IsValid())
- return lldb::eRefetch;
-
- auto element_type = deque_type.GetTypeTemplateArgument(0);
- if (!element_type)
- return lldb::eRefetch;
- auto element_size = element_type.GetByteSize(nullptr);
- if (!element_size)
+ auto block_size = getBlockSize(deque_type);
+ if (!block_size)
return lldb::eRefetch;
auto offset_sp = storage_sp->GetChildMemberWithName("_Myoff");
@@ -138,9 +160,19 @@ lldb_private::formatters::MsvcStlDequeSyntheticFrontEnd::Update() {
if (!ok)
return lldb::eRefetch;
+ auto element_type = deque_type.GetTypeTemplateArgument(0);
+ if (!element_type) {
+ element_type = map_sp->GetCompilerType().GetPointeeType().GetPointeeType();
+ if (!element_type)
+ return lldb::eRefetch;
+ }
+ auto element_size = element_type.GetByteSize(nullptr);
+ if (!element_size)
+ return lldb::eRefetch;
+
m_map = map_sp.get();
m_exe_ctx_ref = m_backend.GetExecutionContextRef();
- m_block_size = block_size.ULongLong();
+ m_block_size = *block_size;
m_offset = offset;
m_map_size = map_size;
m_element_size = *element_size;
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/deque/TestDataFormatterGenericDeque.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/deque/TestDataFormatterGenericDeque.py
index 2332eff7b10dd..2b22281a87318 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/deque/TestDataFormatterGenericDeque.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/deque/TestDataFormatterGenericDeque.py
@@ -5,6 +5,8 @@
class GenericDequeDataFormatterTestCase(TestBase):
+ TEST_WITH_PDB_DEBUG_INFO = True
+
def findVariable(self, name):
var = self.frame().FindVariable(name)
self.assertTrue(var.IsValid())
|
Yes please
I think as a temporary measure shell-tests seem reasonable. Getting API tests (and subsequently all the formatter tests) to run against MSVC would be great, but before doing that, a good first step is to ask on Discourse about the level of support/infrastructure we're willing to dedicate to testing MSVC. AFAIU the general practice is "if you're using MSVC, use the debugger that goes with it". I'd be interested to hear about the use-cases where a user compiled with MSVC but doesn't want to use the associated debugger. Are we talking about programs that were partially compiled with clang-cl and partially with MSVC? Is that a common workflow? I just don't work on Windows enough to be able to answer these questions personally.
Lookup into anonymous scopes is a bit of a can of worms. There are a couple of issues with it and would be nice to fix eventually. But it's a bit more complicated than it seems because we have to decide whether we want to break existing SBAPI users relying on the old lookup behaviour (which doesn't account for anonymous scopes consistently). I think an MSVC-specific workaround seems fine for now. |
Similar to the other PRs, this looks up the type from a member variable. Here, we can use the type of
_Mapptr. On its own, that's enough to pass the test with clang-cl.