🍒[lldb] Add support for new libc++ __compressed_pair layout #10257
Conversation
This file contains 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 |
|
@swift-ci test Linux |
|
@swift-ci test Windows |
|
@swift-ci test |
adrian-prantl
approved these changes
Mar 14, 2025
…vm#100674) This patch performs following NFC changes to TypeSystemClang::GetBitSize: * Factor out the Objective-C logic into a helper function. * Introduce a new case for `FunctionProto`. I don't see a good reason for special-casing this in the `default` case. * We had a redundant check for `GetCompleteType` in the `RecordType` case. We used to allow zero-size types for records and function prototypes, so we can just fold them into the same case. * Introduce a new case for `IncompleteArray`. The motivation for this is that there are a few issues around VLAs (i.e., `Type::IncompleteArray`) whose fixes will be easier to reason about after this refactor. Generally I don't think treating a type with 0 size as an error is the right thing to do (at least not for array types). But that's not something I wanted to fold into this NFC change. (cherry picked from commit 11a7ee5)
…rrayType (llvm#100710) Depends on llvm#100674 Currently, we treat VLAs declared as `int[]` and `int[0]` identically. I.e., we create them as `IncompleteArrayType`s. However, the `DW_AT_count` for `int[0]` *does* exist, and is retrievable without an execution context. This patch decouples the notion of "has 0 elements" from "has no known `DW_AT_count`". This aligns with how Clang represents `int[0]` in the AST (it treats it as a `ConstantArrayType` of 0 size). This issue was surfaced when adapting LLDB to llvm#93069. There, the `__compressed_pair_padding` type has a `char[0]` member. If we previously got the `__compressed_pair_padding` out of a module (where clang represents `char[0]` as a `ConstantArrayType`), and try to merge the AST with one we got from DWARF (where LLDB used to represent `char[0]` as an `IncompleteArrayType`), the AST structural equivalence check fails, resulting in silent ASTImporter failures. This manifested in a failure in `TestNonModuleTypeSeparation.py`. **Implementation** 1. Adjust `ParseChildArrayInfo` to store the element counts of each VLA dimension as an `optional<uint64_t>`, instead of a regular `uint64_t`. So when we pass this down to `CreateArrayType`, we have a better heuristic for what is an `IncompleteArrayType`. 2. In `TypeSystemClang::GetBitSize`, if we encounter a `ConstantArrayType` simply return the size that it was created with. If we couldn't determine the authoritative bound from DWARF during parsing, we would've created an `IncompleteArrayType`. This ensures that `GetBitSize` on arrays with `DW_AT_count 0x0` returns `0` (whereas previously it would've returned a `std::nullopt`, causing that `FieldDecl` to just get dropped during printing) (cherry picked from commit d72c8b0)
…ords" (llvm#100903) This reverts commit 88e5206. The original change went in a while ago (last year) in https://reviews.llvm.org/D145057. The specific reason I'm proposing a revert is that this is now causing exactly the issue that @balazske predicted in https://reviews.llvm.org/D145057#4164717: > Problematic case is if the attribute has pointer to a Decl or Type that is imported here in a state when the field is already created but not initialized. Another problem is that attributes are added a second time in Import(Decl *) This now came up in the testing of LLDB support for llvm#93069. There, `__compressed_pair`s are now replaced with fields that have an `alignof(...)` and `[[no_unique_address]]` attribute. In the specific failing case, we're importing following `std::list` method: ``` size_type& __sz() _NOEXCEPT { return __size_; } ``` During this process, we create a new `__size_` `FieldDecl` (but don't initialize it yet). Then we go down the `ImportAttrs` codepath added in D145057. This imports the `alignof` expression which then references the uninitialized `__size_` and we trigger an assertion. Important to note, this codepath was added specifically to support `[[no_unique_address]]` in LLDB, and was supposed to land with https://reviews.llvm.org/D143347. But the LLDB side of that never landed, and the way we plan to support `[[no_unique_address]]` doesn't require things like the `markEmpty` method added here. So really, this is a dead codepath, which as pointed out in the original review isn't fully sound. (cherry picked from commit 31769e4)
This patch is in preparation for the `__compressed_pair` refactor in llvm#76756. This is mostly reviewable now. With the new layout we no longer need to unwrap the `__compressed_pair`. Instead, we just need to look for child members. E.g., to get to the underlying pointer of `std::unique_ptr` we no longer do, ``` GetFirstValueOfCXXCompressedPair(GetChildMemberWithName("__ptr_")) ``` but instead do ``` GetChildMemberWithName("__ptr_") ``` We need to be slightly careful because previously the `__compressed_pair` had a member called `__value_`, whereas now `__value_` might be a member of the class that used to hold the `__compressed_pair`. So before unwrapping the pair, we added checks for `isOldCompressedLayout` (not sure yet whether folding this check into `GetFirstValueOfCXXCompressedPair` is better). (cherry picked from commit 9e9b117)
For the same reasons as 6cfac49. This test was added in llvm#100710. It fails because when we're linking with link.exe, -gdwarf has no effect and we get a PDB file anyway. The Windows on Arm lldb bot uses link.exe. "C:\\Program Files\\Microsoft Visual Studio\\2022\\Community\\VC\\Tools\\MSVC\\14.34.31933\\bin\\Hostx86\\arm64\\link.exe" <...> 08/01/2024 01:47 PM 2,956,488 vla.cpp.ilk 08/01/2024 01:47 PM 6,582,272 vla.cpp.pdb 08/01/2024 01:47 PM 734,208 vla.cpp.tmp (cherry picked from commit 229a165)
Michael137
force-pushed
the
lldb/compressed-pair-to-20240723
branch
from
95287ab to
372d155
Compare
|
@swift-ci test |
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.
Cherry-picks all the changes required for the libc++ __compressed_pair refactor in llvm#93069
Fixes #9881
rdar://146964266