[lldb] Fix libstdc++ std::string formatter after #147835 #152993
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvm#147835 made changes that caused libstdc++ std::string tests to fail: ``` ====================================================================== FAIL: test_unavailable_summary_libstdcxx_dwo (TestDataFormatterStdString.StdStringDataFormatterTestCase) ---------------------------------------------------------------------- Traceback (most recent call last): File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1805, in test_method return attrvalue(self) File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/string/TestDataFormatterStdString.py", line 223, in test_unavailable_summary_libstdcxx self.do_test_summary_unavailable() File "/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/API/functionalities/data-formatter/data-formatter-stl/generic/string/TestDataFormatterStdString.py", line 213, in do_test_summary_unavailable self.assertEqual(summary, "Summary Unavailable", "No summary for bad value") AssertionError: '(null)' != 'Summary Unavailable' - (null) + Summary Unavailable : No summary for bad value Config=aarch64-/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang ---------------------------------------------------------------------- ``` This test constructs an invalid std::string by starting with a null pointer. Somehow this improvement in Clang uncovered a bug in the formatter. The formatter tries to access `_M_p` and checked whether the resulting ValueObjectSP was null, but not that it did not contain an error value. I think that error value can be there if you are able to access one part of the path, `_M_dataplus`, but another part fails. Since the layout looks like this: ``` struct _Alloc_hider : { pointer _M_p; // The actual data. }; _Alloc_hider _M_dataplus; void _M_data(pointer __p) { _M_dataplus._M_p = __p; } ``` So I think we were able to read `_M_dataplus` just by offset, but then failed because it contains, or points to something containing nulls or a null pointer. Or perhaps an error value means we know what the class member is, but could not read from it. I found this by comparing with the libcxx formatter, so I've copied the same handling from there to fix the issue.
|
@llvm/pr-subscribers-lldb Author: David Spickett (DavidSpickett) Changes#147835 made changes that caused libstdc++ std::string tests to fail: This test constructs an invalid std::string by starting with a null pointer. Somehow this improvement in Clang uncovered a bug in the formatter. The formatter tries to access I think that error value can be there if you are able to access one part of the path, So I think we were able to read Or perhaps an error value means we know what the class member is, but could not read from it. I found this by comparing with the libcxx formatter, so I've copied the same handling from there to fix the issue. Full diff: https://github.com/llvm/llvm-project/pull/152993.diff 1 Files Affected:
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
index 595e835b37df9..f4a695e036999 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcpp.cpp
@@ -241,10 +241,11 @@ VectorIteratorSyntheticFrontEnd::GetIndexOfChildWithName(ConstString name) {
bool lldb_private::formatters::LibStdcppStringSummaryProvider(
ValueObject &valobj, Stream &stream, const TypeSummaryOptions &options) {
ValueObjectSP ptr = valobj.GetChildAtNamePath({"_M_dataplus", "_M_p"});
- if (!ptr)
- return false;
+ if (!ptr || !ptr->GetError().Success())
+ stream << "Summary Unavailable";
+ else
+ stream << ptr->GetSummaryAsCString();
- stream << ptr->GetSummaryAsCString();
return true;
}
|
Michael137
approved these changes
Aug 11, 2025
I guess that before we didn't have the full type of this nested struct member. Then clang made improvements so we did, and then knew to format it as a C string. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#147835 made changes that caused libstdc++ std::string tests to fail:
This test constructs an invalid std::string by starting with a null pointer.
Somehow this improvement in Clang uncovered a bug in the formatter. Perhaps because we now know the type of the field we tried to access is char *, so fall back to a C string formatter that produces
(null).The formatter tries to access
_M_pand checked whether the resulting ValueObjectSP was null, but not that it did not contain an error value. I think that error value can be there if you are able to access one part of the path,_M_dataplus, but another part fails. Since the layout looks like this:So I think we were able to read
_M_dataplusjust by offset, but then failed because it contains, or points to something containing nulls or a null pointer.Or perhaps an error value means we know what the class member is, but could not read from it.
I found this by comparing with the libcxx formatter, so I've copied the same handling from there to fix the issue.