RFA: m32r sim: Add prototypes for functions that pass/return DI values

RFA: m32r sim: Add prototypes for functions that pass/return DI values

[Nick Clifton]

Hi Doug, Hi Andrew,

  Jeff Law has tracked down a set of problems with the m32r simulator
  which turned out to be because the m32rbf_h_accum_get_handler()
  function returns a DI value, but it was not prototyped.  So callers
  would assume that it returned an int, and happiness ensued...

  I am proposing the attached patch as a workaround for the problem.  A
  proper fix would be to update the scheme files used to generate the
  cpu headers.  But I am not a scheme programmer, and besides getting
  cgen to work with modern versions of guile seems to be quite
  difficult.  So instead the patch adds prototypes into one of the m32r
  sim header files that is not auto-generated.

  OK to apply ?

Cheers
  Nick

sim/m32r/ChangeLog
2020-11-10  Nick Clifton  <nickc@redhat.com>

	* m32r-sim.h (m32rbf_h_accum_get_handler): Always provide a
	prototype for this function.
	(m32rbf_h_accum_set_handler): Likewise.
	(m32r2f_h_accums_get_handler): Prototype.
	(m32r2f_h_accums_set_handler): Prototype.

diff --git a/sim/m32r/m32r-sim.h b/sim/m32r/m32r-sim.h
index 9f3e270f39..6f9f4610c0 100644
--- a/sim/m32r/m32r-sim.h
+++ b/sim/m32r/m32r-sim.h
@@ -61,10 +61,19 @@ extern void m32rbf_h_psw_set_handler (SIM_CPU *, UQI);
   XCONCAT2 (WANT_CPU,_h_psw_set_handler) (current_cpu, (val))
 #endif
 
-#ifndef  GET_H_ACCUM
+/* FIXME: These prototypes are necessary because the cgen generated
+   cpu.h, cpux.h and cpu2.h headers do not provide them, and functions
+   which take or return parameters that are larger than an int must be
+   prototyed in order for them to work correctly.
+
+   The correct solution is to fix the code in cgen/sim.scm to generate
+   prototypes for each of the functions it generates.  */
 extern DI   m32rbf_h_accum_get_handler (SIM_CPU *);
 extern void m32rbf_h_accum_set_handler (SIM_CPU *, DI);
+extern DI   m32r2f_h_accums_get_handler (SIM_CPU *, UINT);
+extern void m32r2f_h_accums_set_handler (SIM_CPU *, UINT, DI);
 
+#ifndef  GET_H_ACCUM
 #define GET_H_ACCUM() \
   XCONCAT2 (WANT_CPU,_h_accum_get_handler) (current_cpu)
 #define SET_H_ACCUM(val) \

Re: RFA: m32r sim: Add prototypes for functions that pass/return DI values

[Simon Marchi]

On 2020-11-10 12:42 p.m., Nick Clifton via Gdb-patches wrote:
> Hi Doug, Hi Andrew,
>
>   Jeff Law has tracked down a set of problems with the m32r simulator
>   which turned out to be because the m32rbf_h_accum_get_handler()
>   function returns a DI value, but it was not prototyped.  So callers
>   would assume that it returned an int, and happiness ensued...
>
>   I am proposing the attached patch as a workaround for the problem.  A
>   proper fix would be to update the scheme files used to generate the
>   cpu headers.  But I am not a scheme programmer, and besides getting
>   cgen to work with modern versions of guile seems to be quite
>   difficult.  So instead the patch adds prototypes into one of the m32r
>   sim header files that is not auto-generated.
>
>   OK to apply ?
>
> Cheers
>   Nick

This kind of problem bit us in the past in GDB too.  I'd suggest trying
to make the sim build system enable any or all of these warnings to
catch such problems.

  -Wmissing-declarations
  -Wmissing-prototypes
  -Wold-style-declaration
  -Wold-style-definition
  -Wstrict-prototypes

Simon

Re: RFA: m32r sim: Add prototypes for functions that pass/return DI values

[Nick Clifton]

Hi Simon,

> This kind of problem bit us in the past in GDB too.  I'd suggest trying
> to make the sim build system enable any or all of these warnings to
> catch such problems.
> 
>    -Wmissing-declarations
>    -Wmissing-prototypes
>    -Wold-style-declaration
>    -Wold-style-definition
>    -Wstrict-prototypes

I think that this would be an excellent idea.  Although I think
that it might cause a lot of work for the maintainers of cgen
based simulators.

Cheers
   Nick

Re: RFA: m32r sim: Add prototypes for functions that pass/return DI values

[Andrew Burgess]

* Nick Clifton <nickc@redhat.com> [2020-11-10 17:42:15 +0000]:

> Hi Doug, Hi Andrew,
> 
>   Jeff Law has tracked down a set of problems with the m32r simulator
>   which turned out to be because the m32rbf_h_accum_get_handler()
>   function returns a DI value, but it was not prototyped.  So callers
>   would assume that it returned an int, and happiness ensued...
> 
>   I am proposing the attached patch as a workaround for the problem.  A
>   proper fix would be to update the scheme files used to generate the
>   cpu headers.  But I am not a scheme programmer, and besides getting
>   cgen to work with modern versions of guile seems to be quite
>   difficult.  So instead the patch adds prototypes into one of the m32r
>   sim header files that is not auto-generated.
> 
>   OK to apply ?
> 
> Cheers
>   Nick
> 
> sim/m32r/ChangeLog
> 2020-11-10  Nick Clifton  <nickc@redhat.com>
> 
> 	* m32r-sim.h (m32rbf_h_accum_get_handler): Always provide a
> 	prototype for this function.
> 	(m32rbf_h_accum_set_handler): Likewise.
> 	(m32r2f_h_accums_get_handler): Prototype.
> 	(m32r2f_h_accums_set_handler): Prototype.
> 
> diff --git a/sim/m32r/m32r-sim.h b/sim/m32r/m32r-sim.h
> index 9f3e270f39..6f9f4610c0 100644
> --- a/sim/m32r/m32r-sim.h
> +++ b/sim/m32r/m32r-sim.h
> @@ -61,10 +61,19 @@ extern void m32rbf_h_psw_set_handler (SIM_CPU *, UQI);
>    XCONCAT2 (WANT_CPU,_h_psw_set_handler) (current_cpu, (val))
>  #endif
>  
> -#ifndef  GET_H_ACCUM
> +/* FIXME: These prototypes are necessary because the cgen generated
> +   cpu.h, cpux.h and cpu2.h headers do not provide them, and functions
> +   which take or return parameters that are larger than an int must be
> +   prototyed in order for them to work correctly.
> +
> +   The correct solution is to fix the code in cgen/sim.scm to generate
> +   prototypes for each of the functions it generates.  */
>  extern DI   m32rbf_h_accum_get_handler (SIM_CPU *);
>  extern void m32rbf_h_accum_set_handler (SIM_CPU *, DI);
> +extern DI   m32r2f_h_accums_get_handler (SIM_CPU *, UINT);
> +extern void m32r2f_h_accums_set_handler (SIM_CPU *, UINT, DI);

It looks like someone already did some of this before, so it seems
hard to argue against it :-)

Feel free to apply.

Thanks,
Andrew



>  
> +#ifndef  GET_H_ACCUM
>  #define GET_H_ACCUM() \
>    XCONCAT2 (WANT_CPU,_h_accum_get_handler) (current_cpu)
>  #define SET_H_ACCUM(val) \
> 

Re: RFA: m32r sim: Add prototypes for functions that pass/return DI values

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 912 bytes --]

On 11 Nov 2020 15:33, Nick Clifton via Gdb-patches wrote:
> > This kind of problem bit us in the past in GDB too.  I'd suggest trying
> > to make the sim build system enable any or all of these warnings to
> > catch such problems.
> > 
> >    -Wmissing-declarations
> >    -Wmissing-prototypes
> >    -Wold-style-declaration
> >    -Wold-style-definition
> >    -Wstrict-prototypes
> 
> I think that this would be an excellent idea.  Although I think
> that it might cause a lot of work for the maintainers of cgen
> based simulators.

well i've turned on the full set of warnings (copied from gdb in days
past) for all sim ports now :).  when possible, i've turned on -Werror,
but you're right that all the cgen ports need a lot of love ...

i see gdb has split their warning code out into gdbsupport/.  i wonder
if we can reuse that, or maybe just sync the updated set of warnings.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]