Re: [PATCH] D21453: Add support for attribute "overallocated"

[Akira Hatanaka via cfe-commits]

ahatanak added a comment.

ping


https://reviews.llvm.org/D21453



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21453: Add support for attribute "overallocated"

[Aaron Ballman via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM with one small testing nit, but you should wait for @rsmith to chime in 
since he had comments previously.



Comment at: test/SemaCXX/flexible-array-attr.cpp:52
@@ +51,3 @@
+  int a[4];
+  int b[4] __attribute__((flexible_array)); // expected-error 
{{'flexible_array' attribute only applies to the last member of a non-union 
class}}
+};

Would be good to change this to b[1] so that it looks like an otherwise 
well-formed construct. Should not change the diagnostic.


https://reviews.llvm.org/D21453



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21453: Add support for attribute "overallocated"

[Akira Hatanaka via cfe-commits]

ahatanak added a comment.

ping


https://reviews.llvm.org/D21453



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21453: Add support for attribute "overallocated"

[Akira Hatanaka via cfe-commits]

ahatanak added inline comments.


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2170
@@ +2169,3 @@
+  "'flexible_array' attribute only applies to %select{"
+  "the last member of a struct|members of structs or classes|"
+  "fixed sized array members|array members that have at least one element}0">;

aaron.ballman wrote:
> I think "members of structs or classes" and "the last member of a struct" 
> could be combined into "the last member of a non-union class", instead of 
> using separate diagnostic text. What do you think?
I agree, I think these two diagnostics should be combined.


http://reviews.llvm.org/D21453



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21453: Add support for attribute "overallocated"

[Akira Hatanaka via cfe-commits]

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

Change diagnostic messages.


http://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/Decl.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenTBAA.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.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
@@ -2641,6 +2641,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,57 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+int g0[16] __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to non-static data members}}
+
+struct S0 {
+  int a[4];
+  int foo1() __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to non-static data members}}
+};
+
+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 non-union class}}
+  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
+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[1] __attribute__((flexible_array));
+};
+
+struct S7 : S6 { // expected-error {{base class 'S6' has a flexible array member}}
+};
+
+struct S8 {
+  int a;
+  S6 s6; // expected-error {{struct with a member marked 'flexible_array' cannot be nested}}
+};
+
+union U0 {
+  int a[4];
+  int b[4] __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to the last member of a non-union class}}
+};
+
+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}}
+}
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
@@ -517,3 +517,19 @@
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size([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);
+}
Index: lib/Serialization/ASTWriterDecl.cpp
===
--- lib/Serialization/ASTWriterDecl.cpp
+++ lib/Serialization/ASTWriterDecl.cpp
@@ -457,6 +457,7 @@
 void 

Re: [PATCH] D21453: Add support for attribute "overallocated"

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: include/clang/AST/Decl.h:3249
@@ -3248,1 +3248,3 @@
 
+  /// This is true if this struct ends with an array marked 'flexible_array'.
+  bool HasFlexibleArrayAttr : 1;

ahatanak wrote:
> Probably it can be looked up although it would require incrementing the 
> field_iterator until it reaches the last non-static data member of the record.
Ah shoot, I forgot that these are forward iterators, not bidirectional ones. I 
think the bit is probably the better way to go; we have bits to spare here.


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2170
@@ +2169,3 @@
+  "'flexible_array' attribute only applies to %select{"
+  "the last member of a struct|members of structs or classes|"
+  "fixed sized array members|array members that have at least one element}0">;

I think "members of structs or classes" and "the last member of a struct" could 
be combined into "the last member of a non-union class", instead of using 
separate diagnostic text. What do you think?


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2171
@@ +2170,3 @@
+  "the last member of a struct|members of structs or classes|"
+  "fixed sized array members|array members that have at least one element}0">;
+def err_flexible_array_nested : Error<

Since you can only have one such member, I think we want to drop the plural 
from both of these. I think the first may read better as "a non-flexible array 
member" (since there are "arrays" and "flexible arrays", but not "fixed-sized 
arrays" in our diagnostics), and the second may read better as "an array member 
that has at least one element".


Comment at: lib/AST/ExprConstant.cpp:170
@@ +169,3 @@
+/// Indicator of whether the last array added is marked flexible_array.
+bool IsFlexibleArray : 1;
+

ahatanak wrote:
> I don't think there is a way to do that reliably. The FieldDecl for the array 
> isn't always available in struct LValue, as far as I can tell, so it looks 
> like we'll need a bit here.
Okay, that seems reasonable to me -- I don't think anyone is relying on that 
bit being stolen from `MostDerivedPathLength`.


http://reviews.llvm.org/D21453



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21453: Add support for attribute "overallocated"

[Akira Hatanaka via cfe-commits]

ahatanak marked 7 inline comments as done.


Comment at: include/clang/AST/Decl.h:3249
@@ -3248,1 +3248,3 @@
 
+  /// This is true if this struct ends with an array marked 'flexible_array'.
+  bool HasFlexibleArrayAttr : 1;

Probably it can be looked up although it would require incrementing the 
field_iterator until it reaches the last non-static data member of the record.


Comment at: lib/AST/ExprConstant.cpp:170
@@ +169,3 @@
+/// Indicator of whether the last array added is marked flexible_array.
+bool IsFlexibleArray : 1;
+

I don't think there is a way to do that reliably. The FieldDecl for the array 
isn't always available in struct LValue, as far as I can tell, so it looks like 
we'll need a bit here.


http://reviews.llvm.org/D21453



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21453: Add support for attribute "overallocated"

[Akira Hatanaka via cfe-commits]

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

Address review comments.


http://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/Decl.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenTBAA.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.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
@@ -2641,6 +2641,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,57 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+
+int g0[16] __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to non-static data members}}
+
+struct S0 {
+  int a[4];
+  int foo1() __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to non-static data members}}
+};
+
+struct S1 {
+  int a[4];
+  int *b __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to fixed sized array members}}
+};
+
+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 fixed sized array members}}
+};
+
+struct S4 {
+  int a[4];
+  int b[0] __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to array members that have at least one element}}
+};
+
+template
+struct S5 {
+  int a[4];
+  int b[N] __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to fixed sized array members}}
+};
+
+struct S6 {
+  int a[4];
+  int b[1] __attribute__((flexible_array));
+};
+
+struct S7 : S6 { // expected-error {{base class 'S6' has a flexible array member}}
+};
+
+struct S8 {
+  int a;
+  S6 s6; // expected-error {{struct with a member marked 'flexible_array' cannot be nested}}
+};
+
+union U0 {
+  int a[4];
+  int b[4] __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to members of structs or classes}}
+};
+
+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}}
+}
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
@@ -517,3 +517,19 @@
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size([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);
+}
Index: lib/Serialization/ASTWriterDecl.cpp
===
--- lib/Serialization/ASTWriterDecl.cpp
+++ lib/Serialization/ASTWriterDecl.cpp
@@ -457,6 +457,7 @@
 void ASTDeclWriter::VisitRecordDecl(RecordDecl *D) {
   

Re: [PATCH] D21453: Add support for attribute "overallocated"

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: include/clang/AST/Decl.h:3249
@@ -3248,1 +3248,3 @@
 
+  /// This is true if this struct ends with an array marked 'flexible_array'.
+  bool HasFlexibleArrayAttr : 1;

Does this require a bit, or can this simply be looked up as needed?


Comment at: include/clang/Basic/Attr.td:18
@@ -17,2 +17,3 @@
 def DocCatVariable : DocumentationCategory<"Variable Attributes">;
+def DocCatField : DocumentationCategory<"Field Attributes">;
 def DocCatType : DocumentationCategory<"Type Attributes">;

Instead of "field", how about "non-static data member"? That should make it 
more clear as to what it pertains to.


Comment at: include/clang/Basic/Attr.td:2282
@@ +2281,3 @@
+  let Subjects = SubjectList<[Field], ErrorDiag,
+ "ExpectedField">;
+  let Documentation = [FlexibleArrayDocs];

Should be able to drop the string literal here; it should work by default. If 
it doesn't, then ClangAttrEmitter.cpp should be updated to properly handle it.


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2165
@@ +2164,3 @@
+def err_flexible_array_attribute : Error<
+  "'flexible_array' attribute only applies to %0">;
+def err_flexible_array_nested : Error<

This should be updated to use a %select instead of passing string literals (for 
eventual localization purposes).


Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2520
@@ -2516,1 +2519,3 @@
+  "variables, functions, methods, types, enumerations, enumerators, labels, 
and non-static data members|"
+  "fields}1">,
   InGroup;

non-static data members instead of fields.


Comment at: lib/AST/ExprConstant.cpp:170
@@ +169,3 @@
+/// Indicator of whether the last array added is marked flexible_array.
+bool IsFlexibleArray : 1;
+

Is the bit required here as well, or can this be queried as-needed?


Comment at: lib/AST/ExprConstant.cpp:1133
@@ -1128,1 +1132,3 @@
+void addArray(EvalInfo , const Expr *E, const ConstantArrayType *CAT,
+  bool HasFlexibleArrayAttr) {
   if (checkSubobject(Info, E, CSK_ArrayToPointer))

Perhaps this should be a default argument, since it's false almost everywhere?


Comment at: lib/AST/ExprConstant.cpp:5184-5185
@@ +5183,4 @@
+  bool HasFlexibleArrayAttr = false;
+  if (auto *ME = dyn_cast(SubExpr)) {
+auto *FD = dyn_cast(ME->getMemberDecl());
+HasFlexibleArrayAttr = FD && FD->hasAttr();

Should be `const` qualified.


Comment at: lib/Sema/SemaDeclAttr.cpp:1808-1809
@@ +1807,4 @@
+const AttributeList ) {
+  auto *FD = cast(D);
+  auto *CAT = dyn_cast(FD->getType().getTypePtr());
+

Should be `const`


http://reviews.llvm.org/D21453



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21453: Add support for attribute "overallocated"

[Akira Hatanaka via cfe-commits]

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

The new patch defines a new attribute "flexible_array", which gets attached to 
the last array member of a struct.

I made changes to clang to treat arrays marked "flexible_array" as C99's 
flexible array members where it made sense to do so. There are several places 
where C99's flexible arrays and "flexible_array" are treated differently. For 
example, the attribute doesn't change the way arguments are passed or values 
are returned from a function. It doesn't change objective-c's type encoding 
either.


http://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/Decl.cpp
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CodeGenTBAA.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaExpr.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CodeGen/object-size.c
  test/CodeGenCXX/catch-undef-behavior.cpp
  test/SemaCXX/flexible-array-attr.cpp

Index: test/SemaCXX/flexible-array-attr.cpp
===
--- /dev/null
+++ test/SemaCXX/flexible-array-attr.cpp
@@ -0,0 +1,57 @@
+// 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 fixed sized array members}}
+};
+
+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 fixed sized array members}}
+};
+
+struct S4 {
+  int a[4];
+  int b[0] __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to array members that have at least one element}}
+};
+
+template
+struct S5 {
+  int a[4];
+  int b[N] __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to fixed sized array members}}
+};
+
+struct S6 {
+  int a[4];
+  int b[1] __attribute__((flexible_array));
+};
+
+struct S7 : S6 { // expected-error {{base class 'S6' has a flexible array member}}
+};
+
+struct S8 {
+  int a;
+  S6 s6; // expected-error {{struct with a member marked 'flexible_array' cannot be nested}}
+};
+
+union U0 {
+  int a[4];
+  int b[4] __attribute__((flexible_array)); // expected-error {{'flexible_array' attribute only applies to members of structs or classes}}
+};
+
+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}}
+}
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
@@ -517,3 +517,19 @@
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size([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);
+}
Index: lib/Serialization/ASTWriterDecl.cpp
===
--- lib/Serialization/ASTWriterDecl.cpp
+++ lib/Serialization/ASTWriterDecl.cpp
@@ -457,6 +457,7 @@
 void ASTDeclWriter::VisitRecordDecl(RecordDecl *D) {
   VisitTagDecl(D);
   Record.push_back(D->hasFlexibleArrayMember());
+  Record.push_back(D->hasFlexibleArrayAttr());
   Record.push_back(D->isAnonymousStructOrUnion());
   

Re: [PATCH] D21453: Add support for attribute "overallocated"

[Richard Smith via cfe-commits]

rsmith requested changes to this revision.


Comment at: include/clang/Basic/AttrDocs.td:2073-2079
@@ +2072,9 @@
+  let Content = [{
+Use ``overallocated`` to indicate a class or union can have extra memory
+allocated at its end. This attribute is primarily used when we want
+__builtin_object_size to return a conservative value for the distance between
+the pointer and the end of the subobject the pointer points to.
+
+For example:
+
+.. code-block:: c++

No, this approach is not reasonable. Just changing what `__builtin_object_size` 
returns does not change the fact that code that tries to use bytes off the end 
of the struct would have undefined behavior. Lying in the result of 
`__builtin_object_size` is actively harmful.

Note that in your example below, you cannot access more than four `char`s 
through `((struct S*)p)->b`, despite the attribute, because the attribute does 
not affect the behaviour of the array member of `S`.

The right thing to do here would presumably be to have an attribute that makes 
an array be treated as a flexible array member, *even if* its bound is 
specified (and greater than 0). This would affect `__builtin_object_size`, 
sanitizers, alias analysis, diagnostics for flexible array members in the 
middle of a type, and so on.


http://reviews.llvm.org/D21453



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21453: Add support for attribute "overallocated"

[Aaron Ballman via cfe-commits]

On Mon, Jun 27, 2016 at 2:07 PM, Duncan P. N. Exon Smith
 wrote:
>
>> On 2016-Jun-27, at 11:02, Aaron Ballman  wrote:
>>
>> aaron.ballman requested changes to this revision.
>> aaron.ballman added a comment.
>> This revision now requires changes to proceed.
>>
>> Missing Sema tests for the attribute.
>>
>>
>> 
>> Comment at: include/clang/Basic/AttrDocs.td:2082
>> @@ +2081,3 @@
>> +
>> +struct S {
>> +  char a[4], char b[4];
>> 
>> I *think* this code example will give doxygen fits because it's not 
>> indented; have you tried generating the docs locally? You can test this by 
>> running: `clang-tblgen -gen-attr-docs -I E:\llvm\llvm\tools\clang\include 
>> E:\llvm\llvm\tools\clang\include\clang\Basic\Attr.td -o 
>> E:\llvm\llvm\tools\clang\docs\AttributeReference.rst` and then building the 
>> docs as normal. Note, you will have to revert AttributeReference.rst when 
>> you are done.
>>
>> 
>> Comment at: lib/AST/ExprConstant.cpp:1051-1057
>> @@ -1050,4 +1050,9 @@
>> CharUnits Offset;
>> bool InvalidBase : 1;
>> -unsigned CallIndex : 31;
>> +
>> +// Indicates the enclosing struct is marked overallocated. This is used 
>> in
>> +// computation of __builtin_object_size.
>> +bool OverAllocated : 1;
>> +
>> +unsigned CallIndex : 30;
>> SubobjectDesignator Designator;
>> 
>> All three of these should be using `unsigned`, otherwise you do not get the 
>> behavior you expect in MSVC (it allocates bit-fields of different types on 
>> their own boundaries).
>
> It looks like it's already bloated for MSVC because of `InvalidBase`.  I
> think that should be cleaned up in a prep commit.

Agreed.

~Aaron

>
>>
>>
>> http://reviews.llvm.org/D21453
>>
>>
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21453: Add support for attribute "overallocated"

[Duncan P. N. Exon Smith via cfe-commits]

> On 2016-Jun-27, at 11:02, Aaron Ballman  wrote:
> 
> aaron.ballman requested changes to this revision.
> aaron.ballman added a comment.
> This revision now requires changes to proceed.
> 
> Missing Sema tests for the attribute.
> 
> 
> 
> Comment at: include/clang/Basic/AttrDocs.td:2082
> @@ +2081,3 @@
> +
> +struct S {
> +  char a[4], char b[4];
> 
> I *think* this code example will give doxygen fits because it's not indented; 
> have you tried generating the docs locally? You can test this by running: 
> `clang-tblgen -gen-attr-docs -I E:\llvm\llvm\tools\clang\include 
> E:\llvm\llvm\tools\clang\include\clang\Basic\Attr.td -o 
> E:\llvm\llvm\tools\clang\docs\AttributeReference.rst` and then building the 
> docs as normal. Note, you will have to revert AttributeReference.rst when you 
> are done.
> 
> 
> Comment at: lib/AST/ExprConstant.cpp:1051-1057
> @@ -1050,4 +1050,9 @@
> CharUnits Offset;
> bool InvalidBase : 1;
> -unsigned CallIndex : 31;
> +
> +// Indicates the enclosing struct is marked overallocated. This is used 
> in
> +// computation of __builtin_object_size.
> +bool OverAllocated : 1;
> +
> +unsigned CallIndex : 30;
> SubobjectDesignator Designator;
> 
> All three of these should be using `unsigned`, otherwise you do not get the 
> behavior you expect in MSVC (it allocates bit-fields of different types on 
> their own boundaries).

It looks like it's already bloated for MSVC because of `InvalidBase`.  I
think that should be cleaned up in a prep commit.

> 
> 
> http://reviews.llvm.org/D21453
> 
> 
> 

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21453: Add support for attribute "overallocated"

[Aaron Ballman via cfe-commits]

aaron.ballman requested changes to this revision.
aaron.ballman added a comment.
This revision now requires changes to proceed.

Missing Sema tests for the attribute.



Comment at: include/clang/Basic/AttrDocs.td:2082
@@ +2081,3 @@
+
+struct S {
+  char a[4], char b[4];

I *think* this code example will give doxygen fits because it's not indented; 
have you tried generating the docs locally? You can test this by running: 
`clang-tblgen -gen-attr-docs -I E:\llvm\llvm\tools\clang\include 
E:\llvm\llvm\tools\clang\include\clang\Basic\Attr.td -o 
E:\llvm\llvm\tools\clang\docs\AttributeReference.rst` and then building the 
docs as normal. Note, you will have to revert AttributeReference.rst when you 
are done.


Comment at: lib/AST/ExprConstant.cpp:1051-1057
@@ -1050,4 +1050,9 @@
 CharUnits Offset;
 bool InvalidBase : 1;
-unsigned CallIndex : 31;
+
+// Indicates the enclosing struct is marked overallocated. This is used in
+// computation of __builtin_object_size.
+bool OverAllocated : 1;
+
+unsigned CallIndex : 30;
 SubobjectDesignator Designator;

All three of these should be using `unsigned`, otherwise you do not get the 
behavior you expect in MSVC (it allocates bit-fields of different types on 
their own boundaries).


http://reviews.llvm.org/D21453



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21453: Add support for attribute "overallocated"

[Akira Hatanaka via cfe-commits]

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

Address review comments and change the wording in AttrDocs.td to explain what 
the attribute means and how it is used. Also, fixed the code in VisitMemberExpr 
to set LValue::OverAllocated before the base class of the member expression is 
visited. This change was needed because the base class of the innermost 
MemberExpr has to be examined to see whether the member belongs to an 
overallocated class. Without this change, the last check in 
test/CodeGen/object-size.c fails.


http://reviews.llvm.org/D21453

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGen/object-size.c
  test/CodeGen/object-size.cpp

Index: test/CodeGen/object-size.cpp
===
--- test/CodeGen/object-size.cpp
+++ test/CodeGen/object-size.cpp
@@ -62,3 +62,29 @@
   // CHECK: store i32 16
   gi = __builtin_object_size(>bs[0].buf[0], 3);
 }
+
+struct S0 {
+  int a[16], b[16];
+} __attribute__((overallocated));
+
+struct S1 : S0 {
+};
+
+struct S2 : S1 {
+} __attribute__((overallocated));
+
+// CHECK-LABEL: define void @_Z5test3v()
+void test3() {
+  struct S0 *s0;
+  struct S1 *s1;
+  struct S2 *s2;
+
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(s0->b, 1);
+
+  // CHECK: store i32 64, i32* @gi
+  gi = __builtin_object_size(s1->b, 1);
+
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(s2->b, 1);
+}
Index: test/CodeGen/object-size.c
===
--- test/CodeGen/object-size.c
+++ test/CodeGen/object-size.c
@@ -517,3 +517,52 @@
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size([9].snd[0], 1);
 }
+
+union U0 {
+  int a[16], b[16];
+} __attribute__((overallocated));
+
+struct S0 {
+  int a[16], b[16];
+};
+
+struct S1 {
+  int a[16], b[16];
+} __attribute__((overallocated));
+
+struct S2 {
+  int a[16], b[16];
+  struct S1 s1;
+  struct S0 s0;
+} __attribute__((overallocated));
+
+// CHECK-LABEL: @test32
+void test32() {
+  union U0 *u0;
+  struct S1 *s1;
+  struct S2 *s2;
+
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(u0->a, 1);
+
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(u0->b, 1);
+
+  // CHECK: store i32 64, i32* @gi
+  gi = __builtin_object_size(s1->a, 1);
+
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(s1->b, 1);
+
+  // CHECK: store i32 64, i32* @gi
+  gi = __builtin_object_size(s2->b, 1);
+
+  // CHECK: store i32 64, i32* @gi
+  gi = __builtin_object_size(s2->s1.b, 1);
+
+  // CHECK: store i32 128, i32* @gi
+  gi = __builtin_object_size(>s0, 1);
+
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(s2->s0.b, 1);
+}
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5394,6 +5394,9 @@
   case AttributeList::AT_X86ForceAlignArgPointer:
 handleX86ForceAlignArgPointerAttr(S, D, Attr);
 break;
+  case AttributeList::AT_OverAllocated:
+handleSimpleAttribute(S, D, Attr);
+break;
   case AttributeList::AT_DLLExport:
   case AttributeList::AT_DLLImport:
 handleDLLAttr(S, D, Attr);
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -1049,7 +1049,12 @@
 APValue::LValueBase Base;
 CharUnits Offset;
 bool InvalidBase : 1;
-unsigned CallIndex : 31;
+
+// Indicates the enclosing struct is marked overallocated. This is used in
+// computation of __builtin_object_size.
+bool OverAllocated : 1;
+
+unsigned CallIndex : 30;
 SubobjectDesignator Designator;
 
 const APValue::LValueBase getLValueBase() const { return Base; }
@@ -1059,6 +1064,8 @@
 SubobjectDesignator () { return Designator; }
 const SubobjectDesignator () const { return Designator;}
 
+LValue() : OverAllocated(false) {}
+
 void moveInto(APValue ) const {
   if (Designator.Invalid)
 V = APValue(Base, Offset, APValue::NoLValuePath(), CallIndex);
@@ -4559,19 +4566,34 @@
 
   bool VisitMemberExpr(const MemberExpr *E) {
 // Handle non-static data members.
+
+// Check to see if the record is marked overallocated.
+auto IsOverAllocated = [](QualType RT, const MemberExpr *ME) {
+  const TagDecl *TD = RT->getAsTagDecl();
+
+  if (isa(TD))
+TD = ME->getBase()->getBestDynamicClassType();
+
+  return TD->hasAttr();
+};
+
 QualType BaseTy;
 bool EvalOK;
 if (E->isArrow()) {
-  EvalOK = 

Re: [PATCH] D21453: Add support for attribute "overallocated"

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: include/clang/Basic/Attr.td:2279
@@ +2278,3 @@
+  let Spellings = [GNU<"overallocated">, CXX11<"clang", "overallocated">];
+  let Subjects = SubjectList<[Record], ErrorDiag, "ExpectedStructOrUnion">;
+  let Documentation = [OverAllocatedDocs];

Can drop the "Expected" string literal. A record type should automatically do 
the right thing.


Comment at: lib/Sema/SemaDeclAttr.cpp:4932
@@ -4931,1 +4931,3 @@
 
+static void handleOverAllocatedAttr(Sema , Decl *D, const AttributeList 
) {
+  D->addAttr(::new (S.Context)

This isn't required.


Comment at: lib/Sema/SemaDeclAttr.cpp:5404
@@ +5403,3 @@
+  case AttributeList::AT_OverAllocated:
+handleOverAllocatedAttr(S, D, Attr);
+break;

Can just call `handleSimpleAttribute()` instead.


http://reviews.llvm.org/D21453



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21453: Add support for attribute "overallocated"

[Akira Hatanaka via cfe-commits]

ahatanak added inline comments.


Comment at: include/clang/Basic/AttrDocs.td:2073-2079
@@ +2072,9 @@
+  let Content = [{
+Use ``overallocated`` to indicate a struct or union is over-allocated. For 
example,
+
+.. code-block:: c++
+
+struct S {
+  char a[4], char b[4];
+} __attribute__((overallocated));
+

I intended overallocated to mean there might be extra memory at the end of a 
struct or union regardless of whether or not the last member is an array. This 
seemed more useful than restricting it to structs with array members as was 
discussed in the cfe-dev thread. For example, Hal mentioned that we might have 
caught the bugs you fixed in r262891 had we used this attribute on 
over-allocated classes.

Does this sound reasonable to you or do you see any problem with this approach? 
Alternatively, we can use an attribute (variable_length_array or gcc's 
bnd_variable_size) that will be attached to the array, which is good enough for 
the use case we care about (we want __builtin_object_size to return an exact or 
a conservative value for struct sockaddr_un that was mentioned in the thread).


http://reviews.llvm.org/D21453



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D21453: Add support for attribute "overallocated"

[Akira Hatanaka via cfe-commits]

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

Fix a bug in tryEvaluateBuiltinObjectSize. If the pointer passed to 
__builtin_object_size doesn't point to an array, it should be able to compute 
the exact size of the subobject the pointer points to. Therefore, it should be 
able to tell the last call to __builtin_object_size in 
test/CodeGen/object-size.c should return 128.


http://reviews.llvm.org/D21453

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/AttrDocs.td
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaDeclAttr.cpp
  test/CodeGen/object-size.c
  test/CodeGen/object-size.cpp

Index: test/CodeGen/object-size.cpp
===
--- test/CodeGen/object-size.cpp
+++ test/CodeGen/object-size.cpp
@@ -62,3 +62,29 @@
   // CHECK: store i32 16
   gi = __builtin_object_size(>bs[0].buf[0], 3);
 }
+
+struct S0 {
+  int a[16], b[16];
+} __attribute__((overallocated));
+
+struct S1 : S0 {
+};
+
+struct S2 : S1 {
+} __attribute__((overallocated));
+
+// CHECK-LABEL: define void @_Z5test3v()
+void test3() {
+  struct S0 *s0;
+  struct S1 *s1;
+  struct S2 *s2;
+
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(s0->b, 1);
+
+  // CHECK: store i32 64, i32* @gi
+  gi = __builtin_object_size(s1->b, 1);
+
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(s2->b, 1);
+}
Index: test/CodeGen/object-size.c
===
--- test/CodeGen/object-size.c
+++ test/CodeGen/object-size.c
@@ -517,3 +517,49 @@
   // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
   gi = __builtin_object_size([9].snd[0], 1);
 }
+
+union U0 {
+  int a[16], b[16];
+} __attribute__((overallocated));
+
+struct S0 {
+  int a[16], b[16];
+};
+
+struct S1 {
+  int a[16], b[16];
+} __attribute__((overallocated));
+
+struct S2 {
+  int a[16], b[16];
+  struct S1 s1;
+  struct S0 s0;
+} __attribute__((overallocated));
+
+// CHECK-LABEL: @test32
+void test32() {
+  union U0 *u0;
+  struct S1 *s1;
+  struct S2 *s2;
+
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(u0->a, 1);
+
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(u0->b, 1);
+
+  // CHECK: store i32 64, i32* @gi
+  gi = __builtin_object_size(s1->a, 1);
+
+  // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false)
+  gi = __builtin_object_size(s1->b, 1);
+
+  // CHECK: store i32 64, i32* @gi
+  gi = __builtin_object_size(s2->b, 1);
+
+  // CHECK: store i32 64, i32* @gi
+  gi = __builtin_object_size(s2->s1.b, 1);
+
+  // CHECK: store i32 128, i32* @gi
+  gi = __builtin_object_size(>s0, 1);
+}
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -4929,6 +4929,12 @@
   }
 }
 
+static void handleOverAllocatedAttr(Sema , Decl *D, const AttributeList ) {
+  D->addAttr(::new (S.Context)
+ OverAllocatedAttr(Attr.getLoc(), S.Context,
+   Attr.getAttributeSpellingListIndex()));
+}
+
 static void handleAMDGPUNumVGPRAttr(Sema , Decl *D,
 const AttributeList ) {
   uint32_t NumRegs;
@@ -5394,6 +5400,9 @@
   case AttributeList::AT_X86ForceAlignArgPointer:
 handleX86ForceAlignArgPointerAttr(S, D, Attr);
 break;
+  case AttributeList::AT_OverAllocated:
+handleOverAllocatedAttr(S, D, Attr);
+break;
   case AttributeList::AT_DLLExport:
   case AttributeList::AT_DLLImport:
 handleDLLAttr(S, D, Attr);
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -1049,7 +1049,12 @@
 APValue::LValueBase Base;
 CharUnits Offset;
 bool InvalidBase : 1;
-unsigned CallIndex : 31;
+
+// Indicates the enclosing struct is marked overallocated. This is used in
+// computation of __builtin_object_size.
+bool OverAllocated = 1;
+
+unsigned CallIndex : 30;
 SubobjectDesignator Designator;
 
 const APValue::LValueBase getLValueBase() const { return Base; }
@@ -1059,6 +1064,8 @@
 SubobjectDesignator () { return Designator; }
 const SubobjectDesignator () const { return Designator;}
 
+LValue() : OverAllocated(false) {}
+
 void moveInto(APValue ) const {
   if (Designator.Invalid)
 V = APValue(Base, Offset, APValue::NoLValuePath(), CallIndex);
@@ -4572,6 +4579,15 @@
   EvalOK = this->Visit(E->getBase());
   BaseTy = E->getBase()->getType();
 }
+
+// Check to see if the parent record is marked overallocated.
+const TagDecl *TD = BaseTy->getAsTagDecl();
+
+if (isa(TD))
+  TD = E->getBase()->getBestDynamicClassType();
+
+Result.OverAllocated = TD->hasAttr();
+
 if 

Re: [PATCH] D21453: Add support for attribute "overallocated"

[Richard Smith via cfe-commits]

rsmith added inline comments.


Comment at: include/clang/Basic/AttrDocs.td:2073-2079
@@ +2072,9 @@
+  let Content = [{
+Use ``overallocated`` to indicate a struct or union is over-allocated. For 
example,
+
+.. code-block:: c++
+
+struct S {
+  char a[4], char b[4];
+} __attribute__((overallocated));
+

I don't think this says what you mean. "Overallocated" alone does not imply 
that you can use S::b to access more than 4 bytes. If you want that to work, I 
think the right model is for the attribute to imply that the final member 
within the struct is treated as a flexible array member, no matter what its 
array bound is.

This should then apply to everywhere we consider flexible array members, not 
just to builtin_object_size.


http://reviews.llvm.org/D21453



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits