[SLOF] [PATCH] libc: The arguments of puts() can be marked as "const"

[Alexey Kardashevskiy]

On 12/07/17 14:53, Thomas Huth wrote:
> 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:
> 
> 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;
> 
> ?


You also need to fix:

slof/prim.code|207| PRIM(DEPTH) PUSH; TOS.u = dp - the_data_stack; MIRP

as otherwise SLOF stops at its prompt with "1 >" rather than "0 >".

What is uglier is a question.

> 
>  Thomas
> 


-- 
Alexey