[LLDB] Avoid crashes when inspecting MSVC STL types #140761

Nerixyz · 2025-05-20T16:48:18Z

When inspecting/printing types from MSVC's STL, LLDB would crash because it assumes these types were from libstdc++. Specifically, std::shared_ptr and std::optional would crash because of a null pointer dereference. I added a minimal test that tests the types with C++ helpers for libstdc++ (only tests for crashes).

Fixes [lldb] resolving optional crashes lldb(-dap) (clang/windows/msvc) #115216
Fixes [LLDB] frame variable crashes when printing a shared_ptr when program uses MSVC STL #120310

This still has one unresolved discussion: What about MS STL types? This is #24834, but there was a bit of discussion in #120310 as well. The main issue is that MSVC's STL uses the same type names as libstdc++ (i.e. neither uses an inline namespace like libc++ for some types).

llvmbot · 2025-05-20T16:48:55Z

@llvm/pr-subscribers-lldb

Author: nerix (Nerixyz)

Changes

When inspecting/printing types from MSVC's STL, LLDB would crash because it assumes these types were from libstdc++. Specifically, std::shared_ptr and std::optional would crash because of a null pointer dereference. I added a minimal test that tests the types with C++ helpers for libstdc++ (only tests for crashes).

Fixes #120310. This still has one unresolved discussion: What about MS STL types? This is #24834, but there was a bit of discussion in #120310 as well. The main issue is that MSVC's STL uses the same type names as libstdc++ (i.e. neither uses an inline namespace like libc++ for some types).

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

4 Files Affected:

(modified) lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp (+5-3)
(modified) lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp (+1-1)
(added) lldb/test/Shell/Process/Windows/msstl_smoke.cpp (+57)
(modified) lldb/test/Shell/helper/build.py (+3-3)

diff --git a/lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp b/lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
index b1fdc0fe37763..ed4536cb5c775 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp
@@ -74,9 +74,11 @@ lldb::ChildCacheState GenericOptionalFrontend::Update() {
 
   if (m_stdlib == StdLib::LibCxx)
     engaged_sp = m_backend.GetChildMemberWithName("__engaged_");
-  else if (m_stdlib == StdLib::LibStdcpp)
-    engaged_sp = m_backend.GetChildMemberWithName("_M_payload")
-                     ->GetChildMemberWithName("_M_engaged");
+  else if (m_stdlib == StdLib::LibStdcpp) {
+    ValueObjectSP payload = m_backend.GetChildMemberWithName("_M_payload");
+    if (payload)
+      engaged_sp = payload->GetChildMemberWithName("_M_engaged");
+  }
 
   if (!engaged_sp)
     return lldb::ChildCacheState::eRefetch;
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index 02113baf64b8c..54158d9a0d85a 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -379,7 +379,7 @@ LibStdcppSharedPtrSyntheticFrontEnd::CalculateNumChildren() {
 
 lldb::ValueObjectSP
 LibStdcppSharedPtrSyntheticFrontEnd::GetChildAtIndex(uint32_t idx) {
-  if (idx == 0)
+  if (idx == 0 && m_ptr_obj)
     return m_ptr_obj->GetSP();
   if (idx == 1) {
     if (m_ptr_obj && !m_obj_obj) {
diff --git a/lldb/test/Shell/Process/Windows/msstl_smoke.cpp b/lldb/test/Shell/Process/Windows/msstl_smoke.cpp
new file mode 100644
index 0000000000000..4ae8f4256f24e
--- /dev/null
+++ b/lldb/test/Shell/Process/Windows/msstl_smoke.cpp
@@ -0,0 +1,57 @@
+// clang-format off
+
+// REQUIRES: target-windows
+// RUN: %build --compiler=clang-cl -o %t.exe --std c++20 -- %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -o "b main" -o "run" -o "fr v" -o c | FileCheck %s
+
+#include <bitset>
+#include <coroutine>
+#include <deque>
+#include <forward_list>
+#include <list>
+#include <map>
+#include <memory>
+#include <optional>
+#include <set>
+#include <string>
+#include <tuple>
+#include <unordered_map>
+#include <unordered_set>
+#include <variant>
+#include <vector>
+
+int main() {
+  std::shared_ptr<int> foo;
+  std::weak_ptr<int> weak = foo;
+  std::unique_ptr<int> unique(new int(42));
+  std::optional<int> opt;
+  std::string str = "str";
+  std::string longStr =
+      "string that is long enough such that no SSO can happen";
+  std::wstring wStr = L"wstr";
+  std::wstring longWStr =
+      L"string that is long enough such that no SSO can happen";
+  std::tuple<int, bool, float> tuple{1, false, 4.2};
+  std::coroutine_handle<> coroHandle;
+  std::bitset<16> bitset(123);
+
+  std::map<int, int> map{{1, 2}, {2, 4}, {3, 6}, {4, 8}, {5, 10}};
+  auto mapIt = map.find(3);
+  auto mapItEnd = map.find(9);
+  std::set<int> set{1, 2, 3};
+  std::multimap<int, int> mMap{{1, 2}, {1, 1}, {2, 4}};
+  std::multiset<int> mSet{1, 2, 3};
+
+  std::variant<int, float, std::string, std::monostate> variant;
+  std::list<int> list{1, 2, 3};
+  std::forward_list<int> fwList{1, 2, 3};
+
+  std::unordered_map<int, int> uMap{{1, 2}, {2, 4}, {3, 6}};
+  std::unordered_set<int> uSet{1, 2, 4};
+  std::unordered_multimap<int, int> uMMap{{1, 2}, {1, 1}, {2, 4}};
+  std::unordered_multiset<int> uMSet{1, 1, 2};
+  std::deque<int> deque{1, 2, 3};
+  std::vector<int> vec{1, 2, 3};
+}
+
+// CHECK: Process {{.*}} exited with status = 0 (0x00000000)
diff --git a/lldb/test/Shell/helper/build.py b/lldb/test/Shell/helper/build.py
index b2b8146e88c75..caaa14f90af1c 100755
--- a/lldb/test/Shell/helper/build.py
+++ b/lldb/test/Shell/helper/build.py
@@ -683,14 +683,14 @@ def _get_compilation_command(self, source, obj):
             args.append("-fms-compatibility-version=19")
         args.append("/c")
 
+        if self.std:
+            args.append("/std:" + self.std)
+
         args.append("/Fo" + obj)
         if self.toolchain_type == "clang-cl":
             args.append("--")
         args.append(source)
 
-        if self.std:
-            args.append("/std:" + self.std)
-
         return ("compiling", [source], obj, self.compile_env, args)
 
     def _get_link_command(self):

lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp

lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp

Michael137 · 2025-05-21T09:01:49Z

lldb/test/Shell/Process/Windows/msstl_smoke.cpp

+// RUN: %build --compiler=clang-cl -o %t.exe --std c++20 -- %s
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb -f %t.exe -o "b main" -o "run" -o "fr v" -o c | FileCheck %s
+
+#include <bitset>


Are we not running the data-formatter tests on Windows at all? There's definitely tests for all of these elsewhere in the API test-suite

Do you have any specific tests? Ideally ones that test optional and shared_ptr. I can look at them later.

The tests in lldb/test/API/functionalities/data-formatter/data-formatter-stl

Most of these tests are unsupported due to being in the libc++/libstdc++ categories which aren't supported on Windows ("Don't know how to build with libc++ on windows"). The only tests that run are libstdcpp/unique_ptr/invalid and libcxx-simulators/*.

Hmm right that makes sense. Should we then have a Windows data-formatter subdirectory in lldb/test/API/functionalities/data-formatter/data-formatter-stl? I think we should ideally have two test configurations:

Tests that ensure we at least don't crash when using the MSVC STL. E.g., at least for std::optional and std::shared_ptr. This would test your current PR

Tests for libc++ on Windows (which AFAIU, isn't the recommended configuration on Windows but is definitely used in the wild)

Not sure how easy it is to set up (2). That shouldn't block this PR, but would be nice to get around to it.

But (1) should be doable. Though not sure how equipped the API test infrastructure is to compile against MSVC STL. Worst case we could go for Shell tests like you did here

CC @DavidSpickett @labath @omjavaid for opinions on Windows testing

Hmm I wonder if that was leftover documentation from when the data-formatters were all written in python? But yea, now they're all in the cplusplus category IIUC

Yup looks like it is. At least when browsing back to:

commit 78453eed1219b83dff87d34a4bbbe48cc65eda70 (HEAD) Author: Enrico Granata <egranata@apple.com> Date: Thu May 3 01:08:44 2012 +0000 Adding a new 'type category disable *' feature that disables all categories - This is intended as a quick kill switch for data formatters for cases where the user wants as little extra processing as possible to be done on their target, or to override major data formatters bug, should they occur llvm-svn: 156044

And searching the repo for gnu-libstdc++, you'll see that the libcxx and lisbtdcpp formatters were written in python and were added to separate categories. Not 100% why they got combined back into the same category. Presumably the idea was that you never want to have both categories enabled anyway, so if a cplusplus category can just turn on/off the STL formatters for you wholesale, that'd be more intuitive.

I presume all API tests on Windows are compiling against MSVC STL when compiled on Windows (it has nothing else to use). If you're wondering whether there's a good way to detect that is the case, yeah, that might be tricky.

For number 2, I'm not into the idea of changing the existing bot to build everything against libcxx, but if we can enable the runtime and then libcxx specific tests will be run as well, that's fine. Windows is supported and tested for libcxx as long as clang/clang-cl is the compiler, which we use already on the bot.

Thanks for the additional context David. I'm not super familiar with the Windows test bot setup

Not sure what the best testing strategy here is then. Since the STL formatters in LLDB currently don't support MSVC STL, and we just want to make sure they don't crash, maybe we could even omit tests for this PR until we get proper support for it? Though I don't mind the smoke test added here either tbh, maybe with a FIXME? Just diverges a bit from the rest of our formatter testing.

If this smoke test ensures they don't crash horribly, which I think it does, we should accept it. If we have issue(s) talking about MSVC formatter support link to them, or if we don't have an overall one, open one and link to that.

If this test fails way too often and it's holding up formatter development elsewhere, then we can revisit it of course.

lldb/test/Shell/helper/build.py

Michael137

Seems like a fine stop-gap for now to avoid crashing

Just left some clarification questions

Michael137 · 2025-05-21T09:03:02Z

FYI @charles-zablit

Nerixyz · 2025-05-21T09:11:44Z

Seems like a fine stop-gap for now to avoid crashing

Yes, exactly. The formatted output for these types is still incomplete (often, types are formatted without any children).
I'd love to help with support for MSVC's STL, but it's probably better to discuss that in #24834.

This is still leftover from the days when the libc++ and libstdc++ formatters were both written in python and in separate categories. Since then we group libstdc++ and libc++ formatters into the same cateogry. This patch removes references to the obsolete `gnu-libstdc++` category from the docs (and a test). See [this thread](llvm#140761 (comment)) for more context

#141610) This is still leftover from the days when the libc++ and libstdc++ formatters were both written in python and in separate categories. Since then we group libstdc++ and libc++ formatters into the same cateogry. This patch removes references to the obsolete `gnu-libstdc++` category from the docs (and a test). See [this thread](#140761 (comment)) for more context

…c++ category (#141610) This is still leftover from the days when the libc++ and libstdc++ formatters were both written in python and in separate categories. Since then we group libstdc++ and libc++ formatters into the same cateogry. This patch removes references to the obsolete `gnu-libstdc++` category from the docs (and a test). See [this thread](llvm/llvm-project#140761 (comment)) for more context

lldb/test/Shell/Process/Windows/msstl_smoke.cpp

Michael137 · 2025-05-28T10:43:45Z

lldb/test/Shell/Process/Windows/msstl_smoke.cpp

@@ -0,0 +1,57 @@
+// clang-format off
+
+// REQUIRES: target-windows


Can you add a comment here to explain why this test exists? I.e., that this just ensures that debugging with MSVC STL doesn't crash, etc.

Michael137

LGTM (just left small test comments)

DavidSpickett

Request changes just so the remaining comment doesn't get forgotten, the rest Michael has approved and if he's cool with it so am I.

lldb/test/Shell/helper/build.py

From #140761. `MsvcBuilder` passed `/std:<value>` (if specified) after `--`, so the compiler would interpret this as a file. This moves the argument before the `--`.

Michael137 · 2025-05-30T08:09:13Z

Think this is good to go after rebase

From llvm/llvm-project#140761. `MsvcBuilder` passed `/std:<value>` (if specified) after `--`, so the compiler would interpret this as a file. This moves the argument before the `--`.

Michael137 · 2025-05-30T15:13:19Z

@Nerixyz do you need us to merge this for you?

Nerixyz · 2025-05-30T15:51:59Z

@Nerixyz do you need us to merge this for you?

Yeah, that would be great!

Nerixyz · 2025-05-30T19:34:10Z

I put a summary and some ideas about MSVC STL support in #24834 (comment).

llvm#141610) This is still leftover from the days when the libc++ and libstdc++ formatters were both written in python and in separate categories. Since then we group libstdc++ and libc++ formatters into the same cateogry. This patch removes references to the obsolete `gnu-libstdc++` category from the docs (and a test). See [this thread](llvm#140761 (comment)) for more context

From llvm#140761. `MsvcBuilder` passed `/std:<value>` (if specified) after `--`, so the compiler would interpret this as a file. This moves the argument before the `--`.

Nerixyz requested a review from JDevlieghere as a code owner May 20, 2025 16:48

llvmbot added the lldb label May 20, 2025

Michael137 reviewed May 21, 2025

View reviewed changes

lldb/source/Plugins/Language/CPlusPlus/GenericOptional.cpp Outdated Show resolved Hide resolved

Michael137 reviewed May 21, 2025

View reviewed changes

lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp Outdated Show resolved Hide resolved

Michael137 reviewed May 21, 2025

View reviewed changes

lldb/test/Shell/helper/build.py Outdated Show resolved Hide resolved

Michael137 reviewed May 21, 2025

View reviewed changes

Michael137 requested a review from adrian-prantl May 21, 2025 09:02

Nerixyz force-pushed the fix/msstl-crashes branch from 15c47a0 to 355cdcd Compare May 21, 2025 15:47

Michael137 mentioned this pull request May 24, 2025

[LLDB] Fix std::shared_ptr formatter crash on windows #141348

Closed

Michael137 mentioned this pull request May 27, 2025

[lldb][docs][NFC] Remove references to obsolete gnu-libstdc++ category #141610

Merged

Michael137 approved these changes May 28, 2025

View reviewed changes

Michael137 reviewed May 28, 2025

View reviewed changes

lldb/test/Shell/Process/Windows/msstl_smoke.cpp Outdated Show resolved Hide resolved

Michael137 reviewed May 28, 2025

View reviewed changes

Michael137 approved these changes May 28, 2025

View reviewed changes

DavidSpickett requested changes May 28, 2025

View reviewed changes

lldb/test/Shell/helper/build.py Outdated Show resolved Hide resolved

Nerixyz mentioned this pull request May 28, 2025

[LLDB] Pass /std:... before -- on MSVC #141782

Merged

Nerixyz force-pushed the fix/msstl-crashes branch from 355cdcd to af77293 Compare May 28, 2025 14:46

[LLDB] Avoid crashes when inspecting MS STL types

9ee1b61

Nerixyz force-pushed the fix/msstl-crashes branch from af77293 to 9ee1b61 Compare May 30, 2025 09:49

Michael137 merged commit 9e9626b into llvm:main May 30, 2025
10 checks passed

Nerixyz deleted the fix/msstl-crashes branch May 30, 2025 19:46

		@@ -0,0 +1,57 @@
		// clang-format off

		// REQUIRES: target-windows

[LLDB] Avoid crashes when inspecting MSVC STL types #140761

[LLDB] Avoid crashes when inspecting MSVC STL types #140761

Uh oh!

Conversation

Nerixyz commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented May 20, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Michael137 May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Michael137 May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Michael137 left a comment

Choose a reason for hiding this comment

Uh oh!

Michael137 commented May 21, 2025

Uh oh!

Nerixyz commented May 21, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Michael137 left a comment

Choose a reason for hiding this comment

Uh oh!

DavidSpickett left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Michael137 commented May 30, 2025

Uh oh!

Michael137 commented May 30, 2025

Uh oh!

Nerixyz commented May 30, 2025

Uh oh!

Nerixyz commented May 30, 2025

Uh oh!

Uh oh!

Uh oh!

Nerixyz commented May 20, 2025 •

edited

Loading

Michael137 May 22, 2025 •

edited

Loading

Michael137 May 22, 2025 •

edited

Loading