[PATCH 1/3] sim: callback: drop unused printf helpers

[PATCH 1/3] sim: callback: drop unused printf helpers

[Mike Frysinger]

These cover functions aren't used anywhere, so drop them.  There was
one caller, but it's old DOS code that most likely hasn't been tested
in years, so just delete that too.
---
 sim/common/callback.c  | 61 ++----------------------------------------
 sim/common/sim-utils.h |  7 -----
 2 files changed, 2 insertions(+), 66 deletions(-)

diff --git a/sim/common/callback.c b/sim/common/callback.c
index c0ace6e4c8e5..f773de1c19f0 100644
--- a/sim/common/callback.c
+++ b/sim/common/callback.c
@@ -49,12 +49,6 @@
 #define PIPE_BUF 512
 #endif
 
-/* ??? sim_cb_printf should be cb_printf, but until the callback support is
-   broken out of the simulator directory, these are here to not require
-   sim-utils.h.  */
-void sim_cb_printf (host_callback *, const char *, ...);
-void sim_cb_eprintf (host_callback *, const char *, ...);
-
 extern CB_TARGET_DEFS_MAP cb_init_syscall_map[];
 extern CB_TARGET_DEFS_MAP cb_init_errno_map[];
 extern CB_TARGET_DEFS_MAP cb_init_signal_map[];
@@ -148,43 +142,21 @@ os_close (host_callback *p, int fd)
 /* taken from gdb/util.c:notice_quit() - should be in a library */
 
 
-#if defined(__GO32__) || defined (_MSC_VER)
+#if defined(_MSC_VER)
 static int
 os_poll_quit (host_callback *p)
 {
-#if defined(__GO32__)
-  int kbhit ();
-  int getkey ();
-  if (kbhit ())
-    {
-      int k = getkey ();
-      if (k == 1)
-	{
-	  return 1;
-	}
-      else if (k == 2)
-	{
-	  return 1;
-	}
-      else
-	{
-	  sim_cb_eprintf (p, "CTRL-A to quit, CTRL-B to quit harder\n");
-	}
-    }
-#endif
-#if defined (_MSC_VER)
   /* NB - this will not compile! */
   int k = win32pollquit ();
   if (k == 1)
     return 1;
   else if (k == 2)
     return 1;
-#endif
   return 0;
 }
 #else
 #define os_poll_quit 0
-#endif /* defined(__GO32__) || defined(_MSC_VER) */
+#endif /* defined(_MSC_VER) */
 
 static int
 os_get_errno (host_callback *p)
@@ -1078,35 +1050,6 @@ cb_host_to_target_stat (host_callback *cb, const struct stat *hs, void *ts)
   return p - (char *) ts;
 }
 \f
-/* Cover functions to the vfprintf callbacks.
-
-   ??? If one thinks of the callbacks as a subsystem onto itself [or part of
-   a larger "remote target subsystem"] with a well defined interface, then
-   one would think that the subsystem would provide these.  However, until
-   one is allowed to create such a subsystem (with its own source tree
-   independent of any particular user), such a critter can't exist.  Thus
-   these functions are here for the time being.  */
-
-void
-sim_cb_printf (host_callback *p, const char *fmt, ...)
-{
-  va_list ap;
-
-  va_start (ap, fmt);
-  p->vprintf_filtered (p, fmt, ap);
-  va_end (ap);
-}
-
-void
-sim_cb_eprintf (host_callback *p, const char *fmt, ...)
-{
-  va_list ap;
-
-  va_start (ap, fmt);
-  p->evprintf_filtered (p, fmt, ap);
-  va_end (ap);
-}
-
 int
 cb_is_stdin (host_callback *cb, int fd)
 {
diff --git a/sim/common/sim-utils.h b/sim/common/sim-utils.h
index bfc8ac72d08e..1af4ea206b82 100644
--- a/sim/common/sim-utils.h
+++ b/sim/common/sim-utils.h
@@ -73,13 +73,6 @@ void sim_do_commandf (SIM_DESC sd, const char *fmt, ...)
     ATTRIBUTE_PRINTF (2, 3);
 
 
-/* These are defined in callback.c as cover functions to the vprintf
-   callbacks.  */
-
-void sim_cb_printf (host_callback *, const char *, ...);
-void sim_cb_eprintf (host_callback *, const char *, ...);
-
-
 /* sim-basics.h defines a number of enumerations, convert each of them
    to a string representation */
 const char *map_to_str (unsigned map);
-- 
2.31.1

[PATCH 2/3] sim: callback: add printf attributes

[Mike Frysinger]

This helps these funcs get printf format checking coverage.

The sim-io.c hack as a result is a bit unfortunate, but the compiler
throws warnings when printing with empty strings.  In this one case,
we actually want that due to the side-effect of the callback halting
execution for us.
---
 include/sim/callback.h | 12 ++++++++----
 sim/common/sim-io.c    |  3 ++-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/include/sim/callback.h b/include/sim/callback.h
index be72f4503e1a..06aa2d4be790 100644
--- a/include/sim/callback.h
+++ b/include/sim/callback.h
@@ -113,18 +113,22 @@ struct host_callback_struct
   int (*init)     (host_callback *);
 
   /* depreciated, use vprintf_filtered - Talk to the user on a console.  */
-  void (*printf_filtered) (host_callback *, const char *, ...);
+  void (*printf_filtered) (host_callback *, const char *, ...)
+    ATTRIBUTE_PRINTF_2;
 
   /* Talk to the user on a console.  */
-  void (*vprintf_filtered) (host_callback *, const char *, va_list);
+  void (*vprintf_filtered) (host_callback *, const char *, va_list)
+    ATTRIBUTE_PRINTF (2, 0);
 
   /* Same as vprintf_filtered but to stderr.  */
-  void (*evprintf_filtered) (host_callback *, const char *, va_list);
+  void (*evprintf_filtered) (host_callback *, const char *, va_list)
+    ATTRIBUTE_PRINTF (2, 0);
 
   /* Print an error message and "exit".
      In the case of gdb "exiting" means doing a longjmp back to the main
      command loop.  */
-  void (*error) (host_callback *, const char *, ...) ATTRIBUTE_NORETURN;
+  void (*error) (host_callback *, const char *, ...)
+    ATTRIBUTE_NORETURN ATTRIBUTE_PRINTF_2;
 
   int last_errno;		/* host format */
 
diff --git a/sim/common/sim-io.c b/sim/common/sim-io.c
index e09a4af7ba2e..0d14e0dfc452 100644
--- a/sim/common/sim-io.c
+++ b/sim/common/sim-io.c
@@ -309,7 +309,8 @@ sim_io_error (SIM_DESC sd,
     va_start (ap, fmt);
     STATE_CALLBACK (sd)->evprintf_filtered (STATE_CALLBACK (sd), fmt, ap);
     va_end (ap);
-    STATE_CALLBACK (sd)->error (STATE_CALLBACK (sd), "");
+    /* Printing a space here avoids empty printf compiler warnings.  */
+    STATE_CALLBACK (sd)->error (STATE_CALLBACK (sd), " ");
   }
 }
 
-- 
2.31.1

[PATCH 3/3] sim: io: add printf attributes to vprintf funcs too

[Mike Frysinger]

The compiler can still do basic format checks with vprintf style
funcs, so add the printf attribute to these.
---
 sim/common/sim-io.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/sim/common/sim-io.h b/sim/common/sim-io.h
index a78e8f59d540..9536e4b587a2 100644
--- a/sim/common/sim-io.h
+++ b/sim/common/sim-io.h
@@ -65,13 +65,15 @@ void sim_io_printf (SIM_DESC sd,
 		    const char *fmt,
 		    ...) ATTRIBUTE_PRINTF (2, 3);
 
-void sim_io_vprintf (SIM_DESC sd, const char *fmt, va_list ap);
+void sim_io_vprintf (SIM_DESC sd, const char *fmt, va_list ap)
+  ATTRIBUTE_PRINTF (2, 0);
 
 void sim_io_eprintf (SIM_DESC sd,
 		     const char *fmt,
 		     ...) ATTRIBUTE_PRINTF (2, 3);
 
-void sim_io_evprintf (SIM_DESC sd, const char *fmt, va_list ap);
+void sim_io_evprintf (SIM_DESC sd, const char *fmt, va_list ap)
+  ATTRIBUTE_PRINTF (2, 0);
 
 void sim_io_error (SIM_DESC sd,
 		   const char *fmt,
-- 
2.31.1

RE: [PATCH 2/3] sim: callback: add printf attributes

[Aktemur, Tankut Baris]

On Monday, June 28, 2021 5:25 AM, Mike Frysinger wrote:
> This helps these funcs get printf format checking coverage.
> 
> The sim-io.c hack as a result is a bit unfortunate, but the compiler
> throws warnings when printing with empty strings.  In this one case,
> we actually want that due to the side-effect of the callback halting
> execution for us.
> ---
>  include/sim/callback.h | 12 ++++++++----
>  sim/common/sim-io.c    |  3 ++-
>  2 files changed, 10 insertions(+), 5 deletions(-)

Hi,

This patch resulted in the following build error:

/path/to/sim/ppc/sim_calls.c: In function ‘sim_io_error’:
/path/to/sim/ppc/sim_calls.c:382:32: error: zero-length gnu_printf format string [-Werror=format-zero-length]
   callbacks->error (callbacks, "");
                                ^~
/path/to/sim/ppc/sim_calls.c: In function ‘error’:
/path/to/sim/ppc/sim_calls.c:394:32: error: zero-length gnu_printf format string [-Werror=format-zero-length]
   callbacks->error (callbacks, "");
                                ^~

Thanks
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 2/3] sim: callback: add printf attributes

[Mike Frysinger]

On 29 Jun 2021 07:01, Aktemur, Tankut Baris wrote:
> On Monday, June 28, 2021 5:25 AM, Mike Frysinger wrote:
> > This helps these funcs get printf format checking coverage.
> > 
> > The sim-io.c hack as a result is a bit unfortunate, but the compiler
> > throws warnings when printing with empty strings.  In this one case,
> > we actually want that due to the side-effect of the callback halting
> > execution for us.
> > ---
> >  include/sim/callback.h | 12 ++++++++----
> >  sim/common/sim-io.c    |  3 ++-
> >  2 files changed, 10 insertions(+), 5 deletions(-)
> 
> This patch resulted in the following build error:
> 
> /path/to/sim/ppc/sim_calls.c: In function ‘sim_io_error’:
> /path/to/sim/ppc/sim_calls.c:382:32: error: zero-length gnu_printf format string [-Werror=format-zero-length]
>    callbacks->error (callbacks, "");
>                                 ^~
> /path/to/sim/ppc/sim_calls.c: In function ‘error’:
> /path/to/sim/ppc/sim_calls.c:394:32: error: zero-length gnu_printf format string [-Werror=format-zero-length]
>    callbacks->error (callbacks, "");
>                                 ^~

are you forcing extra -W flags in your build ?
-mike

RE: [PATCH 2/3] sim: callback: add printf attributes

[Aktemur, Tankut Baris]

On Tuesday, June 29, 2021 4:32 PM, Mike Frysinger wrote:
> On 29 Jun 2021 07:01, Aktemur, Tankut Baris wrote:
> > On Monday, June 28, 2021 5:25 AM, Mike Frysinger wrote:
> > > This helps these funcs get printf format checking coverage.
> > >
> > > The sim-io.c hack as a result is a bit unfortunate, but the compiler
> > > throws warnings when printing with empty strings.  In this one case,
> > > we actually want that due to the side-effect of the callback halting
> > > execution for us.
> > > ---
> > >  include/sim/callback.h | 12 ++++++++----
> > >  sim/common/sim-io.c    |  3 ++-
> > >  2 files changed, 10 insertions(+), 5 deletions(-)
> >
> > This patch resulted in the following build error:
> >
> > /path/to/sim/ppc/sim_calls.c: In function ‘sim_io_error’:
> > /path/to/sim/ppc/sim_calls.c:382:32: error: zero-length gnu_printf format string [-
> Werror=format-zero-length]
> >    callbacks->error (callbacks, "");
> >                                 ^~
> > /path/to/sim/ppc/sim_calls.c: In function ‘error’:
> > /path/to/sim/ppc/sim_calls.c:394:32: error: zero-length gnu_printf format string [-
> Werror=format-zero-length]
> >    callbacks->error (callbacks, "");
> >                                 ^~
> 
> are you forcing extra -W flags in your build ?
> -mike

No, I'm not.

Interestingly a fresh `configure --enable-targets=all; make` is now
giving me:

/path/to/sim/bpf/../common/cgen-run.c: In function ‘sim_resume’:
/path/to/sim/bpf/../common/cgen-run.c:254:5: error: ‘engine_fns$’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
    (* engine_fns[next_cpu_nr]) (cpu);
    ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/path/to/sim/bpf/../common/cgen-run.c:232:14: note: ‘engine_fns$’ was declared here
   ENGINE_FN *engine_fns[MAX_NR_PROCESSORS];
              ^~~~~~~~~~

Bisection points at

  78484bcab9a sim: bpf: enable -Werror usage

Thanks
-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Re: [PATCH 2/3] sim: callback: add printf attributes

[Mike Frysinger]

On 29 Jun 2021 16:03, Aktemur, Tankut Baris wrote:
> On Tuesday, June 29, 2021 4:32 PM, Mike Frysinger wrote:
> > On 29 Jun 2021 07:01, Aktemur, Tankut Baris wrote:
> > > On Monday, June 28, 2021 5:25 AM, Mike Frysinger wrote:
> > > > This helps these funcs get printf format checking coverage.
> > > >
> > > > The sim-io.c hack as a result is a bit unfortunate, but the compiler
> > > > throws warnings when printing with empty strings.  In this one case,
> > > > we actually want that due to the side-effect of the callback halting
> > > > execution for us.
> > > > ---
> > > >  include/sim/callback.h | 12 ++++++++----
> > > >  sim/common/sim-io.c    |  3 ++-
> > > >  2 files changed, 10 insertions(+), 5 deletions(-)
> > >
> > > This patch resulted in the following build error:
> > >
> > > /path/to/sim/ppc/sim_calls.c: In function ‘sim_io_error’:
> > > /path/to/sim/ppc/sim_calls.c:382:32: error: zero-length gnu_printf format string [-
> > Werror=format-zero-length]
> > >    callbacks->error (callbacks, "");
> > >                                 ^~
> > > /path/to/sim/ppc/sim_calls.c: In function ‘error’:
> > > /path/to/sim/ppc/sim_calls.c:394:32: error: zero-length gnu_printf format string [-
> > Werror=format-zero-length]
> > >    callbacks->error (callbacks, "");
> > >                                 ^~
> > 
> > are you forcing extra -W flags in your build ?
> 
> No, I'm not.

-Wformat-zero-length is implied by -Wformat which is enabled by -Wall.
we aren't using any of those flags, so you shouldn't see these.

> Interestingly a fresh `configure --enable-targets=all; make` is now
> giving me:
> 
> /path/to/sim/bpf/../common/cgen-run.c: In function ‘sim_resume’:
> /path/to/sim/bpf/../common/cgen-run.c:254:5: error: ‘engine_fns$’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>     (* engine_fns[next_cpu_nr]) (cpu);
>     ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> /path/to/sim/bpf/../common/cgen-run.c:232:14: note: ‘engine_fns$’ was declared here
>    ENGINE_FN *engine_fns[MAX_NR_PROCESSORS];
>               ^~~~~~~~~~

this depends on the gcc version it seems.  assert hints were added as so the
compiler can see that we don't use the array uninitialized.  i'm guessing
you're using an older gcc version.  gcc-11 works for me.

gdb disabled -Wmaybe-uninitialized entirely.  prob should do the same for sim
rather than fight buggy compilers.
-mike

[PATCH] sim: ppc: fix printf warnings

[Mike Frysinger]

This code hits some format-zero-length warnings, so hack the code
like we did in the common layers.
---
 sim/ppc/ChangeLog   |  6 ++++++
 sim/ppc/main.c      |  2 ++
 sim/ppc/sim_calls.c | 10 ++++++++--
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/sim/ppc/ChangeLog b/sim/ppc/ChangeLog
index b41d20230d95..7a23d68842bd 100644
--- a/sim/ppc/ChangeLog
+++ b/sim/ppc/ChangeLog
@@ -1,3 +1,9 @@
+2021-06-29  Mike Frysinger  <vapier@gentoo.org>
+
+	* main.c (sim_io_error): Add comment
+	* sim_calls.c (sim_io_error): Likewise.  Change "" to " ".
+	(error): Likewise.
+
 2021-06-20  Mike Frysinger  <vapier@gentoo.org>
 
 	* Makefile.in (INLINE_CFLAGS): Change to $(SIM_INLINE).
diff --git a/sim/ppc/main.c b/sim/ppc/main.c
index 0c1a070fc2c2..2d4d7e402753 100644
--- a/sim/ppc/main.c
+++ b/sim/ppc/main.c
@@ -214,6 +214,8 @@ sim_io_flush_stdoutput(void)
   }
 }
 
+/* Glue to use sim-fpu module.  */
+
 void
 sim_io_error (SIM_DESC sd, const char *msg, ...)
 {
diff --git a/sim/ppc/sim_calls.c b/sim/ppc/sim_calls.c
index 14d4d6586c78..ee9d123b1872 100644
--- a/sim/ppc/sim_calls.c
+++ b/sim/ppc/sim_calls.c
@@ -372,6 +372,8 @@ sim_io_flush_stdoutput(void)
   }
 }
 
+/* Glue to use sim-fpu module.  */
+
 void
 sim_io_error (SIM_DESC sd, const char *fmt, ...)
 {
@@ -379,7 +381,9 @@ sim_io_error (SIM_DESC sd, const char *fmt, ...)
   va_start(ap, fmt);
   callbacks->evprintf_filtered (callbacks, fmt, ap);
   va_end(ap);
-  callbacks->error (callbacks, "");
+  /* Printing a space here avoids empty printf compiler warnings.  Not ideal,
+     but we want error's side-effect where it halts processing.  */
+  callbacks->error (callbacks, " ");
 }
 
 /****/
@@ -391,7 +395,9 @@ error (const char *msg, ...)
   va_start(ap, msg);
   callbacks->evprintf_filtered (callbacks, msg, ap);
   va_end(ap);
-  callbacks->error (callbacks, "");
+  /* Printing a space here avoids empty printf compiler warnings.  Not ideal,
+     but we want error's side-effect where it halts processing.  */
+  callbacks->error (callbacks, " ");
 }
 
 void *
-- 
2.31.1

Re: [PATCH 2/3] sim: callback: add printf attributes

[Mike Frysinger]

both issues should be fixed in the lastest branch if you want to try again
-mike

RE: [PATCH 2/3] sim: callback: add printf attributes

[Aktemur, Tankut Baris]

On Friday, July 2, 2021 2:35 AM, Mike Frysinger wrote:
> both issues should be fixed in the lastest branch if you want to try again
> -mike

Using (7eb1f99adaa "sim: unify reserved instruction bits settings") as HEAD,
I don't see build problems anymore.  Thank you.

-Baris


Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928