From patchwork Mon Nov 24 16:41:48 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Marek Polacek X-Patchwork-Id: 414038 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3EF0D140185 for ; Tue, 25 Nov 2014 03:42:04 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:content-transfer-encoding:in-reply-to; q=dns; s= default; b=JzijudF8R7VcsynxjjcHBd2KdnLJ0k/ZdY5iz/pEneK0xeiJLrAQs i85eTmUg4dUUdHqw4R46nyrPxGMmDXy4n83mcexxw6eFcYpU/kU17REm39uMIiJI ai3+iVnzIsWZowBP7Ko779azQOdiMK7AAzm2/ff8HmA5cW/qkE1pVg= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:date :from:to:cc:subject:message-id:references:mime-version :content-type:content-transfer-encoding:in-reply-to; s=default; bh=lSgo2CZd2IqT/g9wl+AFKQN1+Hk=; b=JL8HkWFX3npUuZKxkB7wBUGCmsKC 26yCVdXLYyoW/Wa7kS47bA+BP2ZlQGl4efeLQmAlxp9mZppLKnjeohC91U5WjSRh mT4kP4Y/PbwF9lDOLE5hRcfMBwwbwmiftm25TbD+/eQFH2O0akLf5mmny3J00sGS 7t56DGGDXKbq6bk= Received: (qmail 13010 invoked by alias); 24 Nov 2014 16:41:56 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 12997 invoked by uid 89); 24 Nov 2014 16:41:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Mon, 24 Nov 2014 16:41:55 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sAOGfqJt030194 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Mon, 24 Nov 2014 11:41:53 -0500 Received: from redhat.com (ovpn-116-21.ams2.redhat.com [10.36.116.21]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sAOGfmie002845 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Mon, 24 Nov 2014 11:41:51 -0500 Date: Mon, 24 Nov 2014 17:41:48 +0100 From: Marek Polacek To: Jakub Jelinek Cc: Jason Merrill , GCC Patches Subject: Re: [C++ PATCH] Detect UB in shifts in constexpr functions Message-ID: <20141124164148.GR29446@redhat.com> References: <20141124135114.GO29446@redhat.com> <20141124154925.GI1674@tucnak.redhat.com> <20141124155822.GQ29446@redhat.com> <20141124160508.GJ1674@tucnak.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20141124160508.GJ1674@tucnak.redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) On Mon, Nov 24, 2014 at 05:05:08PM +0100, Jakub Jelinek wrote: > On Mon, Nov 24, 2014 at 04:58:22PM +0100, Marek Polacek wrote: > > > Consider say: > > > > > > constexpr int p = 1; > > > constexpr int foo (int a) > > > { > > > return a << (int) &p; > > > } > > > constexpr int bar (int a) > > > { > > > return ((int) &p) << a; > > > } > > > constexpr int q = foo (5); > > > constexpr int r = bar (2); > > > constexpr int s = bar (0); > > > > > > Now, for foo (5) and bar (2) fold_binary_loc returns NULL and thus > > > your cxx_eval_check_shift_p is not called, for bar (0) it returns > > > non-NULL and while the result still is not a constant expression and > > > right now is diagnosed, with your patch it would ICE. > > > > > > So, I'd just return false if either lhs or rhs are not INTEGER_CSTs. > > > > Ok, I'll add that. Thank for pointing that out. > > Note, the above only with -m32 obviously, or supposedly for bar > you could change return type and the cast and type of r/s to > __PTRDIFF_TYPE__ or so. foo, as gcc always casts the shift count to int, > would supposedly need to stay the way it is and might produce different > diagnostics between -m32 and -m64. I changed int to __PTRDIFF_TYPE__. You're right: with -m64 I get: i.C: In function ‘constexpr int foo(int)’: i.C:4:22: error: cast from ‘const int*’ to ‘int’ loses precision [-fpermissive] return a << (int) &p; ^ but not with -m32. I dropped the foo from the testcase. > I meant for the above computation, there you don't check any overflow. I see. I'm happy to change it, whatever you/Jason prefer. The following is a version with INTEGER_CST check and a new test. 2014-11-24 Marek Polacek * constexpr.c (cxx_eval_check_shift_p): New function. (cxx_eval_binary_expression): Call it. * g++.dg/cpp0x/constexpr-shift1.C: New test. * g++.dg/cpp1y/constexpr-shift1.C: New test. Marek diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c index 2678223..049ec0f 100644 --- gcc/cp/constexpr.c +++ gcc/cp/constexpr.c @@ -1451,6 +1451,45 @@ verify_constant (tree t, bool allow_non_constant, bool *non_constant_p, return *non_constant_p; } +/* Return true if the shift operation on LHS and RHS is undefined. */ + +static bool +cxx_eval_check_shift_p (enum tree_code code, tree lhs, tree rhs) +{ + if ((code != LSHIFT_EXPR && code != RSHIFT_EXPR) + || TREE_CODE (lhs) != INTEGER_CST + || TREE_CODE (rhs) != INTEGER_CST) + return false; + + tree lhstype = TREE_TYPE (lhs); + unsigned HOST_WIDE_INT uprec = TYPE_PRECISION (TREE_TYPE (lhs)); + + /* [expr.shift] The behavior is undefined if the right operand + is negative, or greater than or equal to the length in bits + of the promoted left operand. */ + if (tree_int_cst_sgn (rhs) == -1 || compare_tree_int (rhs, uprec) >= 0) + return true; + + /* The value of E1 << E2 is E1 left-shifted E2 bit positions; [...] + if E1 has a signed type and non-negative value, and E1x2^E2 is + representable in the corresponding unsigned type of the result type, + then that value, converted to the result type, is the resulting value; + otherwise, the behavior is undefined. */ + if (code == LSHIFT_EXPR && !TYPE_UNSIGNED (lhstype)) + { + if (tree_int_cst_sgn (lhs) == -1) + return true; + tree t = build_int_cst (unsigned_type_node, uprec - 1); + t = fold_build2 (MINUS_EXPR, unsigned_type_node, t, rhs); + tree ulhs = fold_convert (unsigned_type_for (lhstype), lhs); + t = fold_build2 (RSHIFT_EXPR, TREE_TYPE (ulhs), ulhs, t); + if (tree_int_cst_lt (integer_one_node, t)) + return true; + } + + return false; +} + /* Subroutine of cxx_eval_constant_expression. Attempt to reduce the unary expression tree T to a compile time value. If successful, return the value. Otherwise issue a diagnostic @@ -1506,7 +1545,7 @@ cxx_eval_binary_expression (const constexpr_ctx *ctx, tree t, enum tree_code code = TREE_CODE (t); tree type = TREE_TYPE (t); r = fold_binary_loc (loc, code, type, lhs, rhs); - if (r == NULL_TREE) + if (r == NULL_TREE || cxx_eval_check_shift_p (code, lhs, rhs)) { if (lhs == orig_lhs && rhs == orig_rhs) r = t; diff --git gcc/testsuite/g++.dg/cpp0x/constexpr-shift1.C gcc/testsuite/g++.dg/cpp0x/constexpr-shift1.C index e69de29..2551fbe 100644 --- gcc/testsuite/g++.dg/cpp0x/constexpr-shift1.C +++ gcc/testsuite/g++.dg/cpp0x/constexpr-shift1.C @@ -0,0 +1,73 @@ +// { dg-do compile { target c++11 } } + +constexpr int +fn1 (int i, int j) +{ + return i << j; // { dg-error "is not a constant expression" } +} + +constexpr int i1 = fn1 (1, -1); + +constexpr int +fn2 (int i, int j) +{ + return i << j; // { dg-error "is not a constant expression" } +} + +constexpr int i2 = fn2 (1, 200); + +constexpr int +fn3 (int i, int j) +{ + return i << j; // { dg-error "is not a constant expression" } +} + +constexpr int i3 = fn3 (-1, 2); + +constexpr int +fn4 (int i, int j) +{ + return i << j; // { dg-error "is not a constant expression" } +} + +constexpr int i4 = fn4 (__INT_MAX__, 2); + +constexpr int +fn5 (int i, int j) +{ + return i << j; +} + +constexpr int i5 = fn5 (__INT_MAX__, 1); + +constexpr int +fn6 (unsigned int i, unsigned int j) +{ + return i << j; // { dg-error "is not a constant expression" } +} + +constexpr int i6 = fn6 (1, -1); + +constexpr int +fn7 (int i, int j) +{ + return i >> j; // { dg-error "is not a constant expression" } +} + +constexpr int i7 = fn7 (1, -1); + +constexpr int +fn8 (int i, int j) +{ + return i >> j; +} + +constexpr int i8 = fn8 (-1, 1); + +constexpr int +fn9 (int i, int j) +{ + return i >> j; // { dg-error "is not a constant expression" } +} + +constexpr int i9 = fn9 (1, 200); diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-shift1.C gcc/testsuite/g++.dg/cpp1y/constexpr-shift1.C index e69de29..a739fd2 100644 --- gcc/testsuite/g++.dg/cpp1y/constexpr-shift1.C +++ gcc/testsuite/g++.dg/cpp1y/constexpr-shift1.C @@ -0,0 +1,9 @@ +// { dg-do compile { target c++14 } } + +constexpr int p = 1; +constexpr __PTRDIFF_TYPE__ bar (int a) +{ + return ((__PTRDIFF_TYPE__) &p) << a; // { dg-error "is not a constant expression" } +} +constexpr __PTRDIFF_TYPE__ r = bar (2); +constexpr __PTRDIFF_TYPE__ s = bar (0); // { dg-error "conversion from pointer" }