forked from llvm/llvm-project
-
Notifications
You must be signed in to change notification settings - Fork 340
[lldb][DataFormatter] Surface CalculateNumChildren errors in std::vector summary (#135944) #10498
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
adrian-prantl
merged 9 commits into
swift/release/6.2
from
lldb/vector-summary-errors-to-6.2
Apr 17, 2025
Merged
[lldb][DataFormatter] Surface CalculateNumChildren errors in std::vector summary (#135944) #10498
adrian-prantl
merged 9 commits into
swift/release/6.2
from
lldb/vector-summary-errors-to-6.2
Apr 17, 2025
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
@swift-ci test |
adrian-prantl
approved these changes
Apr 16, 2025
@swift-ci test |
…ests (llvm#99012) This is a follow-up to llvm#98330 for the upcoming `__compressed_pair` refactor in llvm#93069 This patch just adds the 2 new copies of `_LIBCPP_COMPRESSED_PAIR` layouts to the simulator tests. (cherry picked from commit 765e106)
…luator This would've caught the failures in llvm#105865 in the libc++ data-formatter CI. (cherry picked from commit 19f604e)
…or current layout (llvm#108362) IIUC, the history of `std::string`'s `__short` structure in the alternate ABI layout (as recorded by the simulator test) looks as follows: * First layout ( `SUBCLASS_PADDING` is defined): ``` struct __short { value_type __data_[__min_cap]; struct : __padding<value_type> { unsigned char __size_; }; }; ``` * Then: ``` struct __short { value_type __data_[__min_cap]; unsigned char __padding[sizeof(value_type) - 1]; unsigned char __size_; }; ``` * Then, post-`BITMASKS`: ``` struct __short { value_type __data_[__min_cap]; unsigned char __padding[sizeof(value_type) - 1]; unsigned char __size_ : 7; unsigned char __is_long_ : 1; }; ``` Which is the one that's [on top-of-tree](https://github.com/llvm/llvm-project/blob/89c10e27d8b4d5f44998aad9abd2590d9f96c5df/libcxx/include/string#L854-L859). But for `REVISION > 1`, `BITMASKS` is never set, so for those tests we lose the `__padding` member. This patch fixes this by splitting out the `SUBCLASS_PADDING` out of the ifdef. Drive-by: * Also run expression evaluator on the string to provide is with some extra coverage. (cherry picked from commit 4ca8fb1)
…ng layout (llvm#108375) Depends on llvm#108362 and llvm#108343. Adds new layout for llvm#105865. (cherry picked from commit d5f6e88)
…ew padding layout (llvm#108375)" This reverts commit d5f6e88. Caused failure on Windows CI. Following test failed: ``` Config=aarch64-C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\clang.exe ====================================================================== FAIL: test_r5_c2_ALTERNATE_LAYOUT (TestDataFormatterLibcxxStringSimulator.LibcxxStringDataFormatterSimulatorTestCase.test_r5_c2_ALTERNATE_LAYOUT) partial(func, *args, **keywords) - new function with partial application ---------------------------------------------------------------------- Traceback (most recent call last): File "C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\API\functionalities\data-formatter\data-formatter-stl\libcxx-simulators\string\TestDataFormatterLibcxxStringSimulator.py", line 23, in _run_test self.expect_var_path("longstring", summary='"I am a very long string"') File "C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\packages\Python\lldbsuite\test\lldbtest.py", line 2552, in expect_var_path value_check.check_value(self, eval_result, str(eval_result)) File "C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\packages\Python\lldbsuite\test\lldbtest.py", line 321, in check_value test_base.assertEqual( AssertionError: '"I am a very long string"' != '""' - "I am a very long string" + "" : (std::__lldb::string) longstring = "" Checking SBValue: (std::__lldb::string) longstring = "" ``` We may need to use `msvc::no_unique_address` around the simulators. (cherry picked from commit ee4dd14)
…ew padding layout" (llvm#111123) Relands llvm#108375 which had to be reverted because it was failing on the Windows buildbot. Trying to reland this with `msvc::no_unique_address` on Windows. (cherry picked from commit d148548)
…tor summary (llvm#135944) When the data-formatters happen to break (e.g., due to layout changes in libc++), there's no clear indicator of them failing from a user's perspective. E.g., for `std::vector`s we would just show: ``` (std::vector<int>) v = size=0 {} ``` which is highly misleading, especially if `v.size()` returns a non-zero size. This patch surfaces the various errors that could occur when calculating the number of children of a vector. rdar://146964266 (cherry picked from commit 419fa1b)
…line namespace warnings Fixes: ``` /Users/ec2-user/jenkins/workspace/apple-llvm-project-pr-macos/branch-swift/release/6.2/llvm-project/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libcxx-simulators/invalid-vector/main.cpp:5:11: warning: inline namespace reopened as a non-inline namespace [-Winline-namespace-reopened-noninline] 5 | namespace __1 { | ^ ``` Drive-by: compile test as C++20 (in an attempt to fix another buildbot issue) (cherry picked from commit dfed3d2)
2a2ee18
to
4159e41
Compare
@swift-ci test |
Had to pull in some simulator tests commits that weren't cherry-picked |
This fails on one of the compressed_pair layouts with: ``` Assertion failed: (M.Offset >= Tail && "Bitfield access unit is not clipped"), function checkBitfieldClipping, file CGRecordLayoutBuilder.cpp, line 960. PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace. Stack dump: 0. <eof> parser at end of file 1. <lldb wrapper prefix>:43:1: Generating code for declaration '$__lldb_expr' Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it): 0 liblldb.19.1.5.dylib 0x0000000143fc03fc llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 56 1 liblldb.19.1.5.dylib 0x0000000143fbe720 llvm::sys::RunSignalHandlers() + 112 2 liblldb.19.1.5.dylib 0x0000000143fc0a90 SignalHandler(int, __siginfo*, void*) + 360 3 libsystem_platform.dylib 0x00000001809c7624 _sigtramp + 56 4 libsystem_pthread.dylib 0x000000018098d88c pthread_kill + 296 5 libsystem_c.dylib 0x0000000180896c60 abort + 124 6 libsystem_c.dylib 0x0000000180895eec err + 0 7 liblldb.19.1.5.dylib 0x0000000144042708 (anonymous namespace)::CGRecordLowering::lower(bool) (.cold.70) + 0 8 liblldb.19.1.5.dylib 0x0000000141ab9f5c (anonymous namespace)::CGRecordLowering::lower(bool) + 10428 9 liblldb.19.1.5.dylib 0x0000000141ab6d80 clang::CodeGen::CodeGenTypes::ComputeRecordLayout(clang::RecordDecl const*, llvm::StructType*) + 240 10 liblldb.19.1.5.dylib 0x0000000141b7b980 clang::CodeGen::CodeGenTypes::ConvertRecordDeclType(clang::RecordDecl const*) + 352 11 liblldb.19.1.5.dylib 0x0000000141b7a6b4 clang::CodeGen::CodeGenTypes::ConvertTypeForMem(clang::QualType) + 172 12 liblldb.19.1.5.dylib 0x0000000141abb0e0 (anonymous namespace)::CGRecordLowering::getStorageType(clang::FieldDecl const*) const + 48 13 liblldb.19.1.5.dylib 0x0000000141ab7904 (anonymous namespace)::CGRecordLowering::lower(bool) + 612 ``` But only on the Apple LLVM fork. Will need to investigate this separately
@swift-ci test |
@swift-ci test Windows |
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.
When the data-formatters happen to break (e.g., due to layout changes in libc++), there's no clear indicator of them failing from a user's perspective. E.g., for
std::vector
s we would just show:which is highly misleading, especially if
v.size()
returns a non-zero size.This patch surfaces the various errors that could occur when calculating the number of children of a vector.
rdar://146964266
(cherry picked from commit 419fa1b)