This is an archive of the discontinued LLVM Phabricator instance.

[Clang][CodeGen] Fixing mismatch between memory layout and const expressions for oversized bitfields
ClosedPublic

Authored by pratlucas on Mar 30 2020, 3:44 AM.

Details

Summary

The construction of constants for structs/unions was conflicting the
expected memory layout for over-sized bit-fields. When building the
necessary bits for those fields, clang was ignoring the size information
computed for the struct/union memory layout and using the original data
from the AST's FieldDecl information. This caused an issue in big-endian
targets, where the field's contant was incorrectly misplaced due to
endian calculations.

This patch aims to separate the constant value from the necessary
padding bits, using the proper size information for each one of them.
With this, the layout of constants for over-sized bit-fields matches the
ABI requirements.

Diff Detail

Event Timeline

pratlucas created this revision.Mar 30 2020, 3:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 30 2020, 3:44 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
pratlucas updated this revision to Diff 253568.Mar 30 2020, 5:44 AM

Formatting.

efriedma added inline comments.
clang/lib/CodeGen/CGExprConstant.cpp
613

The existing code in ConstantAggregateBuilder::add should skip over padding. I'm not sure what you're trying to accomplish by adding more code dealing with padding here.

pratlucas updated this revision to Diff 254156.Apr 1 2020, 4:00 AM

Removing unecessary handling of padding bits.

pratlucas marked 2 inline comments as done.Apr 1 2020, 4:02 AM
pratlucas added inline comments.
clang/lib/CodeGen/CGExprConstant.cpp
613

You're right, I missed the fact that FieldOffsets take care of this.

pratlucas marked an inline comment as done.Apr 1 2020, 4:02 AM
efriedma accepted this revision.Apr 1 2020, 9:49 AM

LGTM

This revision is now accepted and ready to land.Apr 1 2020, 9:49 AM
This revision was automatically updated to reflect the committed changes.