On 05/23/2011 05:04 PM, Nathan Froyd wrote:
> On 05/22/2011 02:24 PM, Tom de Vries wrote:
>> Now that struct tree_type does not exist anymore, 'sizeof (struct tree_type)'
>> generates an error in the following assert in fold_checksum_tree:
>> ...
>> gcc_assert ((sizeof (struct tree_exp) + 5 * sizeof (tree)
>> <= sizeof (struct tree_function_decl))
>> && sizeof (struct tree_type) <= sizeof (struct
>> tree_function_decl));
>> ...
>>
>> This error is triggered with -enable-checking=fold.
>
> Doh. Thanks for the report.
>
> The easy fix is s/tree_type/tree_type_non_common/. But I don't see why the
> assert has to even care about tree_type; doesn't:
>
> gcc_assert ((sizeof (struct tree_exp) + 5 * sizeof (tree)
> <= sizeof (union tree_node));
>
> accomplish the same thing?
>
> -Nathan
>
>
I don't know for sure what the assert is trying to check, but I'm guessing it's
trying to check that the memcpys are save. A naive implementation would be:
Index: fold-const.c
===================================================================
--- fold-const.c (revision 173703)
+++ fold-const.c (working copy)
@@ -13792,6 +13789,7 @@ recursive_label:
&& DECL_ASSEMBLER_NAME_SET_P (expr))
{
/* Allow DECL_ASSEMBLER_NAME to be modified. */
+ gcc_assert (tree_size (expr) <= sizeof (buf));
memcpy ((char *) &buf, expr, tree_size (expr));
SET_DECL_ASSEMBLER_NAME ((tree)&buf, NULL);
expr = (tree) &buf;
@@ -13805,6 +13803,7 @@ recursive_label:
{
/* Allow these fields to be modified. */
tree tmp;
+ gcc_assert (tree_size (expr) <= sizeof (buf));
memcpy ((char *) &buf, expr, tree_size (expr));
expr = tmp = (tree) &buf;
TYPE_CONTAINS_PLACEHOLDER_INTERNAL (tmp) = 0;
But that turns it into a runtime check.
On the other hand, I'm not sure the original assert still makes sense. Neither
tcc_type nor tcc_declaration have variable size, so the '5 * sizeof (tree)' does
not seem applicable anymore.
If we want checks cheaper than the naive, but more maintainable than the
current, we would want something like:
+ gcc_assert (tree_class_max_size (tcc_declaration) <= sizeof (buf));
+ gcc_assert (tree_class_max_size (tcc_type) <= sizeof (buf));
We would want those checks moved out of the hot path, and we would need to
implement and maintain tree_class_max_size alongside tree_size and
tree_code_size. But I'm not sure it's worth the effort.
Thanks,
- Tom
