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(&array)] = 10 * j; /* segfaults */

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

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

[Bartc]

"Charles Sullivan" <cw******@triad.rr.comwrote in message
news:48**********************@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]

On Jun 1, 2:27*pm, Charles Sullivan <[e-mail]:

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(&array)] = 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(&array)) = 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]

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(&array)] = 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(&array) = 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]

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]

Gene wrote:

On Jun 1, 2:27 pm, Charles Sullivan <[e-mail]:

>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(&array)] = 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(&array)) = 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]

Bartc wrote:

"Charles Sullivan" <cw******@triad.rr.comwrote in message
news:48**********************@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]

jacob navia <ja***@nospam.comwrites:

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_Keith) ks***@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]

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

"Charles Sullivan" <cw******@triad.rr.comwrote in message
news:48**********************@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(&array)] = 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(&array);
for ( j = 0; j < 10; j++ )
array[nextindex(&array)] = 10 * j;

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

Regards,
Charles Sullivan

[Kenny McCormack]

In article <ln************@nuthaus.mib.org>,
Keith Thompson <ks***@mib.orgwrote:

>jacob navia <ja***@nospam.comwrites:

>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(&array)] = 10 * j; /* segfaults */

As if it was:

NULL[nextindex(&array)]

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

[Thad Smith]

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(&array)] = 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]

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(&array)] = 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(&array);
int temp2 = 10*j;
*temp1 = temp2;

which attempts to dereference the original array value of NULL.

--
Thad

[Eric Sosman]

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(&array)] = 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
es*****@ieee-dot-org.invalid

[Jens Thoms Toerring]

jacob navia <ja***@nospam.comwrote:

Gene wrote:

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

Bad code. The seg fault line is the same as

*(array + nextindex(&array)) = 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(&array)) = 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 ___ [e-mail]
\__________________________ http://toerring.de

[Barry Schwarz]

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

>Gene wrote:

>On Jun 1, 2:27 pm, Charles Sullivan <[e-mail]:

>>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(&array)] = 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(&array)) = 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

[Barry Schwarz]

On Sun, 1 Jun 2008 19:24:19 +0000 (UTC), Willem <wi****@stack.nl>
wrote:

>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(&array)] = 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.

There is a sequence point before and after the call to nextindex. The
undefined behavior comes during execution due to dereferencing a NULL
pointer.

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

And have nextindex return *arrayp + ++lastindex;
SaSW, Willem

Remove del for email

[Charles Sullivan]

Thanks guys. I now have a much better understanding of what's
happening.

Since the actual production code also uses realloc() to extend the array,
I took the easy way out and recoded:

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

instead as:

int i;
for ( j = 0; j < 10; j++ ) {
i = nextindex(&array);
array[i] = 10 * j;
}

Many thanks to all who contributed to the discussion.

Regards,
Charles Sullivan

[Peter Nilsson]

Thad Smith <[e-mail]:

jacob navia wrote:

Willem wrote:

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(&array)] = 10 * j; */* segfaults */

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

* *return 0;
}
>
I think this is an instance of 'modifying a variable
and using its value more than once between two sequence
points'.

You think wrong.

*You know, like 'i + i++'.

Which variable is being MODIFIED ???

array.

The function call is a sequence point. The array object
is not modified between 'previous and next' sequence
points.

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

It is modified by nextindex().

Which is incidental to Willem's claim.

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.

It would if it was an array, but it isn't, it's a
pointer which begins its life as a null pointer.

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

The UB comes simply from the unspecified order of
evaluation, and potential dereferencing of a null
pointer, not 6.5p2 as people have expressed.

--
Peter

[Thad Smith]

Barry Schwarz wrote:

On Sun, 1 Jun 2008 19:24:19 +0000 (UTC), Willem <wi****@stack.nl>
wrote:

>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(&array)] = 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.

There is a sequence point before and after the call to nextindex.

Yes. One of those sequence points then separates accessing array for
dereferencing and assigning array from within nextindex.

The
undefined behavior comes during execution due to dereferencing a NULL
pointer.

As I read the standard, it is first _unspecified_ which value array has for
dereferencing (Section 6.5 p3), then if array null, an attempt to
dereference results in undefined behavior. Taken together, the result is
undefined behavior due to possibility (implemention choice) of
dereferencing a null pointer.

--
Thad

[Nick Keighley]

On 1 Jun, 20:38, Keith Thompson <[e-mail]:

jacob navia <[e-mail]:

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.

almost certainly *does* modify the value of array as calloc()
is called

--
Nick keighley