From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31193 invoked by alias); 23 Jan 2014 12:34:26 -0000 Mailing-List: contact gcc-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-owner@gcc.gnu.org Received: (qmail 31140 invoked by uid 89); 23 Jan 2014 12:34:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: smtp.mozilla.org Received: from mx2.corp.phx1.mozilla.com (HELO smtp.mozilla.org) (63.245.216.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 23 Jan 2014 12:34:24 +0000 Received: from tsaunders-iceball.corp.tor1.mozilla.com (69-165-234-187.cable.teksavvy.com [69.165.234.187]) (Authenticated sender: tsaunders@mozilla.com) by mx2.mail.corp.phx1.mozilla.com (Postfix) with ESMTPSA id 84ABAF2822 for ; Thu, 23 Jan 2014 04:34:22 -0800 (PST) Date: Thu, 23 Jan 2014 13:16:00 -0000 From: Trevor Saunders To: gcc@gcc.gnu.org Subject: Re: -Wformat-security warnings generated in gcc build Message-ID: <20140123123413.GA13854@tsaunders-iceball.corp.tor1.mozilla.com> References: <87d2jjc4rx.fsf@seketeli.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <87d2jjc4rx.fsf@seketeli.org> User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes X-SW-Source: 2014-01/txt/msg00203.txt.bz2 On Thu, Jan 23, 2014 at 12:32:34PM +0100, Dodji Seketeli wrote: > "Joseph S. Myers" a écrit: > > > On Wed, 22 Jan 2014, Prathamesh Kulkarni wrote: > > > >> Unfortunately, I am not clear on how to check for format specifiers in string. > >> Should I do it manually by checking the format string for specifiers > >> and call abort if found a no-argument specifier, > >> or is there a better way to do it ? > > > > I'll leave it to Dodji as diagnostics maintainer to advise on the best way > > to implement error_at_no_args. > > Thanks for the heads-up, Joseph. > > Assuming we want to do something more than just segfaulting if > error_at_no_args is given a format specifier that needs an argument, I > think the function pretty-print.c:pp_format is the place where the > core of the change has to be done. Basically, that function walks the > format string and expands format specifiers. > > Thus, in that function, if a format specifier needs an argument (that > is, each time we try to access text->args_ptr) but we are in a context > where we want to accept only no-argument specifiers, we can call abort > or internal_error with a meaningful error message. > > I guess we can assume that we know that we are in a > oo-argument-specifier context if text->args_ptr is NULL in pp_format() > That text->args_ptr is actually the va_ap that you (Prathamesh) set to > NULL when you did: > > void > error_at_no_args (location_t loc, const char *gmsgid) > { > diagnostic_info diagnostic; > diagnostic_set_info (&diagnostic, gmsgid, NULL, loc, DK_ERROR); > ^^^^ > ^ > | > here: __| > > report_diagnostic (&diagnostic); > } > > > And then, just keep the error_at_no_args() implementation like what > you did already. > > Also, you'd need to modify cp/error.c:cp_printer in a similar way, to > issue an internal_error each time we try to access a null test->args_ptr. > > But then, I guess some people might argue that we could as well let > the code as is and just segfault on accessing a NULL test->args_ptr. > I would personally lean towards aborting with a meaningful error > message, but I'd like to hear what others think about this. It seems like this is prime teritory for an assert, and then documenting the API to be if you use format specifiers then you must pass args as well. Considering that both makes it easy to see what went wrong, and doesn't trade performance to get most of what you get with internal_error() Trev > > I hope this helps. > > -- > Dodji