> On Apr 27, 2014, at 10:09 AM, Prathamesh Kulkarni > <bilbotheelffri...@gmail.com> wrote: > >> On Sun, Apr 27, 2014 at 8:48 PM, Trevor Saunders <tsaund...@mozilla.com> >> wrote: >>> On Sun, Apr 27, 2014 at 06:21:20PM +0530, Prathamesh Kulkarni wrote: >>>> On Sun, Apr 27, 2014 at 5:31 PM, Trevor Saunders <tsaund...@mozilla.com> >>>> wrote: >>>>> On Sun, Apr 27, 2014 at 02:31:46AM +0530, Prathamesh Kulkarni wrote: >>>>> Hi, >>>>> Shall it a good idea to add new warning -Wsizeof-array-argument that >>>>> warns when sizeof is applied on parameter declared as an array ? >>>> >>>> Seems reasonable enough. >>>> >>>>> Similar to clang's -Wsizeof-array-argument: >>>>> http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20110613/042812.html >>>>> This was also reported as PR6940: >>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=6940 >>>>> >>>>> I have attached a patch that adds the warning to C front-end. >>>> >>>> if we're doing this for C, we should probably do it for C++ too. >>>> >>>>> I implemented it by adding a new member BOOL_BITFIELD is_array_parm to >>>>> tree_parm_decl. Not sure if that's a good approach. >>>> >>>> I'm about the last one who should comment on this, but it seems pretty >>>> crazy you can't use data that's already stored. >>> AFAIU, the information about declarator is stored in c_declarator. >>> c_declarator->kind == cdk_array holds true >>> if the declarator is an array. However in push_parm_decl, call to >>> grokdeclarator returns decl of pointer_type, corresponding to array >>> declarator if the array is parameter (TREE_TYPE (decl) is >>> pointer_type). So I guess we lose that information there. >> >> I guess that sort of makes sense, so I'll shut up ;) >> >>>>> Index: gcc/tree-core.h >>>>> =================================================================== >>>>> --- gcc/tree-core.h (revision 209800) >>>>> +++ gcc/tree-core.h (working copy) >>>>> @@ -1411,6 +1411,7 @@ struct GTY(()) tree_const_decl { >>>>> struct GTY(()) tree_parm_decl { >>>>> struct tree_decl_with_rtl common; >>>>> rtx incoming_rtl; >>>>> + BOOL_BITFIELD is_array_parm; >>>> >>>> BOOL_BITFIELD only makes sense if you declare it as an actually bitfield >>>> with size less than that of unisgned int, otherwise you might as well >>>> use that directly. On the other hand I wonder if we can't just nuke >>>> BOOL_BITFIELD, it seems to be legacy from a time of C and bool not being >>>> a builtin type? >>> Thanks, I wasn't aware of that. Could we now use C++ bool type instead ? >> >> you can certainly do |bool x;| as struct fields, that's already all >> over. However its not entirely clear to me if |bool x : 1;| will work >> everywhere and take the single bit you'd expect, istr there being >> compilers that do stupid things if you use multiple types next to each >> other in bitfields, but I'm not sure if we need to care about any of >> those. > Changed to bool is_array_parm; > and from macros to inline functions.
I don't like this field being part of the generic code as it increases the size of the struct for all front-ends and even during LTO. Is there a way to do this using one of the language specific bitfields that are already there (language flags iirc)? Thanks, Andrew > > [gcc] > * tree-core.h (tree_parm_decl): Add new member bool is_array_parm > * tree.h (set_parm_decl_is_array): New function. > (parm_decl_array_p): New function. > > [gcc/c] > * c-decl.c (push_parm_decl): Call set_parm_decl_is_array. > * c-typeck.c (c_expr_sizeof_expr): Add check for sizeof-array-argument > warning. > > [gcc/c-family] > * c.opt (-Wsizeof-array-argument): New option. > > [gcc/testsuite/gcc.dg] > * sizeof-array-argument.c: New test-case. > > Thanks and Regards, > Prathamesh >> >> Trev >> >>>> >>>>> Index: gcc/tree.h >>>>> =================================================================== >>>>> --- gcc/tree.h (revision 209800) >>>>> +++ gcc/tree.h (working copy) >>>>> @@ -1742,6 +1742,7 @@ extern void protected_set_expr_location >>>>> #define TYPE_LANG_SPECIFIC(NODE) \ >>>>> (TYPE_CHECK (NODE)->type_with_lang_specific.lang_specific) >>>>> >>>>> + >>>>> #define TYPE_VALUES(NODE) (ENUMERAL_TYPE_CHECK >>>>> (NODE)->type_non_common.values) >>>>> #define TYPE_DOMAIN(NODE) (ARRAY_TYPE_CHECK >>>>> (NODE)->type_non_common.values) >>>>> #define TYPE_FIELDS(NODE) \ >>>>> @@ -2258,6 +2259,12 @@ extern void decl_value_expr_insert (tree >>>>> #define DECL_INCOMING_RTL(NODE) \ >>>>> (PARM_DECL_CHECK (NODE)->parm_decl.incoming_rtl) >>>>> >>>>> +#define SET_PARM_DECL_IS_ARRAY(NODE, val) \ >>>>> + (PARM_DECL_CHECK (NODE)->parm_decl.is_array_parm = (val)) >>>> >>>> if we're adding more stuff here is there a reason it needs to be a macro >>>> not a inline function? >>> No, shall change that to inline function. >>>> >>>> Trev > <sizeof-array-argument.patch>
