rogfer01 updated this revision to Diff 82114.
rogfer01 added a comment.

Minor issues addressed


https://reviews.llvm.org/D23325

Files:
  include/clang/AST/Decl.h
  include/clang/AST/Expr.h
  include/clang/AST/Stmt.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/Specifiers.h
  include/clang/Sema/Initialization.h
  lib/AST/ASTDumper.cpp
  lib/AST/Decl.cpp
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprMember.cpp
  lib/Sema/SemaFixItUtils.cpp
  lib/Sema/SemaInit.cpp
  test/CodeGenCXX/pod-member-memcpys.cpp
  test/SemaCXX/bind-packed-member.cpp

Index: test/SemaCXX/bind-packed-member.cpp
===================================================================
--- test/SemaCXX/bind-packed-member.cpp
+++ test/SemaCXX/bind-packed-member.cpp
@@ -0,0 +1,107 @@
+// 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}}
Index: test/CodeGenCXX/pod-member-memcpys.cpp
===================================================================
--- test/CodeGenCXX/pod-member-memcpys.cpp
+++ test/CodeGenCXX/pod-member-memcpys.cpp
@@ -168,7 +168,7 @@
 // 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:
@@ -183,7 +183,7 @@
 // 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)
Index: lib/Sema/SemaInit.cpp
===================================================================
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -3111,6 +3111,7 @@
   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:
@@ -4252,11 +4253,12 @@
                                  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.
@@ -4313,8 +4315,8 @@
         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;
     }
@@ -4364,6 +4366,9 @@
         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;
@@ -4450,11 +4455,16 @@
       return;
     }
 
-    if (RefRelationship == Sema::Ref_Compatible &&
-        isRValueRef && InitCategory.isLValue()) {
-      Sequence.SetFailed(
-        InitializationSequence::FK_RValueReferenceBindingToLValue);
-      return;
+    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);
@@ -7488,6 +7498,12 @@
     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()
@@ -7795,6 +7811,10 @@
       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;
Index: lib/Sema/SemaFixItUtils.cpp
===================================================================
--- lib/Sema/SemaFixItUtils.cpp
+++ lib/Sema/SemaFixItUtils.cpp
@@ -130,7 +130,8 @@
     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,
Index: lib/Sema/SemaExprMember.cpp
===================================================================
--- lib/Sema/SemaExprMember.cpp
+++ lib/Sema/SemaExprMember.cpp
@@ -1780,14 +1780,14 @@
   ExprValueKind VK = VK_LValue;
   ExprObjectKind OK = OK_Ordinary;
   if (!IsArrow) {
-    if (BaseExpr->getObjectKind() == OK_Ordinary)
-      VK = BaseExpr->getValueKind();
-    else
-      VK = VK_RValue;
+    VK = BaseExpr->getValueKind();
+    if (BaseExpr->getObjectKind() == OK_PackedField ||
+        Field->isPackedField(Context))
+      OK = OK_PackedField;
   }
   if (VK != VK_RValue && Field->isBitField())
-    OK = OK_BitField;
-  
+      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>()) {
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -1775,6 +1775,8 @@
     // 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
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -14503,6 +14503,16 @@
     }
     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;
Index: lib/Sema/SemaCast.cpp
===================================================================
--- lib/Sema/SemaCast.cpp
+++ lib/Sema/SemaCast.cpp
@@ -1919,6 +1919,9 @@
     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;
Index: lib/AST/Decl.cpp
===================================================================
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -3585,6 +3585,13 @@
                                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
 //===----------------------------------------------------------------------===//
Index: lib/AST/ASTDumper.cpp
===================================================================
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -1888,6 +1888,9 @@
     switch (Node->getObjectKind()) {
     case OK_Ordinary:
       break;
+    case OK_PackedField:
+      OS << " packedfield";
+      break;
     case OK_BitField:
       OS << " bitfield";
       break;
Index: include/clang/Sema/Initialization.h
===================================================================
--- include/clang/Sema/Initialization.h
+++ include/clang/Sema/Initialization.h
@@ -834,6 +834,8 @@
     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
Index: include/clang/Basic/Specifiers.h
===================================================================
--- include/clang/Basic/Specifiers.h
+++ include/clang/Basic/Specifiers.h
@@ -123,6 +123,9 @@
     /// 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,
 
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -1714,6 +1714,11 @@
 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<
Index: include/clang/AST/Stmt.h
===================================================================
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -126,13 +126,13 @@
     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;
Index: include/clang/AST/Expr.h
===================================================================
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -409,9 +409,10 @@
     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.
@@ -425,13 +426,21 @@
 
 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.
   ///
Index: include/clang/AST/Decl.h
===================================================================
--- include/clang/AST/Decl.h
+++ include/clang/AST/Decl.h
@@ -2383,6 +2383,10 @@
            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(); }
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to