Message ID | 20200129175943.1035-1-sergiodj@redhat.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 92390 invoked by alias); 29 Jan 2020 17:59:56 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 92379 invoked by uid 89); 29 Jan 2020 17:59:56 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-16.2 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=inferiors, our 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, 29 Jan 2020 17:59:55 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1580320793; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=fyxcvuG8nvypVpk5wXNo7tFmSD/L73aRwRb8rrEEiNc=; b=bExVvjJVMmIBA9kVmhPXyT4hXJMUTX1im3CjNhTsDHJv9kmYy+0zzJYonQkmY75MciNnuV qcOxxVDKcS5BZs+QjGDJ1TqCfou5RERribxkpt+banorGXEqlBgkiWHNZCsRPHWGWs5f8f rV7XfallXSvqCj5vT8OYwWzJtafFk8k= 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-244-BAVEEklyMCSrJ1nQ85bvsw-1; Wed, 29 Jan 2020 12:59:51 -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 049DA107ACC7 for <gdb-patches@sourceware.org>; Wed, 29 Jan 2020 17:59:51 +0000 (UTC) Received: from psique.yyz.redhat.com (unused-10-15-17-196.yyz.redhat.com [10.15.17.196]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3F74660BF3; Wed, 29 Jan 2020 17:59:48 +0000 (UTC) From: Sergio Durigan Junior <sergiodj@redhat.com> To: GDB Patches <gdb-patches@sourceware.org> Cc: Sergio Durigan Junior <sergiodj@redhat.com> Subject: [PATCH] Assert that 'length' > 0 on infcmd.c:construct_inferior_arguments Date: Wed, 29 Jan 2020 12:59:43 -0500 Message-Id: <20200129175943.1035-1-sergiodj@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes |
Commit Message
Sergio Durigan Junior
Jan. 29, 2020, 5:59 p.m. UTC
While testing a GCC 10 build of our git HEAD, I noticed an error triggered by -Werror-stringop on infcmd.c:construct_inferior_arguments. One of the things the function does is calculate the length of the string that will hold the inferior's arguments. GCC warns us that 'length' can be 0, which can lead to undesired behaviour: ../../gdb/infcmd.c: In function 'char* construct_inferior_arguments(int, char**)': ../../gdb/infcmd.c:369:17: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=] 369 | result[0] = '\0'; | ~~~~~~~~~~^~~~~~ ../../gdb/infcmd.c:368:33: note: at offset 0 to an object with size 0 allocated by 'xmalloc' here 368 | result = (char *) xmalloc (length); | ~~~~~~~~^~~~~~~~ The solution here is to explicit check that 'length' is greater than 0. Since we're dealing with 'argc', I think it's pretty much guaranteed that it's going to be at least 1. Tested by rebuilding. OK? gdb/ChangeLog: 2020-01-29 Sergio Durigan Junior <sergiodj@redhat.com> * infcmd.c (construct_inferior_arguments): Assert that 'length' is greater than 0. Change-Id: Ide8407cbedcb4921de1843a6a15bbcb7676c7d26 --- gdb/infcmd.c | 5 +++++ 1 file changed, 5 insertions(+)
Comments
On 1/29/20 5:59 PM, Sergio Durigan Junior wrote: > While testing a GCC 10 build of our git HEAD, I noticed an error > triggered by -Werror-stringop on > infcmd.c:construct_inferior_arguments. One of the things the function > does is calculate the length of the string that will hold the > inferior's arguments. GCC warns us that 'length' can be 0, which can > lead to undesired behaviour: > > ../../gdb/infcmd.c: In function 'char* construct_inferior_arguments(int, char**)': > ../../gdb/infcmd.c:369:17: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=] > 369 | result[0] = '\0'; > | ~~~~~~~~~~^~~~~~ > ../../gdb/infcmd.c:368:33: note: at offset 0 to an object with size 0 allocated by 'xmalloc' here > 368 | result = (char *) xmalloc (length); > | ~~~~~~~~^~~~~~~~ > > The solution here is to explicit check that 'length' is greater than > 0. Since we're dealing with 'argc', I think it's pretty much > guaranteed that it's going to be at least 1. It's a certainly -- there's only one caller to construct_inferior_arguments, which does: const char * get_inferior_args (void) { if (current_inferior ()->argc != 0) { char *n; n = construct_inferior_arguments (current_inferior ()->argc, current_inferior ()->argv); (construct_inferior_arguments could be made static, btw.) > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -365,6 +365,11 @@ construct_inferior_arguments (int argc, char **argv) > length += strlen (argv[i]) + 1; > } > > + /* argc should always be at least 1, but we double check this Uppercase ARGC. But putting gdb_assert (argc > 0); at the top of the function instead as I originally suggested also works for me (tried current gcc master), which seems a bit better to me, as it covers both branches at once. Did it not work for you? This makes gcc see that the loops always run at least once. Thanks, Pedro Alves
diff --git a/gdb/infcmd.c b/gdb/infcmd.c index b44adca88d..f468c788b6 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -365,6 +365,11 @@ construct_inferior_arguments (int argc, char **argv) length += strlen (argv[i]) + 1; } + /* argc should always be at least 1, but we double check this + here. This is also needed to silence -Werror-stringop + warnings. */ + gdb_assert (length > 0); + result = (char *) xmalloc (length); result[0] = '\0'; for (i = 0; i < argc; ++i)