[Lldb-commits] [clang] [lldb] [clang][lldb] Don't assert structure layout correctness for layouts provided by LLDB (PR #93809)

[Michael Buch via lldb-commits]

Michael137 wrote:

> Here's the smallest patch that would put explicit alignment on any packed structure:
> 
> ```
> diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp
> index a072475ba770..bbb13ddd593b 100644
> --- a/clang/lib/CodeGen/CGDebugInfo.cpp
> +++ b/clang/lib/CodeGen/CGDebugInfo.cpp
> @@ -64,7 +64,7 @@ static uint32_t getTypeAlignIfRequired(const Type *Ty, const ASTContext &Ctx) {
>    // MaxFieldAlignmentAttr is the attribute added to types
>    // declared after #pragma pack(n).
>    if (auto *Decl = Ty->getAsRecordDecl())
> -    if (Decl->hasAttr<MaxFieldAlignmentAttr>())
> +    if (Decl->hasAttr<MaxFieldAlignmentAttr>() || Decl->hasAttr<PackedAttr>())
>        return TI.Align;
>  
>    return 0;
> ```
> 
> But I don't think that's the right approach - I think what we should do is compute the natural alignment of the structure, then compare that to the actual alignment - and if they differ, we should put an explicit alignment on the structure. This avoids the risk that other alignment-influencing effects might be missed (and avoids the case of putting alignment on a structure that, when packed, just has the same alignment anyway - which is a minor issue, but nice to get right (eg: packed struct of a single char probably shouldn't have an explicit alignment - since it's the same as the implicit alignment anyway))

Thanks for the analysis! If we can emit alignment for packed attributes consistently then we probably can get rid of most of the `InferAlignment` logic in the `RecordLayoutBuilder` (it seems to me most of that logic was put introduced there for the purpose of packed structs), which would address the issue I saw with laying out `[[no_unique_address]]` fields. Trying this now

https://github.com/llvm/llvm-project/pull/93809