From patchwork Wed Oct 5 11:45:55 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 58729 From: aburgess@redhat.com (Andrew Burgess) Date: Wed, 05 Oct 2022 12:45:55 +0100 List-Id: gdb-patches mailing list Subject: [PATCH 4/4] sim: Suppress non-literal printf warning In-Reply-To: <35eaee9855aa3882a1ecc9de9d62c0f8b2fe0e7c.1663073826.git.research_trasio@irq.a4lg.com> References: <35eaee9855aa3882a1ecc9de9d62c0f8b2fe0e7c.1663073826.git.research_trasio@irq.a4lg.com> Message-ID: <87h70ifq8c.fsf@redhat.com> 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? My motivation is to avoid disabling diagnostics as much as possible. 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