[PATCH 1/3] sim/common: convert sim-arange to use sim-inline

[PATCH 1/3] sim/common: convert sim-arange to use sim-inline

[Stafford Horne]

This fixes a TODO item and also fixes an error which we get when
building with no optimizations (-O0) in at least gcc 8.2.1.

Tested with sims that use cgen code lm32, or1k, cris, m32r and inlining
is working corretly.

Reference Error:

gcc -DHAVE_CONFIG_H -DWITH_DEFAULT_MODEL='"or1200"' -DWITH_ALIGNMENT=STRICT_ALIGNMENT \
 -DWITH_TARGET_WORD_BITSIZE=32 -DWITH_TARGET_WORD_MSB=31 -DWITH_TARGET_ADDRESS_BITSIZE=32 \
 -DWITH_TARGET_BYTE_ORDER=BFD_ENDIAN_BIG   -DDEFAULT_INLINE=0  -DWITH_SCACHE=16384 \
 -I. -I../../../binutils-gdb/sim/or1k -I../common -I../../../binutils-gdb/sim/or1k/../common \
 -I../../include -I../../../binutils-gdb/sim/or1k/../../include -I../../bfd \
 -I../../../binutils-gdb/sim/or1k/../../bfd -I../../opcodes -I../../../binutils-gdb/sim/or1k/../../opcodes \
 -g -o run nrun.o libsim.a ../../bfd/libbfd.a ../../opcodes/libopcodes.a  ../../libiberty/libiberty.a \
 -ldl  -lz -lm

/usr/bin/ld: libsim.a(mloop.o): in function `extract':
/home/shorne/work/openrisc/gdb-musl/sim/or1k/mloop.c:82: undefined reference to `sim_addr_range_hit_p'
/usr/bin/ld: /home/shorne/work/openrisc/gdb-musl/sim/or1k/mloop.c:83: undefined reference to `sim_addr_range_hit_p'
collect2: error: ld returned 1 exit status
make[3]: *** [Makefile:305: run] Error 1

sim/common/ChangeLog:

	* Make-common.in (sim-arange_h): Remove sim-arange.c
	* sim-arange.c: Remove SIM_ARANGE_C.
	Add ifdef for _SIM_ARANGE_C_.
	Include "sim-arange.h".
	Remove include for unused "sim-assert.h".
	Remove DEFINE_INLINE_P.  Remove DEFINE_NON_INLINE_P.
	(sim_addr_range_add): Declare as INLINE_SIM_ARANGE.
	(sim_addr_range_delete): Declare as INLINE_SIM_ARANGE.
	(sim_addr_range_hit_p): Change from SIM_ARANGE_INLINE to
	INLINE_SIM_ARANGE.
	* sim-arange.h (sim_addr_range_add): Declare as
	INLINE_SIM_ARANGE.
	(sim_addr_range_delete): Declare as INLINE_SIM_ARANGE.
	(sim_addr_range_hit_p) Declare as INLINE_SIM_ARANGE.
	Remove definition of SIM_ARANGE_INLINE.
	Remove [HAVE_INLINE].
	Wrap include "sim-arange.c" in H_REVEALS_MODULE_P.
	* sim-base.h: Include "sim-arange.h"
	* sim-basics.h: Remove include of "sim-arange.h"
	* sim-inline.c: Include "sim-arange.c"
	* sim-inline.h: Define INLINE_SIM_ARANGE.
	Define SIM_ARANGE_INLINE.  Define EXTERN_SIM_ARANGE_P.
	Define STATIC_INLINE_SIM_ARANGE.  Define STATIC_SIM_ARANGE.
---
 sim/common/Make-common.in |  3 +--
 sim/common/sim-arange.c   | 26 ++++++++++----------------
 sim/common/sim-arange.h   | 31 ++++++++++---------------------
 sim/common/sim-base.h     |  1 +
 sim/common/sim-basics.h   |  1 -
 sim/common/sim-inline.c   |  5 ++++-
 sim/common/sim-inline.h   | 32 ++++++++++++++++++++++++++++++++
 7 files changed, 58 insertions(+), 41 deletions(-)

diff --git a/sim/common/Make-common.in b/sim/common/Make-common.in
index 1e61927e82..2e449b330e 100644
--- a/sim/common/Make-common.in
+++ b/sim/common/Make-common.in
@@ -319,8 +319,7 @@ SIM_MAIN_DEPS = \
 	$(sim_main_headers)
 
 sim-alu_h = $(srccom)/sim-alu.h
-sim-arange_h = $(srccom)/sim-arange.h \
-		$(srccom)/sim-arange.c
+sim-arange_h = $(srccom)/sim-arange.h
 sim-assert_h = $(srccom)/sim-assert.h
 sim-base_h = $(srccom)/sim-base.h \
 		$(sim-module_h) \
diff --git a/sim/common/sim-arange.c b/sim/common/sim-arange.c
index 5a8779f061..75cd1e5119 100644
--- a/sim/common/sim-arange.c
+++ b/sim/common/sim-arange.c
@@ -17,12 +17,12 @@ GNU General Public License for more details.
 You should have received a copy of the GNU General Public License
 along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-/* Tell sim-arange.h it's us.  */
-#define SIM_ARANGE_C
+#ifndef _SIM_ARANGE_C_
+#define _SIM_ARANGE_C_
 
 #include "libiberty.h"
 #include "sim-basics.h"
-#include "sim-assert.h"
+#include "sim-arange.h"
 
 #ifdef HAVE_STDLIB_H
 #include <stdlib.h>
@@ -32,11 +32,6 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 #include <string.h>
 #endif
 
-#define DEFINE_INLINE_P (! defined (SIM_ARANGE_C_INCLUDED))
-#define DEFINE_NON_INLINE_P defined (SIM_ARANGE_C_INCLUDED)
-
-#if DEFINE_NON_INLINE_P
-
 /* Insert a range.  */
 
 static void
@@ -254,7 +249,8 @@ build_search_tree (ADDR_RANGE *ar)
   free (asrtab);
 }
 
-void
+INLINE_SIM_ARANGE\
+(void)
 sim_addr_range_add (ADDR_RANGE *ar, address_word start, address_word end)
 {
   frob_range (ar, start, end, 0);
@@ -267,7 +263,8 @@ sim_addr_range_add (ADDR_RANGE *ar, address_word start, address_word end)
   build_search_tree (ar);
 }
 
-void
+INLINE_SIM_ARANGE\
+(void)
 sim_addr_range_delete (ADDR_RANGE *ar, address_word start, address_word end)
 {
   frob_range (ar, start, end, 1);
@@ -280,11 +277,8 @@ sim_addr_range_delete (ADDR_RANGE *ar, address_word start, address_word end)
   build_search_tree (ar);
 }
 
-#endif /* DEFINE_NON_INLINE_P */
-
-#if DEFINE_INLINE_P
-
-SIM_ARANGE_INLINE int
+INLINE_SIM_ARANGE\
+(int)
 sim_addr_range_hit_p (ADDR_RANGE *ar, address_word addr)
 {
   ADDR_RANGE_TREE *t = ar->range_tree;
@@ -301,4 +295,4 @@ sim_addr_range_hit_p (ADDR_RANGE *ar, address_word addr)
   return 0;
 }
 
-#endif /* DEFINE_INLINE_P */
+#endif /* _SIM_ARANGE_C_ */
diff --git a/sim/common/sim-arange.h b/sim/common/sim-arange.h
index 45eb1746e4..e615cfde15 100644
--- a/sim/common/sim-arange.h
+++ b/sim/common/sim-arange.h
@@ -51,36 +51,25 @@ typedef struct _addr_range {
 } ADDR_RANGE;
 
 /* Add address range START,END to AR.  */
-extern void sim_addr_range_add (ADDR_RANGE * /*ar*/,
-				address_word /*start*/,
-				address_word /*end*/);
+INLINE_SIM_ARANGE (void) sim_addr_range_add (ADDR_RANGE * /*ar*/,
+					     address_word /*start*/,
+					     address_word /*end*/);
 
 /* Delete address range START,END from AR.  */
-extern void sim_addr_range_delete (ADDR_RANGE * /*ar*/,
-				   address_word /*start*/,
-				   address_word /*end*/);
-
-/* TODO: This should get moved into sim-inline.h.  */
-#ifdef HAVE_INLINE
-#ifdef SIM_ARANGE_C
-#define SIM_ARANGE_INLINE INLINE
-#else
-#define SIM_ARANGE_INLINE EXTERN_INLINE
-#endif
-#else
-#define SIM_ARANGE_INLINE EXTERN
-#endif
+INLINE_SIM_ARANGE (void) sim_addr_range_delete (ADDR_RANGE * /*ar*/,
+						address_word /*start*/,
+						address_word /*end*/);
 
 /* Return non-zero if ADDR is in range AR, traversing the entire tree.
    If no range is specified, that is defined to mean "everything".  */
-SIM_ARANGE_INLINE int
-sim_addr_range_hit_p (ADDR_RANGE * /*ar*/, address_word /*addr*/);
+
+INLINE_SIM_ARANGE (int) sim_addr_range_hit_p (ADDR_RANGE * /*ar*/,
+					      address_word /*addr*/);
 #define ADDR_RANGE_HIT_P(ar, addr) \
   ((ar)->range_tree == NULL || sim_addr_range_hit_p ((ar), (addr)))
 
-#ifdef HAVE_INLINE
+#if H_REVEALS_MODULE_P (SIM_ARANGE_INLINE)
 #include "sim-arange.c"
 #endif
-#define SIM_ARANGE_C_INCLUDED
 
 #endif /* SIM_ARANGE_H */
diff --git a/sim/common/sim-base.h b/sim/common/sim-base.h
index 524195e8ad..c9bb800bb3 100644
--- a/sim/common/sim-base.h
+++ b/sim/common/sim-base.h
@@ -78,6 +78,7 @@ typedef struct _sim_cpu sim_cpu;
 
 #include "sim-module.h"
 
+#include "sim-arange.h"
 #include "sim-trace.h"
 #include "sim-core.h"
 #include "sim-events.h"
diff --git a/sim/common/sim-basics.h b/sim/common/sim-basics.h
index 37d228f19f..6ece00a17c 100644
--- a/sim/common/sim-basics.h
+++ b/sim/common/sim-basics.h
@@ -144,7 +144,6 @@ typedef enum {
 #include "sim-bits.h"
 #include "sim-endian.h"
 #include "sim-signal.h"
-#include "sim-arange.h"
 
 #include "sim-utils.h"
 
diff --git a/sim/common/sim-inline.c b/sim/common/sim-inline.c
index 2bd23ed184..80a577c6e7 100644
--- a/sim/common/sim-inline.c
+++ b/sim/common/sim-inline.c
@@ -29,6 +29,10 @@
 #include "sim-inline.h"
 #include "sim-main.h"
 
+#if C_REVEALS_MODULE_P (SIM_ARANGE_INLINE)
+#include "sim-arange.c"
+#endif
+
 
 #if C_REVEALS_MODULE_P (SIM_BITS_INLINE)
 #include "sim-bits.c"
@@ -64,7 +68,6 @@
 #include "sim-main.c"
 #endif
 
-
 #if C_REVEALS_MODULE_P (ENGINE_INLINE)
 /* #include "engine.c" - handled by generator */
 #endif
diff --git a/sim/common/sim-inline.h b/sim/common/sim-inline.h
index b2a3fc32f3..0cec86b33f 100644
--- a/sim/common/sim-inline.h
+++ b/sim/common/sim-inline.h
@@ -348,6 +348,38 @@
 
 
 \f
+
+/* sim_arange */
+
+#if !defined (SIM_ARANGE_INLINE) && (DEFAULT_INLINE)
+# define SIM_ARANGE_INLINE (ALL_H_INLINE)
+#endif
+
+#if ((H_REVEALS_MODULE_P (SIM_ARANGE_INLINE) || defined (SIM_INLINE_C)) \
+     && !defined (SIM_ARANGE_C) \
+     && (REVEAL_MODULE_P (SIM_ARANGE_INLINE)))
+# if (SIM_ARANGE_INLINE & INLINE_GLOBALS)
+#  define INLINE_SIM_ARANGE(TYPE) static INLINE TYPE UNUSED
+#  define EXTERN_SIM_ARANGE_P 0
+# else
+#  define INLINE_SIM_ARANGE(TYPE) static TYPE UNUSED
+#  define EXTERN_SIM_ARANGE_P 0
+# endif
+#else
+# define INLINE_SIM_ARANGE(TYPE) TYPE
+# define EXTERN_SIM_ARANGE_P 1
+#endif
+
+#if (SIM_ARANGE_INLINE & INLINE_LOCALS)
+# define STATIC_INLINE_SIM_ARANGE(TYPE) static INLINE TYPE
+#else
+# define STATIC_INLINE_SIM_ARANGE(TYPE) static TYPE
+#endif
+
+#define STATIC_SIM_ARANGE(TYPE) static TYPE
+
+
+
 /* *****
    sim-bits and sim-endian are treated differently from the rest
    of the modules below.  Their default value is ALL_H_INLINE.
-- 
2.19.1

[PATCH 2/3] include/dis-asm.h: Fix warning: Include "string.h" for strchr

[Stafford Horne]

I got this warning when building the OpenRISC sim target.

Warning:

In file included from ../../../binutils-gdb/sim/or1k/../common/cgen-trace.c:26:
../../../binutils-gdb/sim/or1k/../../include/dis-asm.h: In function ‘next_disassembler_option’:
../../../binutils-gdb/sim/or1k/../../include/dis-asm.h:343:21: warning: implicit declaration of function ‘strchr’ [-Wimplicit-function-declaration]
   const char *opt = strchr (options, ',');
                     ^~~~~~
../../../binutils-gdb/sim/or1k/../../include/dis-asm.h:343:21: warning: incompatible implicit declaration of built-in function ‘strchr’
../../../binutils-gdb/sim/or1k/../../include/dis-asm.h:343:21: note: include ‘<string.h>’ or provide a declaration of ‘strchr’
../../../binutils-gdb/sim/or1k/../../include/dis-asm.h:40:1:
+#include <string.h>

../../../binutils-gdb/sim/or1k/../../include/dis-asm.h:343:21:
   const char *opt = strchr (options, ',');
                     ^~~~~~

include/ChangeLog:

	dis-asm.h: Include "string.h"
---
 include/dis-asm.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/dis-asm.h b/include/dis-asm.h
index 84627950c0..823bba5b45 100644
--- a/include/dis-asm.h
+++ b/include/dis-asm.h
@@ -32,6 +32,10 @@ extern "C" {
 #endif
 
 #include <stdio.h>
+/* For strchr.  */
+#ifdef HAVE_STRING_H
+#include <string.h>
+#endif
 #include "bfd.h"
 
   typedef int (*fprintf_ftype) (void *, const char*, ...) ATTRIBUTE_FPTR_PRINTF_2;
-- 
2.19.1

[PATCH 3/3] sim/common: Fix warnings: "warning: implicit declaration of function..."

[Stafford Horne]

During building of several cgen simulator's I notices the below
warnings.  Adding includes fixes these.

Including config.h allows stdio.h to properly configure itself to expose
asprintf().

The other warnings for abort, free, memset, strlen are trivial.

Warnings:

../../../binutils-gdb/sim/or1k/../common/sim-watch.c: In function ‘sim_watchpoint_install’:
../../../binutils-gdb/sim/or1k/../common/sim-watch.c:415:10: warning: implicit declaration of function ‘asprintf’; did you mean ‘vasprintf’? [-Wimplicit-function-declaration]
      if (asprintf (&name, "watch-%s-%s",
          ^~~~~~~~
          vasprintf

../../../binutils-gdb/sim/lm32/../common/hw-device.c: In function ‘hw_strdup’:
../../../binutils-gdb/sim/lm32/../common/hw-device.c:59:34: warning: implicit declaration of function ‘strlen’ [-Wimplicit-function-declaration]
       char *dup = hw_zalloc (me, strlen (str) + 1);
                                  ^~~~~~

../../../binutils-gdb/sim/lm32/../common/hw-events.c: In function ‘hw_event_queue_schedule’:
../../../binutils-gdb/sim/lm32/../common/hw-events.c:92:3: warning: implicit declaration of function ‘memset’ [-Wimplicit-function-declaration]
   memset (&dummy, 0, sizeof dummy);
   ^~~~~~

../../../binutils-gdb/sim/lm32/../common/hw-handles.c: In function ‘hw_handle_remove_ihandle’:
../../../binutils-gdb/sim/lm32/../common/hw-handles.c:211:4: warning: implicit declaration of function ‘free’ [-Wimplicit-function-declaration]
    free (delete);
    ^~~~

../../../binutils-gdb/sim/lm32/../common/sim-fpu.c: In function ‘pack_fpu’:
../../../binutils-gdb/sim/lm32/../common/sim-fpu.c:292:7: warning: implicit declaration of function ‘abort’ [-Wimplicit-function-declaration]
       abort ();
       ^~~~~

sim/common/ChangeLog:

	* sim-options.c: Include "config.h".
	Include <stdio.h>.
	* sim-watch.c: Include "config.h".
	Include <stdio.h>.
	* hw-device.c: Include "string.h".
	* hw-events.c: Include "string.h".
	* hw-handles.c: Include <stdlib.h>.
	* sim-fpu.c: Include <stdlib.h>.
---
 sim/common/hw-device.c   | 4 ++++
 sim/common/hw-events.c   | 3 +++
 sim/common/hw-handles.c  | 3 +++
 sim/common/sim-fpu.c     | 3 +++
 sim/common/sim-options.c | 2 ++
 sim/common/sim-watch.c   | 2 ++
 6 files changed, 17 insertions(+)

diff --git a/sim/common/hw-device.c b/sim/common/hw-device.c
index eef3373151..4dee1eca38 100644
--- a/sim/common/hw-device.c
+++ b/sim/common/hw-device.c
@@ -27,6 +27,10 @@
 #include <stdlib.h>
 #endif
 
+#if HAVE_STRING_H
+#include <string.h>
+#endif
+
 /* Address methods */
 
 const hw_unit *
diff --git a/sim/common/hw-events.c b/sim/common/hw-events.c
index d59c39ee9f..ca2063fa84 100644
--- a/sim/common/hw-events.c
+++ b/sim/common/hw-events.c
@@ -23,6 +23,9 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "sim-events.h"
 
+#if HAVE_STRING_H
+#include <string.h>
+#endif
 
 /* The hw-events object is implemented using sim-events */
 
diff --git a/sim/common/hw-handles.c b/sim/common/hw-handles.c
index 1d03c2b97e..57690db101 100644
--- a/sim/common/hw-handles.c
+++ b/sim/common/hw-handles.c
@@ -23,6 +23,9 @@
 #include "hw-main.h"
 #include "hw-base.h"
 
+#if HAVE_STDLIB_H
+#include <stdlib.h>
+#endif
 
 struct hw_handle_mapping
 {
diff --git a/sim/common/sim-fpu.c b/sim/common/sim-fpu.c
index ce5fafbf4c..3d4c34c971 100644
--- a/sim/common/sim-fpu.c
+++ b/sim/common/sim-fpu.c
@@ -41,6 +41,9 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "sim-io.h"
 #include "sim-assert.h"
 
+#ifdef HAVE_STDLIB_H
+#include <stdlib.h>
+#endif
 
 /* Debugging support.
    If digits is -1, then print all digits.  */
diff --git a/sim/common/sim-options.c b/sim/common/sim-options.c
index 96c6bb8509..3a05d8e964 100644
--- a/sim/common/sim-options.c
+++ b/sim/common/sim-options.c
@@ -17,6 +17,7 @@ GNU General Public License for more details.
 You should have received a copy of the GNU General Public License
 along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#include "config.h"
 #include "sim-main.h"
 #ifdef HAVE_STRING_H
 #include <string.h>
@@ -29,6 +30,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 #include <stdlib.h>
 #endif
 #include <ctype.h>
+#include <stdio.h>
 #include "libiberty.h"
 #include "sim-options.h"
 #include "sim-io.h"
diff --git a/sim/common/sim-watch.c b/sim/common/sim-watch.c
index b29ac3bab2..d7eddeeb4e 100644
--- a/sim/common/sim-watch.c
+++ b/sim/common/sim-watch.c
@@ -17,12 +17,14 @@ GNU General Public License for more details.
 You should have received a copy of the GNU General Public License
 along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
+#include "config.h"
 #include "sim-main.h"
 #include "sim-options.h"
 
 #include "sim-assert.h"
 
 #include <ctype.h>
+#include <stdio.h>
 
 #ifdef HAVE_STRING_H
 #include <string.h>
-- 
2.19.1

[PATCH 0/3] Fix build failure with sim-arange inlining

[Stafford Horne]

Hello,

When building the OpenRISC sim without any CLFAGS set or -O0, I get a failure
due to some bad inlining definitions in sim-arange.  To fix this I have
converted sim-arange inlining to use the more standard sim-inline inlining
framework.  This seems to work fine and makes the code a bit easier to follow in
my opinion.

I have tested this on a few other targets, nios, arm, lm32 etc and it seems to
not break the build or cause issues.

During the build I also noticed several other warnings.  Which I think is good
to also fix up so I am including these in the series.

Stafford Horne (3):
  sim/common: convert sim-arange to use sim-inline
  include/dis-asm.h: Fix warning: Include "string.h" for strchr
  sim/common: Fix warnings: "warning: implicit declaration of
    function..."

 include/dis-asm.h         |  4 ++++
 sim/common/Make-common.in |  3 +--
 sim/common/hw-device.c    |  4 ++++
 sim/common/hw-events.c    |  3 +++
 sim/common/hw-handles.c   |  3 +++
 sim/common/sim-arange.c   | 26 ++++++++++----------------
 sim/common/sim-arange.h   | 31 ++++++++++---------------------
 sim/common/sim-base.h     |  1 +
 sim/common/sim-basics.h   |  1 -
 sim/common/sim-fpu.c      |  3 +++
 sim/common/sim-inline.c   |  5 ++++-
 sim/common/sim-inline.h   | 32 ++++++++++++++++++++++++++++++++
 sim/common/sim-options.c  |  2 ++
 sim/common/sim-watch.c    |  2 ++
 14 files changed, 79 insertions(+), 41 deletions(-)

-- 
2.19.1

Re: [PATCH 2/3] include/dis-asm.h: Fix warning: Include "string.h" for strchr

[Simon Marchi]

On 2018-12-02 8:05 a.m., Stafford Horne wrote:
> I got this warning when building the OpenRISC sim target.
> 
> Warning:
> 
> In file included from ../../../binutils-gdb/sim/or1k/../common/cgen-trace.c:26:
> ../../../binutils-gdb/sim/or1k/../../include/dis-asm.h: In function ‘next_disassembler_option’:
> ../../../binutils-gdb/sim/or1k/../../include/dis-asm.h:343:21: warning: implicit declaration of function ‘strchr’ [-Wimplicit-function-declaration]
>    const char *opt = strchr (options, ',');
>                      ^~~~~~
> ../../../binutils-gdb/sim/or1k/../../include/dis-asm.h:343:21: warning: incompatible implicit declaration of built-in function ‘strchr’
> ../../../binutils-gdb/sim/or1k/../../include/dis-asm.h:343:21: note: include ‘<string.h>’ or provide a declaration of ‘strchr’
> ../../../binutils-gdb/sim/or1k/../../include/dis-asm.h:40:1:
> +#include <string.h>
> 
> ../../../binutils-gdb/sim/or1k/../../include/dis-asm.h:343:21:
>    const char *opt = strchr (options, ',');
>                      ^~~~~~
> 
> include/ChangeLog:
> 
> 	dis-asm.h: Include "string.h"
> ---
>  include/dis-asm.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/dis-asm.h b/include/dis-asm.h
> index 84627950c0..823bba5b45 100644
> --- a/include/dis-asm.h
> +++ b/include/dis-asm.h
> @@ -32,6 +32,10 @@ extern "C" {
>  #endif
>  
>  #include <stdio.h>
> +/* For strchr.  */
> +#ifdef HAVE_STRING_H
> +#include <string.h>
> +#endif
>  #include "bfd.h"
>  
>    typedef int (*fprintf_ftype) (void *, const char*, ...) ATTRIBUTE_FPTR_PRINTF_2;
> 

Hi Stafford,

It looks like this patchset fell through the cracks (I found it while searching for
something else).  A version of this patch went in, witout the HAVE_STRING_H ifdef.
Do you think the ifdef is necessary in practice?

Simon

Re: [PATCH 2/3] include/dis-asm.h: Fix warning: Include "string.h" for strchr

[Stafford Horne]

On Tue, Jan 22, 2019 at 05:07:43PM +0000, Simon Marchi wrote:
> On 2018-12-02 8:05 a.m., Stafford Horne wrote:
> > I got this warning when building the OpenRISC sim target.
> > 
> > Warning:
> > 
> > In file included from ../../../binutils-gdb/sim/or1k/../common/cgen-trace.c:26:
> > ../../../binutils-gdb/sim/or1k/../../include/dis-asm.h: In function ‘next_disassembler_option’:
> > ../../../binutils-gdb/sim/or1k/../../include/dis-asm.h:343:21: warning: implicit declaration of function ‘strchr’ [-Wimplicit-function-declaration]
> >    const char *opt = strchr (options, ',');
> >                      ^~~~~~
> > ../../../binutils-gdb/sim/or1k/../../include/dis-asm.h:343:21: warning: incompatible implicit declaration of built-in function ‘strchr’
> > ../../../binutils-gdb/sim/or1k/../../include/dis-asm.h:343:21: note: include ‘<string.h>’ or provide a declaration of ‘strchr’
> > ../../../binutils-gdb/sim/or1k/../../include/dis-asm.h:40:1:
> > +#include <string.h>
> > 
> > ../../../binutils-gdb/sim/or1k/../../include/dis-asm.h:343:21:
> >    const char *opt = strchr (options, ',');
> >                      ^~~~~~
> > 
> > include/ChangeLog:
> > 
> > 	dis-asm.h: Include "string.h"
> > ---
> >  include/dis-asm.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/include/dis-asm.h b/include/dis-asm.h
> > index 84627950c0..823bba5b45 100644
> > --- a/include/dis-asm.h
> > +++ b/include/dis-asm.h
> > @@ -32,6 +32,10 @@ extern "C" {
> >  #endif
> >  
> >  #include <stdio.h>
> > +/* For strchr.  */
> > +#ifdef HAVE_STRING_H
> > +#include <string.h>
> > +#endif
> >  #include "bfd.h"
> >  
> >    typedef int (*fprintf_ftype) (void *, const char*, ...) ATTRIBUTE_FPTR_PRINTF_2;
> > 
> 
> Hi Stafford,
> 
> It looks like this patchset fell through the cracks (I found it while searching for
> something else).  A version of this patch went in, witout the HAVE_STRING_H ifdef.
> Do you think the ifdef is necessary in practice?

Sorry for the late reply, I just got back from vacation with the family in
Hawaii :).

I thought about this when I made this patch, we currently have a lot of these
HAVE_XXX_H checks throughout the sim code.  I am not sure of the history, but
perhaps I can try a patch set to eliminate a few of them like HAVE_STRING_H vs
HAVE_STRINGS_H (BSD).  I would think everything we need is in on every machine
in <string.h>.

I am not an expert in C/C++ compatibility.  Is there at minimum standard c/c++
required for gdb?  Perhaps this has changed since we moved to allow C++.

-Stafford

Re: [PATCH 2/3] include/dis-asm.h: Fix warning: Include "string.h" for strchr

[Tom Tromey]

>>>>> "Stafford" == Stafford Horne <shorne@gmail.com> writes:

I didn't see a reply to this, apologies if I missed it somehow.

Stafford> I thought about this when I made this patch, we currently have a lot of these
Stafford> HAVE_XXX_H checks throughout the sim code.  I am not sure of the history, but
Stafford> perhaps I can try a patch set to eliminate a few of them like HAVE_STRING_H vs
Stafford> HAVE_STRINGS_H (BSD).  I would think everything we need is in on every machine
Stafford> in <string.h>.

The old BSD one is surely long obsolete.

The sim could also consider using gnulib.  This isn't totally trivial --
we'd probably have to move it to the top level, though I've wanted us to
do that for a while now -- but it would mean not having to care so much
about checking for standard headers.

Stafford> I am not an expert in C/C++ compatibility.  Is there at minimum standard c/c++
Stafford> required for gdb?  Perhaps this has changed since we moved to allow C++.

gdb uses C++11.  The rest of the tree uses C89 as far as I know.

Tom