ahatanak updated this revision to Diff 81090.
ahatanak added a comment.

Address review comments.

- Arrays marked "flexible_array" are now treated as flexible arrays.
- __builtin_object_size returns a more accurate numbers for normal C99 flexible 
arrays (see test/CodeGen/object-size.c).

Note that a couple of tests fail with this patch applied since this patch 
depends on https://reviews.llvm.org/D24999.


https://reviews.llvm.org/D21453

Files:
  include/clang/AST/Decl.h
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/AttributeList.h
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGExpr.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGen/object-size.c
  test/CodeGenCXX/catch-undef-behavior.cpp
  test/SemaCXX/flexible-array-attr.cpp
  utils/TableGen/ClangAttrEmitter.cpp

Index: utils/TableGen/ClangAttrEmitter.cpp
===================================================================
--- utils/TableGen/ClangAttrEmitter.cpp
+++ utils/TableGen/ClangAttrEmitter.cpp
@@ -2653,6 +2653,7 @@
     case GenericRecord:
       return "(S.getLangOpts().CPlusPlus ? ExpectedStructOrUnionOrClass : "
                                            "ExpectedStructOrUnion)";
+    case Field: return "ExpectedField";
     case Func | ObjCMethod | Block: return "ExpectedFunctionMethodOrBlock";
     case Func | ObjCMethod | Class: return "ExpectedFunctionMethodOrClass";
     case Func | Param:
Index: test/SemaCXX/flexible-array-attr.cpp
===================================================================
--- /dev/null
+++ test/SemaCXX/flexible-array-attr.cpp
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+int g0[16] __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to fields}}
+
+struct S0 {
+  int a[4];
+  int foo1() __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to fields}}
+};
+
+struct S1 {
+  int a[4];
+  int *b __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to a non-flexible array member}}
+};
+
+struct S2 {
+  int a[4];
+  int b[4] __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to the last member of a struct}}
+  int c[4];
+};
+
+struct S3 {
+  int a[4];
+  int b[] __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to a non-flexible array member}}
+};
+
+struct S4 {
+  int a[4];
+  int b[0] __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to an array member that has at least one element}}
+};
+
+template<int N>
+struct S5 {
+  int a[4];
+  int b[N] __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to a non-flexible array member}}
+};
+
+struct S6 {
+  int a[4];
+  int b[2] __attribute__((flexible_array));
+};
+
+struct S7 : S6 { // expected-error {{base class 'S6' has a flexible array member}}
+};
+
+int lambda_capture(S6 a) { // expected-note {{'a' declared here}}
+  return [a](){ return 0; }(); // expected-error {{variable 'a' with flexible array member cannot be captured in a lambda expression}}
+}
+
+void pointer_arithmetic(S6 *s6) {
+  int *p = &s6->b[4]; // No warnings.
+}
Index: test/CodeGenCXX/catch-undef-behavior.cpp
===================================================================
--- test/CodeGenCXX/catch-undef-behavior.cpp
+++ test/CodeGenCXX/catch-undef-behavior.cpp
@@ -327,6 +327,17 @@
   return incomplete[n];
 }
 
+struct FlexibleArray {
+  int a1[4];
+  int a2[4] __attribute__((flexible_array));
+};
+
+// CHECK-LABEL: @_Z14flexible_array
+int flexible_array(FlexibleArray *p, int n) {
+  // CHECK-NOT: call void @__ubsan_handle_out_of_bounds(
+  return p->a2[n];
+}
+
 typedef __attribute__((ext_vector_type(4))) int V4I;
 // CHECK-LABEL: @_Z12vector_index
 int vector_index(V4I v, int n) {
Index: test/CodeGen/object-size.c
===================================================================
--- test/CodeGen/object-size.c
+++ test/CodeGen/object-size.c
@@ -425,6 +425,15 @@
   gi = __builtin_object_size(dv->snd, 3);
 
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(dv->fst, 0);
+  // CHECK: store i32 16
+  gi = __builtin_object_size(dv->fst, 1);
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true)
+  gi = __builtin_object_size(dv->fst, 2);
+  // CHECK: store i32 16
+  gi = __builtin_object_size(d0->fst, 3);
+
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size(d0->snd, 0);
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size(d0->snd, 1);
@@ -518,6 +527,22 @@
   gi = __builtin_object_size(&dsv[9].snd[0], 1);
 }
 
+struct S0 {
+  int a[16];
+  int b[16] __attribute__((flexible_array));
+};
+
+// CHECK-LABEL: @test32
+void test32() {
+  struct S0 *s0;
+
+  // CHECK: store i32 64, i32* @gi
+  gi = __builtin_object_size(s0->a, 1);
+
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(s0->b, 1);
+}
+
 // CHECK-LABEL: @PR30346
 void PR30346() {
   struct sa_family_t {};
Index: lib/Sema/SemaDeclAttr.cpp
===================================================================
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -1804,6 +1804,25 @@
       Attr.getRange(), S.Context, Attr.getAttributeSpellingListIndex()));
 }
 
+static void handleFlexibleArrayAttr(Sema &S, Decl *D,
+                                    const AttributeList &Attr) {
+  const auto *FD = cast<FieldDecl>(D);
+  const auto *CAT = dyn_cast<ConstantArrayType>(FD->getType().getTypePtr());
+
+  if (!CAT) {
+    S.Diag(Attr.getLoc(), diag::err_flexible_array_attribute) << 1;
+    return;
+  }
+
+  if (CAT->getSize().ult(1)) {
+    S.Diag(Attr.getLoc(), diag::err_flexible_array_attribute) << 2;
+    return;
+  }
+
+  D->addAttr(::new (S.Context) FlexibleArrayAttr(
+      Attr.getRange(), S.Context, Attr.getAttributeSpellingListIndex()));
+}
+
 static void handleDisableTailCallsAttr(Sema &S, Decl *D,
                                        const AttributeList &Attr) {
   if (checkAttrMutualExclusion<NakedAttr>(S, D, Attr.getRange(),
@@ -5799,6 +5818,9 @@
   case AttributeList::AT_NotTailCalled:
     handleNotTailCalledAttr(S, D, Attr);
     break;
+  case AttributeList::AT_FlexibleArray:
+    handleFlexibleArrayAttr(S, D, Attr);
+    break;
   case AttributeList::AT_DisableTailCalls:
     handleDisableTailCallsAttr(S, D, Attr);
     break;
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -14298,6 +14298,15 @@
       continue;
     }
 
+    // Set HasFlexibleArrayMember. Only the last member of a struct can be
+    // marked flexible_array.
+    if (FD->hasAttr<FlexibleArrayAttr>()) {
+      if (i + 1 == end)
+        Record->setHasFlexibleArrayMember(true);
+      else
+        Diag(FD->getLocation(), diag::err_flexible_array_attribute) << 0;
+    }
+
     // C99 6.7.2.1p2:
     //   A structure or union shall not contain a member with
     //   incomplete or function type (hence, a structure shall not
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -10498,6 +10498,11 @@
     if (IsTailPaddedMemberArray(*this, size, ND))
       return;
 
+    // Don't warn if ND is a flexible array.
+    if (auto *FD = dyn_cast_or_null<FieldDecl>(ND))
+      if (FD->getParent()->hasFlexibleArrayMember())
+        return;
+
     // Suppress the warning if the subscript expression (as identified by the
     // ']' location) and the index expression are both from macro expansions
     // within a system header.
Index: lib/CodeGen/CGExpr.cpp
===================================================================
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -691,27 +691,30 @@
   // For compatibility with existing code, we treat arrays of length 0 or
   // 1 as flexible array members.
   const ArrayType *AT = E->getType()->castAsArrayTypeUnsafe();
+  E = E->IgnoreParens();
+  const auto *ME = dyn_cast<MemberExpr>(E);
+  const auto *FD = ME ? dyn_cast<FieldDecl>(ME->getMemberDecl()) : nullptr;
+
+  // FIXME: If the base type of the member expr is not FD->getParent(),
+  // this should not be treated as a flexible array member access.
+
   if (const auto *CAT = dyn_cast<ConstantArrayType>(AT)) {
-    if (CAT->getSize().ugt(1))
+    if (!(CAT->getSize().ule(1) ||
+          (FD && FD->getParent()->hasFlexibleArrayMember())))
       return false;
   } else if (!isa<IncompleteArrayType>(AT))
     return false;
 
-  E = E->IgnoreParens();
-
   // A flexible array member must be the last member in the class.
-  if (const auto *ME = dyn_cast<MemberExpr>(E)) {
-    // FIXME: If the base type of the member expr is not FD->getParent(),
-    // this should not be treated as a flexible array member access.
-    if (const auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl())) {
-      RecordDecl::field_iterator FI(
-          DeclContext::decl_iterator(const_cast<FieldDecl *>(FD)));
-      return ++FI == FD->getParent()->field_end();
-    }
-  } else if (const auto *IRE = dyn_cast<ObjCIvarRefExpr>(E)) {
-    return IRE->getDecl()->getNextIvar() == nullptr;
+  if (FD) {
+    RecordDecl::field_iterator FI(
+        DeclContext::decl_iterator(const_cast<FieldDecl *>(FD)));
+    return ++FI == FD->getParent()->field_end();
   }
 
+  if (const auto *IRE = dyn_cast<ObjCIvarRefExpr>(E))
+    return IRE->getDecl()->getNextIvar() == nullptr;
+
   return false;
 }
 
Index: lib/AST/ExprConstant.cpp
===================================================================
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -120,10 +120,11 @@
     Type = Base;
     for (unsigned I = 0, N = Path.size(); I != N; ++I) {
       if (Type->isArrayType()) {
-        const ConstantArrayType *CAT =
-          cast<ConstantArrayType>(Ctx.getAsArrayType(Type));
-        Type = CAT->getElementType();
-        ArraySize = CAT->getSize().getZExtValue();
+        const ArrayType *AT = Ctx.getAsArrayType(Type);
+        Type = AT->getElementType();
+        ArraySize = 0;
+        if (const auto *CAT = dyn_cast<ConstantArrayType>(AT))
+          ArraySize = CAT->getSize().getZExtValue();
         MostDerivedLength = I + 1;
         IsArray = true;
       } else if (Type->isAnyComplexType()) {
@@ -165,9 +166,13 @@
     /// Indicator of whether the most-derived object is an array element.
     unsigned MostDerivedIsArrayElement : 1;
 
+    /// Indicator of whether the last array added is either a C99 flexible array
+    /// or an array marked flexible_array.
+    bool LastIsFlexibleArray : 1;
+
     /// The length of the path to the most-derived object of which this is a
     /// subobject.
-    unsigned MostDerivedPathLength : 29;
+    unsigned MostDerivedPathLength : 28;
 
     /// The size of the array of which the most-derived object is an element.
     /// This will always be 0 if the most-derived object is not an array
@@ -187,13 +192,14 @@
 
     explicit SubobjectDesignator(QualType T)
         : Invalid(false), IsOnePastTheEnd(false),
-          MostDerivedIsArrayElement(false), MostDerivedPathLength(0),
-          MostDerivedArraySize(0), MostDerivedType(T) {}
+          MostDerivedIsArrayElement(false), LastIsFlexibleArray(false),
+          MostDerivedPathLength(0), MostDerivedArraySize(0),
+          MostDerivedType(T) {}
 
     SubobjectDesignator(ASTContext &Ctx, const APValue &V)
         : Invalid(!V.isLValue() || !V.hasLValuePath()), IsOnePastTheEnd(false),
-          MostDerivedIsArrayElement(false), MostDerivedPathLength(0),
-          MostDerivedArraySize(0) {
+          MostDerivedIsArrayElement(false), LastIsFlexibleArray(false),
+          MostDerivedPathLength(0), MostDerivedArraySize(0) {
       if (!Invalid) {
         IsOnePastTheEnd = V.isLValueOnePastTheEnd();
         ArrayRef<PathEntry> VEntries = V.getLValuePath();
@@ -236,16 +242,19 @@
     bool checkSubobject(EvalInfo &Info, const Expr *E, CheckSubobjectKind CSK);
 
     /// Update this designator to refer to the first element within this array.
-    void addArrayUnchecked(const ConstantArrayType *CAT) {
+    void addArrayUnchecked(const ArrayType *AT, bool IsFlexibleArray) {
       PathEntry Entry;
       Entry.ArrayIndex = 0;
       Entries.push_back(Entry);
 
       // This is a most-derived object.
-      MostDerivedType = CAT->getElementType();
+      MostDerivedType = AT->getElementType();
       MostDerivedIsArrayElement = true;
-      MostDerivedArraySize = CAT->getSize().getZExtValue();
+      MostDerivedArraySize = 0;
+      if (const auto *CAT = dyn_cast<ConstantArrayType>(AT))
+        MostDerivedArraySize = CAT->getSize().getZExtValue();
       MostDerivedPathLength = Entries.size();
+      LastIsFlexibleArray = IsFlexibleArray;
     }
     /// Update this designator to refer to the given base or member of this
     /// object.
@@ -283,7 +292,8 @@
       if (MostDerivedPathLength == Entries.size() &&
           MostDerivedIsArrayElement) {
         Entries.back().ArrayIndex += N;
-        if (Entries.back().ArrayIndex > MostDerivedArraySize) {
+        if (Entries.back().ArrayIndex > MostDerivedArraySize &&
+            !LastIsFlexibleArray) {
           diagnosePointerArithmetic(Info, E, Entries.back().ArrayIndex);
           setInvalid();
         }
@@ -1133,9 +1143,10 @@
       if (checkSubobject(Info, E, isa<FieldDecl>(D) ? CSK_Field : CSK_Base))
         Designator.addDeclUnchecked(D, Virtual);
     }
-    void addArray(EvalInfo &Info, const Expr *E, const ConstantArrayType *CAT) {
+    void addArray(EvalInfo &Info, const Expr *E, const ArrayType *AT,
+                  bool IsFlexibleArray = false) {
       if (checkSubobject(Info, E, CSK_ArrayToPointer))
-        Designator.addArrayUnchecked(CAT);
+        Designator.addArrayUnchecked(AT, IsFlexibleArray);
     }
     void addComplex(EvalInfo &Info, const Expr *E, QualType EltTy, bool Imag) {
       if (checkSubobject(Info, E, Imag ? CSK_Imag : CSK_Real))
@@ -5207,7 +5218,7 @@
       return true;
     }
   }
-  case CK_ArrayToPointerDecay:
+  case CK_ArrayToPointerDecay: {
     if (SubExpr->isGLValue()) {
       if (!EvaluateLValue(SubExpr, Result, Info))
         return false;
@@ -5218,12 +5229,20 @@
         return false;
     }
     // The result is a pointer to the first element of the array.
-    if (const ConstantArrayType *CAT
-          = Info.Ctx.getAsConstantArrayType(SubExpr->getType()))
-      Result.addArray(Info, E, CAT);
-    else
+    const auto *AT = Info.Ctx.getAsArrayType(SubExpr->getType());
+    if (const auto *CAT = dyn_cast<ConstantArrayType>(AT)) {
+      bool IsFlexibleArray = false;
+      if (const auto *ME = dyn_cast<MemberExpr>(SubExpr)) {
+        const auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl());
+        IsFlexibleArray = FD && FD->hasAttr<FlexibleArrayAttr>();
+      }
+      Result.addArray(Info, E, CAT, IsFlexibleArray);
+    } else if (isa<IncompleteArrayType>(AT)) {
+      Result.addArray(Info, E, AT, true);
+    } else
       Result.Designator.setInvalid();
     return true;
+  }
 
   case CK_FunctionToPointerDecay:
     return EvaluateLValue(SubExpr, Result, Info);
@@ -6950,6 +6969,8 @@
   if (End.InvalidBase && SubobjectOnly && Type == 1 &&
       End.Designator.Entries.size() == End.Designator.MostDerivedPathLength &&
       End.Designator.MostDerivedIsArrayElement &&
+      (End.Designator.MostDerivedArraySize < 2 ||
+       End.Designator.LastIsFlexibleArray) &&
       isDesignatorAtObjectEnd(Info.Ctx, End))
     return false;
 
Index: include/clang/Sema/AttributeList.h
===================================================================
--- include/clang/Sema/AttributeList.h
+++ include/clang/Sema/AttributeList.h
@@ -925,7 +925,8 @@
   ExpectedVariableEnumFieldOrTypedef,
   ExpectedFunctionMethodEnumOrClass,
   ExpectedStructClassVariableFunctionOrInlineNamespace,
-  ExpectedForMaybeUnused
+  ExpectedForMaybeUnused,
+  ExpectedField
 };
 
 }  // end namespace clang
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2257,6 +2257,10 @@
     : Error<"%0 attribute is not supported for this target">;
 // The err_*_attribute_argument_not_int are seperate because they're used by
 // VerifyIntegerConstantExpression.
+def err_flexible_array_attribute : Error<
+  "'flexible_array' attribute only applies to %select{"
+  "the last member of a struct|a non-flexible array member|"
+  "an array member that has at least one element}0">;
 def err_aligned_attribute_argument_not_int : Error<
   "'aligned' attribute requires integer constant">;
 def err_align_value_attribute_argument_not_int : Error<
@@ -2649,7 +2653,8 @@
   "|variables, enums, fields and typedefs"
   "|functions, methods, enums, and classes"
   "|structs, classes, variables, functions, and inline namespaces"
-  "|variables, functions, methods, types, enumerations, enumerators, labels, and non-static data members}1">,
+  "|variables, functions, methods, types, enumerations, enumerators, labels, and non-static data members"
+  "|fields}1">,
   InGroup<IgnoredAttributes>;
 def err_attribute_wrong_decl_type : Error<warn_attribute_wrong_decl_type.Text>;
 def warn_type_attribute_wrong_type : Warning<
Index: include/clang/Basic/AttrDocs.td
===================================================================
--- include/clang/Basic/AttrDocs.td
+++ include/clang/Basic/AttrDocs.td
@@ -2295,6 +2295,28 @@
   }];
 }
 
+def FlexibleArrayDocs : Documentation {
+  let Category = DocCatField;
+  let Content = [{
+``flexible_array`` indicates to the compiler that an array that is the last member of a struct can have extra memory allocated at its end.
+
+  .. code-block:: c
+
+    struct S0 {
+      char a[4];
+      char b[4] __attribute__((flexible_array));
+    };
+
+    struct S0 *p = (struct S0 *)malloc(sizeof(S0) + 4);
+
+The attribute causes the compiler to treat such arrays as if they were C99 flexible arrays and has the following effects:
+
+- array bound checking will be skipped
+- __builtin_object_size will be more conservative when computing the size of the array.
+- tbaa.struct metadata for the struct will not be emitted.
+  }];
+}
+
 def InternalLinkageDocs : Documentation {
   let Category = DocCatFunction;
   let Content = [{
Index: include/clang/Basic/Attr.td
===================================================================
--- include/clang/Basic/Attr.td
+++ include/clang/Basic/Attr.td
@@ -15,6 +15,8 @@
 }
 def DocCatFunction : DocumentationCategory<"Function Attributes">;
 def DocCatVariable : DocumentationCategory<"Variable Attributes">;
+def DocCatField :
+    DocumentationCategory<"Field Attributes">;
 def DocCatType : DocumentationCategory<"Type Attributes">;
 def DocCatStmt : DocumentationCategory<"Statement Attributes">;
 // Attributes listed under the Undocumented category do not generate any public
@@ -2335,6 +2337,13 @@
   let Documentation = [LoopHintDocs, UnrollHintDocs];
 }
 
+def FlexibleArray : InheritableAttr {
+  let Spellings = [GNU<"flexible_array">,
+                   CXX11<"clang", "flexible_array">];
+  let Subjects = SubjectList<[Field], ErrorDiag>;
+  let Documentation = [FlexibleArrayDocs];
+}
+
 def CapturedRecord : InheritableAttr {
   // This attribute has no spellings as it is only ever created implicitly.
   let Spellings = [];
Index: include/clang/AST/Decl.h
===================================================================
--- include/clang/AST/Decl.h
+++ include/clang/AST/Decl.h
@@ -3283,7 +3283,8 @@
 class RecordDecl : public TagDecl {
   // FIXME: This can be packed into the bitfields in Decl.
   /// HasFlexibleArrayMember - This is true if this struct ends with a flexible
-  /// array member (e.g. int X[]) or if this union contains a struct that does.
+  /// array member (e.g. int X[]) or an array marked 'flexible_array' or if this
+  /// union contains a struct that does.
   /// If so, this cannot be contained in arrays or other structs as a member.
   bool HasFlexibleArrayMember : 1;
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to