Patchwork [review,v3,ARM,sim] Fix build error and warnings

login
register
mail settings
Submitter Simon Marchi (Code Review)
Date Nov. 28, 2019, 1:33 p.m.
Message ID <20191128133313.8EB9120AF6@gnutoolchain-gerrit.osci.io>
Download mbox | patch
Permalink /patch/36351/
State New
Headers show

Comments

Simon Marchi (Code Review) - Nov. 28, 2019, 1:33 p.m.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/726
......................................................................

[ARM, sim] Fix build error and warnings

Newer GCC's have switched to -fno-common by default, and this breaks the build
for the ARM sim, like this:

binutils-gdb.git~gdb-8.3-release/sim/arm/maverick.c:65: multiple definition of `DSPsc'; libsim.a(wrapper.o):binutils-gdb.git~gdb-8.3-release/sim/arm/wrapper.c:134: first defined here
binutils-gdb.git~gdb-8.3-release/sim/arm/maverick.c:64: multiple definition of `DSPacc'; libsim.a(wrapper.o):binutils-gdb.git~gdb-8.3-release/sim/arm/wrapper.c:133: first defined here
binutils-gdb.git~gdb-8.3-release/sim/arm/maverick.c:63: multiple definition of `DSPregs'; libsim.a(wrapper.o):binutils-gdb.git~gdb-8.3-release/sim/arm/wrapper.c:132: first defined here

I also noticed a few warnings due to mismatching types, as follows:

binutils-gdb/sim/arm/wrapper.c:870:31: warning: passing argument 1 of ‘sim_target_parse_arg_array’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
  sim_target_parse_arg_array (argv);

binutils-gdb/sim/arm/wrapper.c:775:1: note: expected ‘char **’ but argument is of type ‘char * const*’
  sim_target_parse_arg_array (char ** argv)

The following patch fixes both of the above.

sim/arm/ChangeLog:

2019-11-28  Luis Machado  <luis.machado@linaro.org>

	* armemu.c (isize): Move this declaration ...
	* arminit.c (isize): ... here.
	* wrapper.c: Include "maverick.h".
	(<struct maverick_regs>, <union maverick_acc_regs>): Remove and update
	comment.
	(DSPregs): Make extern.
	(DSPacc): Likewise.
	(DSPsc): Likewise.
	(sim_create_inferior): Cast variables to proper type.
	(sim_open): Likewise.
	* maverick.c: Include "maverick.h".
	(<struct maverick_regs>, <union maverick_acc_regs>): Move
	declarations to maverick.h and update comment.
	* maverick.h: New file.

Signed-off-by: Luis Machado <luis.machado@linaro.org>
Change-Id: I21db699d3b61b2de8c44053e47be4387285af28f
---
M sim/arm/armemu.c
M sim/arm/arminit.c
M sim/arm/maverick.c
A sim/arm/maverick.h
M sim/arm/wrapper.c
5 files changed, 58 insertions(+), 67 deletions(-)

Patch

diff --git a/sim/arm/armemu.c b/sim/arm/armemu.c
index 76f398b..3a72277 100644
--- a/sim/arm/armemu.c
+++ b/sim/arm/armemu.c
@@ -1140,10 +1140,6 @@ 
 
 /* EMULATION of ARM6.  */
 
-/* The PC pipeline value depends on whether ARM
-   or Thumb instructions are being executed.  */
-ARMword isize;
-
 ARMword
 #ifdef MODE32
 ARMul_Emulate32 (ARMul_State * state)
diff --git a/sim/arm/arminit.c b/sim/arm/arminit.c
index 851d356..3a626c8 100644
--- a/sim/arm/arminit.c
+++ b/sim/arm/arminit.c
@@ -40,6 +40,10 @@ 
 ARMword ARMul_ImmedTable[4096];	/* immediate DP LHS values */
 char ARMul_BitList[256];	/* number of bits in a byte table */
 
+/* The PC pipeline value depends on whether ARM
+   or Thumb instructions are being executed.  */
+ARMword isize;
+
 /***************************************************************************\
 *         Call this routine once to set up the emulator's tables.           *
 \***************************************************************************/
diff --git a/sim/arm/maverick.c b/sim/arm/maverick.c
index c112692..cbcf55b 100644
--- a/sim/arm/maverick.c
+++ b/sim/arm/maverick.c
@@ -19,6 +19,7 @@ 
 #include "armdefs.h"
 #include "ansidecl.h"
 #include "armemu.h"
+#include "maverick.h"
 
 /*#define CIRRUS_DEBUG 1	*/
 #if CIRRUS_DEBUG
@@ -30,36 +31,9 @@ 
 #define POS64(i) ( (~(i)) >> 63 )
 #define NEG64(i) ( (i) >> 63 )
 
-/* Define Co-Processor instruction handlers here.  */
-
-/* Here's ARMulator's DSP definition.  A few things to note:
-   1) it has 16 64-bit registers and 4 72-bit accumulators
-   2) you can only access its registers with MCR and MRC.  */
-
-/* We can't define these in here because this file might not be linked
-   unless the target is arm9e-*.  They are defined in wrapper.c.
-   Eventually the simulator should be made to handle any coprocessor
-   at run time.  */
-struct maverick_regs
-{
-  union
-  {
-    int i;
-    float f;
-  } upper;
-
-  union
-  {
-    int i;
-    float f;
-  } lower;
-};
-
-union maverick_acc_regs
-{
-  long double ld;		/* Acc registers are 72-bits.  */
-};
-
+/* These variables are defined here and made extern in wrapper.c for now.
+   Eventually the simulator should be made to handle any coprocessor at run
+   time.  */
 struct maverick_regs DSPregs[16];
 union maverick_acc_regs DSPacc[4];
 ARMword DSPsc;
diff --git a/sim/arm/maverick.h b/sim/arm/maverick.h
new file mode 100644
index 0000000..9e081a4
--- /dev/null
+++ b/sim/arm/maverick.h
@@ -0,0 +1,42 @@ 
+/*  maverick.h -- Cirrus/DSP co-processor interface header
+    Copyright (C) 2003-2019 Free Software Foundation, Inc.
+    Contributed by Aldy Hernandez (aldyh@redhat.com).
+
+    This program is free software; you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation; either version 3 of the License, or
+    (at your option) any later version.
+
+    This program is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    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/>. */
+
+/* Define Co-Processor instruction handlers here.  */
+
+/* Here's ARMulator's DSP definition.  A few things to note:
+   1) it has 16 64-bit registers and 4 72-bit accumulators
+   2) you can only access its registers with MCR and MRC.  */
+
+struct maverick_regs
+{
+  union
+  {
+    int i;
+    float f;
+  } upper;
+
+  union
+  {
+    int i;
+    float f;
+  } lower;
+};
+
+union maverick_acc_regs
+{
+  long double ld;		/* Acc registers are 72-bits.  */
+};
diff --git a/sim/arm/wrapper.c b/sim/arm/wrapper.c
index fde5d8c..5b7bd04 100644
--- a/sim/arm/wrapper.c
+++ b/sim/arm/wrapper.c
@@ -37,6 +37,7 @@ 
 #include "gdb/signals.h"
 #include "libiberty.h"
 #include "iwmmxt.h"
+#include "maverick.h"
 
 /* TODO: This should get pulled from the SIM_DESC.  */
 host_callback *sim_callback;
@@ -101,37 +102,11 @@ 
   fprintf (stderr, " %*s\n", size, opbuf);
 }
 
-/* Cirrus DSP registers.
-
-   We need to define these registers outside of maverick.c because
-   maverick.c might not be linked in unless --target=arm9e-* in which
-   case wrapper.c will not compile because it tries to access Cirrus
-   registers.  This should all go away once we get the Cirrus and ARM
-   Coprocessor to coexist in armcopro.c-- aldyh.  */
-
-struct maverick_regs
-{
-  union
-  {
-    int i;
-    float f;
-  } upper;
-
-  union
-  {
-    int i;
-    float f;
-  } lower;
-};
-
-union maverick_acc_regs
-{
-  long double ld;		/* Acc registers are 72-bits.  */
-};
-
-struct maverick_regs     DSPregs[16];
-union maverick_acc_regs  DSPacc[4];
-ARMword DSPsc;
+/* These are defined in maverick.c for now.  Once coprocessor
+   handling is unified, they can go away.  */
+extern struct maverick_regs     DSPregs[16];
+extern union maverick_acc_regs  DSPacc[4];
+extern ARMword DSPsc;
 
 static void
 init (void)
@@ -236,7 +211,7 @@ 
 {
   int argvlen = 0;
   int mach;
-  char **arg;
+  char * const *arg;
 
   init ();
 
@@ -867,7 +842,7 @@ 
 
   sim_callback = cb;
 
-  sim_target_parse_arg_array (argv);
+  sim_target_parse_arg_array ((char **) argv);
 
   if (argv[1] != NULL)
     {