[clang] [clang][CGRecordLayout] Remove dependency on isZeroSize (PR #96422)
Eli Friedman via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 8 12:16:23 PDT 2024
efriedma-quic wrote:
> > For CGClass, it's not directly tied to the LLVM structure layout, but I'm not sure the generated code would be semantically correct if an "empty" field that isn't isEmpty() overlaps with actual data.
>
> I haven't addressed this yet. To clarify, are you referring to `addMemcpyableField` and `PushCleanupForField` (both of which check `isZeroSize`? So if we generated an overlap between an "empty" (but not `isEmpty`/`isZeroSize`) field and a non-empty field, previously `addMemcpyableField` wouldn't have considered the field for inclusion in the memcpy? So we should adjust the `isZeroSize` here too? Doing so didn't reflect in the test-suite, so it's possible there's some missing coverage here too
In addMemcpyableField(), if we conclude a field is not zero-size (and the other heuristics pass), we generate a memcpy for it. But if the field is actually empty, it might overlap with some other field, so the memcpy() might actually end up overwriting something unrelated. Similarly, for PushCleanupForField, we'll poison any overlapping data. In both cases, if the field doesn't actually contain any data, we can safely skip the relevant code.
I wouldn't be surprised if there's missing coverage here.
https://github.com/llvm/llvm-project/pull/96422
More information about the cfe-commits
mailing list