[LLDB] Check type before creating std::atomic synthetic children

[Nerixyz]

From #163077 (comment): Currently, std::atomic<T> will always use the MSVC STL synthetic children and summary. When inspecting types from other STLs, the output would not show any children.

This PR adds a check that std::atomic contains _Storage to be classified as coming from MSVC's STL.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: nerix (Nerixyz)

Changes

From #163077 (comment): Currently, std::atomic<T> will always use the MSVC STL synthetic children and summary. When inspecting types from other STLs, the output would not show any children.

This PR adds a check that std::atomic contains _Storage to be classified as coming from MSVC's STL.

---

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

5 Files Affected:

• (modified) lldb/source/Plugins/Language/CPlusPlus/MsvcStl.h (+1)
• (modified) lldb/source/Plugins/Language/CPlusPlus/MsvcStlAtomic.cpp (+9-1)
• (added) lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/invalid-atomic/Makefile (+3)
• (added) lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/invalid-atomic/TestDataFormatterMsvcStlInvalidAtomic.py (+30)
• (added) lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/invalid-atomic/main.cpp (+13)

diff --git a/lldb/source/Plugins/Language/CPlusPlus/MsvcStl.h b/lldb/source/Plugins/Language/CPlusPlus/MsvcStl.h
index 8a4918127584f..e818b88e202ef 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/MsvcStl.h
+++ b/lldb/source/Plugins/Language/CPlusPlus/MsvcStl.h
@@ -89,6 +89,7 @@ MsvcStlVariantSyntheticFrontEndCreator(CXXSyntheticChildren *,
                                        lldb::ValueObjectSP valobj_sp);
 
 // MSVC STL std::atomic<>
+bool IsMsvcStlAtomic(ValueObject &valobj);
 bool MsvcStlAtomicSummaryProvider(ValueObject &valobj, Stream &stream,
                                   const TypeSummaryOptions &options);
 SyntheticChildrenFrontEnd *
diff --git a/lldb/source/Plugins/Language/CPlusPlus/MsvcStlAtomic.cpp b/lldb/source/Plugins/Language/CPlusPlus/MsvcStlAtomic.cpp
index 3ec324577ac76..c8718616c45ab 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/MsvcStlAtomic.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/MsvcStlAtomic.cpp
@@ -83,7 +83,9 @@ llvm::Expected<size_t> lldb_private::formatters::
 lldb_private::SyntheticChildrenFrontEnd *
 lldb_private::formatters::MsvcStlAtomicSyntheticFrontEndCreator(
     CXXSyntheticChildren *, lldb::ValueObjectSP valobj_sp) {
-  return new MsvcStlAtomicSyntheticFrontEnd(valobj_sp);
+  if (valobj_sp && IsMsvcStlAtomic(*valobj_sp))
+    return new MsvcStlAtomicSyntheticFrontEnd(valobj_sp);
+  return nullptr;
 }
 
 bool lldb_private::formatters::MsvcStlAtomicSummaryProvider(
@@ -100,3 +102,9 @@ bool lldb_private::formatters::MsvcStlAtomicSummaryProvider(
   }
   return false;
 }
+
+bool lldb_private::formatters::IsMsvcStlAtomic(ValueObject &valobj) {
+  if (auto valobj_sp = valobj.GetNonSyntheticValue())
+    return valobj_sp->GetChildMemberWithName("_Storage") != nullptr;
+  return false;
+}
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/invalid-atomic/Makefile b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/invalid-atomic/Makefile
new file mode 100644
index 0000000000000..99998b20bcb05
--- /dev/null
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/invalid-atomic/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/invalid-atomic/TestDataFormatterMsvcStlInvalidAtomic.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/invalid-atomic/TestDataFormatterMsvcStlInvalidAtomic.py
new file mode 100644
index 0000000000000..c4d426a0055ee
--- /dev/null
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/invalid-atomic/TestDataFormatterMsvcStlInvalidAtomic.py
@@ -0,0 +1,30 @@
+"""
+Test formatting of `std::atomic`s not from MSVC's STL
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class MsvcStlInvalidAtomicDataFormatterTestCase(TestBase):
+    def test(self):
+        self.build()
+        lldbutil.run_to_source_breakpoint(
+            self, "Set break point at this line.", lldb.SBFileSpec("main.cpp")
+        )
+
+        self.expect_expr(
+            "a",
+            result_children=[
+                ValueCheck(name="foo", value="1"),
+                ValueCheck(name="bar", value="2"),
+            ],
+        )
+        self.expect_expr(
+            "b",
+            result_children=[
+                ValueCheck(name="foo", value="3"),
+                ValueCheck(name="bar", value="4"),
+            ],
+        )
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/invalid-atomic/main.cpp b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/invalid-atomic/main.cpp
new file mode 100644
index 0000000000000..bdac9e52b49f1
--- /dev/null
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/msvcstl/invalid-atomic/main.cpp
@@ -0,0 +1,13 @@
+namespace std {
+template <typename T> struct atomic {
+  int foo;
+  int bar;
+};
+} // namespace std
+
+int main() {
+  std::atomic<int> a{1, 2};
+  std::atomic<void> b{3, 4};
+
+  return 0; // Set break point at this line.
+}

[Michael137]

Nit:

We typically classify these into the "simulator" tests (because they don't rely on the STL). E.g.,:

lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector

Should we create a msvc-simluators subdirectory?

[Michael137]

And this actually doesn't need to be specific to msvc fwiw. Could have a generic "simulators" directory. And put this test there. Then create another structure here in the std::__1:: namespace to confirm that libc++ also does a sane thing.

[Nerixyz]

I put it in a generic simulators directory. libc++ didn't handle this yet, so I added it there as well.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[Michael137]

thanks!