[LLDB] Add formatters for MSVC STL std::variant

[Nerixyz]

Adds a summary and synthetic children for MSVC STL's std::variant.

This one is a bit complicated because of DWARF vs PDB differences. I put the representations in comments. Being able to GetChildMemberWithName a member in an anonymous union would make this a lot simpler (std::optional will have something similar iirc).

Towards #24834.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: nerix (Nerixyz)

Changes

Adds a summary and synthetic children for MSVC STL's std::variant.

This one is a bit complicated because of DWARF vs PDB differences. I put the representations in comments. Being able to GetChildMemberWithName a member in an anonymous union would make this a lot simpler (std::optional will have something similar iirc).

Towards #24834.

---

Full diff: https://github.com/llvm/llvm-project/pull/148554.diff

6 Files Affected:

• (modified) lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt (+1)
• (modified) lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp (+25-8)
• (modified) lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h (+3)
• (modified) lldb/source/Plugins/Language/CPlusPlus/MsvcStl.h (+8)
• (added) lldb/source/Plugins/Language/CPlusPlus/MsvcStlVariant.cpp (+221)
• (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/variant/TestDataFormatterStdVariant.py (+6)

diff --git a/lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt b/lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
index 5905d9b9a6d03..e1dd5bf84d7eb 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
+++ b/lldb/source/Plugins/Language/CPlusPlus/CMakeLists.txt
@@ -36,6 +36,7 @@ add_lldb_library(lldbPluginCPlusPlusLanguage PLUGIN
   MsvcStl.cpp
   MsvcStlSmartPointer.cpp
   MsvcStlTuple.cpp
+  MsvcStlVariant.cpp
   MsvcStlVector.cpp
   MSVCUndecoratedNameParser.cpp
 
diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index a8ebde0b55815..2a06422e86d0c 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -1449,11 +1449,6 @@ static void LoadLibStdcppFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
       SyntheticChildrenSP(new ScriptedSyntheticChildren(
           stl_synth_flags,
           "lldb.formatters.cpp.gnu_libstdcpp.StdForwardListSynthProvider")));
-  cpp_category_sp->AddTypeSynthetic(
-      "^std::variant<.+>$", eFormatterMatchRegex,
-      SyntheticChildrenSP(new ScriptedSyntheticChildren(
-          stl_synth_flags,
-          "lldb.formatters.cpp.gnu_libstdcpp.VariantSynthProvider")));
 
   stl_summary_flags.SetDontShowChildren(false);
   stl_summary_flags.SetSkipPointers(false);
@@ -1509,9 +1504,6 @@ static void LoadLibStdcppFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
       TypeSummaryImplSP(new ScriptSummaryFormat(
           stl_summary_flags,
           "lldb.formatters.cpp.gnu_libstdcpp.ForwardListSummaryProvider")));
-  AddCXXSummary(cpp_category_sp, LibStdcppVariantSummaryProvider,
-                "libstdc++ std::variant summary provider", "^std::variant<.+>$",
-                stl_summary_flags, true);
 
   AddCXXSynthetic(
       cpp_category_sp,
@@ -1648,6 +1640,25 @@ GenericForwardListSyntheticFrontEndCreator(CXXSyntheticChildren *children,
       *valobj_sp);
 }
 
+static SyntheticChildrenFrontEnd *
+GenericVariantSyntheticFrontEndCreator(CXXSyntheticChildren *children,
+                                       lldb::ValueObjectSP valobj_sp) {
+  if (!valobj_sp)
+    return nullptr;
+
+  if (IsMsvcStlVariant(*valobj_sp))
+    return MsvcStlVariantSyntheticFrontEndCreator(children, valobj_sp);
+  return new ScriptedSyntheticChildren::FrontEnd(
+      "lldb.formatters.cpp.gnu_libstdcpp.VariantSynthProvider", *valobj_sp);
+}
+
+static bool GenericVariantSummaryProvider(ValueObject &valobj, Stream &stream,
+                                          const TypeSummaryOptions &options) {
+  if (IsMsvcStlVariant(valobj))
+    return MsvcStlVariantSummaryProvider(valobj, stream, options);
+  return LibStdcppVariantSummaryProvider(valobj, stream, options);
+}
+
 /// Load formatters that are formatting types from more than one STL
 static void LoadCommonStlFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
   if (!cpp_category_sp)
@@ -1712,6 +1723,9 @@ static void LoadCommonStlFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
   AddCXXSynthetic(cpp_category_sp, GenericForwardListSyntheticFrontEndCreator,
                   "std::forward_list synthetic children",
                   "^std::forward_list<.+>(( )?&)?$", stl_synth_flags, true);
+  AddCXXSynthetic(cpp_category_sp, GenericVariantSyntheticFrontEndCreator,
+                  "std::variant synthetic children", "^std::variant<.*>$",
+                  stl_synth_flags, true);
 
   AddCXXSummary(cpp_category_sp, GenericSmartPointerSummaryProvider,
                 "MSVC STL/libstdc++ std::shared_ptr summary provider",
@@ -1739,6 +1753,9 @@ static void LoadCommonStlFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
       TypeSummaryImplSP(new ScriptSummaryFormat(
           stl_summary_flags,
           "lldb.formatters.cpp.gnu_libstdcpp.ForwardListSummaryProvider")));
+  AddCXXSummary(cpp_category_sp, GenericVariantSummaryProvider,
+                "MSVC STL/libstdc++ std::variant summary provider",
+                "^std::variant<.*>$", stl_summary_flags, true);
 }
 
 static void LoadMsvcStlFormatters(lldb::TypeCategoryImplSP cpp_category_sp) {
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h
index 8d2025e940ead..429142f63a4bd 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.h
@@ -61,6 +61,9 @@ SyntheticChildrenFrontEnd *
 LibStdcppUniquePtrSyntheticFrontEndCreator(CXXSyntheticChildren *,
                                            lldb::ValueObjectSP);
 
+bool LibStdcppVariantSummaryProvider(ValueObject &valobj, Stream &stream,
+                                     const TypeSummaryOptions &options);
+
 } // namespace formatters
 } // namespace lldb_private
 
diff --git a/lldb/source/Plugins/Language/CPlusPlus/MsvcStl.h b/lldb/source/Plugins/Language/CPlusPlus/MsvcStl.h
index 0f3db4b50eeaf..c262df644b387 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/MsvcStl.h
+++ b/lldb/source/Plugins/Language/CPlusPlus/MsvcStl.h
@@ -65,6 +65,14 @@ SyntheticChildrenFrontEnd *
 MsvcStlListSyntheticFrontEndCreator(CXXSyntheticChildren *,
                                     lldb::ValueObjectSP valobj_sp);
 
+// MSVC STL std::variant<>
+bool IsMsvcStlVariant(ValueObject &valobj);
+bool MsvcStlVariantSummaryProvider(ValueObject &valobj, Stream &stream,
+                                   const TypeSummaryOptions &options);
+SyntheticChildrenFrontEnd *
+MsvcStlVariantSyntheticFrontEndCreator(CXXSyntheticChildren *,
+                                       lldb::ValueObjectSP valobj_sp);
+
 } // namespace formatters
 } // namespace lldb_private
 
diff --git a/lldb/source/Plugins/Language/CPlusPlus/MsvcStlVariant.cpp b/lldb/source/Plugins/Language/CPlusPlus/MsvcStlVariant.cpp
new file mode 100644
index 0000000000000..e8a6b0e930c2c
--- /dev/null
+++ b/lldb/source/Plugins/Language/CPlusPlus/MsvcStlVariant.cpp
@@ -0,0 +1,221 @@
+//===-- MsvcStlVariant.cpp-------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "MsvcStl.h"
+#include "lldb/DataFormatters/FormattersHelpers.h"
+#include "lldb/Symbol/CompilerType.h"
+#include <optional>
+
+using namespace lldb;
+using namespace lldb_private;
+
+namespace {
+
+// A variant when using DWARF looks as follows:
+// (lldb) fr v -R v1
+// (std::variant<int, double, char>) v1 = {
+//   std::_SMF_control<std::_Variant_base<int, double, char>, int, double, char>
+//   = {
+//     std::_Variant_storage<int, double, char> = {
+//        = {
+//         _Head = 0
+//         _Tail = {
+//            = {
+//             _Head = 2
+//             _Tail = {
+//                = {
+//                 _Head = '\0'
+//                 _Tail = {}
+//               }
+//             }
+//           }
+//         }
+//       }
+//     }
+//     _Which = '\x01'
+//   }
+// }
+//
+// ... when using PDB, it looks like this:
+// (lldb) fr v -R v1
+// (std::variant<int,double,char>) v1 = {
+//   std::_Variant_base<int,double,char> = {
+//     std::_Variant_storage_<1,int,double,char> = {
+//       _Head = 0
+//       _Tail = {
+//         _Head = 2
+//         _Tail = {
+//           _Head = '\0'
+//           _Tail = {}
+//         }
+//       }
+//     }
+//     _Which = '\x01'
+//   }
+// }
+
+ValueObjectSP GetStorageAtIndex(ValueObject &valobj, size_t index) {
+  // PDB flattens the members on unions to the parent
+  if (valobj.GetCompilerType().GetNumFields() == 2)
+    return valobj.GetChildAtIndex(index);
+
+  // DWARF keeps the union
+  ValueObjectSP union_sp = valobj.GetChildAtIndex(0);
+  if (!union_sp)
+    return nullptr;
+  return union_sp->GetChildAtIndex(index);
+}
+
+ValueObjectSP GetHead(ValueObject &valobj) {
+  return GetStorageAtIndex(valobj, 0);
+}
+ValueObjectSP GetTail(ValueObject &valobj) {
+  return GetStorageAtIndex(valobj, 1);
+}
+
+std::optional<int64_t> GetIndexValue(ValueObject &valobj) {
+  ValueObjectSP index_sp = valobj.GetChildMemberWithName("_Which");
+  if (!index_sp)
+    return std::nullopt;
+
+  return {index_sp->GetValueAsSigned(-1)};
+}
+
+ValueObjectSP GetNthStorage(ValueObject &outer, int64_t index) {
+  ValueObjectSP container_sp = outer.GetSP();
+
+  // When using DWARF, we need to find the std::_Variant_storage base class.
+  // There, the top level type doesn't have any fields.
+  if (container_sp->GetCompilerType().GetNumFields() == 0) {
+    // -> std::_SMF_control (typedef to std::_Variant_base)
+    container_sp = container_sp->GetChildAtIndex(0);
+    if (!container_sp)
+      return nullptr;
+    // -> std::_Variant_storage
+    container_sp = container_sp->GetChildAtIndex(0);
+    if (!container_sp)
+      return nullptr;
+  }
+
+  for (int64_t i = 0; i < index; i++) {
+    container_sp = GetTail(*container_sp);
+    if (!container_sp)
+      return nullptr;
+  }
+  return container_sp;
+}
+
+} // namespace
+
+bool formatters::IsMsvcStlVariant(ValueObject &valobj) {
+  if (auto valobj_sp = valobj.GetNonSyntheticValue()) {
+    return valobj_sp->GetChildMemberWithName("_Which") != nullptr;
+  }
+  return false;
+}
+
+bool formatters::MsvcStlVariantSummaryProvider(
+    ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
+  ValueObjectSP valobj_sp(valobj.GetNonSyntheticValue());
+  if (!valobj_sp)
+    return false;
+
+  auto index = GetIndexValue(*valobj_sp);
+  if (!index)
+    return false;
+
+  if (*index < 0) {
+    stream.Printf(" No Value");
+    return true;
+  }
+
+  ValueObjectSP storage = GetNthStorage(*valobj_sp, *index);
+  if (!storage)
+    return false;
+  CompilerType storage_type = storage->GetCompilerType();
+  if (!storage_type)
+    return false;
+  // With DWARF, it's a typedef, with PDB, it's not
+  if (storage_type.IsTypedefType())
+    storage_type = storage_type.GetTypedefedType();
+
+  CompilerType active_type = storage_type.GetTypeTemplateArgument(1, true);
+  if (!active_type) {
+    // not enough debug info, try the type of _Head
+    ValueObjectSP head_sp = GetHead(*storage);
+    if (!head_sp)
+      return false;
+    active_type = head_sp->GetCompilerType();
+    if (!active_type)
+      return false;
+  }
+
+  stream << " Active Type = " << active_type.GetDisplayTypeName() << " ";
+  return true;
+}
+
+namespace {
+class VariantFrontEnd : public SyntheticChildrenFrontEnd {
+public:
+  VariantFrontEnd(ValueObject &valobj) : SyntheticChildrenFrontEnd(valobj) {
+    Update();
+  }
+
+  llvm::Expected<size_t> GetIndexOfChildWithName(ConstString name) override {
+    auto optional_idx = formatters::ExtractIndexFromString(name.GetCString());
+    if (!optional_idx) {
+      return llvm::createStringError("Type has no child named '%s'",
+                                     name.AsCString());
+    }
+    return *optional_idx;
+  }
+
+  lldb::ChildCacheState Update() override;
+  llvm::Expected<uint32_t> CalculateNumChildren() override { return m_size; }
+  ValueObjectSP GetChildAtIndex(uint32_t idx) override;
+
+private:
+  size_t m_size = 0;
+};
+} // namespace
+
+lldb::ChildCacheState VariantFrontEnd::Update() {
+  m_size = 0;
+
+  auto index = GetIndexValue(m_backend);
+  if (index && *index >= 0)
+    m_size = 1;
+
+  return lldb::ChildCacheState::eRefetch;
+}
+
+ValueObjectSP VariantFrontEnd::GetChildAtIndex(uint32_t idx) {
+  if (idx >= m_size)
+    return nullptr;
+
+  auto index = GetIndexValue(m_backend);
+  if (!index)
+    return nullptr;
+
+  ValueObjectSP storage_sp = GetNthStorage(m_backend, *index);
+  if (!storage_sp)
+    return nullptr;
+
+  ValueObjectSP head_sp = GetHead(*storage_sp);
+  if (!head_sp)
+    return nullptr;
+
+  return head_sp->Clone(ConstString("Value"));
+}
+
+SyntheticChildrenFrontEnd *formatters::MsvcStlVariantSyntheticFrontEndCreator(
+    CXXSyntheticChildren *, lldb::ValueObjectSP valobj_sp) {
+  if (valobj_sp)
+    return new VariantFrontEnd(*valobj_sp);
+  return nullptr;
+}
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/variant/TestDataFormatterStdVariant.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/variant/TestDataFormatterStdVariant.py
index 9365cfc96783e..9f32ad97c1f0a 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/variant/TestDataFormatterStdVariant.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/variant/TestDataFormatterStdVariant.py
@@ -83,3 +83,9 @@ def test_libcxx(self):
     def test_libstdcxx(self):
         self.build(dictionary={"USE_LIBSTDCPP": 1})
         self.do_test()
+
+    @add_test_categories(["msvcstl"])
+    def test_msvcstl(self):
+        # No flags, because the "msvcstl" category checks that the MSVC STL is used by default.
+        self.build()
+        self.do_test()

[Michael137]

Appreciate the effort to support both PDB and DWARF, but I don't think we should be jumping through hoops to support PDB when there is no bots running the API tests for it. It's too easy for that to bit-rot. So I would suggest dropping PDB support from this for now.

CC @labath @DavidSpickett for second opinions

[DavidSpickett]

What does running API tests with PDB look like? I assume Linaro is not doing that because we don't pass any extra flags, and often have to disable tests because link.exe is mangling the DWARF sections.

Or put another way, @Nerixyz how do you make sure PDB is being used when you run these tests?

If PDB could be added as one of the debug info test types, like we have for Dwarf and dysym and DWO, that could be added to existing builders. I don't recall the name of that thing, it runs the test many times with each different debug info format.

[Nerixyz]

Appreciate the effort to support both PDB and DWARF, but I don't think we should be jumping through hoops to support PDB when there is no bots running the API tests for it. It's too easy for that to bit-rot. So I would suggest dropping PDB support from this for now.

My main intention with supporting PDB was that it's the default debug format (and the only supported one on MSVC).

Or put another way, @Nerixyz how do you make sure PDB is being used when you run these tests?

I run them manually. So this is mostly a best-effort.

If PDB could be added as one of the debug info test types, like we have for Dwarf and dysym and DWO, that could be added to existing builders.

That could work - though some tests would probably need to be skipped. The default should still be DWARF on Windows.

One thing to keep in mind is that there are two PDB implementations, the DIA SDK one and the native one. Just today I noticed that they demangle types slightly different - DIA demangles to std::basic_string<char,std::char_traits<char>,std::allocator<char> > whereas the native implementation (using LLVM's demangling) demangles to std::basic_string<char, std::char_traits<char>, std::allocator<char>> (notice the spaces). This causes some formatters to not match on the types because they check for ^std::foo<.+> >(( )?&)?$. Other than that, the native one is usually better, from what I can tell.

I experimented with PDB in API tests a bit, and it seems to workbut requires a few xfail annotations.

[DavidSpickett]

We had a discussion about preferring one or the other PDB implementation but I don't recall the conclusion. I think this is just for reading PDB, right? Clang is going to use llvm's libraries to write PDB and of course MSVC is using Microsoft's implementation.

I think a few XFAILs would be worth it if we can say lldb + MSVC toolchain works out of the box. If we can get it to be an extra mode on top of DWARF/DWO/dysym I can add it to Linaro's Windows bot for sure. Assuming I can stick with clang as the test compiler.

The formatters are not my area of expertise, so if @Michael137 does not want the complexity without regular testing, I suggest we respect that. Might be useful for your own purposes to split out the PDB bit and keep it for later.

I experimented with PDB in API tests a bit, and it seems to workbut requires a few xfail annotations.

Then could you put up a PR for, or just explain how, you did this? I can try it out with or bot's existing configuration. We can work towards getting some testing of PDB, whatever form it happens to be in, and then you can have future PRs with PDB support too.