From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 95633 invoked by alias); 20 Nov 2017 16:33:53 -0000 Mailing-List: contact gdb-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-owner@sourceware.org Received: (qmail 95611 invoked by uid 89); 20 Nov 2017 16:33:53 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-23.5 required=5.0 tests=AWL,BAYES_05,FREEMAIL_FROM,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KB_WAM_FROM_NAME_SINGLEWORD,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=misuse, UD:remote-utils.c, 1071, PyMem_RawMalloc X-HELO: mail-ot0-f174.google.com Received: from mail-ot0-f174.google.com (HELO mail-ot0-f174.google.com) (74.125.82.174) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 20 Nov 2017 16:33:50 +0000 Received: by mail-ot0-f174.google.com with SMTP id b49so8110671otj.5 for ; Mon, 20 Nov 2017 08:33:50 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to; bh=Ec/z4FaE1FVlpysZtRQhT6Xh6sC8a/SPxLQkvkW1WP8=; b=HZaIbOYsv9PKIESyG2f439Cr1z0r1Qg8HxZY2nJh/gTeyAxDWG/ccg5V5tu2KRU35x 0Nb3Q8fHx5rXs7cfPmbIF3bXiQdhJTTkjUP37uXqbr6Qpg4tGtiAX0J60IQn/fQz0eTP F3aI9xMzHsnAReBZN0oR4z5tzoeEKshwYrlYek7EwvEVDdE2kF+E/1p70mXSva4y1jUo pBEx5AZ4s2hj7o5HkM+C3RRl1xWfNoWL9aL/EEH3lZB7uAxJsm2XbmoT4KbP56r0LJvb mKrcW7JineULvm8hmhUA/TMTdfpxwL7o+exN/ZyI0x2HrG15abV261kJ6FFohM5G062Q 0AAA== X-Gm-Message-State: AJaThX7LdEh+tx4YnWoskVdfLgJDCJEU13xsS5c7a5ojhATtoNsCoSYS O33dX5DWUjpEaXfNO4O+13fxFg== X-Google-Smtp-Source: AGs4zMal33Ps7kBd4e/au6VXNvPi3Ru7Oa07fXiQ089opugTUqUUFmOhBO93KOBz9pHHes9/lvo7PA== X-Received: by 10.157.43.244 with SMTP id u107mr8670368ota.121.1511195629117; Mon, 20 Nov 2017 08:33:49 -0800 (PST) Received: from localhost.localdomain (75-171-240-43.hlrn.qwest.net. [75.171.240.43]) by smtp.gmail.com with ESMTPSA id f21sm5084885otj.39.2017.11.20.08.33.47 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 20 Nov 2017 08:33:47 -0800 (PST) Subject: Re: Fail to compile GDB with recent GCC trunk (-Werror=stringop-overflow=, -Werror=stringop-truncation) To: Yao Qi , gcc@gcc.gnu.org, gdb@sourceware.org References: <86r2strlrd.fsf@gmail.com> Cc: msebor@redhat.com From: Martin Sebor Message-ID: <1a24cb6d-efa0-7f07-76cc-ca3708ee9d14@gmail.com> Date: Mon, 20 Nov 2017 16:33: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: <86r2strlrd.fsf@gmail.com> Content-Type: multipart/mixed; boundary="------------0622EAEEDC51466749B52F55" X-SW-Source: 2017-11/txt/msg00018.txt.bz2 This is a multi-part message in MIME format. --------------0622EAEEDC51466749B52F55 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-length: 3410 On 11/20/2017 08:51 AM, Yao Qi wrote: > > Hi, > I failed to compile GDB with GCC trunk (8.0.0 20171117) because of some > -Werror=stringop-overflow= and -Werror=stringop-truncation warnings. > Some of them are not necessary to me, I have the attached patch for two of these but I have been waiting to submit it until the latest GCC patch has been approved that adjusts the checker a bit. > > 1. ../../binutils-gdb/gdb/python/py-gdb-readline.c:79:15: error: ‘char* strncpy(char*, const char*, size_t)’ output truncated before terminating nul copying as many bytes from a string as its length [-Werror=stringop-truncation] > strncpy (q, p, n); > ~~~~~~~~^~~~~~~~~ > ../../binutils-gdb/gdb/python/py-gdb-readline.c:73:14: note: length computed here > n = strlen (p); > ~~~~~~~^~~ > > the code is simple, > > n = strlen (p); > > /* Copy the line to Python and return. */ > q = (char *) PyMem_RawMalloc (n + 2); > if (q != NULL) > { > strncpy (q, p, n); > q[n] = '\n'; > q[n + 1] = '\0'; > } > > I don't see the point of warning here. The overall purpose of the warning is to help find likely misuses of strncpy and strncat. As with any warning that's based on intent, it cannot avoid highlighting some safe uses, or missing some unsafe ones. The case above is based on a heuristic designed to find bugs where the bound depends on the length of the source rather the size of the destination, as in: strncpy (d, s, strlen (s)); This is, unfortunately, a common misuse/mistake. It's often seen in legacy code that's being updated in response to a security mandate to replace strcpy with strncpy. The GDB use case, although safe, is also not how the function is intended to be used. The intended use is to specify the size of the destination, typically a statically allocated array, and have the function fill it with data (not necessarily a string, and not necessarily containing a terminating nul). When the array is allocated dynamically and sized to store the entire string it's preferable to use some other function (e.g., memcpy or strcpy). > > 2. ../../binutils-gdb/gdb/cp-namespace.c:1071:11: error: ‘char* strncpy(char*, const char*, size_t)’ output truncated before terminating nul copying 2 bytes from a string of the same length [-Werror=stringop-truncation] > strncpy (full_name + scope_length, "::", 2); > ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > full_name = (char *) alloca (scope_length + 2 + strlen (name) + 1); > strncpy (full_name, scope, scope_length); > strncpy (full_name + scope_length, "::", 2); This is safe, although also not the intended use of the function. The call above can be replaced either by memcpy or strcpy. There also is no good way to avoid warning on it without compromising the efficacy of the checker. > strcpy (full_name + scope_length + 2, name); > > the code looks right to me, > > Likewise, > > ../../../binutils-gdb/gdb/gdbserver/remote-utils.c:1204:14: error: ‘char* strncpy(char*, const char*, size_t)’ output truncated before terminating nul copying 6 bytes from a string of the same length [-Werror=stringop-truncation] > strncpy (buf, "watch:", 6); > ~~~~~~~~^~~~~~~~~~~~~~~~~~ > > strncpy (buf, "watch:", 6); > buf += 6; > .... > *buf = '\0'; As above, memcpy or strcpy are the preferred alternatives. Martin --------------0622EAEEDC51466749B52F55 Content-Type: message/rfc822; name="Attached Message" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="Attached Message" Content-length: 3761 X-Identity-Key: id4 X-Account-Key: account4 From: Martin Sebor Subject: [PATCH] avoid GCC 8.0 -Wstringop-truncation To: gdb-patches@sourceware.org Message-ID: <07b8393f-35c1-e7bf-158f-3ab4e137afb1@gmail.com> Date: Fri, 17 Nov 2017 17:47:06 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="------------4B27CCFB67596E6EB55D67F6" This is a multi-part message in MIME format. --------------4B27CCFB67596E6EB55D67F6 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-length: 836 The latest revision of GCC 8.0 adds a -Wstringop-truncation option to detect common misuses of the strncpy and strncat functions that may truncate the copy and leave the result without a terminating nul character. In testing the implementation with GDB sources on x86_64 I found a few instances of the warning that are issued for what's safe but nevertheless not strictly intended uses of the functions (i.e., to create "bounded" non-nul-terminated copies of a string). I adjusted the warning to accept some but not all of these use cases. The attached patch shows the two instances of the warning that I had to suppress in GDB. In general, even though the checker handles some such cases, to avoid the warning, it's best to use strncpy only with a bound that reflects the size of the destination, never that of the source. Martin --------------4B27CCFB67596E6EB55D67F6 Content-Type: text/x-patch; name="gdb-wstringop-trunc.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="gdb-wstringop-trunc.diff" Content-length: 2055 2017-11-10 Martin Sebor * gdb/cli/cli-decode.c (help_list): Use strcpy and memcpy instead of strncpy. * gdb/cp-namespace.c (cp_lookup_transparent_type_loop): Use strcpy instead of strncpy to avoid -Wstringop-truncation. * gdb/gdbserver/remote-utils.c (prepare_resume_reply): Use memcpy instead of strncpy to avoid -Wstringop-truncation. diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c index 87ebed5..968779e 100644 --- a/gdb/cli/cli-decode.c +++ b/gdb/cli/cli-decode.c @@ -1,4 +1,4 @@ -/* Handle lists of commands, their decoding and documentation, for GDB. +t/* Handle lists of commands, their decoding and documentation, for GDB. Copyright (C) 1986-2017 Free Software Foundation, Inc. @@ -1115,9 +1115,8 @@ help_list (struct cmd_list_element *list, const char *cmdtype, if (len) { cmdtype1[0] = ' '; - strncpy (cmdtype1 + 1, cmdtype, len - 1); - cmdtype1[len] = 0; - strncpy (cmdtype2, cmdtype, len - 1); + strcpy (cmdtype1 + 1, cmdtype); + memcpy (cmdtype2, cmdtype, len - 1); strcpy (cmdtype2 + len - 1, " sub"); } diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c index 214b7e1..fabe87a 100644 --- a/gdb/cp-namespace.c +++ b/gdb/cp-namespace.c @@ -1068,7 +1068,7 @@ cp_lookup_transparent_type_loop (const char *name, full_name = (char *) alloca (scope_length + 2 + strlen (name) + 1); strncpy (full_name, scope, scope_length); - strncpy (full_name + scope_length, "::", 2); + strcpy (full_name + scope_length, "::"); strcpy (full_name + scope_length + 2, name); return basic_lookup_transparent_type (full_name); diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c index 66e0652..e314447 100644 --- a/gdb/gdbserver/remote-utils.c +++ b/gdb/gdbserver/remote-utils.c @@ -1201,7 +1201,7 @@ prepare_resume_reply (char *buf, ptid_t ptid, CORE_ADDR addr; int i; - strncpy (buf, "watch:", 6); + memcpy (buf, "watch:", 6); buf += 6; addr = (*the_target->stopped_data_address) (); --------------4B27CCFB67596E6EB55D67F6-- --------------0622EAEEDC51466749B52F55--