From patchwork Tue Mar 12 02:27:00 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Sebor X-Patchwork-Id: 1055036 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=gcc.gnu.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=gcc-patches-return-497727-incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="WNxyyijo"; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="WQix54a0"; dkim-atps=neutral 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 44JJlW1NZBz9s70 for ; Tue, 12 Mar 2019 13:27:16 +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:to :from:subject:message-id:date:mime-version:content-type; q=dns; s=default; b=O/Y6W6I2Aa1XOy4HFFYwznRZ41xINEJrni3nORmviGSe4yyzEr KqclUz5xlR7tlb1IqYbNDJxF637TFRCIQW40J+qFweXHWsUq76mK0+8YB5f+wlDf FZhpkO/N6YZhnr1dF2Mh8FfTC6Z63uM583O85T5/Sr5aWu8wsJYy7lKzA= 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:to :from:subject:message-id:date:mime-version:content-type; s= default; bh=alK4Fk2/o3Ka3BfyP8zdVJCMC3E=; b=WNxyyijo7Hm4E0rh1bwy bD2TCydhoK63jq6hUAnbiNohZr1iB3EfTjgAZR++nF5Y0D6fW60Pn6UL7q+lkxH4 EdS7eKjAloiWXaJKf/4Lv5dUaJ7aMhAS8WAQx3MnbEi/PcnrYKdnHRziuYvO60Cm bZZCHJlqyW4/fs3j+3PO1rk= Received: (qmail 122630 invoked by alias); 12 Mar 2019 02:27:07 -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 122621 invoked by uid 89); 12 Mar 2019 02:27:07 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-16.2 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=corrects X-HELO: mail-qt1-f180.google.com Received: from mail-qt1-f180.google.com (HELO mail-qt1-f180.google.com) (209.85.160.180) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 12 Mar 2019 02:27:05 +0000 Received: by mail-qt1-f180.google.com with SMTP id u7so918376qtg.9 for ; Mon, 11 Mar 2019 19:27:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=to:from:subject:message-id:date:user-agent:mime-version :content-language; bh=yi/qiQo19hdR+vgPOUdmyQnOelyEoAte78tMQWmFbIQ=; b=WQix54a0CDjwQ9mN/TeaevjfA2/Z+7AW47MLZ4d8coiYiLgUhbqYxt7pYhvpYNpIcd m50eFR89mkTvMQboSXZlZaf0GJk5w6QAGtTI4m8xU62LKLvYRpRg5spHSVXZX3OsCnBL jkg1U6/nCttYfrXBJuAVNHWN8xuujsZFnuygI9mKGN8U3SekuQPHuZfKFot5XCZdgteV W7jQlkWOtyrECi+AKGgkg5rzIpADxv8VwpjPvP5ogvOzoLFUqkTgUsKwK72ibUOxtDQQ g+mwrWhLD1znG9XfY9on5I7dS+hyhNtoxHYLFANwATPJQcLLoYkkTLw61HvvjhwsZM2f 6/RQ== Received: from [192.168.0.106] (174-16-104-92.hlrn.qwest.net. [174.16.104.92]) by smtp.gmail.com with ESMTPSA id k33sm4285942qte.8.2019.03.11.19.27.01 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 11 Mar 2019 19:27:02 -0700 (PDT) To: "gcc-patches@gcc.gnu.org" From: Martin Sebor Subject: [PATCH] avoid assuming strncpy arrays are nul-terminated (PR 89664) Message-ID: <24f17d93-7af3-2358-8e26-35d0cf23571b@gmail.com> Date: Mon, 11 Mar 2019 20:27:00 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.3.1 MIME-Version: 1.0 X-IsSubscribed: yes The -Wstringop-truncation handling for strncpy/stpncpy neglects to consider that character arrays tracked by the strlen pass are not necessarily nul-terminated. It unconditionally adds one when computing the size of each sequence to account for the nul. This leads to false positive warnings when checking the validity of indices/pointers computed by the built-ins. The attached patch corrects this by adding one for the nul only when the character array is known to be nul-terminated. Since GCC 7 does not issue the warning this is a 8/9 regression that I would like to fix in both releases. Is the patch okay for trunk/gcc-8-branch? Tested on x86_64-linux. Martin PR tree-optimization/89644 - False-positive -Warray-bounds diagnostic on strncpy gcc/ChangeLog: PR tree-optimization/89644 * tree-ssa-strlen.c (handle_builtin_stxncpy): Consider unterminated arrays in determining sequence sizes in strncpy and stpncpy. gcc/testsuite/ChangeLog: PR tree-optimization/89644 * gcc.dg/Wstringop-truncation-8.c: New test. diff --git a/gcc/testsuite/gcc.dg/Wstringop-truncation-8.c b/gcc/testsuite/gcc.dg/Wstringop-truncation-8.c new file mode 100644 index 00000000000..1745da50a37 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wstringop-truncation-8.c @@ -0,0 +1,94 @@ +/* PR tree-optimization/89644 - False-positive -Warray-bounds diagnostic + on strncpy + { dg-do compile } + { dg-options "-O2 -Wall -ftrack-macro-expansion=0" } */ + +#define NONSTR __attribute__ ((nonstring)) + +typedef __SIZE_TYPE__ size_t; + +size_t strlen (const char*); +extern char* stpncpy (char*, const char*, size_t); +extern char* strncpy (char*, const char*, size_t); + +void sink (char*, ...); + +char f0 (char *s) +{ + char a[6] NONSTR = { 1, 2, 3, 4, 5, 6 }; + if (*s) + strncpy (a, s, sizeof a); /* { dg-bogus "\\\[-Warray-bounds" } */ + return a[0]; +} + +void f1 (char *s) +{ + char a[6] NONSTR = { 1, 2, 3, 4, 5, 6 }; + if (*s) + strncpy (a, s, sizeof a); /* { dg-bogus "\\\[-Warray-bounds" } */ + sink (a); +} + +char f2 (void) +{ + char a[6] NONSTR = { 1, 2, 3, 4, 5, 6 }; + char b[6] NONSTR = { 6, 5, 4, 3, 2, 1 }; + strncpy (a, b + 1, 5); /* { dg-bogus "\\\[-Warray-bounds" } */ + return a[0]; +} + +void f3 (void) +{ + char a[6] NONSTR = { 1, 2, 3, 4, 5, 6 }; + char b[6] NONSTR = { 6, 5, 4, 3, 2, 1 }; + strncpy (a, b + 2, 4); /* { dg-bogus "\\\[-Warray-bounds" } */ + sink (a); +} + +void f4 (NONSTR char *d) +{ + char b[6] NONSTR = { 6, 5, 4, 3, 2, 1 }; + strncpy (d, b + 3, 3); /* { dg-bogus "\\\[-Warray-bounds" } */ + sink (d); +} + + +char g0 (char *s) +{ + char a[6] NONSTR = { 1, 2, 3, 4, 5, 6 }; + if (*s) + stpncpy (a, s, sizeof a); /* { dg-bogus "\\\[-Warray-bounds" } */ + return a[0]; +} + +void g1 (char *s) +{ + char a[6] NONSTR = { 1, 2, 3, 4, 5, 6 }; + char *p = 0; + if (*s) + p = stpncpy (a, s, sizeof a); /* { dg-bogus "\\\[-Warray-bounds" } */ + sink (a, p); +} + +char g2 (void) +{ + char a[6] NONSTR = { 1, 2, 3, 4, 5, 6 }; + char b[6] NONSTR = { 6, 5, 4, 3, 2, 1 }; + stpncpy (a, b + 1, 5); /* { dg-bogus "\\\[-Warray-bounds" } */ + return a[0]; +} + +void g3 (void) +{ + char a[6] NONSTR = { 1, 2, 3, 4, 5, 6 }; + char b[6] NONSTR = { 6, 5, 4, 3, 2, 1 }; + char *p = stpncpy (a, b + 2, 4); /* { dg-bogus "\\\[-Warray-bounds" } */ + sink (a, p); +} + +void g4 (NONSTR char *d) +{ + char b[6] NONSTR = { 6, 5, 4, 3, 2, 1 }; + char *p = stpncpy (d, b + 3, 3); /* { dg-bogus "\\\[-Warray-bounds" } */ + sink (d, p); +} diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c index 721832e3f19..1969c728dc7 100644 --- a/gcc/tree-ssa-strlen.c +++ b/gcc/tree-ssa-strlen.c @@ -2191,13 +2191,21 @@ handle_builtin_stxncpy (built_in_function, gimple_stmt_iterator *gsi) int didx = get_stridx (dst); if (strinfo *sidst = didx > 0 ? get_strinfo (didx) : NULL) { - /* Compute the size of the destination string including the NUL. */ + /* Compute the size of the destination string including the nul + if it is known to be nul-terminated. */ if (sidst->nonzero_chars) { - tree type = TREE_TYPE (sidst->nonzero_chars); - dstsize = fold_build2 (PLUS_EXPR, type, sidst->nonzero_chars, - build_int_cst (type, 1)); + if (sidst->endptr) + { + /* String is known to be nul-terminated. */ + tree type = TREE_TYPE (sidst->nonzero_chars); + dstsize = fold_build2 (PLUS_EXPR, type, sidst->nonzero_chars, + build_int_cst (type, 1)); + } + else + dstsize = sidst->nonzero_chars; } + dst = sidst->ptr; } @@ -2209,12 +2217,18 @@ handle_builtin_stxncpy (built_in_function, gimple_stmt_iterator *gsi) over the terminating nul so SISRC->DONT_INVALIDATE must be left clear. */ - /* Compute the size of the source string including the NUL. */ + /* Compute the size of the source string including the terminating + nul if its known to be nul-terminated. */ if (sisrc->nonzero_chars) { - tree type = TREE_TYPE (sisrc->nonzero_chars); - srcsize = fold_build2 (PLUS_EXPR, type, sisrc->nonzero_chars, - build_int_cst (type, 1)); + if (sisrc->endptr) + { + tree type = TREE_TYPE (sisrc->nonzero_chars); + srcsize = fold_build2 (PLUS_EXPR, type, sisrc->nonzero_chars, + build_int_cst (type, 1)); + } + else + srcsize = sisrc->nonzero_chars; } src = sisrc->ptr;