[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:

While fixing the libc++ formatters in preparation for the [compressed_pair change](https://github.com/llvm/llvm-project/issues/93069), i encountered another issue which I'm not sure entirely how to best reconcile. There's [this assumption in `RecordLayoutBuilder`](https://github.com/llvm/llvm-project/blob/f782ff8fc6426890863be0791a9ace2394da3887/clang/lib/AST/RecordLayoutBuilder.cpp#L2258-L2264) for external layouts, where, if we encounter overlapping field offsets, we assume the structure is packed and set the alignment back to `1`:
```
uint64_t
ItaniumRecordLayoutBuilder::updateExternalFieldOffset(const FieldDecl *Field,
                                                      uint64_t ComputedOffset) {
  uint64_t ExternalFieldOffset = External.getExternalFieldOffset(Field);

  if (InferAlignment && ExternalFieldOffset < ComputedOffset) {
    // The externally-supplied field offset is before the field offset we
    // computed. Assume that the structure is packed.
    Alignment = CharUnits::One();
    PreferredAlignment = CharUnits::One();
    InferAlignment = false;
  }
  ...
```

The assumption in that comment doesn't hold for layouts where the overlap occurred because of `[[no_unique_address]]`. In those cases, the alignment of `1` will prevent us from aligning the `FieldOffset` correctly and cause LLDB to read out the fields incorrectly.

We can't guard this with `Field->isZeroSize()` because we don't have the attribute in the AST.
Can we infer packedness more accurately here?
Or maybe that's just putting a bandaid on a bigger underlying issue

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