Bad code or bad compiler?

[Charles Sullivan]

A C program with code typified by the following pared-down example
has been running after compilation on numerous compilers for several
years. However with a fairly recent GCC compiler it results in a
segmentation fault at the indicated instruction.

Briefly, the problem occurs when writing to an array element when
the array pointer is allocated by a function in the array index.

My question: Is this bad C code, "unwise" C code, or a compiler bug?

(Note: This simple example is obviously not production code.)

Thanks for your help.

Regards,
Charles Sullivan

-----------------------------------------
#include <stdio.h>
#include <stdlib.h>

int nextindex ( int **arrayp )
{
static int lastindex = -1;

if ( *arrayp == NULL )
*arrayp = calloc(100, sizeof(int));

return ++lastindex;
}

int main ( void )
{
int j;
int *array = NULL;

for ( j = 0; j < 10; j++ )
array[nextindex(&arra y)] = 10 * j; /* segfaults */

for ( j = 0; j < 5; j++ )
printf("array[%d] = %d\n", j, array[j]);

return 0;
}
--------------------------------------

[Bartc]

Re: Bad code or bad compiler?


"Charles Sullivan" <cwsulliv@triad .rr.comwrote in message
news:4842ea2a$0 $7078$4c368faf@ roadrunner.com. ..

>
A C program with code typified by the following pared-down example
has been running after compilation on numerous compilers for several
years. However with a fairly recent GCC compiler it results in a
segmentation fault at the indicated instruction.
>
Briefly, the problem occurs when writing to an array element when
the array pointer is allocated by a function in the array index.
>
My question: Is this bad C code, "unwise" C code, or a compiler bug?
>
(Note: This simple example is obviously not production code.)
>
Thanks for your help.
>
Regards,
Charles Sullivan
>
-----------------------------------------
#include <stdio.h>
#include <stdlib.h>
>
int nextindex ( int **arrayp )
{
static int lastindex = -1;
>
if ( *arrayp == NULL )
*arrayp = calloc(100, sizeof(int));

Aren't you supposed to check whether calloc returns a valid pointer? (I know
it's a little different to malloc)

What's the value of *arrayp (or array in main()) anyway, just before it goes
wrong?

--
Bartc

[Gene]

Re: Bad code or bad compiler?

On Jun 1, 2:27 pm, Charles Sullivan <cwsul...@triad .rr.comwrote:

A C program with code typified by the following pared-down example
has been running after compilation on numerous compilers for several
years.  However with a fairly recent GCC compiler it results in a
segmentation fault at the indicated instruction.
>
Briefly, the problem occurs when writing to an array element when
the array pointer is allocated by a function in the array index.
>
My question: Is this bad C code, "unwise" C code, or a compiler bug?
>
(Note: This simple example is obviously not production code.)
>
Thanks for your help.
>
Regards,
Charles Sullivan
>
-----------------------------------------
#include <stdio.h>
#include <stdlib.h>
>
int nextindex ( int **arrayp )
{
   static int lastindex = -1;
>
   if ( *arrayp == NULL )
      *arrayp = calloc(100, sizeof(int));
>
   return ++lastindex;
>
}
>
int main ( void )
{
   int j;
   int *array = NULL;
>
   for ( j = 0; j < 10; j++ )
      array[nextindex(&arra y)] = 10 * j;  /* segfaults */
>
   for ( j = 0; j < 5; j++ )
      printf("array[%d] = %d\n", j, array[j]);
>
   return 0;}
>
--------------------------------------

Bad code. The seg fault line is the same as

*(array + nextindex(&arra y)) = 10 * j;

The operands can be evaluated in either order. So the compiler is
allowed to e.g generate code that loads the value of array into a
register, followed by the (potentially inlined) nextindex
computation. My guess is that this is what's happening.

[Willem]

Re: Bad code or bad compiler?

Charles wrote:
) A C program with code typified by the following pared-down example
) has been running after compilation on numerous compilers for several
) years. However with a fairly recent GCC compiler it results in a
) segmentation fault at the indicated instruction.

<snip code for nextindex, which modifies what its argument points to>

) for ( j = 0; j < 10; j++ )
) array[nextindex(&arra y)] = 10 * j; /* segfaults */

I think this is an instance of 'modifying a variable and using its value
more than once between two sequence points'. You know, like 'i + i++'.
So that is UB, and therefore the segfault can be expected.
If I'm correct, you were simply lucky it worked for so long.

You can rewrite it to:
for ( j = 0; j < 10; j++ )
nextindex(&arra y) = 10 * j;

And have nextindex return *arrayp + ++lastindex;


SaSW, Willem
--
Disclaimer: I am in no way responsible for any of the statements
made in the above text. For all I know I might be
drugged or something..
No I'm not paranoid. You all think I'm paranoid, don't you !
#EOT

[jacob navia]

Re: Bad code or bad compiler?

Willem wrote:

I think this is an instance of 'modifying a variable and using its value
more than once between two sequence points'. You know, like 'i + i++'.

????

Which variable is being MODIFIED ???

Array is just being READ, not modified in any way!

The array position at array+nextindex () is being
modified, but the variable array is not modified at all.
It always point to the first element of the array.



--
jacob navia
jacob at jacob point remcomp point fr
logiciels/informatique

http://www.cs.virginia.edu/~lcc-win32

[jacob navia]

Re: Bad code or bad compiler?

Gene wrote:

On Jun 1, 2:27 pm, Charles Sullivan <cwsul...@triad .rr.comwrote:

>A C program with code typified by the following pared-down example
>has been running after compilation on numerous compilers for several
>years. However with a fairly recent GCC compiler it results in a
>segmentation fault at the indicated instruction.
>>
>Briefly, the problem occurs when writing to an array element when
>the array pointer is allocated by a function in the array index.
>>
>My question: Is this bad C code, "unwise" C code, or a compiler bug?
>>
>(Note: This simple example is obviously not production code.)
>>
>Thanks for your help.
>>
>Regards,
>Charles Sullivan
>>
>-----------------------------------------
>#include <stdio.h>
>#include <stdlib.h>
>>
>int nextindex ( int **arrayp )
>{
> static int lastindex = -1;
>>
> if ( *arrayp == NULL )
> *arrayp = calloc(100, sizeof(int));
>>
> return ++lastindex;
>>
>}
>>
>int main ( void )
>{
> int j;
> int *array = NULL;
>>
> for ( j = 0; j < 10; j++ )
> array[nextindex(&arra y)] = 10 * j; /* segfaults */
>>
> for ( j = 0; j < 5; j++ )
> printf("array[%d] = %d\n", j, array[j]);
>>
> return 0;}
>>
>--------------------------------------

>
Bad code. The seg fault line is the same as
>
*(array + nextindex(&arra y)) = 10 * j;

yes.

>
The operands can be evaluated in either order. So the compiler is
allowed to e.g generate code that loads the value of array into a
register, followed by the (potentially inlined) nextindex
computation.

Correct


My guess is that this is what's happening.

But why should that segment fault???

It is perfectly legal!

--
jacob navia
jacob at jacob point remcomp point fr
logiciels/informatique

http://www.cs.virginia.edu/~lcc-win32

[jacob navia]

Re: Bad code or bad compiler?

Bartc wrote:

"Charles Sullivan" <cwsulliv@triad .rr.comwrote in message
news:4842ea2a$0 $7078$4c368faf@ roadrunner.com. ..

>A C program with code typified by the following pared-down example
>has been running after compilation on numerous compilers for several
>years. However with a fairly recent GCC compiler it results in a
>segmentation fault at the indicated instruction.
>>
>Briefly, the problem occurs when writing to an array element when
>the array pointer is allocated by a function in the array index.
>>
>My question: Is this bad C code, "unwise" C code, or a compiler bug?
>>
>(Note: This simple example is obviously not production code.)
>>
>Thanks for your help.
>>
>Regards,
>Charles Sullivan
>>
>-----------------------------------------
>#include <stdio.h>
>#include <stdlib.h>
>>
>int nextindex ( int **arrayp )
>{
> static int lastindex = -1;
>>
> if ( *arrayp == NULL )
> *arrayp = calloc(100, sizeof(int));

>
>
Aren't you supposed to check whether calloc returns a valid pointer? (I know
it's a little different to malloc)
>
What's the value of *arrayp (or array in main()) anyway, just before it goes
wrong?
>

With lcc-win you program produces the expected results.
I do not see anything wrong with it.


--
jacob navia
jacob at jacob point remcomp point fr
logiciels/informatique

http://www.cs.virginia.edu/~lcc-win32

[Keith Thompson]

Re: Bad code or bad compiler?

jacob navia <jacob@nospam.c omwrites:

Willem wrote:

>I think this is an instance of 'modifying a variable and using its value
>more than once between two sequence points'. You know, like 'i + i++'.

>
????
>
Which variable is being MODIFIED ???
>
Array is just being READ, not modified in any way!
>
The array position at array+nextindex () is being
modified, but the variable array is not modified at all.
It always point to the first element of the array.

&array is being passed as an argument to a function, which can modify
the value of array.

--
Keith Thompson (The_Other_Keit h) kst-u@mib.org <http://www.ghoti.net/~kst>
Nokia
"We must do something. This is something. Therefore, we must do this."
-- Antony Jay and Jonathan Lynn, "Yes Minister"

[Charles Sullivan]

Re: Bad code or bad compiler?

On Sun, 01 Jun 2008 18:52:21 +0000, Bartc wrote:

"Charles Sullivan" <cwsulliv@triad .rr.comwrote in message
news:4842ea2a$0 $7078$4c368faf@ roadrunner.com. ..

>>
>A C program with code typified by the following pared-down example has
>been running after compilation on numerous compilers for several years.
> However with a fairly recent GCC compiler it results in a segmentation
>fault at the indicated instruction.
>>
>Briefly, the problem occurs when writing to an array element when the
>array pointer is allocated by a function in the array index.
>>
>My question: Is this bad C code, "unwise" C code, or a compiler bug?
>>
>(Note: This simple example is obviously not production code.)
>>
>Thanks for your help.
>>
>Regards,
>Charles Sullivan
>>
>----------------------------------------- #include <stdio.h>
>#include <stdlib.h>
>>
>int nextindex ( int **arrayp )
>{
> static int lastindex = -1;
>>
> if ( *arrayp == NULL )
> *arrayp = calloc(100, sizeof(int));

>
>
Aren't you supposed to check whether calloc returns a valid pointer? (I
know it's a little different to malloc)

As I said, this is not production code - just a very simple example
which replicates the problem. The production code checks for a valid
pointer within the function nextindex() and it's always been valid.

What's the value of *arrayp (or array in main()) anyway, just before it
goes wrong?

I had assumed that the index of an array would be computed before
attempting to write to the array, but evidently that's not happening
with this compiler (GCC 4.3.0).

If instead of:
array[nextindex(&arra y)] = 10 * j;

the code is changed to:
int i;
for ( j = 0; j < 10; j++ ) {
i = nextindex[&array];
array[i] = 10 * j;
}

-- or even just --
nextindex(&arra y);
for ( j = 0; j < 10; j++ )
array[nextindex(&arra y)] = 10 * j;

There's no segfault (although in the latter I'm now skipping element 0 of
array[]).

Regards,
Charles Sullivan

[Kenny McCormack]

Re: Bad code or bad compiler?

In article <lnmym5c5ec.fsf @nuthaus.mib.or g>,
Keith Thompson <kst-u@mib.orgwrote:

>jacob navia <jacob@nospam.c omwrites:

>Willem wrote:

>>I think this is an instance of 'modifying a variable and using its value
>>more than once between two sequence points'. You know, like 'i + i++'.

>>
>????
>>
>Which variable is being MODIFIED ???
>>
>Array is just being READ, not modified in any way!
>>
>The array position at array+nextindex () is being
>modified, but the variable array is not modified at all.
>It always point to the first element of the array.

>
>&array is being passed as an argument to a function, which can modify
>the value of array.

Right. Another way to look at it is that the first time through the
loop, array (an "int *" type variable) has the value NULL.

So, the compiler is allowed to compile the LHS of:

array[nextindex(&arra y)] = 10 * j; /* segfaults */

As if it was:

NULL[nextindex(&arra y)]

which is legal syntax, but is unlikely to survive at runtime.

[Thad Smith]

Re: Bad code or bad compiler?

Charles Sullivan wrote:

int nextindex ( int **arrayp )
{
static int lastindex = -1;
>
if ( *arrayp == NULL )
*arrayp = calloc(100, sizeof(int));
>
return ++lastindex;
}
>
int main ( void )
{
int j;
int *array = NULL;
>
for ( j = 0; j < 10; j++ )
array[nextindex(&arra y)] = 10 * j; /* segfaults */
>
for ( j = 0; j < 5; j++ )
printf("array[%d] = %d\n", j, array[j]);
>
return 0;
}

jacob navia wrote:

Willem wrote:

>I think this is an instance of 'modifying a variable and using its value
>more than once between two sequence points'. You know, like 'i + i++'.

Which variable is being MODIFIED ???

array.

Array is just being READ, not modified in any way!

It is modified by nextindex().

The array position at array+nextindex () is being
modified, but the variable array is not modified at all.
It always point to the first element of the array.

On first call to nextindex(), array does not point to any array.

--
Thad

[Thad Smith]

Re: Bad code or bad compiler?

Charles Sullivan wrote:

I had assumed that the index of an array would be computed before
attempting to write to the array, but evidently that's not happening
with this compiler (GCC 4.3.0).

The index is necessarily computed before attempting to write to an indexed
array element.

If instead of:
array[nextindex(&arra y)] = 10 * j;
>
the code is changed to:
int i;
for ( j = 0; j < 10; j++ ) {
i = nextindex[&array];
array[i] = 10 * j;
}

....

There's no segfault...

That's because the latter has a sequence point between the first call to
nextindex() and referencing array for the assignment. The former does not.
A compiler is free to generate code for the former as if written;


int *temp1 = array;
temp1 += nextindex(&arra y);
int temp2 = 10*j;
*temp1 = temp2;

which attempts to dereference the original array value of NULL.

--
Thad

[Eric Sosman]

Re: Bad code or bad compiler?

Charles Sullivan wrote:

A C program with code typified by the following pared-down example
has been running after compilation on numerous compilers for several
years. However with a fairly recent GCC compiler it results in a
segmentation fault at the indicated instruction.
>
Briefly, the problem occurs when writing to an array element when
the array pointer is allocated by a function in the array index.
>
My question: Is this bad C code, "unwise" C code, or a compiler bug?

Bad C code. There's undefined behavior here:

array[nextindex(&arra y)] = 10 * j; /* segfaults */

^^^^^ ^^^^^^^^^^^^^^^ ^^
1 2

.... because it is unspecified whether part (1) or (2) of
the expression is evaluated first. If (2) is evaluated
first then the `array' pointer is set non-NULL inside the
function, and all's well. But if (1) is evaluated first,
you get `((int*)NULL)[...] = ...' and undefined behavior.

--
Eric Sosman
esosman@ieee-dot-org.invalid

[Jens Thoms Toerring]

Re: Bad code or bad compiler?

jacob navia <jacob@nospam.c omwrote:

Gene wrote:

array[nextindex(&arra y)] = 10 * j; /* segfaults */

Bad code. The seg fault line is the same as

*(array + nextindex(&arra y)) = 10 * j;

The operands can be evaluated in either order. So the compiler is
allowed to e.g generate code that loads the value of array into a
register, followed by the (potentially inlined) nextindex
computation.

Correct

My guess is that this is what's happening.

But why should that segment fault???

It is perfectly legal!

If you have

*(array + nextindex(&arra y)) = 10 * j;

and it first evaluates the 'array' part, which is NULL (at least
initially, and then adds the return value of the nextindex() call,
then you get either again the NULL pointer (if nextindex() did
return 0) or some value near to NULL, which rather likely also
isn't a pointer that can be dereferenced. Thus the segmentation
fault. So I don't think that this is legal code when it depends
on the order of the evaluation of the expression in parentheses.

Regards, Jens
--
\ Jens Thoms Toerring ___ jt@toerring.de
\______________ ____________ http://toerring.de

[Barry Schwarz]

Re: Bad code or bad compiler?

On Sun, 01 Jun 2008 21:34:30 +0200, jacob navia <jacob@nospam.c om>
wrote:

>Gene wrote:

>On Jun 1, 2:27 pm, Charles Sullivan <cwsul...@triad .rr.comwrote:

>>A C program with code typified by the following pared-down example
>>has been running after compilation on numerous compilers for several
>>years. However with a fairly recent GCC compiler it results in a
>>segmentatio n fault at the indicated instruction.
>>>
>>Briefly, the problem occurs when writing to an array element when
>>the array pointer is allocated by a function in the array index.
>>>
>>My question: Is this bad C code, "unwise" C code, or a compiler bug?
>>>
>>(Note: This simple example is obviously not production code.)
>>>
>>Thanks for your help.
>>>
>>Regards,
>>Charles Sullivan
>>>
>>-----------------------------------------
>>#include <stdio.h>
>>#include <stdlib.h>
>>>
>>int nextindex ( int **arrayp )
>>{
>> static int lastindex = -1;
>>>
>> if ( *arrayp == NULL )
>> *arrayp = calloc(100, sizeof(int));
>>>
>> return ++lastindex;
>>>
>>}
>>>
>>int main ( void )
>>{
>> int j;
>> int *array = NULL;
>>>
>> for ( j = 0; j < 10; j++ )
>> array[nextindex(&arra y)] = 10 * j; /* segfaults */
>>>
>> for ( j = 0; j < 5; j++ )
>> printf("array[%d] = %d\n", j, array[j]);
>>>
>> return 0;}
>>>
>>--------------------------------------

>>
>Bad code. The seg fault line is the same as
>>
>*(array + nextindex(&arra y)) = 10 * j;

>
>yes.
>

>>
>The operands can be evaluated in either order. So the compiler is
>allowed to e.g generate code that loads the value of array into a
>register, followed by the (potentially inlined) nextindex
>computation.

>
>Correct
>
>
>My guess is that this is what's happening.
>
>But why should that segment fault???

At the first iteration through the loop, array is NULL. NULL +
anything does not point to memory you own.

>
>It is perfectly legal!

You can't dereference memory you don't own.


Remove del for email