summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--clang/include/clang/AST/ASTContext.h2
-rw-r--r--clang/include/clang/Basic/DiagnosticGroups.td3
-rw-r--r--clang/include/clang/Basic/DiagnosticSemaKinds.td13
-rw-r--r--clang/include/clang/Basic/SourceManager.h2
-rw-r--r--clang/lib/AST/ASTContext.cpp4
-rw-r--r--clang/lib/AST/RecordLayoutBuilder.cpp177
-rw-r--r--clang/test/CodeGenCXX/warn-padded-packed.cpp76
7 files changed, 254 insertions, 23 deletions
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index ae4ee946fe2..b1369a47572 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -316,6 +316,8 @@ public:
const LangOptions& getLangOptions() const { return LangOpts; }
+ Diagnostic &getDiagnostics() const;
+
FullSourceLoc getFullLoc(SourceLocation Loc) const {
return FullSourceLoc(Loc,SourceMgr);
}
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index fa7c5cfc91e..c7585e1ee0a 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -84,7 +84,8 @@ def OutOfLineDeclaration : DiagGroup<"out-of-line-declaration">;
def : DiagGroup<"overflow">;
def OverlengthStrings : DiagGroup<"overlength-strings">;
def : DiagGroup<"overloaded-virtual">;
-def : DiagGroup<"packed">;
+def Packed : DiagGroup<"packed">;
+def Padded : DiagGroup<"padded">;
def PointerArith : DiagGroup<"pointer-arith">;
def PoundWarning : DiagGroup<"#warnings">,
DiagCategory<"#warning Directive">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 5cb242f9ca5..0927de34da4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1825,6 +1825,19 @@ def err_vm_func_decl : Error<
def err_array_too_large : Error<
"array is too large (%0 elements)">;
+// -Wpadded, -Wpacked
+def warn_padded_struct_field : Warning<
+ "padding %select{struct|class}0 %1 with %2 %select{byte|bit}3%select{|s}4 "
+ "to align %5">, InGroup<Padded>, DefaultIgnore;
+def warn_padded_struct_anon_field : Warning<
+ "padding %select{struct|class}0 %1 with %2 %select{byte|bit}3%select{|s}4 "
+ "to align anonymous bit-field">, InGroup<Padded>, DefaultIgnore;
+def warn_padded_struct_size : Warning<
+ "padding size of %0 with %1 %select{byte|bit}2%select{|s}3 "
+ "to alignment boundary">, InGroup<Padded>, DefaultIgnore;
+def warn_unnecessary_packed : Warning<
+ "packed attribute is unnecessary for %0">, InGroup<Packed>, DefaultIgnore;
+
def err_typecheck_negative_array_size : Error<"array size is negative">;
def warn_typecheck_function_qualifiers : Warning<
"qualifier on function type %0 has unspecified behavior">;
diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h
index 7a66117f207..fd4f3f4f7a7 100644
--- a/clang/include/clang/Basic/SourceManager.h
+++ b/clang/include/clang/Basic/SourceManager.h
@@ -436,6 +436,8 @@ public:
void clearIDTables();
+ Diagnostic &getDiagnostics() const { return Diag; }
+
//===--------------------------------------------------------------------===//
// MainFileID creation and querying methods.
//===--------------------------------------------------------------------===//
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 4591a0f3c55..4b4c56502b2 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -369,6 +369,10 @@ void ASTContext::InitBuiltinTypes() {
InitBuiltinType(NullPtrTy, BuiltinType::NullPtr);
}
+Diagnostic &ASTContext::getDiagnostics() const {
+ return SourceMgr.getDiagnostics();
+}
+
AttrVec& ASTContext::getDeclAttrs(const Decl *D) {
AttrVec *&Result = DeclAttrs[D];
if (!Result) {
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index 13fae299d87..0a095886ff9 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -14,6 +14,7 @@
#include "clang/AST/Expr.h"
#include "clang/AST/RecordLayout.h"
#include "clang/Basic/TargetInfo.h"
+#include "clang/Sema/SemaDiagnostic.h"
#include "llvm/Support/Format.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/Support/MathExtras.h"
@@ -529,6 +530,9 @@ protected:
/// Alignment - The current alignment of the record layout.
unsigned Alignment;
+ /// \brief The alignment if attribute packed is not used.
+ unsigned UnpackedAlignment;
+
llvm::SmallVector<uint64_t, 16> FieldOffsets;
/// Packed - Whether the record is packed or not.
@@ -583,9 +587,9 @@ protected:
RecordLayoutBuilder(ASTContext &Context, EmptySubobjectMap *EmptySubobjects)
: Context(Context), EmptySubobjects(EmptySubobjects), Size(0), Alignment(8),
- Packed(false), IsUnion(false), IsMac68kAlign(false),
- UnfilledBitsInLastByte(0), MaxFieldAlignment(0), DataSize(0),
- NonVirtualSize(0), NonVirtualAlignment(8), PrimaryBase(0),
+ UnpackedAlignment(Alignment), Packed(false), IsUnion(false),
+ IsMac68kAlign(false), UnfilledBitsInLastByte(0), MaxFieldAlignment(0),
+ DataSize(0), NonVirtualSize(0), NonVirtualAlignment(8), PrimaryBase(0),
PrimaryBaseIsVirtual(false), FirstNearlyEmptyVBase(0) { }
void Layout(const RecordDecl *D);
@@ -594,7 +598,8 @@ protected:
void LayoutFields(const RecordDecl *D);
void LayoutField(const FieldDecl *D);
- void LayoutWideBitField(uint64_t FieldSize, uint64_t TypeSize);
+ void LayoutWideBitField(uint64_t FieldSize, uint64_t TypeSize,
+ bool FieldPacked, const FieldDecl *D);
void LayoutBitField(const FieldDecl *D);
/// BaseSubobjectInfoAllocator - Allocator for BaseSubobjectInfo objects.
@@ -661,9 +666,18 @@ protected:
/// FinishLayout - Finalize record layout. Adjust record size based on the
/// alignment.
- void FinishLayout();
+ void FinishLayout(const NamedDecl *D);
+
+ void UpdateAlignment(unsigned NewAlignment, unsigned UnpackedNewAlignment);
+ void UpdateAlignment(unsigned NewAlignment) {
+ UpdateAlignment(NewAlignment, NewAlignment);
+ }
+
+ void CheckFieldPadding(uint64_t Offset, uint64_t UnpaddedOffset,
+ uint64_t UnpackedOffset, unsigned UnpackedAlign,
+ bool isPacked, const FieldDecl *D);
- void UpdateAlignment(unsigned NewAlignment);
+ DiagnosticBuilder Diag(SourceLocation Loc, unsigned DiagID);
RecordLayoutBuilder(const RecordLayoutBuilder&); // DO NOT IMPLEMENT
void operator=(const RecordLayoutBuilder&); // DO NOT IMPLEMENT
@@ -1146,7 +1160,7 @@ void RecordLayoutBuilder::Layout(const RecordDecl *D) {
// Finally, round the size of the total struct up to the alignment of the
// struct itself.
- FinishLayout();
+ FinishLayout(D);
}
void RecordLayoutBuilder::Layout(const CXXRecordDecl *RD) {
@@ -1167,7 +1181,7 @@ void RecordLayoutBuilder::Layout(const CXXRecordDecl *RD) {
// Finally, round the size of the total struct up to the alignment of the
// struct itself.
- FinishLayout();
+ FinishLayout(RD);
#ifndef NDEBUG
// Check that we have base offsets for all bases.
@@ -1215,7 +1229,7 @@ void RecordLayoutBuilder::Layout(const ObjCInterfaceDecl *D) {
// Finally, round the size of the total struct up to the alignment of the
// struct itself.
- FinishLayout();
+ FinishLayout(D);
}
void RecordLayoutBuilder::LayoutFields(const RecordDecl *D) {
@@ -1227,7 +1241,9 @@ void RecordLayoutBuilder::LayoutFields(const RecordDecl *D) {
}
void RecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize,
- uint64_t TypeSize) {
+ uint64_t TypeSize,
+ bool FieldPacked,
+ const FieldDecl *D) {
assert(Context.getLangOptions().CPlusPlus &&
"Can only have wide bit-fields in C++!");
@@ -1258,6 +1274,7 @@ void RecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize,
UnfilledBitsInLastByte = 0;
uint64_t FieldOffset;
+ uint64_t UnpaddedFieldOffset = DataSize - UnfilledBitsInLastByte;
if (IsUnion) {
DataSize = std::max(DataSize, FieldSize);
@@ -1276,6 +1293,9 @@ void RecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize,
// Place this field at the current location.
FieldOffsets.push_back(FieldOffset);
+ CheckFieldPadding(FieldOffset, UnpaddedFieldOffset, FieldOffset,
+ TypeAlign, FieldPacked, D);
+
// Update the size.
Size = std::max(Size, DataSize);
@@ -1285,7 +1305,8 @@ void RecordLayoutBuilder::LayoutWideBitField(uint64_t FieldSize,
void RecordLayoutBuilder::LayoutBitField(const FieldDecl *D) {
bool FieldPacked = Packed || D->hasAttr<PackedAttr>();
- uint64_t FieldOffset = IsUnion ? 0 : (DataSize - UnfilledBitsInLastByte);
+ uint64_t UnpaddedFieldOffset = DataSize - UnfilledBitsInLastByte;
+ uint64_t FieldOffset = IsUnion ? 0 : UnpaddedFieldOffset;
uint64_t FieldSize = D->getBitWidth()->EvaluateAsInt(Context).getZExtValue();
std::pair<uint64_t, unsigned> FieldInfo = Context.getTypeInfo(D->getType());
@@ -1293,29 +1314,47 @@ void RecordLayoutBuilder::LayoutBitField(const FieldDecl *D) {
unsigned FieldAlign = FieldInfo.second;
if (FieldSize > TypeSize) {
- LayoutWideBitField(FieldSize, TypeSize);
+ LayoutWideBitField(FieldSize, TypeSize, FieldPacked, D);
return;
}
+ // The align if the field is not packed. This is to check if the attribute
+ // was unnecessary (-Wpacked).
+ unsigned UnpackedFieldAlign = FieldAlign;
+ uint64_t UnpackedFieldOffset = FieldOffset;
+ if (!Context.Target.useBitFieldTypeAlignment())
+ UnpackedFieldAlign = 1;
+
if (FieldPacked || !Context.Target.useBitFieldTypeAlignment())
FieldAlign = 1;
FieldAlign = std::max(FieldAlign, D->getMaxAlignment());
+ UnpackedFieldAlign = std::max(UnpackedFieldAlign, D->getMaxAlignment());
// The maximum field alignment overrides the aligned attribute.
- if (MaxFieldAlignment)
+ if (MaxFieldAlignment) {
FieldAlign = std::min(FieldAlign, MaxFieldAlignment);
+ UnpackedFieldAlign = std::min(UnpackedFieldAlign, MaxFieldAlignment);
+ }
// Check if we need to add padding to give the field the correct alignment.
if (FieldSize == 0 || (FieldOffset & (FieldAlign-1)) + FieldSize > TypeSize)
FieldOffset = llvm::RoundUpToAlignment(FieldOffset, FieldAlign);
+ if (FieldSize == 0 ||
+ (UnpackedFieldOffset & (UnpackedFieldAlign-1)) + FieldSize > TypeSize)
+ UnpackedFieldOffset = llvm::RoundUpToAlignment(UnpackedFieldOffset,
+ UnpackedFieldAlign);
+
// Padding members don't affect overall alignment.
if (!D->getIdentifier())
- FieldAlign = 1;
+ FieldAlign = UnpackedFieldAlign = 1;
// Place this field at the current location.
FieldOffsets.push_back(FieldOffset);
+ CheckFieldPadding(FieldOffset, UnpaddedFieldOffset, UnpackedFieldOffset,
+ UnpackedFieldAlign, FieldPacked, D);
+
// Update DataSize to include the last byte containing (part of) the bitfield.
if (IsUnion) {
// FIXME: I think FieldSize should be TypeSize here.
@@ -1331,7 +1370,7 @@ void RecordLayoutBuilder::LayoutBitField(const FieldDecl *D) {
Size = std::max(Size, DataSize);
// Remember max struct/class alignment.
- UpdateAlignment(FieldAlign);
+ UpdateAlignment(FieldAlign, UnpackedFieldAlign);
}
void RecordLayoutBuilder::LayoutField(const FieldDecl *D) {
@@ -1340,6 +1379,8 @@ void RecordLayoutBuilder::LayoutField(const FieldDecl *D) {
return;
}
+ uint64_t UnpaddedFieldOffset = DataSize - UnfilledBitsInLastByte;
+
// Reset the unfilled bits.
UnfilledBitsInLastByte = 0;
@@ -1366,16 +1407,26 @@ void RecordLayoutBuilder::LayoutField(const FieldDecl *D) {
FieldAlign = FieldInfo.second;
}
+ // The align if the field is not packed. This is to check if the attribute
+ // was unnecessary (-Wpacked).
+ unsigned UnpackedFieldAlign = FieldAlign;
+ uint64_t UnpackedFieldOffset = FieldOffset;
+
if (FieldPacked)
FieldAlign = 8;
FieldAlign = std::max(FieldAlign, D->getMaxAlignment());
+ UnpackedFieldAlign = std::max(UnpackedFieldAlign, D->getMaxAlignment());
// The maximum field alignment overrides the aligned attribute.
- if (MaxFieldAlignment)
+ if (MaxFieldAlignment) {
FieldAlign = std::min(FieldAlign, MaxFieldAlignment);
+ UnpackedFieldAlign = std::min(UnpackedFieldAlign, MaxFieldAlignment);
+ }
// Round up the current record size to the field's alignment boundary.
FieldOffset = llvm::RoundUpToAlignment(FieldOffset, FieldAlign);
+ UnpackedFieldOffset = llvm::RoundUpToAlignment(UnpackedFieldOffset,
+ UnpackedFieldAlign);
if (!IsUnion && EmptySubobjects) {
// Check if we can place the field at this offset.
@@ -1388,6 +1439,9 @@ void RecordLayoutBuilder::LayoutField(const FieldDecl *D) {
// Place this field at the current location.
FieldOffsets.push_back(FieldOffset);
+ CheckFieldPadding(FieldOffset, UnpaddedFieldOffset, UnpackedFieldOffset,
+ UnpackedFieldAlign, FieldPacked, D);
+
// Reserve space for this field.
if (IsUnion)
Size = std::max(Size, FieldSize);
@@ -1398,29 +1452,102 @@ void RecordLayoutBuilder::LayoutField(const FieldDecl *D) {
DataSize = Size;
// Remember max struct/class alignment.
- UpdateAlignment(FieldAlign);
+ UpdateAlignment(FieldAlign, UnpackedFieldAlign);
}
-void RecordLayoutBuilder::FinishLayout() {
+void RecordLayoutBuilder::FinishLayout(const NamedDecl *D) {
// In C++, records cannot be of size 0.
if (Context.getLangOptions().CPlusPlus && Size == 0)
Size = 8;
// Finally, round the size of the record up to the alignment of the
// record itself.
+ uint64_t UnpaddedSize = Size - UnfilledBitsInLastByte;
+ uint64_t UnpackedSize = llvm::RoundUpToAlignment(Size, UnpackedAlignment);
Size = llvm::RoundUpToAlignment(Size, Alignment);
+
+ unsigned CharBitNum = Context.Target.getCharWidth();
+ if (const RecordDecl *RD = dyn_cast<RecordDecl>(D)) {
+ // Warn if padding was introduced to the struct/class/union.
+ if (Size > UnpaddedSize) {
+ unsigned PadSize = Size - UnpaddedSize;
+ bool InBits = true;
+ if (PadSize % CharBitNum == 0) {
+ PadSize = PadSize / CharBitNum;
+ InBits = false;
+ }
+ Diag(RD->getLocation(), diag::warn_padded_struct_size)
+ << Context.getTypeDeclType(RD)
+ << PadSize
+ << (InBits ? 1 : 0) /*(byte|bit)*/ << (PadSize > 1); // plural or not
+ }
+
+ // Warn if we packed it unnecessarily. If the alignment is 1 byte don't
+ // bother since there won't be alignment issues.
+ if (Packed && UnpackedAlignment > CharBitNum && Size == UnpackedSize)
+ Diag(D->getLocation(), diag::warn_unnecessary_packed)
+ << Context.getTypeDeclType(RD);
+ }
}
-void RecordLayoutBuilder::UpdateAlignment(unsigned NewAlignment) {
+void RecordLayoutBuilder::UpdateAlignment(unsigned NewAlignment,
+ unsigned UnpackedNewAlignment) {
// The alignment is not modified when using 'mac68k' alignment.
if (IsMac68kAlign)
return;
- if (NewAlignment <= Alignment)
+ if (NewAlignment > Alignment) {
+ assert(llvm::isPowerOf2_32(NewAlignment && "Alignment not a power of 2"));
+ Alignment = NewAlignment;
+ }
+
+ if (UnpackedNewAlignment > UnpackedAlignment) {
+ assert(llvm::isPowerOf2_32(UnpackedNewAlignment &&
+ "Alignment not a power of 2"));
+ UnpackedAlignment = UnpackedNewAlignment;
+ }
+}
+
+void RecordLayoutBuilder::CheckFieldPadding(uint64_t Offset,
+ uint64_t UnpaddedOffset,
+ uint64_t UnpackedOffset,
+ unsigned UnpackedAlign,
+ bool isPacked,
+ const FieldDecl *D) {
+ // We let objc ivars without warning, objc interfaces generally are not used
+ // for padding tricks.
+ if (isa<ObjCIvarDecl>(D))
return;
- assert(llvm::isPowerOf2_32(NewAlignment && "Alignment not a power of 2"));
+ unsigned CharBitNum = Context.Target.getCharWidth();
- Alignment = NewAlignment;
+ // Warn if padding was introduced to the struct/class.
+ if (!IsUnion && Offset > UnpaddedOffset) {
+ unsigned PadSize = Offset - UnpaddedOffset;
+ bool InBits = true;
+ if (PadSize % CharBitNum == 0) {
+ PadSize = PadSize / CharBitNum;
+ InBits = false;
+ }
+ if (D->getIdentifier())
+ Diag(D->getLocation(), diag::warn_padded_struct_field)
+ << (D->getParent()->isStruct() ? 0 : 1) // struct|class
+ << Context.getTypeDeclType(D->getParent())
+ << PadSize
+ << (InBits ? 1 : 0) /*(byte|bit)*/ << (PadSize > 1) // plural or not
+ << D->getIdentifier();
+ else
+ Diag(D->getLocation(), diag::warn_padded_struct_anon_field)
+ << (D->getParent()->isStruct() ? 0 : 1) // struct|class
+ << Context.getTypeDeclType(D->getParent())
+ << PadSize
+ << (InBits ? 1 : 0) /*(byte|bit)*/ << (PadSize > 1); // plural or not
+ }
+
+ // Warn if we packed it unnecessarily. If the alignment is 1 byte don't
+ // bother since there won't be alignment issues.
+ if (isPacked && UnpackedAlign > CharBitNum && Offset == UnpackedOffset)
+ Diag(D->getLocation(), diag::warn_unnecessary_packed)
+ << D->getIdentifier();
}
const CXXMethodDecl *
@@ -1463,6 +1590,12 @@ RecordLayoutBuilder::ComputeKeyFunction(const CXXRecordDecl *RD) {
return 0;
}
+DiagnosticBuilder
+RecordLayoutBuilder::Diag(SourceLocation Loc, unsigned DiagID) {
+ return Context.getDiagnostics().Report(
+ FullSourceLoc(Loc, Context.getSourceManager()), DiagID);
+}
+
// This class implements layout specific to the Microsoft ABI.
class MSRecordLayoutBuilder: public RecordLayoutBuilder {
public:
diff --git a/clang/test/CodeGenCXX/warn-padded-packed.cpp b/clang/test/CodeGenCXX/warn-padded-packed.cpp
new file mode 100644
index 00000000000..034b02588e6
--- /dev/null
+++ b/clang/test/CodeGenCXX/warn-padded-packed.cpp
@@ -0,0 +1,76 @@
+// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify %s -emit-llvm -o %t
+
+struct S1 {
+ char c;
+ short s; // expected-warning {{padding struct 'S1' with 1 byte to align 's'}}
+ long l; // expected-warning {{padding struct 'S1' with 4 bytes to align 'l'}}
+};
+
+struct S2 { // expected-warning {{padding size of 'S2' with 3 bytes to alignment boundary}}
+ int i;
+ char c;
+};
+
+struct S3 {
+ char c;
+ int i;
+} __attribute__((packed));
+
+struct S4 {
+ int i; // expected-warning {{packed attribute is unnecessary for 'i'}}
+ char c;
+} __attribute__((packed));
+
+struct S5 {
+ char c;
+ union {
+ char c;
+ int i;
+ } u; // expected-warning {{padding struct 'S5' with 3 bytes to align 'u'}}
+};
+
+struct S6 { // expected-warning {{padding size of 'S6' with 30 bits to alignment boundary}}
+ int i : 2;
+};
+
+struct S7 { // expected-warning {{padding size of 'S7' with 7 bytes to alignment boundary}}
+ char c;
+ virtual void m();
+};
+
+struct B {
+ char c;
+};
+
+struct S8 : B {
+ int i; // expected-warning {{padding struct 'S8' with 3 bytes to align 'i'}}
+};
+
+struct S9 { // expected-warning {{packed attribute is unnecessary for 'S9'}}
+ int x; // expected-warning {{packed attribute is unnecessary for 'x'}}
+ int y; // expected-warning {{packed attribute is unnecessary for 'y'}}
+} __attribute__((packed));
+
+struct S10 { // expected-warning {{packed attribute is unnecessary for 'S10'}}
+ int x; // expected-warning {{packed attribute is unnecessary for 'x'}}
+ char a,b,c,d;
+} __attribute__((packed));
+
+
+struct S11 {
+ bool x;
+ char a,b,c,d;
+} __attribute__((packed));
+
+struct S12 {
+ bool b : 1;
+ char c; // expected-warning {{padding struct 'S12' with 7 bits to align 'c'}}
+};
+
+struct S13 { // expected-warning {{padding size of 'S13' with 6 bits to alignment boundary}}
+ char c;
+ bool b : 10; // expected-warning {{size of bit-field 'b' (10 bits) exceeds the size of its type}}
+};
+
+// The warnings are emitted when the layout of the structs is computed, so we have to use them.
+void f(S1*, S2*, S3*, S4*, S5*, S6*, S7*, S8*, S9*, S10*, S11*, S12*, S13*) { }
OpenPOWER on IntegriCloud