Skip to content

[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
merged 9 commits into from
Apr 17, 2025

Conversation

Michael137
Copy link

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::vectors 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)

@Michael137 Michael137 requested a review from a team as a code owner April 16, 2025 16:02
@Michael137
Copy link
Author

@swift-ci test

@Michael137
Copy link
Author

@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)
@Michael137 Michael137 force-pushed the lldb/vector-summary-errors-to-6.2 branch from 2a2ee18 to 4159e41 Compare April 17, 2025 07:59
@Michael137
Copy link
Author

@swift-ci test

@Michael137
Copy link
Author

Michael137 commented Apr 17, 2025

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
@Michael137
Copy link
Author

@swift-ci test

@Michael137
Copy link
Author

@swift-ci test Windows

@adrian-prantl adrian-prantl merged commit edd19f7 into swift/release/6.2 Apr 17, 2025
3 checks passed
@adrian-prantl adrian-prantl deleted the lldb/vector-summary-errors-to-6.2 branch April 17, 2025 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants