[Vm-dev] [commit][3753] Do not use -O3 optimization, -O2 is safer and works well.

[Ben Coman]

Sorry, ignore that last one.  It fired off incomplete.

On Wed, Dec 21, 2016 at 10:35 PM, David T. Lewis <lewis at mail.msen.com>
wrote:
>
> On Wed, Dec 21, 2016 at 10:11:39AM +0000, Jan Vrany wrote:
>>
>> On Tue, 2016-12-20 at 21:54 -0500, David T. Lewis wrote:
>> > ??
>> > On Wed, Dec 21, 2016 at 10:40:32AM +0800, Ben Coman wrote:
>> > > ??
>> > > I'd be interested to know the reason sends get slower (if known).
>> > > cheers -ben
>> >
>> > No clue, I was just sanity checking to make sure that -O2 was not
>> > horribly
>> > worse. It was not. But I suspect all of this is likely to vary
>> > depending on
>> > gcc compiler version and phase of the moon.
>>
>> This post:
>>
>> http://blog.llvm.org/2011/05/what-every-c-programmer-should-know_14.html
>>
>> explains the dependency on moon phases (and compiler versions) :-)
>>
>
> Thanks. Indeed there is probably some undefined C behavior in there,
although
> I was not able to spot it. If anyone is interested in lending their eyes
to
> the problem, I was able to localize the crash to intermittent segfaults
that
> occurred in OSProcessPlugin>>fixPointersInArrayOfStrings: which is
generated
> in C (for a V3 image, not Spur) as:
>
> /*      Use the address offsets in offsetArray to fix up the pointers in
cStringArray.
>         The result is a C array of pointers to char, used for argv and
env vectors. */
>
> static sqInt fixPointersInArrayOfStringswithOffsetscount(char
*flattenedArrayOfStrings, sqInt *offsetArray, sqInt count) {
>     sqInt idx;
>     char **ptr;
>
>         ptr = ((char **) flattenedArrayOfStrings);
>         idx = 0;
>         while (idx < count) {
>                 ptr[idx] = (flattenedArrayOfStrings +
(((offsetArray[idx]) >> 1)));
>                 idx += 1;
>         }
>         return null;
> }
>
>


I've taken a guess at its usage and turned it into an executable test case.
Could you confirm this...

#include <stdio.h>
typedef int sqInt;
int null = 0;
static sqInt
fixPointersInArrayOfStringswithOffsetscount(char *flattenedArrayOfStrings,
sqInt *offsetArray, sqInt count) {
    sqInt idx;
    char **ptr;
        ptr = ((char **) flattenedArrayOfStrings);
        idx = 0;
        while (idx < count) {
                ptr[idx] = (flattenedArrayOfStrings + (((offsetArray[idx])
>> 1)));
                idx += 1;
        }
        return null;
}
int main()
{
        char *flattenedArrayOfStrings = "abcd\0efgh\0ijkl\0";
        sqInt offsetArray[] = {0, 5, 10};
        printf("%s\n", flattenedArrayOfStrings);
        printf("%d %d %d\n", offsetArray[0], offsetArray[1],
offsetArray[2]);

fixPointersInArrayOfStringswithOffsetscount( flattenedArrayOfStrings,
offsetArray, 2 );
        printf("%s\n", flattenedArrayOfStrings);
}

$ cc test.c ; ./a.out
abcd
0 5 10
Segmentation fault

ptr is being defined as a pointer to "one" string, but its being
accessed as a consecutive list of strings.
Even the first assignment to ptr seems wrong.  flattenedArrayOfStrings
is a string. The first dereference is a char, and then when it assigns
to ptr[idx], it tries to dereferenced the char and boom!

If the first ptr assignment is changed as follows, it doesn't crash...
       ptr = &flattenedArrayOfStrings;

But I'm not sure it does what its supposed to ??
$ cc test.c ; ./a.out
abcd
0 5 10
abcd
cd
abcd


Now if I do this...
static sqInt fixPointersInArrayOfStringswithOffsetscount(char
*flattenedArrayOfStrings, sqInt offsetArray[], sqInt count) {
    sqInt idx;
    char *ptr[count+1];
        idx = 0;
        while (idx < count) {
                ptr[idx] = flattenedArrayOfStrings + offsetArray[idx];
                idx += 1;
        }
    idx = 0;
    while (idx < count) {
            printf("%s\n", ptr[idx]);
            idx += 1;
    }
    return null;
}

I can call it with count=3 and get this...
$ cc test.c ; ./a.out
abcd
0 5 10
abcd
efgh
ijkl
abcd

but the original flattenedArrayOfStrings is unmodified, so I'm not sure if
that is how its meant to behave?

cheers -ben
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/vm-dev/attachments/20161222/d34dc087/attachment-0001.html>