From patchwork Wed Jul 12 04:53:38 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Thomas Huth X-Patchwork-Id: 787001 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3x6mvg6QKGz9s65 for ; Wed, 12 Jul 2017 14:59:27 +1000 (AEST) Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 3x6mvg4yDCzDqgX for ; Wed, 12 Jul 2017 14:59:27 +1000 (AEST) X-Original-To: slof@lists.ozlabs.org Delivered-To: slof@lists.ozlabs.org X-Greylist: delayed 337 seconds by postgrey-1.36 at bilbo; Wed, 12 Jul 2017 14:59:21 AEST Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3x6mvY1B9BzDqbZ for ; Wed, 12 Jul 2017 14:59:21 +1000 (AEST) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 88A075D686; Wed, 12 Jul 2017 04:53:42 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 88A075D686 Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=thuth@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 88A075D686 Received: from [10.36.116.105] (ovpn-116-105.ams2.redhat.com [10.36.116.105]) by smtp.corp.redhat.com (Postfix) with ESMTPS id BC9BC5D6A3; Wed, 12 Jul 2017 04:53:40 +0000 (UTC) To: Alexey Kardashevskiy , Segher Boessenkool References: <1496828499-30874-1-git-send-email-thuth@redhat.com> <1322f20e-4fa8-1cc6-65a6-d58cf1bc7734@ozlabs.ru> <20170608171335.GK19687@gate.crashing.org> From: Thomas Huth Message-ID: <88dbbef6-96cb-a32b-fba8-2bb512039c64@redhat.com> Date: Wed, 12 Jul 2017 06:53:38 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Wed, 12 Jul 2017 04:53:42 +0000 (UTC) Subject: Re: [SLOF] [PATCH] libc: The arguments of puts() can be marked as "const" X-BeenThere: slof@lists.ozlabs.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Patches for https://github.com/aik/SLOF" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: slof@lists.ozlabs.org Errors-To: slof-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "SLOF" On 12.07.2017 05:46, Alexey Kardashevskiy wrote: > On 09/06/17 03:13, Segher Boessenkool wrote: >> On Thu, Jun 08, 2017 at 08:54:06AM +0200, Thomas Huth wrote: >>> On 08.06.2017 08:12, Alexey Kardashevskiy wrote: >>>> /home/aik/p/slof/slof/paflof.c: In function ‘engine’: >>>> /home/aik/p/slof/slof/paflof.c:84:23: warning: array subscript is below >>>> array bounds [-Warray-bounds] >>>> dp = the_data_stack - 1; >>>> ~~~~~~~~~~~~~~~^~~ >>>> /home/aik/p/slof/slof/paflof.c:85:22: warning: array subscript is below >>>> array bounds [-Warray-bounds] >>>> rp = handler_stack - 1; >>>> ~~~~~~~~~~~~~~^~~ >>>> >>>> with gcc (GCC) 6.3.1 20161221 (Red Hat 6.3.1-1) from Fedora24/BE. >>>> >>>> Can you please take a look on this? Thanks. >>> >>> See Segher's suggestions here: >>> >>> https://lists.ozlabs.org/pipermail/slof/2016-August/001221.html >>> >>> IMHO we could also just simply sacrifice the first stack entry and only >>> use "dp = the_data_stack", without adding the inline asm here (to keep >>> paflof.c portable). >> >> Or you simply make the_*_stack declared as a pointer in paflof.c, not as >> an array. Where is it actually defined? If it is defined in assembler >> code all is fine; if it is defined in C code, well, don't lie to the >> compiler or it will take its revenge (if using full-program optimisation >> it can still see you're accessing the array out-of-bounds for example, >> or worse, assume you don't do undefined things and optimise accordingly). > > > slof/paflof.c includes slof/ppc64.c (via #include ISTR(TARG,c)) which > defines the_data_stack. > > handler_stack is defined right in slof/paflof.c's engine(). > >> An easy way around is to have the_*_stack just an external symbol, and >> have the_real_*_stack for the arrays of cells, and then equate the two >> in a linker script. > > Harsh. > >> Or, ignore the warning. If ever things break (and they won't), it will >> do so with lots of fireworks; it won't silently break. > > This shuts the gcc up (and we loose 1 cell in each stack): > > diff --git a/slof/paflof.c b/slof/paflof.c > index 50b4adf..1e7874a 100644 > --- a/slof/paflof.c > +++ b/slof/paflof.c > @@ -68,7 +68,8 @@ long engine(int mode, long param_1, long param_2) > > cell *restrict ip; > cell *restrict cfa; > - static cell handler_stack[160]; > + static cell handler_stack_[160]; > + static cell *handler_stack = handler_stack_ + 1; > static cell c_return[2]; > static cell dummy; > > diff --git a/slof/ppc64.c b/slof/ppc64.c > index 83a8e82..76c1bdf 100644 > --- a/slof/ppc64.c > +++ b/slof/ppc64.c > @@ -26,7 +26,8 @@ cell the_exception_frame[0x400 / CELLSIZE] __attribute__ > ((aligned(PAGE_SIZE))); > cell the_client_frame[0x1000 / CELLSIZE] __attribute__ ((aligned(0x100))); > cell the_client_stack[0x8000 / CELLSIZE] __attribute__ ((aligned(0x100))); > /* THE forth stack */ > -cell the_data_stack[0x2000 / CELLSIZE] __attribute__ ((aligned(0x100))); > +cell the_data_stack_[0x2000 / CELLSIZE] __attribute__ ((aligned(0x100))); > +cell *the_data_stack = the_data_stack_ + 1; > /* the forth return stack */ > cell the_return_stack[0x2000 / CELLSIZE] __attribute__ ((aligned(0x100))); That's kind of ugly. Why not simply: ? Thomas diff --git a/slof/paflof.c b/slof/paflof.c index 50b4adf..ea3c145 100644 --- a/slof/paflof.c +++ b/slof/paflof.c @@ -81,8 +81,8 @@ long engine(int mode, long param_1, long param_2) LAST_ELEMENT(xt_FORTH_X2d_WORDLIST).a = xt_LASTWORD; // stack-pointers - dp = the_data_stack - 1; - rp = handler_stack - 1; + dp = the_data_stack; + rp = handler_stack; // return-address for "evaluate" personality dummy.a = &&over;