From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4841 invoked by alias); 9 Nov 2018 01:25:20 -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 4786 invoked by uid 89); 9 Nov 2018 01:25:19 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.7 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=drivers, deferring, triggering, spending X-HELO: mail-yw1-f67.google.com Received: from mail-yw1-f67.google.com (HELO mail-yw1-f67.google.com) (209.85.161.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 09 Nov 2018 01:25:17 +0000 Received: by mail-yw1-f67.google.com with SMTP id z72-v6so435566ywa.0 for ; Thu, 08 Nov 2018 17:25:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=H4TPyjgHZLdnpU7Au2P+b88IRjh0TfAqgLfwNDVK1jk=; b=FT21o+efhZLzFQDPuX7QAPtAVQqOIi83QbTjrwnNaUrE+smbtTyUlautol+h6n7KIw GNzv/Xi6eX4KAG1ZRRCbRNlKb2DjYU4Qt2D+lEYtzcvUyHs5170vvBnqdfjN9Zh89urJ bQkdV7AxWhy2tBcYb+/mcvHBprLTqq6Ny0bFRdfhO8/PZwF198MYS6JlMncmXLZhz5me dvup1FlJLlocfJa7mGyGJ8lmhKRu/Zdb3MqFSaMeTKaJu/JsCiXeItjci2q6iV+y721R m7h4cMhfiSeCITLBi+mo4j/UlwFltiDp92sgaDvOlK2AGGkpU2pjnpO19pWJQkZ3NiwY SsHw== Return-Path: Received: from localhost.localdomain (184-96-239-209.hlrn.qwest.net. [184.96.239.209]) by smtp.gmail.com with ESMTPSA id y14sm2990628qkb.2.2018.11.08.17.25.13 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 08 Nov 2018 17:25:13 -0800 (PST) Subject: Re: [PATCH] avoid warning on constant strncpy until next statement is reachable (PR 87028) To: Jeff Law , Richard Biener References: <88de1ee3-6ee4-d8d9-3e57-3a42474a4169@redhat.com> <27235a6d-6f2c-1f2d-d456-d4cd9b941894@redhat.com> <23ea3d13-d9c5-1b02-f01c-d2a0e11f3a10@redhat.com> <9e3f6d62-47b9-b80f-b8ac-5711628579c5@redhat.com> <072d934c-8bce-86d2-a842-562b40d6fda6@redhat.com> Cc: GCC Patches From: Martin Sebor Message-ID: <0e80bf80-3556-cc33-3237-e44487122e98@gmail.com> Date: Fri, 09 Nov 2018 01:25:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <072d934c-8bce-86d2-a842-562b40d6fda6@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2018-11/txt/msg00676.txt.bz2 On 11/07/2018 02:28 PM, Jeff Law wrote: > On 10/20/18 6:01 PM, Martin Sebor wrote: > > >> >> The warning only triggers when the bound is less than or equal >> to the length of the constant source string (i.e, when strncpy >> truncates). So IIUC, your suggestion would defer folding only >> such strncpy calls and let gimple_fold_builtin_strncpy fold >> those with a constant bound that's greater than the length of >> the constant source string. That would be fine with me, but >> since strncpy calls with a bound that's greater than the length >> of the source are pointless I don't think they are important >> enough to worry about folding super early. The constant ones >> that serve any purpose (and that are presumably important to >> optimize) are those that truncate. > I was focused exclusively on the case where we have to look for a > subsequent statement that handled termination. The idea was to only > leave in the cases that we might need to warn for because we couldn't > search subsequent statement for the termination. > > Splitting up was primarily meant to get the warning out of the folder > with a minimal impact on code generation. But if the common case would > result in deferral of folding, then I'd fully expect Richi to object. The test case from the bug is: struct S { char dest[5]; }; const char src[] = "1234567890"; void f (struct S *s) { strncpy (s->dest, src, sizeof (s->dest) - 1); s->dest [sizeof (s->dest) - 1] = '\0'; } The strncpy call truncates but it's safe because of the assignment. This is representative of the use case that the fix is needed for (one with a constant source string) and that needs folding to be deferred. I don't think this use case is a pervasive one, or even terribly common among all strncpy uses, but it is the only one that the early folding code handles. By deferring the folding, this use case will be transformed to memcpy slightly later than it is now (during forwprop1 to be precise, so after early folding). As a data point, in a build of the Linux kernel where I expect strncpy is used as intended more than in most other code, of the nearly 500 distinct strncpy calls, 21 instances are folded before the CFG is complete. The effect of the patch would fold the 21 instances later. I.e., just 4% of all calls. Most of these calls (12 out of the 21) are in SCSI drivers, in code that responds to the INQUIRY command with things like vendor and product names, hardly something that matters for efficiency. But at -O1 even they still are ultimately folded to memcpy. >> That said, when optimization isn't enabled, I don't think users >> expect calls to library functions to be transformed to calls to >> other functions, or inlined. Yet that's just what GCC does. >> For example, besides triggering the warning, the following: > I don't think we should drag this into the issue at hand. Though I do > generally agree that folding this stuff into low level memory operations > is not what most would expect at -O0. Folding calls to library functions so early that the CFG hasn't been constructed yet is the root cause of the issue. I'm not suggesting to prevent it for all functions as part of this fix, but if agree that this early folding is a poor choice in general then we should not be uncomfortable with a patch that defers it for just a subset of use cases of a single function. FWIW, I would view it as entirely appropriate to do our due diligence before making a decision about the early folding of all library functions, but I'm finding it hard to justify to myself spending this much time and effort on either the false positive or on the question of whether something that's by all measures as inconsequential for efficiency as strncpy should be transformed to memcpy at point 1 or point 2. What do you suggest next? Martin