Re: Fix for PR objc/47832 ("[4.6 Regression] ObjC errors on structures with flexible data members")
Nicola Pero
nicola.pero@meta-innovation.com
Tue Feb 22 03:59:00 GMT 2011
> It doesn't really do what you claim you want to do:
>
> + error ("instance variable %qs has unknown size", ivar_name);
>
You are right; the error message is incorrect. Thanks for spotting that. :-)
It should be
error ("instance variable %qs uses flexible array member", ivar_name);
I did look at the actual code, and having clarified that we are trying to detect
flexible array members being used in instance variables, it seems that the only
way to do it is using flexible_array_type_p(), as my patch did. Checking
C_TYPE_VARIABLE_SIZE won't work (experimentally, and supported by the fact that
c-decl.c uses flexible_array_type_p() for this check too!). Checking COMPLETE_TYPE_P
won't work either; we already have that check in there as a separate check and it
catches stuff such as 'int[]', but not 'struct { int x; int[] y; }'.
This also clarifies why this is not appropriate for C++. There are no flexible array
members in C++, so there is nothing to check for. ;-)
I added more testcases to test more; in particular, while trying more testcases,
I noticed yet another case that had escaped --
typedef struct
{
unsigned long int a;
double b[];
} test_type;
@interface A
{
test_type d[4];
}
@end
This should also be rejected. This revised patch has an additional testcase detecting
this as well, and the code has been fixed to detect it properly. I also updated the
comments.
Ok to commit ?
Thanks
Index: objc/ChangeLog
===================================================================
--- objc/ChangeLog (revision 170370)
+++ objc/ChangeLog (working copy)
@@ -1,9 +1,18 @@
+2011-02-22 Nicola Pero <nicola.pero@meta-innovation.com>
+
+ PR objc/47832
+ * objc-act.c (flexible_array_type_p): New.
+ (add_instance_variable): Produce an error if an instance variable
+ uses flexible array members.
+ (encode_array): Do not emit an error if encoding a flexible array
+ type while generating instance variables.
+
2011-02-20 Nicola Pero <nicola.pero@meta-innovation.com>
- * objc-gnu-runtime-abi-01.c (TARGET_64BIT): Removed. Removed
- usage of padding fields. Do not include tm.h.
- * objc-act.c (objc_write_global_declaration): Set input_location
- to BUILTINS_LOCATION while generating runtime metadata.
+ * objc-gnu-runtime-abi-01.c (TARGET_64BIT): Removed. Removed
+ usage of padding fields. Do not include tm.h.
+ * objc-act.c (objc_write_global_declaration): Set input_location
+ to BUILTINS_LOCATION while generating runtime metadata.
2011-01-20 Nicola Pero <nicola.pero@meta-innovation.com>
Index: objc/objc-act.c
===================================================================
--- objc/objc-act.c (revision 170370)
+++ objc/objc-act.c (working copy)
@@ -5925,6 +5925,58 @@ add_category (tree klass, tree category)
}
}
+#ifndef OBJCPLUS
+/* A flexible array member is a C99 extension where you can use
+ "type[]" at the end of a struct to mean a variable-length array.
+
+ In Objective-C, instance variables are fundamentally members of a
+ struct, but the struct can always be extended by subclassing; hence
+ we need to detect and forbid all instance variables declared using
+ flexible array members.
+
+ No check for this is needed in Objective-C++, since C++ does not
+ have flexible array members. */
+
+/* Determine whether TYPE is a structure with a flexible array member,
+ a union containing such a structure (possibly recursively) or an
+ array of such structures or unions. These are all invalid as
+ instance variable. */
+static bool
+flexible_array_type_p (tree type)
+{
+ tree x;
+ switch (TREE_CODE (type))
+ {
+ case RECORD_TYPE:
+ x = TYPE_FIELDS (type);
+ if (x == NULL_TREE)
+ return false;
+ while (DECL_CHAIN (x) != NULL_TREE)
+ x = DECL_CHAIN (x);
+ if (TREE_CODE (TREE_TYPE (x)) == ARRAY_TYPE
+ && TYPE_SIZE (TREE_TYPE (x)) == NULL_TREE
+ && TYPE_DOMAIN (TREE_TYPE (x)) != NULL_TREE
+ && TYPE_MAX_VALUE (TYPE_DOMAIN (TREE_TYPE (x))) == NULL_TREE)
+ return true;
+ return false;
+ case UNION_TYPE:
+ for (x = TYPE_FIELDS (type); x != NULL_TREE; x = DECL_CHAIN (x))
+ {
+ if (flexible_array_type_p (TREE_TYPE (x)))
+ return true;
+ }
+ return false;
+ /* Note that we also check for arrays of something that uses a flexible array member. */
+ case ARRAY_TYPE:
+ if (flexible_array_type_p (TREE_TYPE (type)))
+ return true;
+ return false;
+ default:
+ return false;
+ }
+}
+#endif
+
/* Called after parsing each instance variable declaration. Necessary to
preserve typedefs and implement public/private...
@@ -5958,6 +6010,27 @@ add_instance_variable (tree klass, objc_ivar_visib
return klass;
}
+#ifndef OBJCPLUS
+ /* Also, in C reject a struct with a flexible array member. Ie,
+
+ struct A { int x; int[] y; };
+
+ @interface X
+ {
+ struct A instance_variable;
+ }
+ @end
+
+ is not valid because if the class is subclassed, we wouldn't be able
+ to calculate the offset of the next instance variable. */
+ if (flexible_array_type_p (field_type))
+ {
+ error ("instance variable %qs uses flexible array member", ivar_name);
+ /* Return class as is without adding this ivar. */
+ return klass;
+ }
+#endif
+
#ifdef OBJCPLUS
/* Check if the ivar being added has a non-POD C++ type. If so, we will
need to either (1) warn the user about it or (2) generate suitable
@@ -9926,27 +9999,23 @@ encode_array (tree type, int curtype, int format)
if (an_int_cst == NULL)
{
/* We are trying to encode an incomplete array. An incomplete
- array is forbidden as part of an instance variable. */
- if (generating_instance_variables)
- {
- /* TODO: Detect this error earlier. */
- error ("instance variable has unknown size");
- return;
- }
+ array is forbidden as part of an instance variable; but it
+ may occur if the instance variable is a pointer to such an
+ array. */
- /* So the only case in which an incomplete array could occur is
- if we are encoding the arguments or return value of a method.
- In that case, an incomplete array argument or return value
- (eg, -(void)display: (char[])string) is treated like a
- pointer because that is how the compiler does the function
- call. A special, more complicated case, is when the
- incomplete array is the last member of a struct (eg, if we
- are encoding "struct { unsigned long int a;double b[];}"),
- which is again part of a method argument/return value. In
- that case, we really need to communicate to the runtime that
- there is an incomplete array (not a pointer!) there. So, we
- detect that special case and encode it as a zero-length
- array.
+ /* So the only case in which an incomplete array could occur
+ (without being pointed to) is if we are encoding the
+ arguments or return value of a method. In that case, an
+ incomplete array argument or return value (eg,
+ -(void)display: (char[])string) is treated like a pointer
+ because that is how the compiler does the function call. A
+ special, more complicated case, is when the incomplete array
+ is the last member of a struct (eg, if we are encoding
+ "struct { unsigned long int a;double b[];}"), which is again
+ part of a method argument/return value. In that case, we
+ really need to communicate to the runtime that there is an
+ incomplete array (not a pointer!) there. So, we detect that
+ special case and encode it as a zero-length array.
Try to detect that we are part of a struct. We do this by
searching for '=' in the type encoding for the current type.
Index: testsuite/ChangeLog
===================================================================
--- testsuite/ChangeLog (revision 170370)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,10 @@
+2011-02-22 Nicola Pero <nicola.pero@meta-innovation.com>
+
+ PR objc/47832
+ * objc.dg/type-size-3.m: Updated error message.
+ * objc.dg/type-size-4.m: New test.
+ * objc.dg/type-size-5.m: New test.
+
2011-02-21 Jeff Law <law@redhat.com>
PR rtl-optimization/46178
Index: testsuite/objc.dg/type-size-3.m
===================================================================
--- testsuite/objc.dg/type-size-3.m (revision 170370)
+++ testsuite/objc.dg/type-size-3.m (working copy)
@@ -1,4 +1,4 @@
-/* Reject ivars with an unknown size. */
+/* Reject ivars that use flexible array members. */
/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com> */
/* { dg-do compile } */
@@ -10,11 +10,9 @@ typedef struct
@interface Test
{
- test_type c;
+ test_type c; /* { dg-error "instance variable .c. uses flexible array member" } */
}
@end
@implementation Test
@end
-
-/* { dg-error "instance variable has unknown size" "" { target *-*-* } 0 } */
Index: testsuite/objc.dg/type-size-4.m
===================================================================
--- testsuite/objc.dg/type-size-4.m (revision 0)
+++ testsuite/objc.dg/type-size-4.m (revision 0)
@@ -0,0 +1,23 @@
+/* Allow ivars that are pointers to structs with an unknown size. */
+/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com> */
+/* PR objc/47832 */
+/* { dg-do compile } */
+
+typedef struct
+{
+ unsigned long int a;
+ double b[];
+} test_type;
+
+@interface Test
+{
+ /* These are all fine. */
+ double *a;
+ struct { int x; double y[]; } *b;
+ test_type *c;
+ union union_type { int x; test_type y; } *d;
+}
+@end
+
+@implementation Test
+@end
Index: testsuite/objc.dg/type-size-5.m
===================================================================
--- testsuite/objc.dg/type-size-5.m (revision 0)
+++ testsuite/objc.dg/type-size-5.m (revision 0)
@@ -0,0 +1,22 @@
+/* Reject ivars that use flexible array members. */
+/* Contributed by Nicola Pero <nicola.pero@meta-innovation.com> */
+/* { dg-do compile } */
+
+typedef struct
+{
+ unsigned long int a;
+ double b[];
+} test_type;
+
+@interface Test
+{
+ double a[]; /* { dg-error "instance variable .a. has unknown size" } */
+ struct { int x; double y[]; } b; /* { dg-error "instance variable .b. uses flexible array member" } */
+ test_type c; /* { dg-error "instance variable .c. uses flexible array member" } */
+ test_type d[4]; /* { dg-error "instance variable .d. uses flexible array member" } */
+ union union_type { int x; test_type y; } e; /* { dg-error "instance variable .e. uses flexible array member" } */
+}
+@end
+
+@implementation Test
+@end
More information about the Gcc-patches
mailing list