From: Tom de Vries <tdevries@suse.de>
Date: Wed, 19 May 2021 10:46:47 +0000 (+0200)
Subject: sim: ppc: enable -Wpointer-sign warnings
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=81f839676d98024f6c0a6609866bfa81b5f35325;p=binutils-gdb.git

sim: ppc: enable -Wpointer-sign warnings

When compiling with --enable-werror and CFLAGS="-O0 -g -Wall", we run into:
...
src/sim/ppc/hw_memory.c: In function 'hw_memory_init_address':
src/sim/ppc/hw_memory.c:204:7: error: pointer targets in passing argument 4 \
  of 'device_find_integer_array_property' differ in signedness \
  [-Werror=pointer-sign]
       &new_chunk->size);
       ^
...

Fix these by adding an explicit pointer cast.  It's a bit ugly to use APIs
based on signed integers to read out unsigned values, but in practice, this
is par for the course in the ppc code.

We already use signed APIs and assign the result to unsigned values a lot:
see how device_find_integer_property returns a signed integer (cell), but
then assign it to unsigned types.  The array APIs are not used that often
which is why we don't see many warnings, and we disable warnings when we
assign signed integers to unsigned integers in general.

The dtc/libfdt project (which is the standard in other projects) processes
the fdt blob as a series of bytes without any type information.  Typing is
left to the caller.  They have core APIs that read/write bytes, and a few
helper functions to cast/convert those bytes to the right value (e.g. u32).
In this ppc sim code, the core APIs use signed integers, and the callers
convert to unsigned, usually implicitly.

We could add some core APIs to the ppc sim that deal with raw bytes and then
add some helpers to convert to the right type, but that seems like a lot of
lifting for what boils down to a cast, and is effectively equivalent to all
the implicit assignments we use elsewhere.  Long term, a lot of the ppc code
should either get converted to existing sim common code, or we should stand
up proper APIs in the common code first, or use standard libraries to do all
the processing (e.g. libfdt).  Either way, this device.c code would all get
deleted, and callers (like these hw_*.c files) would get converted.  Which
is also why we go with a cast rather new (but largely unused) APIs.
---

diff --git a/sim/ppc/configure b/sim/ppc/configure
index 1e42a8adac4..a53a30f7fd4 100755
--- a/sim/ppc/configure
+++ b/sim/ppc/configure
@@ -3486,7 +3486,7 @@ sim_warnings="-Wall -Wdeclaration-after-statement -Wpointer-arith
 -Wmissing-declarations
 -Wmissing-prototypes
 -Wdeclaration-after-statement -Wmissing-parameter-type
--Wno-pointer-sign
+-Wpointer-sign
 -Wold-style-declaration -Wold-style-definition
 "
 # Enable -Wno-format by default when using gcc on mingw since many
diff --git a/sim/ppc/configure.ac b/sim/ppc/configure.ac
index 82250818eab..d1e9c09e9cd 100644
--- a/sim/ppc/configure.ac
+++ b/sim/ppc/configure.ac
@@ -427,7 +427,7 @@ sim_warnings="-Wall -Wdeclaration-after-statement -Wpointer-arith
 -Wmissing-declarations
 -Wmissing-prototypes
 -Wdeclaration-after-statement -Wmissing-parameter-type
--Wno-pointer-sign
+-Wpointer-sign
 -Wold-style-declaration -Wold-style-definition
 "
 # Enable -Wno-format by default when using gcc on mingw since many
diff --git a/sim/ppc/hw_memory.c b/sim/ppc/hw_memory.c
index 46b22f7b6e3..c0826b71139 100644
--- a/sim/ppc/hw_memory.c
+++ b/sim/ppc/hw_memory.c
@@ -199,9 +199,9 @@ hw_memory_init_address(device *me)
 	 cell_nr += 2) {
       hw_memory_chunk *new_chunk = ZALLOC(hw_memory_chunk);
       device_find_integer_array_property(me, "available", cell_nr,
-					 &new_chunk->address);
+					 (signed_cell *)&new_chunk->address);
       device_find_integer_array_property(me, "available", cell_nr + 1,
-					 &new_chunk->size);
+					 (signed_cell *)&new_chunk->size);
       new_chunk->available = 1;
       *curr_chunk = new_chunk;
       curr_chunk = &new_chunk->next;
diff --git a/sim/ppc/hw_opic.c b/sim/ppc/hw_opic.c
index 8afe312a7ef..9404204aa2f 100644
--- a/sim/ppc/hw_opic.c
+++ b/sim/ppc/hw_opic.c
@@ -417,10 +417,12 @@ hw_opic_init_data(device *me)
       }
       if (!device_find_integer_array_property(me, "interrupt-ranges",
 					      reg_nr * 2,
-					      &opic->isu_block[isb].int_number)
+					      (signed_cell *)
+					        &opic->isu_block[isb].int_number)
 	  || !device_find_integer_array_property(me, "interrupt-ranges",
 						 reg_nr * 2 + 1,
-						 &opic->isu_block[isb].range))
+						 (signed_cell *)
+						   &opic->isu_block[isb].range))
 	device_error(me, "missing or invalid interrupt-ranges property entry %d", reg_nr);
       /* first reg entry specifies the address of both the IDU and the
          first set of ISU registers, adjust things accordingly */