From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 920 invoked by alias); 6 Nov 2019 18:55:59 -0000 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 Received: (qmail 909 invoked by uid 89); 6 Nov 2019 18:55:59 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: =?ISO-8859-1?Q?No, score=-7.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.1 spammy=It, into=c2?= X-HELO: us-smtp-1.mimecast.com Received: from us-smtp-delivery-1.mimecast.com (HELO us-smtp-1.mimecast.com) (205.139.110.120) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 06 Nov 2019 18:55:56 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1573066555; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:openpgp:openpgp:autocrypt:autocrypt; bh=lDjrwo++PwcIcwf0cWnHZpgktmrgdfxR2+n37cBZum0=; b=Ci0drbI0Mzr16WG7LlnYLb0NWnCvDDjoaCjbTP5pjWeNRCw3ZZEgruVeK5ecEBr87KsXYd 0OFuYI9Dv3h0TfjddPfV1+NmwDq8uJBECOQ20Rsvp2GKQOTbsYUH1Cd3XqwoA11eEcoxj9 FqIGzTjg5jaGqHuLg8lUpF3IvzJO6FI= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-342-eKugdrOkNnes5PbZfiGWEQ-1; Wed, 06 Nov 2019 13:55:53 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id B7EDA1800D6B; Wed, 6 Nov 2019 18:55:51 +0000 (UTC) Received: from localhost.localdomain (ovpn-112-13.rdu2.redhat.com [10.10.112.13]) by smtp.corp.redhat.com (Postfix) with ESMTP id AF53960BF4; Wed, 6 Nov 2019 18:55:50 +0000 (UTC) Subject: Re: [PATCH] include size and offset in -Wstringop-overflow To: Martin Sebor , gcc-patches References: <9d3210bf-e1bd-81ac-b1a0-5c66c8ec6db7@gmail.com> From: Jeff Law Openpgp: preference=signencrypt Message-ID: <6f6bdef6-0d0d-0303-b870-10ff2b036444@redhat.com> Date: Wed, 06 Nov 2019 18:55:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <9d3210bf-e1bd-81ac-b1a0-5c66c8ec6db7@gmail.com> X-Mimecast-Spam-Score: 0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2019-11/txt/msg00432.txt.bz2 On 11/6/19 11:00 AM, Martin Sebor wrote: > The=C2=A0-Wstringop-overflow=C2=A0warnings=C2=A0for=C2=A0single-byte=C2= =A0and=C2=A0multi-byte > stores=C2=A0mention=C2=A0the=C2=A0amount=C2=A0of=C2=A0data=C2=A0being=C2= =A0stored=C2=A0and=C2=A0the=C2=A0amount=C2=A0of > space=C2=A0remaining=C2=A0in=C2=A0the=C2=A0destination,=C2=A0such=C2=A0as: >=20 > warning:=C2=A0writing=C2=A04=C2=A0bytes=C2=A0into=C2=A0a=C2=A0region=C2= =A0of=C2=A0size=C2=A00=C2=A0[-Wstringop-overflow=3D] > =C2=A0=C2=A0123=C2=A0|=C2=A0=C2=A0=C2=A0*p=C2=A0=3D=C2=A00; > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0|=C2=A0=C2=A0=C2=A0~~~^~~ > note:=C2=A0destination=C2=A0object=C2=A0declared=C2=A0here > =C2=A0=C2=A0=C2=A045=C2=A0|=C2=A0=C2=A0=C2=A0char=C2=A0b[N]; > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0|=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0^ >=20 > A=C2=A0warning=C2=A0like=C2=A0this=C2=A0can=C2=A0take=C2=A0some=C2=A0time= =C2=A0to=C2=A0analyze.=C2=A0=C2=A0First,=C2=A0the=C2=A0size > of=C2=A0the=C2=A0destination=C2=A0isn't=C2=A0mentioned=C2=A0and=C2=A0may= =C2=A0not=C2=A0be=C2=A0easy=C2=A0to=C2=A0tell=C2=A0from > the=C2=A0sources.=C2=A0=C2=A0In=C2=A0the=C2=A0note=C2=A0above,=C2=A0when= =C2=A0N's=C2=A0value=C2=A0is=C2=A0the=C2=A0result=C2=A0of > some=C2=A0non-trivial=C2=A0computation,=C2=A0chasing=C2=A0it=C2=A0down=C2= =A0may=C2=A0be=C2=A0a=C2=A0small=C2=A0project > in=C2=A0and=C2=A0of=C2=A0itself.=C2=A0=C2=A0Second,=C2=A0it's=C2=A0also= =C2=A0not=C2=A0clear=C2=A0why=C2=A0the=C2=A0region=C2=A0size > is=C2=A0zero.=C2=A0=C2=A0It=C2=A0could=C2=A0be=C2=A0because=C2=A0the=C2= =A0offset=C2=A0is=C2=A0exactly=C2=A0N,=C2=A0or=C2=A0because > it's=C2=A0negative,=C2=A0or=C2=A0because=C2=A0it's=C2=A0in=C2=A0some=C2= =A0range=C2=A0greater=C2=A0than=C2=A0N. >=20 > Mentioning=C2=A0both=C2=A0the=C2=A0size=C2=A0of=C2=A0the=C2=A0destination= =C2=A0object=C2=A0and=C2=A0the=C2=A0offset > makes=C2=A0the=C2=A0existing=C2=A0messages=C2=A0clearer,=C2=A0are=C2=A0wi= ll=C2=A0become=C2=A0essential=C2=A0when > GCC=C2=A0starts=C2=A0diagnosing=C2=A0overflow=C2=A0into=C2=A0allocated=C2= =A0buffers=C2=A0(as=C2=A0my > follow-on=C2=A0patch=C2=A0does). >=20 > The=C2=A0attached=C2=A0patch=C2=A0enhances=C2=A0-Wstringop-overflow=C2=A0= to=C2=A0do=C2=A0this=C2=A0by > letting=C2=A0compute_objsize=C2=A0return=C2=A0the=C2=A0offset=C2=A0to=C2= =A0its=C2=A0caller,=C2=A0doing > something=C2=A0similar=C2=A0in=C2=A0get_stridx,=C2=A0and=C2=A0adding=C2= =A0a=C2=A0new=C2=A0function=C2=A0to > the=C2=A0strlen=C2=A0pass=C2=A0to=C2=A0issue=C2=A0this=C2=A0enhanced=C2= =A0warning=C2=A0(eventually,=C2=A0I'd > like=C2=A0the=C2=A0function=C2=A0to=C2=A0replace=C2=A0the=C2=A0-Wstringop= -overflow=C2=A0handler=C2=A0in > builtins.c).=C2=A0=C2=A0With=C2=A0the=C2=A0change,=C2=A0the=C2=A0note=C2= =A0above=C2=A0might=C2=A0read=C2=A0something > like: >=20 > note:=C2=A0at=C2=A0offset=C2=A011=C2=A0to=C2=A0object=C2=A0=E2=80=98b=E2= =80=99=C2=A0with=C2=A0size=C2=A08=C2=A0declared=C2=A0here > =C2=A0=C2=A0=C2=A045=C2=A0|=C2=A0=C2=A0=C2=A0char=C2=A0b[N]; > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0|=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0^ >=20 > Tested=C2=A0on=C2=A0x86_64-linux. >=20 > Martin >=20 > gcc-store-offset.diff >=20 > gcc/ChangeLog: >=20 > * builtins.c (compute_objsize): Add an argument and set it to offset > into destination. > * builtins.h (compute_objsize): Add an argument. > * tree-object-size.c (addr_object_size): Add an argument and set it > to offset into destination. > (compute_builtin_object_size): Same. > * tree-object-size.h (compute_builtin_object_size): Add an argument. > * tree-ssa-strlen.c (get_addr_stridx): Add an argument and set it > to offset into destination. > (maybe_warn_overflow): New function. > (handle_store): Call maybe_warn_overflow to issue warnings. >=20 > gcc/testsuite/ChangeLog: >=20 > * c-c++-common/Wstringop-overflow-2.c: Adjust text of expected messages. > * g++.dg/warn/Wstringop-overflow-3.C: Same. > * gcc.dg/Wstringop-overflow-17.c: Same. >=20 > Index: gcc/tree-ssa-strlen.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- gcc/tree-ssa-strlen.c (revision 277886) > +++ gcc/tree-ssa-strlen.c (working copy) > @@ -189,6 +189,52 @@ struct laststmt_struct > static int get_stridx_plus_constant (strinfo *, unsigned HOST_WIDE_INT, = tree); > static void handle_builtin_stxncpy (built_in_function, gimple_stmt_itera= tor *); >=20=20 > +/* Sets MINMAX to either the constant value or the range VAL is in > + and returns true on success. */ > + > +static bool > +get_range (tree val, wide_int minmax[2], const vr_values *rvals =3D NULL) > +{ > + if (tree_fits_uhwi_p (val)) > + { > + minmax[0] =3D minmax[1] =3D wi::to_wide (val); > + return true; > + } > + > + if (TREE_CODE (val) !=3D SSA_NAME) > + return false; > + > + if (rvals) > + { > + gimple *def =3D SSA_NAME_DEF_STMT (val); > + if (gimple_assign_single_p (def) > + && gimple_assign_rhs_code (def) =3D=3D INTEGER_CST) > + { > + /* get_value_range returns [0, N] for constant assignments. */ > + val =3D gimple_assign_rhs1 (def); > + minmax[0] =3D minmax[1] =3D wi::to_wide (val); > + return true; > + } Umm, something seems really off with this hunk. If the SSA_NAME is set via a simple constant assignment, then the range ought to be a singleton ie [CONST,CONST]. Is there are particular test were this is not true? The only way offhand I could see this happening is if originally the RHS wasn't a constant, but due to optimizations it either simplified into a constant or a constant was propagated into an SSA_NAME appearing on the RHS. This would have to happen between the last range analysis and the point where you're making this query. > + > + // FIXME: handle anti-ranges? > + return false; Please don't if we can avoid them. anti-ranges are on the chopping block, so I'd prefer not to add cases where we're trying to handle them if we can reasonably avoid it. No objections elsewhere. So I think we just need to figure out what's going on with the ranges when you've got an INTEGER_CST on the RHS of an assignment. jeff