[WIP] Binding of references to packed fields

[rogfer01]

Summary

This is a WIP for PR28571.

As suggested by @rsmith it introduces a new ObjectKind for PackedField's modeled similarly in spirit to BitField's. If the reference is const, the binding is done using a temporary otherwise an error is diagnosed.

[rogfer01]

lib/Sema/SemaInit.cpp

4272–4274

Rereading this makes no sense to me now. I'll adress this in a later update.

[aaron.ballman]

include/clang/AST/Expr.h

436

s/gl-value/glvalue

438

"that the field has an effectively-reduced alignment"

440

Like bitfields
glvalues

include/clang/AST/Stmt.h

135

This is a bit unfortunate (not that I see a way around it)...

lib/AST/ASTDumper.cpp

1892

packedfield instead?

lib/AST/Decl.cpp

37

This should be removed.

lib/Sema/SemaExprMember.cpp

1790

Formatting of the brace is incorrect.

lib/Sema/SemaInit.cpp

4258

I don't know whether you should use auto here or not, the type is kinda-sorta spelled out in the initialization. At the very least, this should be const auto *.

[rsmith]

include/clang/AST/Expr.h

441

value-kind -> object kind.

include/clang/Basic/Specifiers.h

119–140

Wait a second, how did this fit into 2 bits before your change?!

lib/AST/Decl.cpp

3590

Does this properly handle anonymous struct/union members:

struct A __attribute__((packed)) {
  char a;
  union { int b; };
} a;
int &r = a.b;

lib/CodeGen/CGCall.cpp

3278 ↗

(On Diff #67360)

This looks wrong: we shouldn't be emitting reference bindings to packed fields. We should have rejected them in Sema.

lib/Sema/SemaCast.cpp

1922

Might be worth adding a comment here, "GCC allows a packed field to be explicitly cast to a reference type as a way of removing the 'packed' taint" or similar.

lib/Sema/SemaExprMember.cpp

1782–1783

If the BaseExpr is an OK_PackedField then the resulting OK for the field should also be an OK_PackedField:

struct Q { int k; };
struct __attribute__((packed)) A { Q k; } a;
int &r = a.k.k; // error, binding reference to packed field

You should handle this here...

1793

... not here. This check does the wrong thing for the IsArrow case:

struct Q { int k; };
struct __attribute__((packed)) A { Q *k; } a;
int &r = a.k->k; // should be valid, not a packed field

lib/Sema/SemaInit.cpp

4256

You don't need to special-case packed fields here. They just happen to be the only non-addressable object kind we have right now that can be a class type. If we had others, they should get this treatment too, so it seems better to remove the check.

4256

Why are you treating lvalue references specially here? GCC doesn't seem to, and it's not obvious to me why rvalue references should get special treatment.

4272–4274

Perhaps replace this entire comment with "If the class doesn't have a trivial copy constructor, we can't create a copy of it for the reference binding because doing so would bind it to a reference in the class's own copy/move constructor, so just give up and allow the error to be diagnosed."

test/SemaCXX/bind-packed-member.cpp

69–74

Another interesting case:

struct __attribute__((packed)) G {
  char c;
  NonTrivialCopy x;
};

Here, GCC ignores the packed attribute entirely (with a warning). For ABI compatibility, we should do the same.

[rogfer01]

include/clang/Basic/Specifiers.h

119–140

It didn't.

I was also really confused when some unrelated Obj-C tests started failing after this change.

[rogfer01]

I am retaking this, will upload an updated patch soon.

lib/AST/Decl.cpp

3590

This test (modulo swapping A and __attribute__((packed)) positions) works. Do you think we may be missing some case here?

test/SemaCXX/bind-packed-member.cpp

69–74

Will do this in a later update of this patch.

I still have to check what is the best place to check this, though I think once the class-specifier has been completed.

[rogfer01]

Changes:

• Rebase patch to trunk
• Fixes as pointed out by reviewers

TODO:

• Diagnose and drop the packed attribute of a class with a nontrivially constructed data member.

[rsmith]

include/clang/AST/Expr.h

440

"Likewise bitfields" -> "Like bitfields" still needs to be done.

Also, "packed-fields" -> "packed fields" throughout this comment.

lib/AST/Decl.cpp

3590

No, I think this is OK, so long as we have a unit test for it. I assume that what happens here is that the implicit a.<anonymous union object> produces a packed lvalue, and then the .b just preserves the packedness -- that is, the anonymous union object is a packed field, but b is not?

lib/Sema/SemaExprMember.cpp

1782–1783

I think the existing code here is checking for a condition that can't happen. Do any tests fail if you replace this if/else block with just VK = BaseExpr->getValueKind();?

lib/Sema/SemaInit.cpp

4256

You don't seem to have addressed this comment: you can delete the refersToPackedField check here; the check for a class type is sufficient.

[rogfer01]

Drop attribute packed of a class when one of its fields is a non-packed non-pod for compatibility with GCC.

[rogfer01]

Rebase to current trunk

[aaron.ballman]

include/clang/AST/Decl.h

2388

Should be &Context instead of & context.

include/clang/AST/Expr.h

412

bit-field instead of bitfield (we're starting to standardize on the spelling used by the standards).

432

typo: bit-field.

440

s/bitfields/bit-fields

include/clang/Basic/Specifiers.h

126

Remove the hyphen from packed-field.

lib/Sema/SemaInit.cpp

4459

Move the brace to the preceding line.

[rogfer01]

Minor issues addressed

include/clang/AST/Decl.h

public:
   bool isMutable() const { return Mutable; }
 
   /// \brief Determines whether this field is a bitfield.
   bool isBitField() const {
     return InitStorage.getInt() == ISK_BitWidthOrNothing &&
            InitStorage.getPointer() != nullptr;
   }
 
+  /// \brief Determines whether this field is actually packed, i.e.
+  /// its alignment is smaller than the alignment of its field type.
+  bool isPackedField(const ASTContext &Context) const;
+
   /// @brief Determines whether this is an unnamed bitfield.
   bool isUnnamedBitfield() const { return isBitField() && !getDeclName(); }
 
   /// isAnonymousStructOrUnion - Determines whether this field is a
   /// representative for an anonymous struct or union. Such fields are
   /// unnamed and are implicitly generated by the implementation to
   /// store the data for the anonymous union or struct.
   bool isAnonymousStructOrUnion() const;

include/clang/AST/Expr.h

public:
 
   /// getObjectKind - The object kind that this expression produces.
   /// Object kinds are meaningful only for expressions that yield an
   /// l-value or x-value.
   ExprObjectKind getObjectKind() const {
     return static_cast<ExprObjectKind>(ExprBits.ObjectKind);
   }
 
+  /// States that this object is ordinary, packed field or bit-field
   bool isOrdinaryOrBitFieldObject() const {
     ExprObjectKind OK = getObjectKind();
-    return (OK == OK_Ordinary || OK == OK_BitField);
+    return (OK == OK_Ordinary || OK == OK_PackedField || OK == OK_BitField);
   }
 
   /// setValueKind - Set the value kind produced by this expression.
   void setValueKind(ExprValueKind Cat) { ExprBits.ValueKind = Cat; }
 
   /// setObjectKind - Set the object kind produced by this expression.
   void setObjectKind(ExprObjectKind Cat) { ExprBits.ObjectKind = Cat; }
 
 private:
   Classification ClassifyImpl(ASTContext &Ctx, SourceLocation *Loc) const;
 
 public:
 
-  /// \brief Returns true if this expression is a gl-value that
+  /// \brief Returns true if this expression is a glvalue that
   /// potentially refers to a bit-field.
   ///
-  /// In C++, whether a gl-value refers to a bitfield is essentially
+  /// In C++, whether a glvalue refers to a bit-field is essentially
   /// an aspect of the value-kind type system.
   bool refersToBitField() const { return getObjectKind() == OK_BitField; }
 
+  /// \brief Returns true if this expression is a glvalue that
+  /// potentially refers to a packed field. Here packed field means
+  /// that the field has an effectively-reduced alignment.
+  ///
+  /// Like bit-fields, we model glvalues referring to packed fields as
+  /// an aspect of the object kind type system.
+  bool refersToPackedField() const { return getObjectKind() == OK_PackedField; }
+
   /// \brief If this expression refers to a bit-field, retrieve the
   /// declaration of that bit-field.
   ///
   /// Note that this returns a non-null pointer in subtly different
   /// places than refersToBitField returns true.  In particular, this can
   /// return a non-null pointer even for r-values loaded from
   /// bit-fields, but it will return null for a conditional bit-field.
   FieldDecl *getSourceBitField();

include/clang/AST/Stmt.h

class ExprBitfields {
     friend class CXXDependentScopeMemberExpr; // ctor
     friend class OverloadExpr; // ctor
     friend class PseudoObjectExpr; // ctor
     friend class AtomicExpr; // ctor
     friend class OpaqueValueExpr; // ctor
     unsigned : NumStmtBits;
 
     unsigned ValueKind : 2;
-    unsigned ObjectKind : 2;
+    unsigned ObjectKind : 3;
     unsigned TypeDependent : 1;
     unsigned ValueDependent : 1;
     unsigned InstantiationDependent : 1;
     unsigned ContainsUnexpandedParameterPack : 1;
   };
-  enum { NumExprBits = 16 };
+  enum { NumExprBits = 17 };
 
   class CharacterLiteralBitfields {
     friend class CharacterLiteral;
     unsigned : NumExprBits;
 
     unsigned Kind : 3;
   };
 

include/clang/Basic/DiagnosticSemaKinds.td

 def err_member_function_call_bad_cvr : Error<"member function %0 not viable: "
     "'this' argument has type %1, but function is not marked "
     "%select{const|restrict|const or restrict|volatile|const or volatile|"
     "volatile or restrict|const, volatile, or restrict}2">;
 
 def err_reference_bind_to_bitfield : Error<
   "%select{non-const|volatile}0 reference cannot bind to "
   "bit-field%select{| %1}2">;
+def err_reference_bind_to_packed_field : Error<
+  "reference cannot bind to packed field">;
+def warn_ignoring_packed_due_to_nonpod_data_member : Warning<
+  "ignoring packed attribute because of unpacked non-POD field of type %0">,
+   InGroup<DiagGroup<"attribute-packed-for-nonpod-field">>;
 def err_reference_bind_to_vector_element : Error<
   "%select{non-const|volatile}0 reference cannot bind to vector element">;
 def err_reference_var_requires_init : Error<
   "declaration of reference variable %0 requires an initializer">;
 def err_reference_without_init : Error<
   "reference to type %0 requires an initializer">;
 def note_value_initialization_here : Note<
   "in value-initialization of type %0 here">;

include/clang/Basic/Specifiers.h

enum ExprValueKind {
     /// \brief An x-value expression is a reference to an object with
     /// independent storage but which can be "moved", i.e.
     /// efficiently cannibalized for its resources.
     VK_XValue
   };
 
   /// \brief A further classification of the kind of object referenced by an
   /// l-value or x-value.
   enum ExprObjectKind {
     /// An ordinary object is located at an address in memory.
     OK_Ordinary,
 
     /// A bitfield object is a bitfield on a C or C++ record.
     OK_BitField,
 
+    /// A packedfield is a field on a C or C++ packed record.
+    OK_PackedField,
+
     /// A vector component is an element or range of elements on a vector.
     OK_VectorComponent,
 
     /// An Objective-C property is a logical field of an Objective-C
     /// object which is read and written via Objective-C method calls.
     OK_ObjCProperty,
     
     /// An Objective-C array/dictionary subscripting which reads an
     /// object or writes at the subscripted array/dictionary element via
     /// Objective-C method calls.
     OK_ObjCSubscript
   };
 
   /// \brief Describes the kind of template specialization that a
   /// particular template specialization declaration represents.
   enum TemplateSpecializationKind {
     /// This template specialization was formed from a template-id but
     /// has not yet been declared, defined, or instantiated.
     TSK_Undeclared = 0,
     /// This template specialization was implicitly instantiated from a

include/clang/Sema/Initialization.h

enum FailureKind {
     /// \brief Cannot resolve the address of an overloaded function.
     FK_AddressOfOverloadFailed,
     /// \brief Overloading due to reference initialization failed.
     FK_ReferenceInitOverloadFailed,
     /// \brief Non-const lvalue reference binding to a temporary.
     FK_NonConstLValueReferenceBindingToTemporary,
     /// \brief Non-const lvalue reference binding to a bit-field.
     FK_NonConstLValueReferenceBindingToBitfield,
+    /// \brief Non-const lvalue reference binding to a packed field.
+    FK_NonConstLValueReferenceBindingToPackedField,
     /// \brief Non-const lvalue reference binding to a vector element.
     FK_NonConstLValueReferenceBindingToVectorElement,
     /// \brief Non-const lvalue reference binding to an lvalue of unrelated
     /// type.
     FK_NonConstLValueReferenceBindingToUnrelated,
     /// \brief Rvalue reference binding to an lvalue.
     FK_RValueReferenceBindingToLValue,
     /// \brief Reference binding drops qualifiers.

lib/AST/ASTDumper.cpp

dumpType(Node->getType());
     }
   }
 
   {
     ColorScope Color(*this, ObjectKindColor);
     switch (Node->getObjectKind()) {
     case OK_Ordinary:
       break;
+    case OK_PackedField:
+      OS << " packedfield";
+      break;
     case OK_BitField:
       OS << " bitfield";
       break;
     case OK_ObjCProperty:
       OS << " objcproperty";
       break;
     case OK_ObjCSubscript:
       OS << " objcsubscript";

lib/AST/Decl.cpp

 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/Specifiers.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <algorithm>
 
 using namespace clang;
 
 Decl *clang::getPrimaryMergedDecl(Decl *D) {
   return D->getASTContext().getPrimaryMergedDecl(D);
 }
 
 // Defined here so that it can be inlined into its direct callers.
 bool Decl::isOutOfLine() const {
   return !getLexicalDeclContext()->Equals(getDeclContext());
assert((getParent()->isLambda() || getParent()->isCapturedRecord()) &&
          "capturing type in non-lambda or captured record.");
   assert(InitStorage.getInt() == ISK_BitWidthOrNothing &&
          InitStorage.getPointer() == nullptr &&
          "bit width, initializer or captured type already set");
   InitStorage.setPointerAndInt(const_cast<VariableArrayType *>(VLAType),
                                ISK_CapturedVLAType);
 }
 
+bool FieldDecl::isPackedField(const ASTContext &Context) const {
+  return !isBitField() &&
+         (this->hasAttr<PackedAttr>() || getParent()->hasAttr<PackedAttr>()) &&
+         Context.getDeclAlign(this) <
+             Context.getTypeAlignInChars(this->getType());
+}
+
 //===----------------------------------------------------------------------===//
 // TagDecl Implementation
 //===----------------------------------------------------------------------===//
 
 SourceLocation TagDecl::getOuterLocStart() const {
   return getTemplateOrInnerLocStart(this);
 }
 

lib/Sema/SemaCast.cpp

if (const ReferenceType *DestTypeTmp = DestType->getAs<ReferenceType>()) {
 
     // C++ 5.2.10p10: [...] a reference cast reinterpret_cast<T&>(x) has the
     //   same effect as the conversion *reinterpret_cast<T*>(&x) with the
     //   built-in & and * operators.
 
     const char *inappropriate = nullptr;
     switch (SrcExpr.get()->getObjectKind()) {
     case OK_Ordinary:
+    case OK_PackedField:
+      // GCC allows binding a reference to a packed field by means of a cast
+      // to a reference type.
       break;
     case OK_BitField:
       msg = diag::err_bad_cxx_cast_bitfield;
       return TC_NotApplicable;
       // FIXME: Use a specific diagnostic for the rest of these cases.
     case OK_VectorComponent: inappropriate = "vector element";      break;
     case OK_ObjCProperty:    inappropriate = "property expression"; break;
     case OK_ObjCSubscript:   inappropriate = "container subscripting expression"; 

lib/Sema/SemaDecl.cpp

if (FDTy->isFunctionType()) {
           Record->setHasObjectMember(true);
         else if (BaseType->isObjCObjectPointerType() ||
                  BaseType.isObjCGCStrong())
                Record->setHasObjectMember(true);
       }
     }
     if (Record && FD->getType().isVolatileQualified())
       Record->setHasVolatileMember(true);
+    if (!FDTy->isDependentType() && Record && !Record->isDependentType() &&
+        Record->hasAttr<PackedAttr>() && !FD->getType().isPODType(Context) &&
+        !((FD->getType()->getAsCXXRecordDecl() &&
+           FD->getType()->getAsCXXRecordDecl()->hasAttr<PackedAttr>()) ||
+          FD->hasAttr<PackedAttr>())) {
+      Diag(FD->getLocation(),
+           diag::warn_ignoring_packed_due_to_nonpod_data_member)
+          << FD->getType();
+      Record->dropAttr<PackedAttr>();
+    }
     // Keep track of the number of named members.
     if (FD->getIdentifier())
       ++NumNamedMembers;
   }
 
   // Okay, we successfully defined 'Record'.
   if (Record) {
     bool Completed = false;

lib/Sema/SemaExpr.cpp

if (getLangOpts().ObjCWeak && isa<VarDecl>(D) &&
       !Diags.isIgnored(diag::warn_arc_repeated_use_of_weak, E->getLocStart()))
       recordUseOfEvaluatedWeak(E);
 
   if (FieldDecl *FD = dyn_cast<FieldDecl>(D)) {
     UnusedPrivateFields.remove(FD);
     // Just in case we're building an illegal pointer-to-member.
     if (FD->isBitField())
       E->setObjectKind(OK_BitField);
+    else if (FD->isPackedField(Context))
+      E->setObjectKind(OK_PackedField);
   }
 
   // C++ [expr.prim]/8: The expression [...] is a bit-field if the identifier
   // designates a bit-field.
   if (auto *BD = dyn_cast<BindingDecl>(D))
     if (auto *BE = BD->getBinding())
       E->setObjectKind(BE->getObjectKind());
 

lib/Sema/SemaExprMember.cpp

Sema::BuildFieldReferenceExpr(Expr *BaseExpr, bool IsArrow,
                               FieldDecl *Field, DeclAccessPair FoundDecl,
                               const DeclarationNameInfo &MemberNameInfo) {
   // x.a is an l-value if 'a' has a reference type. Otherwise:
   // x.a is an l-value/x-value/pr-value if the base is (and note
   //   that *x is always an l-value), except that if the base isn't
   //   an ordinary object then we must have an rvalue.
   ExprValueKind VK = VK_LValue;
   ExprObjectKind OK = OK_Ordinary;
   if (!IsArrow) {
-    if (BaseExpr->getObjectKind() == OK_Ordinary)
-      VK = BaseExpr->getValueKind();
-    else
-      VK = VK_RValue;
+    if (BaseExpr->getObjectKind() == OK_PackedField ||
+        Field->isPackedField(Context))
+      OK = OK_PackedField;
   }
   if (VK != VK_RValue && Field->isBitField())
       OK = OK_BitField;
-  
   // Figure out the type of the member; see C99 6.5.2.3p3, C++ [expr.ref]
   QualType MemberType = Field->getType();
   if (const ReferenceType *Ref = MemberType->getAs<ReferenceType>()) {
     MemberType = Ref->getPointeeType();
     VK = VK_LValue;
   } else {
     QualType BaseType = BaseExpr->getType();
     if (IsArrow) BaseType = BaseType->getAs<PointerType>()->getPointeeType();
 
     Qualifiers BaseQuals = BaseType.getQualifiers();
 

lib/Sema/SemaFixItUtils.cpp

bool ConversionFixItGenerator::tryToFixConversion(const Expr *FullExpr,
 
   // Check if the pointer to the argument needs to be passed:
   //   (type -> type *) or (type & -> type *).
   if (isa<PointerType>(ToQTy)) {
     bool CanConvert = false;
     OverloadFixItKind FixKind = OFIK_TakeAddress;
 
     // Only suggest taking address of L-values.
-    if (!Expr->isLValue() || Expr->getObjectKind() != OK_Ordinary)
+    if (!Expr->isLValue() || (Expr->getObjectKind() != OK_Ordinary &&
+                              Expr->getObjectKind() != OK_PackedField))
       return false;
 
     CanConvert = CompareTypes(S.Context.getPointerType(FromQTy), ToQTy,
                               S, Begin, VK_RValue);
     if (CanConvert) {
 
       if (const UnaryOperator *UO = dyn_cast<UnaryOperator>(Expr)) {
         if (UO->getOpcode() == UO_Deref) {

lib/Sema/SemaInit.cpp

bool InitializationSequence::isAmbiguous() const {
   case FK_ArrayNeedsInitListOrStringLiteral:
   case FK_ArrayNeedsInitListOrWideStringLiteral:
   case FK_NarrowStringIntoWideCharArray:
   case FK_WideStringIntoCharArray:
   case FK_IncompatWideStringIntoWideChar:
   case FK_AddressOfOverloadFailed: // FIXME: Could do better
   case FK_NonConstLValueReferenceBindingToTemporary:
   case FK_NonConstLValueReferenceBindingToBitfield:
+  case FK_NonConstLValueReferenceBindingToPackedField:
   case FK_NonConstLValueReferenceBindingToVectorElement:
   case FK_NonConstLValueReferenceBindingToUnrelated:
   case FK_RValueReferenceBindingToLValue:
   case FK_ReferenceInitDropsQualifiers:
   case FK_ReferenceInitFailed:
   case FK_ConversionFailed:
   case FK_ConversionFromPropertyFailed:
   case FK_TooManyInitsForScalar:
if (ResolveOverloadedFunctionForReferenceBinding(S, Initializer, cv2T2, T2,
                                                    T1, Sequence))
     return;
 
   // Delegate everything else to a subfunction.
   TryReferenceInitializationCore(S, Entity, Kind, Initializer, cv1T1, T1,
                                  T1Quals, cv2T2, T2, T2Quals, Sequence);
 }
 
+
 /// Determine whether an expression is a non-referenceable glvalue (one to
 /// which a reference can never bind). Attemting to bind a reference to
 /// such a glvalue will always create a temporary.
 static bool isNonReferenceableGLValue(Expr *E) {
-  return E->refersToBitField() || E->refersToVectorElement();
+  return E->refersToBitField() || E->refersToVectorElement() || E->refersToPackedField();
 }
 
 /// \brief Reference initialization without resolving overloaded functions.
 static void TryReferenceInitializationCore(Sema &S,
                                            const InitializedEntity &Entity,
                                            const InitializationKind &Kind,
                                            Expr *Initializer,
                                            QualType cv1T1, QualType T1,
                                            Qualifiers T1Quals,
                                            QualType cv2T2, QualType T2,
                                            Qualifiers T2Quals,
                                            InitializationSequence &Sequence) {
   QualType DestType = Entity.getType();
   SourceLocation DeclLoc = Initializer->getLocStart();
   // Compute some basic properties of the types and the initializer.
   bool isLValueRef = DestType->isLValueReferenceType();
   bool isRValueRef = !isLValueRef;
   bool DerivedToBase = false;
   bool ObjCConversion = false;
   bool ObjCLifetimeConversion = false;
   Expr::Classification InitCategory = Initializer->Classify(S.Context);
if (InitCategory.isLValue() && !isNonReferenceableGLValue(Initializer) &&
             S.Context.getQualifiedType(T2, T1Quals),
             Initializer->getValueKind());
       if (DerivedToBase)
         Sequence.AddDerivedToBaseCastStep(cv1T1, VK_LValue);
       else if (ObjCConversion)
         Sequence.AddObjCObjectConversionStep(cv1T1);
 
       // We only create a temporary here when binding a reference to a
-      // bit-field or vector element. Those cases are't supposed to be
-      // handled by this bullet, but the outcome is the same either way.
+      // bit-field, vector element or packed field. Those cases are't supposed
+      // to be handled by this bullet, but the outcome is the same either way.
       Sequence.AddReferenceBindingStep(cv1T1, false);
       return;
     }
 
     //     - has a class type (i.e., T2 is a class type), where T1 is not
     //       reference-related to T2, and can be implicitly converted to an
     //       lvalue of type "cv3 T3," where "cv1 T1" is reference-compatible
     //       with "cv3 T3" (this conversion is selected by enumerating the
else {
       switch (RefRelationship) {
       case Sema::Ref_Compatible:
         if (Initializer->refersToBitField())
           FK = InitializationSequence::
               FK_NonConstLValueReferenceBindingToBitfield;
         else if (Initializer->refersToVectorElement())
           FK = InitializationSequence::
               FK_NonConstLValueReferenceBindingToVectorElement;
+        else if (Initializer->refersToPackedField())
+          FK = InitializationSequence::
+              FK_NonConstLValueReferenceBindingToPackedField;
         else
           llvm_unreachable("unexpected kind of compatible initializer");
         break;
       case Sema::Ref_Related:
         FK = InitializationSequence::FK_ReferenceInitDropsQualifiers;
         break;
       case Sema::Ref_Incompatible:
         FK = InitializationSequence::
if (RefRelationship == Sema::Ref_Incompatible) {
       if (ConvOvlResult)
         Sequence.SetOverloadFailure(
             InitializationSequence::FK_ReferenceInitOverloadFailed,
             ConvOvlResult);
 
       return;
     }
 
-    if (RefRelationship == Sema::Ref_Compatible &&
-        isRValueRef && InitCategory.isLValue()) {
+    if (RefRelationship == Sema::Ref_Compatible) {
+      if (Initializer->refersToPackedField()) {
+        Sequence.SetFailed(InitializationSequence::
+                               FK_NonConstLValueReferenceBindingToPackedField);
+        return;
+      } else if (isRValueRef && InitCategory.isLValue()) {
         Sequence.SetFailed(
             InitializationSequence::FK_RValueReferenceBindingToLValue);
         return;
       }
+    }
 
     Sequence.SetFailed(InitializationSequence::FK_ReferenceInitDropsQualifiers);
     return;
   }
 
   //      - Otherwise, a temporary of type "cv1 T1" is created and initialized
   //        from the initializer expression using the rules for a non-reference
   //        copy-initialization (8.5). The reference is then bound to the
S.Diag(Kind.getLocation(), diag::err_reference_bind_to_bitfield)
       << (BitField ? BitField->getDeclName() : DeclarationName())
       << (BitField != nullptr)
       << Args[0]->getSourceRange();
     if (BitField)
       S.Diag(BitField->getLocation(), diag::note_bitfield_decl);
     break;
   }
 
+  case FK_NonConstLValueReferenceBindingToPackedField: {
+    S.Diag(Kind.getLocation(), diag::err_reference_bind_to_packed_field)
+        << Args[0]->getSourceRange();
+    break;
+  }
+
   case FK_NonConstLValueReferenceBindingToVectorElement:
     S.Diag(Kind.getLocation(), diag::err_reference_bind_to_vector_element)
       << DestType.isVolatileQualified()
       << Args[0]->getSourceRange();
     break;
 
   case FK_RValueReferenceBindingToLValue:
     S.Diag(Kind.getLocation(), diag::err_lvalue_to_rvalue_ref)
case FailedSequence: {
     case FK_NonConstLValueReferenceBindingToTemporary:
       OS << "non-const lvalue reference bound to temporary";
       break;
 
     case FK_NonConstLValueReferenceBindingToBitfield:
       OS << "non-const lvalue reference bound to bit-field";
       break;
 
+    case FK_NonConstLValueReferenceBindingToPackedField:
+      OS << "non-const lvalue reference bound to packed field";
+      break;
+
     case FK_NonConstLValueReferenceBindingToVectorElement:
       OS << "non-const lvalue reference bound to vector element";
       break;
 
     case FK_NonConstLValueReferenceBindingToUnrelated:
       OS << "non-const lvalue reference bound to unrelated type";
       break;
 

test/CodeGenCXX/pod-member-memcpys.cpp

 // CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}i64 32, i32 4{{.*}})
 // CHECK: call dereferenceable({{[0-9]+}}) %struct.NonPOD* @_ZN6NonPODaSERKS_
 // CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}i64 16, i32 4{{.*}})
 // CHECK: ret %struct.InnerClassMember*
 
 // PackedMembers copy-assignment:
 // CHECK-LABEL: define linkonce_odr dereferenceable({{[0-9]+}}) %struct.PackedMembers* @_ZN13PackedMembersaSERKS_(%struct.PackedMembers* %this, %struct.PackedMembers* dereferenceable({{[0-9]+}}))
 // CHECK: call dereferenceable({{[0-9]+}}) %struct.NonPOD* @_ZN6NonPODaSERKS_
-// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}i64 16, i32 1{{.*}})
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}i64 16, i32 4{{.*}})
 // CHECK: ret %struct.PackedMembers*
 
 // COPY-CONSTRUCTORS:
 
 // Clang outputs copy-constructors in the reverse of the order that
 // copy-constructor calls are encountered. Add functions that call the copy
 // constructors of the classes above in reverse order here.
 
 #define CALL_CC(T) T callCC##T(const T& b) { return b; }
 
 CALL_CC(PackedMembers)
 // PackedMembers copy-assignment:
 // CHECK-LABEL: define linkonce_odr void @_ZN13PackedMembersC2ERKS_(%struct.PackedMembers* %this, %struct.PackedMembers* dereferenceable({{[0-9]+}}))
 // CHECK: call void @_ZN6NonPODC1ERKS_
-// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}i64 16, i32 1{{.*}})
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}i64 16, i32 4{{.*}})
 // CHECK: ret void
 
 CALL_CC(BitfieldMember2)
 // BitfieldMember2 copy-constructor:
 // CHECK-2-LABEL: define linkonce_odr void @_ZN15BitfieldMember2C2ERKS_(%struct.BitfieldMember2* %this, %struct.BitfieldMember2* dereferenceable({{[0-9]+}}))
 // CHECK-2: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}}i64 16, i32 4, i1 false)
 // CHECK-2: call void @_ZN6NonPODC1ERKS_
 // CHECK-2: ret void

test/SemaCXX/bind-packed-member.cpp

+// RUN: %clang_cc1 -verify %s
+
+struct __attribute__((packed)) A {
+  char x;
+  float y;
+  char z;
+} a;
+
+char &rax = a.x;  // no-error
+float &ray = a.y; // expected-error {{reference cannot bind to packed field}}
+char &raz = a.z;  // no-error
+
+struct __attribute__((packed, aligned(2))) B {
+  // Regardless of aligned(2) the fields are aligned(1) because of packed.
+  // The whole struct is aligned(2), though.
+  short x;
+  float y;
+  short z;
+  char w;
+} b;
+
+const short &crbx = b.x; // no-error
+short &rbx = b.x; // expected-error {{reference cannot bind to packed field}}
+float &rby = b.y; // expected-error {{reference cannot bind to packed field}}
+short &rbz = b.z; // expected-error {{reference cannot bind to packed field}}
+char &rbw = b.w;  // no-error
+
+struct __attribute__((packed)) B2 {
+  short x __attribute__((aligned(2)));
+  float y;
+  short z __attribute__((aligned(4)));
+  char w;
+} b2;
+
+short &rb2x = b2.x; // no-error
+short &rb2z = b2.z; // no-error
+
+struct C {
+  int c;
+};
+
+struct __attribute__((packed)) D {
+  char x;
+  int y;
+  C z;
+} d;
+
+C &rcz = d.z; // expected-error {{reference cannot bind to packed field}}
+int &rczc = d.z.c; // expected-error {{reference cannot bind to packed field}}
+
+struct E {
+    int x __attribute__((packed));
+    C y __attribute__((packed));
+    C z;
+} e;
+
+int& rex = e.x; // expected-error {{reference cannot bind to packed field}}
+C& rey = e.y; // expected-error {{reference cannot bind to packed field}}
+C& rez = e.z;  // no-error
+
+struct NonTrivialCopy
+{
+    int w;
+    NonTrivialCopy();
+    NonTrivialCopy(const NonTrivialCopy&);
+};
+
+struct F
+{
+    char c;
+    NonTrivialCopy x __attribute__((packed));
+    int w __attribute__((packed));
+} f;
+
+
+void fun1(int &);
+void fun2(const int &);
+
+void bar()
+{
+    const NonTrivialCopy& rx = f.x; // expected-error {{reference cannot bind to packed field}}
+    const int &w = f.w; // no-error
+
+    fun1(f.w); // expected-error {{reference cannot bind to packed field}}
+               // expected-note@76 {{passing argument to parameter here}}
+    fun2(f.w); // no-error
+}
+
+struct __attribute__((packed)) IgnorePacked {
+  int c;
+  NonTrivialCopy x; // expected-warning {{ignoring packed attribute because of unpacked non-POD field}}
+} nopacked1;
+
+int &ok1 = nopacked1.c; // no-error
+
+template <typename T>
+struct __attribute__((packed)) IgnorePackedTpl {
+  int c;
+  T x;
+};
+
+IgnorePackedTpl<NonTrivialCopy> nopacked2; // expected-warning@99 {{ignoring packed attribute because of unpacked non-POD field}}
+                                           // expected-note@102 {{in instantiation of template class}}
+int &ok2 = nopacked2.c; // no-error
+
+IgnorePackedTpl<float> packed3;
+int &error3 = packed3.c; // expected-error {{reference cannot bind to packed field}}