Re: [4.5] PATCH: PR c/39323: MAX_OFILE_ALIGNMENT in elfos.h is too big

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Mon, Mar 2, 2009 at 10:27 PM, Ian Lance Taylor <iant@google.com> wrote:
> "H.J. Lu" <hjl.tools@gmail.com> writes:
>
>> gcc/
>>
>> 2009-02-28 ?H.J. Lu ?<hongjiu.lu@intel.com>
>>
>> ? ? ? PR c/39323
>> ? ? ? * c-common.c (handle_aligned_attribute): Check alignment
>> ? ? ? overlow.
>>
>> gcc/testsuite/
>>
>> 2009-02-28 ?H.J. Lu ?<hongjiu.lu@intel.com>
>>
>> ? ? ? PR c/39323
>> ? ? ? * gcc.dg/pr39323.c: New.
>>
>
>> @@ -5954,7 +5956,19 @@ handle_aligned_attribute (tree *node, tr
>> ? ? ? *type = build_variant_type_copy (*type);
>
>> ? ? ? ?TYPE_ALIGN (*type) = (1 << i) * BITS_PER_UNIT;
>> - ? ? ?TYPE_USER_ALIGN (*type) = 1;
>> + ? ? ?/* TYPE_ALIGN and DECL_ALIGN, defined as decl_common.align, may
>> + ? ? ?have different widths. ?Use decl_common.align directly to check
>> + ? ? ?for overflow. ?*/
>> + ? ? ?saved = (*type)->decl_common.align;
>> + ? ? ?(*type)->decl_common.align = (1 << i) * BITS_PER_UNIT;
>> + ? ? ?if ((*type)->decl_common.align != (1 << i) * BITS_PER_UNIT)
>> + ? ? {
>> + ? ? ? error ("requested alignment is too large");
>> + ? ? ? *no_add_attrs = true;
>> + ? ? }
>> + ? ? ?else
>> + ? ? TYPE_USER_ALIGN (*type) = 1;
>> + ? ? ?(*type)->decl_common.align = saved;
>
> It is never OK to refer to decl_common outside of tree.h. ?In any case I
> don't see why it matters here whether decl_common.align overflows. ?What
> matters is whether TYPE_ALIGN overflows.
>
> If you are concerned about the TYPE_ALIGN value overflowing when you
> copy it to DECL_ALIGN, check it at the point of the copy to DECL_ALIGN.
>
> Please use a variable to hold the expression "(1 << i) * BIT_PER_UNIT",
> rather than repeating it.
>
>> @@ -5977,7 +5991,13 @@ handle_aligned_attribute (tree *node, tr
>> ? ?else
>> ? ? ?{
>> ? ? ? ?DECL_ALIGN (decl) = (1 << i) * BITS_PER_UNIT;
>> - ? ? ?DECL_USER_ALIGN (decl) = 1;
>> + ? ? ?if (DECL_ALIGN (decl) != (1 << i) * BITS_PER_UNIT)
>> + ? ? {
>> + ? ? ? error ("requested alignment is too large");
>> + ? ? ? *no_add_attrs = true;
>> + ? ? }
>> + ? ? ?else
>> + ? ? DECL_USER_ALIGN (decl) = 1;
>
> Please use a variable for "(1 << i) * BIT_PER_UNIT".
>

Here is the updated patch. It added decl_align_ok in
tree.h.

There are

coverage.c:      DECL_ALIGN (tree_ctr_tables[counter]) = TYPE_ALIGN
(gcov_type_node);
omp-low.c:    DECL_ALIGN (field) = TYPE_ALIGN (type);
stmt.c:      DECL_ALIGN (decl) = TYPE_ALIGN (type);
stor-layout.c:      DECL_ALIGN (decl) = TYPE_ALIGN (type);
tree-nested.c:    TYPE_ALIGN (type) = DECL_ALIGN (field);
tree-nested.c:	  DECL_ALIGN (field) = TYPE_ALIGN (TREE_TYPE (field));
tree-nested.c:      DECL_ALIGN (field) = TYPE_ALIGN (type);
tree-nested.c:      DECL_ALIGN (field) = TYPE_ALIGN (type);
tree-vect-analyze.c:      DECL_ALIGN (base) = TYPE_ALIGN (vectype);
tree-vectorizer.c:	  DECL_ALIGN (decl) = TYPE_ALIGN (vectype);
cp/class.c:  DECL_ALIGN (decl) = MAX (TYPE_ALIGN (double_type_node),
cp/class.c:	  DECL_ALIGN (decl) = CLASSTYPE_ALIGN (basetype);
cp/class.c:	  DECL_ALIGN (field) = TYPE_ALIGN (integer_type);
fortran/trans-common.c:	    DECL_ALIGN (decl) = TYPE_ALIGN (TREE_TYPE (field));
java/typeck.c:  DECL_ALIGN (arfld) = TYPE_ALIGN (element_type);
ada/gcc-interface/utils.c:	DECL_ALIGN (field_decl) = TYPE_ALIGN (field_type);

If we want to delay DECL_ALIGN overflow check, we may need to
check it at those places above and we may only call sorry ().
I can prepare such a patch if it is desirable.

Thanks.


-- 
H.J.
---
gcc/

2009-02-28  H.J. Lu  <hongjiu.lu@intel.com>

	PR c/39323
	* c-common.c (handle_aligned_attribute): Check alignment
	overflow.

	* tree.h (decl_align_ok): New.

gcc/testsuite/

2009-02-28  H.J. Lu  <hongjiu.lu@intel.com>

	PR c/39323
	* gcc.dg/pr39323.c: New.

Attachment: gcc-pr39323-4.patch
Description: Text document