From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20815 invoked by alias); 26 Feb 2020 23:18:28 -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 20806 invoked by uid 89); 26 Feb 2020 23:18:27 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-7.6 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=H*i:sk:2e21747, H*f:sk:2e21747, H*MI:sk:2e21747, Law X-HELO: mail-qt1-f196.google.com Received: from mail-qt1-f196.google.com (HELO mail-qt1-f196.google.com) (209.85.160.196) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 26 Feb 2020 23:18:26 +0000 Received: by mail-qt1-f196.google.com with SMTP id l21so899187qtr.8 for ; Wed, 26 Feb 2020 15:18:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=+coF7GEYELgb5ATEdwggwTEi1KcVPumt+kG1t+2NRmU=; b=PNdk+S8Eh1WCwzXNA6dlVuXrmNT7YcDfGR0C9SmLRVTdqDLa4hA4hNGlX1ISQzeMQp 9uauoC4LR/o6Q0aR6if6j0zTDlTandJgi+dBf0m5SGfZ70K6X/EGbcrH6uJYIlMfEVls DA2YjSVIRgT/DtemhjoxsAkxt6+wOCnC4RZriRNp9dF7OXlbeaS6vvyGd5ldgE5q2xAz kfvVPJ3dsJPkIy7DZgoUp6z4ppQ1h2RueYQSAUDJ1yU1HxWtlXJt3eFwlhnBVywpm1B7 v8SFaxR1H/Azuw22fMHaqNQFeXAlQQSGugwFla7MEKHZglplp2AsV+5NkQcMnRzs9lqR Ftww== Return-Path: Received: from [192.168.0.41] (174-16-106-242.hlrn.qwest.net. [174.16.106.242]) by smtp.gmail.com with ESMTPSA id y21sm1919146qto.15.2020.02.26.15.18.22 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 26 Feb 2020 15:18:23 -0800 (PST) Subject: Re: [PATCH] use pointer size rather than array size when storing the former (PR 93829) To: law@redhat.com, gcc-patches References: <864beb08-0073-cde2-ad21-a855a7bf7dc9@gmail.com> <2e21747b15a30dd11cd6918667661cf3838bf3d3.camel@redhat.com> From: Martin Sebor Message-ID: Date: Wed, 26 Feb 2020 23:18:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <2e21747b15a30dd11cd6918667661cf3838bf3d3.camel@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2020-02/txt/msg01505.txt.bz2 On 2/26/20 3:09 PM, Jeff Law wrote: > On Wed, 2020-02-19 at 17:26 -0700, Martin Sebor wrote: >> The buffer overflow detection for multi-char stores uses the size >> of a source array even when what's actually being accessed (read >> and stored) is a pointer to the array. That leads to incorrect >> warnings in some cases. >> >> The attached patch corrects the function that computes the size of >> the access to set it to that of a pointer instead if the source is >> an address expression. >> >> Tested on x86_64-linux. > >> if (TREE_CODE (exp) == ADDR_EXPR) >> - exp = TREE_OPERAND (exp, 0); >> + { >> + /* If the size of the access hasn't been determined yet it's that >> + of a pointer. */ >> + if (!nbytes) >> + nbytes = tree_to_uhwi (TYPE_SIZE_UNIT (TREE_TYPE (exp))); >> + exp = TREE_OPERAND (exp, 0); >> + } >> > This doesn't make any sense to me. You're always going to get the size of a > pointer here. Don't you want the size of the TYPE of the operand? The function correctly handles cases when the expression is an array because the array is what's being stored. The bug is in stripping the ADDR_EXPR and then using the size of the operand when what's being stored is actually a pointer. This test case illustrates the difference: struct S { char *p, *q, r[8]; } a; void f (void) { struct S b = { 0, "1234567", "abcdefgh" }; __builtin_memcpy (&a, &b, sizeof a); } The memcpy results in: MEM [(char * {ref-all})&b] = 0B; MEM [(char * {ref-all})&b + 8B] = "1234567"; MEM [(char * {ref-all})&a] = MEM [(char * {ref-all})&b]; The RHS of the second MEM_REF is ADDR_EXPR (STRING_CST). The RHS of the third MEM_REF is the array case (that's handled correctly). I think computing the size of the store could be simplified (it seems it should be the same as type of either the RHS or the LHS) but I didn't want to mess with things too much this late. Martin