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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to