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

[Thomas Huth]

On 12.07.2017 07:47, Alexey Kardashevskiy wrote:
> 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.

Ugh, ok.

What about:

	dp = the_data_stack;
        rp = handler_stack;
	barrier();
	dp--;
	rp--;

?

That should hopefully shut up the compiler, too, and result in the same
initial setting of dp and rp.

 Thomas