Skip to content

Commit d79f3f6

Browse files
yan-wang-googleyan-wang-google
committed
[clang] Change the condition of unnecessary packed warning
Summary: Change the condition of this unnecessary packed warning. The packed is unnecessary when 1. the alignment of the struct/class won't alter. 2. the size is unchanged. 3. the offset of each field is the same. Remove all field-level warning. Reviewers: chh, akyrtzi, rtrieu Reviewed By: chh Subscribers: rsmith, srhines, cfe-commits, xazax.hun Differential Revision: https://reviews.llvm.org/D34114 llvm-svn: 309750
1 parent 51ca757 commit d79f3f6

File tree

2 files changed

+98
-21
lines changed

2 files changed

+98
-21
lines changed

‎clang/lib/AST/RecordLayoutBuilder.cpp

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -632,6 +632,9 @@ class ItaniumRecordLayoutBuilder {
632632
/// pointer, as opposed to inheriting one from a primary base class.
633633
bool HasOwnVFPtr;
634634

635+
/// \brief the flag of field offset changing due to packed attribute.
636+
bool HasPackedField;
637+
635638
typedef llvm::DenseMap<const CXXRecordDecl *, CharUnits> BaseOffsetsMapTy;
636639

637640
/// Bases - base classes and their offsets in the record.
@@ -666,7 +669,7 @@ class ItaniumRecordLayoutBuilder {
666669
NonVirtualSize(CharUnits::Zero()),
667670
NonVirtualAlignment(CharUnits::One()), PrimaryBase(nullptr),
668671
PrimaryBaseIsVirtual(false), HasOwnVFPtr(false),
669-
FirstNearlyEmptyVBase(nullptr) {}
672+
HasPackedField(false), FirstNearlyEmptyVBase(nullptr) {}
670673

671674
void Layout(const RecordDecl *D);
672675
void Layout(const CXXRecordDecl *D);
@@ -1847,7 +1850,6 @@ void ItaniumRecordLayoutBuilder::FinishLayout(const NamedDecl *D) {
18471850
uint64_t UnpaddedSize = getSizeInBits() - UnfilledBitsInLastUnit;
18481851
uint64_t UnpackedSizeInBits =
18491852
llvm::alignTo(getSizeInBits(), Context.toBits(UnpackedAlignment));
1850-
CharUnits UnpackedSize = Context.toCharUnitsFromBits(UnpackedSizeInBits);
18511853
uint64_t RoundedSize =
18521854
llvm::alignTo(getSizeInBits(), Context.toBits(Alignment));
18531855

@@ -1882,10 +1884,11 @@ void ItaniumRecordLayoutBuilder::FinishLayout(const NamedDecl *D) {
18821884
<< (InBits ? 1 : 0); // (byte|bit)
18831885
}
18841886

1885-
// Warn if we packed it unnecessarily. If the alignment is 1 byte don't
1886-
// bother since there won't be alignment issues.
1887-
if (Packed && UnpackedAlignment > CharUnits::One() &&
1888-
getSize() == UnpackedSize)
1887+
// Warn if we packed it unnecessarily, when the unpacked alignment is not
1888+
// greater than the one after packing, the size in bits doesn't change and
1889+
// the offset of each field is identical.
1890+
if (Packed && UnpackedAlignment <= Alignment &&
1891+
UnpackedSizeInBits == getSizeInBits() && !HasPackedField)
18891892
Diag(D->getLocation(), diag::warn_unnecessary_packed)
18901893
<< Context.getTypeDeclType(RD);
18911894
}
@@ -1977,13 +1980,10 @@ void ItaniumRecordLayoutBuilder::CheckFieldPadding(
19771980
<< Context.getTypeDeclType(D->getParent())
19781981
<< PadSize
19791982
<< (InBits ? 1 : 0); // (byte|bit)
1980-
}
1981-
1982-
// Warn if we packed it unnecessarily. If the alignment is 1 byte don't
1983-
// bother since there won't be alignment issues.
1984-
if (isPacked && UnpackedAlign > CharBitNum && Offset == UnpackedOffset)
1985-
Diag(D->getLocation(), diag::warn_unnecessary_packed)
1986-
<< D->getIdentifier();
1983+
}
1984+
if (isPacked && Offset != UnpackedOffset) {
1985+
HasPackedField = true;
1986+
}
19871987
}
19881988

19891989
static const CXXMethodDecl *computeKeyFunction(ASTContext &Context,

‎clang/test/CodeGenCXX/warn-padded-packed.cpp

Lines changed: 85 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ struct S3 {
1717
} __attribute__((packed));
1818

1919
struct S4 {
20-
int i; // expected-warning {{packed attribute is unnecessary for 'i'}}
20+
int i;
2121
char c;
2222
} __attribute__((packed));
2323

@@ -46,18 +46,18 @@ struct S8 : B {
4646
int i; // expected-warning {{padding struct 'S8' with 3 bytes to align 'i'}}
4747
};
4848

49-
struct S9 { // expected-warning {{packed attribute is unnecessary for 'S9'}}
50-
int x; // expected-warning {{packed attribute is unnecessary for 'x'}}
51-
int y; // expected-warning {{packed attribute is unnecessary for 'y'}}
49+
struct S9 {
50+
int x;
51+
int y;
5252
} __attribute__((packed));
5353

54-
struct S10 { // expected-warning {{packed attribute is unnecessary for 'S10'}}
55-
int x; // expected-warning {{packed attribute is unnecessary for 'x'}}
54+
struct S10 {
55+
int x;
5656
char a,b,c,d;
5757
} __attribute__((packed));
5858

5959

60-
struct S11 {
60+
struct S11 { // expected-warning {{packed attribute is unnecessary for 'S11'}}
6161
bool x;
6262
char a,b,c,d;
6363
} __attribute__((packed));
@@ -72,5 +72,82 @@ struct S13 { // expected-warning {{padding size of 'S13' with 6 bits to alignmen
7272
bool b : 10;
7373
};
7474

75+
struct S14 { // expected-warning {{packed attribute is unnecessary for 'S14'}}
76+
char a,b,c,d;
77+
} __attribute__((packed));
78+
79+
struct S15 { // expected-warning {{packed attribute is unnecessary for 'S15'}}
80+
struct S14 s;
81+
char a;
82+
} __attribute__((packed));
83+
84+
struct S16 { // expected-warning {{padding size of 'S16' with 2 bytes to alignment boundary}} expected-warning {{packed attribute is unnecessary for 'S16'}}
85+
char a,b;
86+
} __attribute__((packed, aligned(4)));
87+
88+
struct S17 {
89+
struct S16 s;
90+
char a,b;
91+
} __attribute__((packed, aligned(2)));
92+
93+
struct S18 { // expected-warning {{padding size of 'S18' with 2 bytes to alignment boundary}} expected-warning {{packed attribute is unnecessary for 'S18'}}
94+
struct S16 s;
95+
char a,b;
96+
} __attribute__((packed, aligned(4)));
97+
98+
struct S19 { // expected-warning {{packed attribute is unnecessary for 'S19'}}
99+
bool b;
100+
char a;
101+
} __attribute__((packed, aligned(1)));
102+
103+
struct S20 {
104+
int i;
105+
char a;
106+
} __attribute__((packed, aligned(1)));
107+
108+
struct S21 { // expected-warning {{padding size of 'S21' with 4 bits to alignment boundary}}
109+
unsigned char a : 6;
110+
unsigned char b : 6;
111+
} __attribute__((packed, aligned(1)));
112+
113+
struct S22 { // expected-warning {{packed attribute is unnecessary for 'S22'}}
114+
unsigned char a : 4;
115+
unsigned char b : 4;
116+
} __attribute__((packed));
117+
118+
struct S23 { // expected-warning {{padding size of 'S23' with 4 bits to alignment boundary}} expected-warning {{packed attribute is unnecessary for 'S23'}}
119+
unsigned char a : 2;
120+
unsigned char b : 2;
121+
} __attribute__((packed));
122+
123+
struct S24 {
124+
unsigned char a : 6;
125+
unsigned char b : 6;
126+
unsigned char c : 6;
127+
unsigned char d : 6;
128+
unsigned char e : 6;
129+
unsigned char f : 6;
130+
unsigned char g : 6;
131+
unsigned char h : 6;
132+
} __attribute__((packed));
133+
134+
struct S25 { // expected-warning {{padding size of 'S25' with 7 bits to alignment boundary}} expected-warning {{packed attribute is unnecessary for 'S25'}}
135+
unsigned char a;
136+
unsigned char b : 1;
137+
} __attribute__((packed));
138+
139+
struct S26 { // expected-warning {{packed attribute is unnecessary for 'S26'}}
140+
unsigned char a : 1;
141+
unsigned char b; //expected-warning {{padding struct 'S26' with 7 bits to align 'b'}}
142+
} __attribute__((packed));
143+
144+
struct S27 { // expected-warning {{padding size of 'S27' with 7 bits to alignment boundary}}
145+
unsigned char a : 1;
146+
unsigned char b : 8;
147+
} __attribute__((packed));
148+
149+
75150
// The warnings are emitted when the layout of the structs is computed, so we have to use them.
76-
void f(S1*, S2*, S3*, S4*, S5*, S6*, S7*, S8*, S9*, S10*, S11*, S12*, S13*) { }
151+
void f(S1*, S2*, S3*, S4*, S5*, S6*, S7*, S8*, S9*, S10*, S11*, S12*, S13*,
152+
S14*, S15*, S16*, S17*, S18*, S19*, S20*, S21*, S22*, S23*, S24*, S25*,
153+
S26*, S27*){}

0 commit comments

Comments
 (0)