[Reland] Detect against invalid variant index for LibStdC++ std::variant data formatters

[jeffreytan81]

This is relanding of #69253. TestTemplatePackArgs.py is passing now.

https://github.com/llvm/llvm-project/pull/68012/files added new data formatters for LibStdC++ std::variant.

However, this formatter can crash if std::variant's index field has invalid value (exceeds the number of template arguments).
This can happen if the current IP stops at a place std::variant is not initialized yet.

This patch fixes the crash by ensuring the index is a valid value and fix GetNthTemplateArgument() to make sure it is not crashing.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: None (jeffreytan81)

Changes

This is relanding of #69253. TestTemplatePackArgs.py is passing now.

https://github.com/llvm/llvm-project/pull/68012/files added new data formatters for LibStdC++ std::variant.

However, this formatter can crash if std::variant's index field has invalid value (exceeds the number of template arguments).
This can happen if the current IP stops at a place std::variant is not initialized yet.

This patch fixes the crash by ensuring the index is a valid value and fix GetNthTemplateArgument() to make sure it is not crashing.

---

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

3 Files Affected:

• (modified) lldb/examples/synthetic/gnu_libstdcpp.py (+5)
• (modified) lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp (+2-1)
• (modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py (+26)

diff --git a/lldb/examples/synthetic/gnu_libstdcpp.py b/lldb/examples/synthetic/gnu_libstdcpp.py
index 29c926167fb440c..f778065aaca3771 100644
--- a/lldb/examples/synthetic/gnu_libstdcpp.py
+++ b/lldb/examples/synthetic/gnu_libstdcpp.py
@@ -914,6 +914,11 @@ def get_variant_npos_value(index_byte_size):
     if index == npos_value:
         return " No Value"
 
+    # Invalid index can happen when the variant is not initialized yet.
+    template_arg_count = data_obj.GetType().GetNumberOfTemplateArguments()
+    if index >= template_arg_count:
+        return " <Invalid>"
+
     active_type = data_obj.GetType().GetTemplateArgumentType(index)
     return f" Active Type = {active_type.GetDisplayTypeName()} "
 
diff --git a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
index f1353db2631ddc6..df06ba0ed952af7 100644
--- a/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ b/lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -7183,7 +7183,8 @@ GetNthTemplateArgument(const clang::ClassTemplateSpecializationDecl *decl,
   // (including the ones preceding the parameter pack).
   const auto &pack = args[last_idx];
   const size_t pack_idx = idx - last_idx;
-  assert(pack_idx < pack.pack_size() && "parameter pack index out-of-bounds");
+  if (pack_idx >= pack.pack_size())
+    return nullptr;
   return &pack.pack_elements()[pack_idx];
 }
 
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py
index 96a9c8d30c45b00..ba1641888b6f30f 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/variant/TestDataFormatterLibStdcxxVariant.py
@@ -71,3 +71,29 @@ def test_with_run_command(self):
             substrs=["v_many_types_no_value =  No Value"],
         )
         """
+
+    @add_test_categories(["libstdcxx"])
+    def test_invalid_variant_index(self):
+        """Test LibStdC++ data formatter for std::variant with invalid index."""
+        self.build()
+
+        (self.target, self.process, thread, bkpt) = lldbutil.run_to_source_breakpoint(
+            self, "// break here", lldb.SBFileSpec("main.cpp", False)
+        )
+
+        lldbutil.continue_to_breakpoint(self.process, bkpt)
+
+        self.expect(
+            "frame variable v1",
+            substrs=["v1 =  Active Type = int  {", "Value = 12", "}"],
+        )
+
+        var_v1 = thread.frames[0].FindVariable("v1")
+        var_v1_raw_obj = var_v1.GetNonSyntheticValue()
+        index_obj = var_v1_raw_obj.GetChildMemberWithName("_M_index")
+        self.assertTrue(index_obj and index_obj.IsValid())
+
+        INVALID_INDEX = "100"
+        index_obj.SetValueFromCString(INVALID_INDEX)
+
+        self.expect("frame variable v1", substrs=["v1 =  <Invalid>"])

[DavidSpickett]

Is this change needed now that the formatter is checking the number of arguments up front?

[jeffreytan81]

It is requested by @clayborg's comment in the original patch:

If this is what was crashing, we should modify the GetTemplateArgumentType() to not crash with an inalid index as part of this fix.

[bulbazord]

+1, we should still include this change. We shouldn't crash if the idx argument is too high.