diff options
author | Tom de Vries <tdevries@suse.de> | 2021-05-19 12:46:47 +0200 |
---|---|---|
committer | Mike Frysinger <vapier@gentoo.org> | 2021-09-08 23:33:25 -0400 |
commit | 81f839676d98024f6c0a6609866bfa81b5f35325 (patch) | |
tree | 56d5d325eb60adb1e831e9918b36e04db26ba788 | |
parent | 0c320e1bdcb45e0d46f69a1567a06be326529c34 (diff) | |
download | gdb-81f839676d98024f6c0a6609866bfa81b5f35325.zip gdb-81f839676d98024f6c0a6609866bfa81b5f35325.tar.gz gdb-81f839676d98024f6c0a6609866bfa81b5f35325.tar.bz2 |
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.
-rwxr-xr-x | sim/ppc/configure | 2 | ||||
-rw-r--r-- | sim/ppc/configure.ac | 2 | ||||
-rw-r--r-- | sim/ppc/hw_memory.c | 4 | ||||
-rw-r--r-- | sim/ppc/hw_opic.c | 6 |
4 files changed, 8 insertions, 6 deletions
diff --git a/sim/ppc/configure b/sim/ppc/configure index 1e42a8a..a53a30f 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 8225081..d1e9c09 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 46b22f7..c0826b7 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 8afe312..9404204 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 */ |