[comp.lang.c] Optimization considered harmful

bks@alfa.berkeley.edu (Brad Sherman) (10/31/90)

I have included a program below which is broken by the Microsoft 6.0
compiler on MSDOS.   While this is no great surprise, the circumstances
that break the code have caused some concern in our shop.

The compiler only generates bad code under certain conditions (outlined
below) but the gist is this:  the interaction between the use of the
keyword "register" and the attempts of the compiler to perform
some optimization causes faulty assembly language to be generated.
We guess that this is because the compiler "runs out of registers."
(Is that excessively anthropomorphic?)

We port code to many different platforms (MSDOS, NOVELL, UNIX, VMS, ... )
and attempt to write maximally portable C.  Normally we test and
debug our code and *then* use the optimizer.

Now, which of the following lessons should be drawn from this event:
	1) Don't use "register."
	2) Don't use optimizers (or in this case be sure to disable optimizer).
	3) Don't use "improved" Microsoft products.
Unfortunately, not porting to MSDOS is precluded

-- Brad Sherman (bks@alfa.berkeley.edu)


-------Program listing follows----------------------------------

/*
 *	Program breaks under MSC 6.0.
 *	Compile cl /AL /o prog prog.c.  The program will break
 *	at checkpoint 3, and MSDOS will hang shortly thereafter.
 *	Will compile and run correctly if:
 *		Compiled with MSC 5.1.
 *		Compiled small or medium memory model.
 *		Compiled with no optimization /Od.
 *		Compiled with all optimizations /Ox.
 *		Keyword "register" is removed!
 *	You can look at the assembly language listing and see that
 *	an address stored in a register is getting clobbered.  The
 *	problem is not due to lack of memory.  Substituting pointer
 *	arithmetic [(*(s_array +i))->s] for array notation has no
 *	salutory affect.
 */
#include <stdio.h>
#include <stdlib.h>
#include <malloc.h>

typedef struct string {
	char *s;
	int  len;
} STRING;

main()
{
	register int i;
	STRING **s_array;
	if ((s_array = (STRING **)malloc(100 * sizeof(STRING))) == NULL) {
		printf("Problem at 1\n");
		exit(1);
	}
	for (i = 0; i < 100; i++) {
		if ((s_array[i] = (STRING *)malloc(sizeof(STRING))) == NULL) {
			printf("Problem at 2 with i=%d\n", i);
			exit(1);
		}
		if ((s_array[i]->s = (char *)malloc(10)) == NULL) {
			printf("Problem at 3 with i=%d\n", i);
			exit(1);
		}
	}
}

stanley@phoenix.com (John Stanley) (11/01/90)

bks@alfa.berkeley.edu (Brad Sherman) writes:
> We guess that this is because the compiler "runs out of registers."
> (Is that excessively anthropomorphic?)

   No, but "runs out of registers and spits out stinky assembly to get
back at the programmer who asked for too many" would be. 8-0
> 
> Now, which of the following lessons should be drawn from this event:
> 	1) Don't use "register."
> 	2) Don't use optimizers (or in this case be sure to disable optimizer).

   The TC manual tells me that using the 'register optimizer' is
dangerous. The compiler may be unable to detect certain usages and reuse
a register incorrectly. This may be a similar problem to what you have.

   I haven't had a chance to try your code with TC, but will and let you
know if I find anything interesting.



"Arinth is a beautiful planet." "Oh, have you been there?"
  "Yes, but not yet." The Doctor. (TB)

bks@alfa.berkeley.edu (Brad Sherman) (11/01/90)

In article <1990Oct31.014132.2400@agate.berkeley.edu> bks@alfa.berkeley.edu (Brad Sherman) writes:
>	if ((s_array = (STRING **)malloc(100 * sizeof(STRING))) == NULL) {
This line should read
>	if ((s_array = (STRING **)malloc(100 * sizeof(STRING *))) == NULL) {
                                                            ^^
This was a transcription problem and all other comments in the posting stand.
(The original program, in fact had nothing to do with STRING's.)

Thanks to bruce@seismo.gps.caltech.edu for pointing this out.
-------------------------
	Brad Sherman (bks@alfa.berkeley.edu)

gwyn@smoke.brl.mil (Doug Gwyn) (11/07/90)

In article <1990Oct31.014132.2400@agate.berkeley.edu> bks@alfa.berkeley.edu (Brad Sherman) writes:
>I have included a program below which is broken by the Microsoft 6.0
>compiler on MSDOS.   While this is no great surprise, ...

>	if ((s_array = (STRING **)malloc(100 * sizeof(STRING))) == NULL) {
------------------------------------------------------^^^^^^
Should be STRING *.

>		if ((s_array[i]->s = (char *)malloc(10)) == NULL) {
----------------------------------------------------^^
Should be 10 * sizeof(char), in order to get the right argument type.

>}
Should be preceded by return 0;.

>Now, which of the following lessons should be drawn from this event:
>	1) Don't use "register."
>	2) Don't use optimizers (or in this case be sure to disable optimizer).
>	3) Don't use "improved" Microsoft products.

	0) Make sure the code is supposed to work before looking elsewhere.
Assuming that fixing the code doesn't solve the problem, then
	4) Report the bug to the vendor and devise a work-around.

I can't recommend any of alternatives 1-3.