From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 116709 invoked by alias); 29 Jan 2020 18:44:55 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 116700 invoked by uid 89); 29 Jan 2020 18:44:55 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=uppercase, sk:get_inf X-HELO: us-smtp-delivery-1.mimecast.com Received: from us-smtp-2.mimecast.com (HELO us-smtp-delivery-1.mimecast.com) (207.211.31.81) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 29 Jan 2020 18:44:54 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1580323492; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=4eqnS2KkC2+/Xy6epoHkB9d1uIlEQTHgyKAg5534Qvw=; b=Zeutkw1uAi8GzmUiGfLsvaI3L/Y+X1Wr9VIfeZvjFpD4Jvv9NgMG8bV59d5M/i80b/glIR 99iLlsgIHl4DywOi+MQfYfh2nLdy0V2d4dQnJ9TgQ8GnXq/1562a35mqSrNGVIE5ozPfma zA5Nbo8RJ+N7/oldy9n033WFAU/nAjc= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-199-SqPm1iDENouKJ89OrVaXCA-1; Wed, 29 Jan 2020 13:44:51 -0500 Received: by mail-wm1-f71.google.com with SMTP id p26so301867wmg.5 for ; Wed, 29 Jan 2020 10:44:51 -0800 (PST) Return-Path: Received: from ?IPv6:2001:8a0:f909:7b00:56ee:75ff:fe8d:232b? ([2001:8a0:f909:7b00:56ee:75ff:fe8d:232b]) by smtp.gmail.com with ESMTPSA id k16sm4185678wru.0.2020.01.29.10.44.49 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 29 Jan 2020 10:44:49 -0800 (PST) Subject: Re: [PATCH] Assert that 'length' > 0 on infcmd.c:construct_inferior_arguments To: Sergio Durigan Junior , GDB Patches References: <20200129175943.1035-1-sergiodj@redhat.com> From: Pedro Alves Message-ID: <640bcf24-b8c3-3c96-0ea5-2efdc29f039a@redhat.com> Date: Wed, 29 Jan 2020 19:20:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20200129175943.1035-1-sergiodj@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2020-01/txt/msg00951.txt.bz2 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