From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id 9B9953848011 for ; Thu, 20 May 2021 12:08:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 9B9953848011 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tdevries@suse.de X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id B6241ABE8; Thu, 20 May 2021 12:08:25 +0000 (UTC) Subject: Re: [PATCH] sim: ppc: fix some Wpointer-sign warnings To: gdb-patches@sourceware.org, andrew.burgess@embecosm.com References: <20210519104646.GA11845@delia> From: Tom de Vries Message-ID: Date: Thu, 20 May 2021 14:08:25 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------FFA346B593B66AE51553A7AE" Content-Language: en-US X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_STATUS, NICE_REPLY_A, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 May 2021 12:08:28 -0000 This is a multi-part message in MIME format. --------------FFA346B593B66AE51553A7AE Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit On 5/19/21 6:32 PM, Mike Frysinger wrote: > On 19 May 2021 12:46, Tom de Vries wrote: >> --- a/sim/ppc/hw_memory.c >> +++ b/sim/ppc/hw_memory.c >> @@ -190,7 +190,7 @@ hw_memory_init_address(device *me) >> if (device_find_property(me, "available") != NULL) { >> hw_memory_chunk **curr_chunk = &hw_memory->heap; >> int cell_nr; >> - unsigned_cell dummy; >> + signed_cell dummy; >> int nr_cells = device_find_integer_array_property(me, "available", 0, &dummy); > > this one is fine > >> @@ -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); >> >> --- 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)) > > these ones i'm not sure about. it does fix the warnings, and it doesn't > change the status quo behavior, but i don't think it's the actual fix we > would want. if the device tree has a negative number, it'll get converted > to an unsigned number. i haven't thought hard as to what the right fix > would look like here. > > i think we'd have to look at what other device tree users are doing like > in boot loaders (e.g. u-boot) and in the linux kernel. Ack. That's out of scope for me though, so I'm gonna carry this updated patch locally, and if not fixed otherwise, this'll end up in the openSUSE gdb package. Thanks, - Tom --------------FFA346B593B66AE51553A7AE Content-Type: text/x-patch; charset=UTF-8; name="0001-sim-ppc-silence-some-Wpointer-sign-warnings.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline; filename="0001-sim-ppc-silence-some-Wpointer-sign-warnings.patch" sim: ppc: silence some Wpointer-sign warnings When compiling with --enable-werror and CFLAGS=3D"-O0 -g -Wall", we run i= nto: =2E.. src/sim/ppc/hw_memory.c: In function =E2=80=98hw_memory_init_address=E2=80= =99: src/sim/ppc/hw_memory.c:204:7: error: pointer targets in passing argument=20 4 \ of =E2=80=98device_find_integer_array_property=E2=80=99 differ in signe= dness \ [-Werror=3Dpointer-sign] &new_chunk->size); ^ =2E.. Silence the warnings by adding an explicit pointer cast. This makes the = type conversion explicit, and doesn't change the current behavior. This may actually need a proper fix, but that's out of scope for this pat= ch, which merely aims to fix the build with parameters as used by the openSUS= E gdb package. --- sim/ppc/hw_memory.c | 4 ++-- sim/ppc/hw_opic.c | 6 ++++-- 2 files changed, 6 insertions(+), 4 deletions(-) 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 +=3D 2) { hw_memory_chunk *new_chunk =3D 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 =3D 1; *curr_chunk =3D new_chunk; curr_chunk =3D &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 */ --------------FFA346B593B66AE51553A7AE--