GNU C: Implicit int and implicit function definitions
I just spend an hour hunting down someone else's GCC code generation bug, when it turned out it was caused by an implicit function definition, where the real return type was incompatible with int. C99 got rid of implicit function definitions and implicit ints. Would it be possible to remove them retroactively from the -std=gnu99 and -std=gnu11 language variants (as well as -std=c99 and -std=c11), so that they are rejected by default? Thanks, Florian
Re: GNU C: Implicit int and implicit function definitions
Implicit function declarations result in warnings since C99 or GNU99 and '-pedantic-errors' turns them into errors. The same goes for implicit return types. -- Best regards, lh_mouse 2016-05-20 - 发件人：Florian Weimer 发送日期：2016-05-20 16:17 收件人：GCC 抄送： 主题：GNU C: Implicit int and implicit function definitions I just spend an hour hunting down someone else's GCC code generation bug, when it turned out it was caused by an implicit function definition, where the real return type was incompatible with int. C99 got rid of implicit function definitions and implicit ints. Would it be possible to remove them retroactively from the -std=gnu99 and -std=gnu11 language variants (as well as -std=c99 and -std=c11), so that they are rejected by default? Thanks, Florian
Re: GNU C: Implicit int and implicit function definitions
On 05/20/2016 10:30 AM, lh mouse wrote: Implicit function declarations result in warnings since C99 or GNU99 and '-pedantic-errors' turns them into errors. The same goes for implicit return types. The warnings typically do not stop the build, and thus are not really helpful when you are looking at binaries. Florian
Re: GNU C: Implicit int and implicit function definitions
On 05/20/2016 10:02 AM, Florian Weimer wrote: > On 05/20/2016 10:30 AM, lh mouse wrote: >> Implicit function declarations result in warnings since C99 or GNU99 and >> '-pedantic-errors' turns them into errors. >> The same goes for implicit return types. > > The warnings typically do not stop the build, and thus are not really > helpful when you are looking at binaries. C99 Rationale sez: A new feature of C99: In C89, all type specifiers could be omitted from the declaration specifiers in a declaration. In such a case int was implied. The Committee decided that the inherent danger of this feature outweighed its convenience, and so it was removed. The effect is to guarantee the production of a diagnostic that will catch an additional category of programming errors. After issuing the diagnostic, an implementation may choose to assume an implicit int and continue to translate the program in order to support existing source code that exploits this feature. Given this, I do not understand why GCC does not treat implicit int as a hard error. Andrew.
Re: GNU C: Implicit int and implicit function definitions
Florian Weimer writes: > C99 got rid of implicit function definitions and implicit ints. Would it > be possible to remove them retroactively from the -std=gnu99 and > -std=gnu11 language variants (as well as -std=c99 and -std=c11), so that > they are rejected by default? -Werror=implicit-int -Werror=implicit-function-declaration Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: GNU C: Implicit int and implicit function definitions
On 05/20/2016 11:22 AM, Andreas Schwab wrote: Florian Weimer writes: C99 got rid of implicit function definitions and implicit ints. Would it be possible to remove them retroactively from the -std=gnu99 and -std=gnu11 language variants (as well as -std=c99 and -std=c11), so that they are rejected by default? -Werror=implicit-int -Werror=implicit-function-declaration I want the default changed. (I thought this was sufficiently clear from my message.) Florian
Re: increase alignment of global structs in increase_alignment pass
On 19 May 2016 at 13:19, Richard Biener wrote:
> On Thu, 19 May 2016, Prathamesh Kulkarni wrote:
>
>> On 18 May 2016 at 19:38, Richard Biener wrote:
>> > On Wed, 18 May 2016, Prathamesh Kulkarni wrote:
>> >
>> >> On 17 May 2016 at 18:36, Richard Biener wrote:
>> >> > On Wed, 11 May 2016, Prathamesh Kulkarni wrote:
>> >> >
>> >> >> On 6 May 2016 at 17:20, Richard Biener wrote:
>> >> >> >
>> >> >> > You can't simply use
>> >> >> >
>> >> >> > + offset = int_byte_position (field);
>> >> >> >
>> >> >> > as it can be placed at variable offset which will make
>> >> >> > int_byte_position
>> >> >> > ICE. Note it also returns a truncated byte position (bit position
>> >> >> > stripped) which may be undesirable here. I think you want to use
>> >> >> > bit_position and if and only if DECL_FIELD_OFFSET and
>> >> >> > DECL_FIELD_BIT_OFFSET are INTEGER_CST.
>> >> >> oops, I didn't realize offsets could be variable.
>> >> >> Will that be the case only for VLA member inside struct ?
>> >> >
>> >> > And non-VLA members after such member.
>> >> >
>> >> >> > Your observation about the expensiveness of the walk still stands I
>> >> >> > guess
>> >> >> > and eventually you should at least cache the
>> >> >> > get_vec_alignment_for_record_decl cases. Please make those workers
>> >> >> > _type rather than _decl helpers.
>> >> >> Done
>> >> >> >
>> >> >> > You seem to simply get at the maximum vectorized field/array element
>> >> >> > alignment possible for all arrays - you could restrict that to
>> >> >> > arrays with at least vector size (as followup).
>> >> >> Um sorry, I didn't understand this part.
>> >> >
>> >> > It doesn't make sense to align
>> >> >
>> >> > struct { int a; int b; int c; int d; float b[3]; int e; };
>> >> >
>> >> > because we have a float[3] member. There is no vector size that
>> >> > would cover the float[3] array.
>> >> Thanks for the explanation.
>> >> So we do not want to align struct if sizeof (array_field) < sizeof
>> >> (vector_type).
>> >> This issue is also present without patch for global arrays, so I modified
>> >> get_vec_alignment_for_array_type, to return 0 if sizeof (array_type) <
>> >> sizeof (vectype).
>> >> >
>> >> >> >
>> >> >> > + /* Skip artificial decls like typeinfo decls or if
>> >> >> > + record is packed. */
>> >> >> > + if (DECL_ARTIFICIAL (record_decl) || TYPE_PACKED (type))
>> >> >> > +return 0;
>> >> >> >
>> >> >> > I think we should honor DECL_USER_ALIGN as well and not mess with
>> >> >> > those
>> >> >> > decls.
>> >> >> Done
>> >> >> >
>> >> >> > Given the patch now does quite some extra work it might make sense
>> >> >> > to split the symtab part out of the vect_can_force_dr_alignment_p
>> >> >> > predicate and call that early.
>> >> >> In the patch I call symtab_node::can_increase_alignment_p early. I
>> >> >> tried
>> >> >> moving that to it's callers - vect_compute_data_ref_alignment and
>> >> >> increase_alignment::execute, however that failed some tests in vect,
>> >> >> and
>> >> >> hence I didn't add the following hunk in the patch. Did I miss some
>> >> >> check ?
>> >> >
>> >> > Not sure.
>> >> >
>> >> >> diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
>> >> >> index 7652e21..2c1acee 100644
>> >> >> --- a/gcc/tree-vect-data-refs.c
>> >> >> +++ b/gcc/tree-vect-data-refs.c
>> >> >> @@ -795,7 +795,10 @@ vect_compute_data_ref_alignment (struct
>> >> >> data_reference *dr)
>> >> >>&& TREE_CODE (TREE_OPERAND (base, 0)) == ADDR_EXPR)
>> >> >> base = TREE_OPERAND (TREE_OPERAND (base, 0), 0);
>> >> >>
>> >> >> - if (!vect_can_force_dr_alignment_p (base, TYPE_ALIGN (vectype)))
>> >> >> + if (!(TREE_CODE (base) == VAR_DECL
>> >> >> +&& decl_in_symtab_p (base)
>> >> >> +&& symtab_node::get (base)->can_increase_alignment_p ()
>> >> >> +&& vect_can_force_dr_alignment_p (base, TYPE_ALIGN
>> >> >> (vectype
>> >> >> {
>> >> >>if (dump_enabled_p ())
>> >> >> {
>> >> >
>> >> > + for (tree field = first_field (type);
>> >> > + field != NULL_TREE;
>> >> > + field = DECL_CHAIN (field))
>> >> > +{
>> >> > + /* Skip if not FIELD_DECL or has variable offset. */
>> >> > + if (TREE_CODE (field) != FIELD_DECL
>> >> > + || TREE_CODE (DECL_FIELD_OFFSET (field)) != INTEGER_CST
>> >> > + || TREE_CODE (DECL_FIELD_BIT_OFFSET (field)) != INTEGER_CST
>> >> > + || DECL_USER_ALIGN (field)
>> >> > + || DECL_ARTIFICIAL (field))
>> >> > + continue;
>> >> >
>> >> > you can stop processing the type and return 0 here if the offset
>> >> > is not an INTEGER_CST. All following fields will have the same issue.
>> >> >
>> >> > + /* FIXME: is this check necessary since we check for variable
>> >> > offset above ? */
>> >> > + if (TREE_CODE (offset_tree) != INTEGER_CST)
>> >> > + continue;
>> >> >
>> >> > the check should not be necessary.
>> >> >
>> >> > offset = tree_to_uhwi (offset_tree);
>> >> >
>> >> >
Re: GNU C: Implicit int and implicit function definitions
On 05/20/2016 03:24 AM, Florian Weimer wrote: On 05/20/2016 11:22 AM, Andreas Schwab wrote: Florian Weimer writes: C99 got rid of implicit function definitions and implicit ints. Would it be possible to remove them retroactively from the -std=gnu99 and -std=gnu11 language variants (as well as -std=c99 and -std=c11), so that they are rejected by default? -Werror=implicit-int -Werror=implicit-function-declaration I want the default changed. (I thought this was sufficiently clear from my message.) I think it's worth revisiting as well, burying in -pedantic seems wrong given the kinds of failures we can see. jeff
Re: Question regarding bug 70584
(reposting in gcc@ and adding more information) On Fri, May 20, 2016 at 3:43 PM, Andres Tiraboschi wrote: > While analysing this bug we arrived to the following code at > tree.c:145 (lvalue_kind): > > case VAR_DECL: > if (TREE_READONLY (ref) && ! TREE_STATIC (ref) > && DECL_LANG_SPECIFIC (ref) > && DECL_IN_AGGR_P (ref)) > return clk_none; > > That condition fails so a fall-through to the next case labels causes > to return clk_ordinary, whereas this is about a constexpr value > (rather than a reference). > > As an experiment, we forced the condition above to return clk_none and > the bug is not reproduced. > > We are suspecting either that the condition is too restrictive or a > fall-through is not intended. Why is the condition requiring > DECL_IN_AGGR_P? Just to provide more information: DECL_LANG_SPECIFIC is NULL and DECL_IN_AGGR_P is false. Can somebody provide the rationale of the condition? Thanks, Daniel. > > Thank, > > Andres. -- Daniel F. Gutson Engineering Manager San Lorenzo 47, 3rd Floor, Office 5 Córdoba, Argentina Phone: +54 351 4217888 / +54 351 4218211 Skype:dgutson LinkedIn: http://ar.linkedin.com/in/danielgutson
Re: Question regarding bug 70584
On 05/20/2016 01:18 PM, Daniel Gutson wrote: (reposting in gcc@ and adding more information) On Fri, May 20, 2016 at 3:43 PM, Andres Tiraboschi wrote: While analysing this bug we arrived to the following code at tree.c:145 (lvalue_kind): case VAR_DECL: if (TREE_READONLY (ref) && ! TREE_STATIC (ref) && DECL_LANG_SPECIFIC (ref) && DECL_IN_AGGR_P (ref)) return clk_none; That condition fails so a fall-through to the next case labels causes to return clk_ordinary, whereas this is about a constexpr value (rather than a reference). As an experiment, we forced the condition above to return clk_none and the bug is not reproduced. We are suspecting either that the condition is too restrictive or a fall-through is not intended. Why is the condition requiring DECL_IN_AGGR_P? Just to provide more information: DECL_LANG_SPECIFIC is NULL and DECL_IN_AGGR_P is false. Can somebody provide the rationale of the condition? I'm not really an expert in this code, but it looks like we're returning clk_none for a small subset of nodes that aren't really lvalues. Examples would be certain read-only objects which can't be lvalues. Other VAR_DECLs would be lvalues and should probably return clk_ordinary. At least that how it appears to me. Jeff
Re: GNU C: Implicit int and implicit function definitions
On Fri, 20 May 2016, Jeff Law wrote: > I think it's worth revisiting as well, burying in -pedantic seems wrong given > the kinds of failures we can see. It's not in -pedantic. The warnings are on by default for C99/C11 (and become errors with -pedantic-errors or the -Werror= options). -- Joseph S. Myers jos...@codesourcery.com
Re: GNU C: Implicit int and implicit function definitions
On Fri, 20 May 2016, Andrew Haley wrote: > Given this, I do not understand why GCC does not treat implicit int as > a hard error. Because in C the existing practice has been that we support the union of all features and extensions that can sensibly be supported with the given language version (these are warnings (pedwarns) by default in C99 mode, not restricted to -pedantic). It's possible that C code used in practice has changed sufficiently over the years that continuing to build legacy code using these features (while using default compiler options, which now imply -std=gnu11) is less of a concern; information about how many warnings for implicit int and implicit function declarations there are in distribution builds would be useful. C++ has various standard-required-diagnostics as permerrors (error by default, warning with -fpermissive). That might be a plausible model for C as well (there are plenty of always-on pedwarns, some of which could be considered for making into permerrors), though in the present case we already have more specific options for these diagnostics. -- Joseph S. Myers jos...@codesourcery.com
Re: Question regarding bug 70584
On Fri, May 20, 2016 at 5:42 PM, Jeff Law wrote: > On 05/20/2016 01:18 PM, Daniel Gutson wrote: >> >> (reposting in gcc@ and adding more information) >> >> On Fri, May 20, 2016 at 3:43 PM, Andres Tiraboschi >> wrote: >>> >>> While analysing this bug we arrived to the following code at >>> tree.c:145 (lvalue_kind): >>> >>> case VAR_DECL: >>> if (TREE_READONLY (ref) && ! TREE_STATIC (ref) >>> && DECL_LANG_SPECIFIC (ref) >>> && DECL_IN_AGGR_P (ref)) >>> return clk_none; >>> >>> That condition fails so a fall-through to the next case labels causes >>> to return clk_ordinary, whereas this is about a constexpr value >>> (rather than a reference). >>> >>> As an experiment, we forced the condition above to return clk_none and >>> the bug is not reproduced. >>> >>> We are suspecting either that the condition is too restrictive or a >>> fall-through is not intended. Why is the condition requiring >>> DECL_IN_AGGR_P? >> >> >> Just to provide more information: DECL_LANG_SPECIFIC is NULL and >> DECL_IN_AGGR_P is false. >> Can somebody provide the rationale of the condition? > > I'm not really an expert in this code, but it looks like we're returning > clk_none for a small subset of nodes that aren't really lvalues. Examples > would be certain read-only objects which can't be lvalues. > > Other VAR_DECLs would be lvalues and should probably return clk_ordinary. > > At least that how it appears to me. > > Jeff > Thanks, Jason and us already solved. I think he will commit the patch soon. -- Daniel F. Gutson Engineering Manager San Lorenzo 47, 3rd Floor, Office 5 Córdoba, Argentina Phone: +54 351 4217888 / +54 351 4218211 Skype:dgutson LinkedIn: http://ar.linkedin.com/in/danielgutson
