From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6832 invoked by alias); 7 Jul 2011 11:42:29 -0000 Received: (qmail 6823 invoked by uid 22791); 7 Jul 2011 11:42:28 -0000 X-SWARE-Spam-Status: No, hits=-3.3 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 07 Jul 2011 11:42:12 +0000 Received: from relay2.suse.de (charybdis-ext.suse.de [195.135.221.2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id 56CB679727; Thu, 7 Jul 2011 13:42:11 +0200 (CEST) Date: Thu, 07 Jul 2011 11:44:00 -0000 From: Richard Guenther To: gcc-patches@gcc.gnu.org Cc: "Joseph S. Myers" Subject: [PATCH][C] Fixup pointer-int-sum Message-ID: User-Agent: Alpine 2.00 (LNX 1167 2008-08-23) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-07/txt/msg00460.txt.bz2 This tries to make sense of the comments and code in the code doing the index - size multiplication in pointer-int-sum. It also fixes a bogus integer-constant conversion which results in not properly canonicalized integer constants. The comment in the code claims the index - size multiplication is carried out signed, which doesn't match the code which does it unsigned (the commend dates back to rev. 6733 where we _did_ carry out the multiplication in a signed type, using c_common_type_for_size (TYPE_PRECISION (sizetype), 0)). The following patch makes us preserve the signedness of intop so that for signed intop the multiplication will be known to not overflow (what is actually the C semantics - is the multiplication allowed to overflow for unsigned intop? If not I guess the orginal code of always choosing a signed type was more correct and we should go back to it instead?) The comment also claims there is a sign-extension of t to sizetype - that's not true either, it's just a sign-change. Joseph, do we want an unconditional (un-)signed multiplication (before the patch it's unsigned), or what the patch does? Bootstrapped and tested on x86_64-unknown-linux-gnu. I'll also test the unconditionally signed variant. Thanks, Richard. 2011-07-07 Richard Guenther * c-common.c (pointer_int_sum): Do the index times size multiplication in the signedness of index. Properly strip overflow flags. Index: gcc/c-family/c-common.c =================================================================== --- gcc/c-family/c-common.c (revision 175962) +++ gcc/c-family/c-common.c (working copy) @@ -3737,23 +3737,22 @@ pointer_int_sum (location_t loc, enum tr /* Convert the integer argument to a type the same size as sizetype so the multiply won't overflow spuriously. */ - if (TYPE_PRECISION (TREE_TYPE (intop)) != TYPE_PRECISION (sizetype) - || TYPE_UNSIGNED (TREE_TYPE (intop)) != TYPE_UNSIGNED (sizetype)) + if (TYPE_PRECISION (TREE_TYPE (intop)) != TYPE_PRECISION (sizetype)) intop = convert (c_common_type_for_size (TYPE_PRECISION (sizetype), - TYPE_UNSIGNED (sizetype)), intop); + TYPE_UNSIGNED (TREE_TYPE (intop))), + intop); /* Replace the integer argument with a suitable product by the object size. - Do this multiplication as signed, then convert to the appropriate type - for the pointer operation and disregard an overflow that occured only - because of the sign-extension change in the latter conversion. */ + Do this multiplication in a widened intop type, then convert to the + appropriate type for the pointer operation and disregard an overflow + that occured only because of the sign-change in the latter conversion. */ { tree t = build_binary_op (loc, MULT_EXPR, intop, convert (TREE_TYPE (intop), size_exp), 1); intop = convert (sizetype, t); if (TREE_OVERFLOW_P (intop) && !TREE_OVERFLOW (t)) - intop = build_int_cst_wide (TREE_TYPE (intop), TREE_INT_CST_LOW (intop), - TREE_INT_CST_HIGH (intop)); + intop = double_int_to_tree (sizetype, tree_to_double_int (intop)); } /* Create the sum or difference. */