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

[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?

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

[Barry Schwarz]

Re: Bad code or bad compiler?

On Sun, 1 Jun 2008 19:24:19 +0000 (UTC), Willem <willem@stack.n l>
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(&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.

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(&arra y) = 10 * j;
>
>And have nextindex return *arrayp + ++lastindex;
>
>
>SaSW, Willem

Remove del for email

[Charles Sullivan]

Re: Bad code or bad compiler? THANKS


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

instead as:

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

Many thanks to all who contributed to the discussion.

Regards,
Charles Sullivan

[Thad Smith]

Re: Bad code or bad compiler?

Barry Schwarz wrote:

On Sun, 1 Jun 2008 19:24:19 +0000 (UTC), Willem <willem@stack.n l>
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(&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.

>
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]

Re: Bad code or bad compiler?

On 1 Jun, 20:38, Keith Thompson <ks...@mib.orgw rote:

jacob navia <ja...@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.

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

--
Nick keighley