From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id yziQNhJqPmPIcgYAWB0awg (envelope-from ) for ; Thu, 06 Oct 2022 01:39:30 -0400 Received: by simark.ca (Postfix, from userid 112) id D34C41E112; Thu, 6 Oct 2022 01:39:30 -0400 (EDT) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=gu25Mc1p; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 825041E0CB for ; Thu, 6 Oct 2022 01:39:30 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 253AB384D175 for ; Thu, 6 Oct 2022 05:39:30 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 253AB384D175 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1665034770; bh=ikFCnRHn5mTeQbMPZTDMFmY+O7SyhjlkpvBeG4wvek8=; h=Date:Subject:To:References:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=gu25Mc1p7Lav3KTZ8iHnlYUSTFNSXndUgemmkwa2jD5UlYFjOxq77+kjuq94fHaHE hmTeT+tXMpxDbnSGRU2UJs9V/qQEqLEeKSsdAxqnZLwRzkMHsZj/TOKs2g8XUY8L3e 5ARnI26Ng6c/9eX0Hzn/fTAsBMf1Z/saMl2fZWhQ= Received: from mail-sender-0.a4lg.com (mail-sender-0.a4lg.com [IPv6:2401:2500:203:30b:4000:6bfe:4757:0]) by sourceware.org (Postfix) with ESMTPS id DE19C384D14B for ; Thu, 6 Oct 2022 05:39:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org DE19C384D14B Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail-sender-0.a4lg.com (Postfix) with ESMTPSA id 4E20B300089; Thu, 6 Oct 2022 05:39:08 +0000 (UTC) Message-ID: <22f70b64-0bf9-96f8-a734-d16bbec6504a@irq.a4lg.com> Date: Thu, 6 Oct 2022 14:39:07 +0900 Mime-Version: 1.0 Subject: Re: [PATCH 4/4] sim: Suppress non-literal printf warning Content-Language: en-US To: Andrew Burgess References: <35eaee9855aa3882a1ecc9de9d62c0f8b2fe0e7c.1663073826.git.research_trasio@irq.a4lg.com> <87h70ifq8c.fsf@redhat.com> In-Reply-To: <87h70ifq8c.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Tsukasa OI via Gdb-patches Reply-To: Tsukasa OI Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" On 2022/10/05 20:45, Andrew Burgess wrote: > Tsukasa OI writes: > >> Clang generates a warning if the format parameter of a printf-like function >> is not a literal. However, on hw_vabort, it's unavoidable to use non- >> literal as a format string (unless we make huge redesign). >> >> We have "include/diagnostics.h" to suppress certain warnings only when >> necessary. Because DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL can suppress >> warnings when the format parameter of a printf-like function is not a >> literal, this commit adds this (only where necessary) to suppress this >> error with "-Werror", the default configuration. >> >> sim/ChangeLog: >> >> * common/sim-hw.c (hw_vabort): Suppress non-literal printf warning >> by using DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL. >> --- >> sim/common/sim-hw.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/sim/common/sim-hw.c b/sim/common/sim-hw.c >> index cece5638bc9..36f355d2262 100644 >> --- a/sim/common/sim-hw.c >> +++ b/sim/common/sim-hw.c >> @@ -425,10 +425,13 @@ hw_vabort (struct hw *me, >> strcat (msg, ": "); >> strcat (msg, fmt); >> /* report the problem */ >> + DIAGNOSTIC_PUSH >> + DIAGNOSTIC_IGNORE_FORMAT_NONLITERAL >> sim_engine_vabort (hw_system (me), >> STATE_HW (hw_system (me))->cpu, >> STATE_HW (hw_system (me))->cia, >> msg, ap); >> + DIAGNOSTIC_POP > > Rather than disabling diagnostics, I'd like to propose the patch below > which expands FMT and AP within sim-hw.c, then passes the expanded > string through to sim_engine_abort. What do you think of this? Ah, It took a while to understand but makes sense to me. I just needed to add ATTRIBUTE_PRINTF (2, 0) to suppress "-Werror -Wformat-nonliteral" but I prefer to use your patch instead. > > My motivation is to avoid disabling diagnostics as much as possible. I support your opinion as possible. I sometimes disable some warnings intentionally but it's because I thought disabling the warning is the only viable solution. In this case, it wasn't. Thanks, Tsukasa > > As far as I can tell the host_callback_struct::evprintf_filtered > callback is just the standard printf API, so using vsnprintf should > expand everything correctly. > > Thanks, > Andrew > > --- > > diff --git a/sim/common/sim-hw.c b/sim/common/sim-hw.c > index cece5638bc9..7bfe91e4ae2 100644 > --- a/sim/common/sim-hw.c > +++ b/sim/common/sim-hw.c > @@ -408,8 +408,11 @@ hw_vabort (struct hw *me, > const char *fmt, > va_list ap) > { > + int len; > const char *name; > char *msg; > + va_list cpy; > + > /* find an identity */ > if (me != NULL && hw_path (me) != NULL && hw_path (me) [0] != '\0') > name = hw_path (me); > @@ -419,16 +422,19 @@ hw_vabort (struct hw *me, > name = hw_family (me); > else > name = "device"; > - /* construct an updated format string */ > - msg = alloca (strlen (name) + strlen (": ") + strlen (fmt) + 1); > - strcpy (msg, name); > - strcat (msg, ": "); > - strcat (msg, fmt); > + > + /* Expand FMT and AP into MSG buffer. */ > + va_copy (cpy, ap); > + len = vsnprintf (NULL, 0, fmt, cpy) + 1; > + va_end (cpy); > + msg = alloca (len); > + vsnprintf (msg, len, fmt, ap); > + > /* report the problem */ > - sim_engine_vabort (hw_system (me), > - STATE_HW (hw_system (me))->cpu, > - STATE_HW (hw_system (me))->cia, > - msg, ap); > + sim_engine_abort (hw_system (me), > + STATE_HW (hw_system (me))->cpu, > + STATE_HW (hw_system (me))->cia, > + "%s: %s", name, msg); > } > > void >