#2471 Incorrect Initiaization of Global Array Containing Pointers

[alvin]

sdcc -v
3.5.5. #9501 (MINGW64)

First I will give a test program that produces correct initialization:

typedef struct { unsigned char file[13];} FILE;

extern int printf(char *format,...);
extern int fgetc(FILE *stream);

extern FILE *SOCK0;
extern FILE *SOCK1;
extern FILE *SOCK2;

FILE** socket[3]={&SOCK0,&SOCK1,&SOCK2};

void main()
{
   unsigned char i;

   for (i=0;i<=2;i++)
   {
      printf("SOCK%d says:%#02x\n",i,fgetc(*socket[i]));
   }
}

What I'm looking at is the static initialization of array socket[]:

__xinit__socket:
    .dw _SOCK0
    .dw _SOCK1
    .dw _SOCK2

This is correct - the address of the FILE ptrs SOCK0, SOCK1, SOCK2 corresponding to asm labels -SOCK0, -SOCK1, -SOCK2 are placed into the structure. (I am using a minus sign for underscore here because the underscore messes with formatting).

The compiler does not correctly intialize a slightly different structure. This may not be legal C either, and I will explain that below, but if that is the case the compiler should be producing an error rather than silently compiling incorrect code.

typedef struct { unsigned char file[13];} FILE;

extern int printf(char *format,...);
extern int fgetc(FILE *stream);

extern FILE *SOCK0;
extern FILE *SOCK1;
extern FILE *SOCK2;

FILE* socket[3]={SOCK0,SOCK1,SOCK2};

void main()
{
   unsigned char i;

   for (i=0;i<=2;i++)
   {
      printf("SOCK%d says:%#02x\n",i,fgetc(socket[i]));
   }
}

The difference is the socket[] array now holds FILE ptrs rather than pointers to FILE ptr. The reason why i think this may not be legal C is that the compiler is being expected to find the value of a pointer in another translation unit. This is possible to do at runtime so it is possible if the compiler copies the value of those SOCKn FILE ptrs into the array by executing code placed in gs_init. For this to work this would require sdcc's bss and data sections to be initialized prior to any other code inserted into gs_init. In z88dk this is guaranteed but I'm not sure if that's the case with sdcc's default z80 crt.

Anyway, legal or not, the structure intialization is the same as the first program:

__xinit__socket:
    .dw _SOCK0
    .dw _SOCK1
    .dw _SOCK2

That's not right. At runtime, the socket[] array could be intialized by adding the following code into gs_init:

ld hl,(_SOCK0)
ld (_socket),hl
ld hl,(_SOCK1)
ld (_socket + 2),hl
ld hl,(_SOCK2)
ld (_socket + 4),hl

Or by using a more efficient "ldir" but I know the compiler isn't able to do that yet as an array of static strings is initialized using this method currently.

[alvin]

compile line used to produce output assembly:

sdcc -mz80 -S test.c

[Ben Shi]

An error should rise when parsing the AST

[Maarten Brock]

Whether this initialization is allowed depends on the storage class of the socket array, I think. If socket is an auto (local, on stack) it is ok. But if socket is static or global it is invalid. It should never generate code in gs_init or xinit.

[Ben Shi]

Fixed in reversion [r9504]. Now an error will rise with the above wrong code.

[alvin]

Hi Ben, the fix works on the test cases thanks.

I took a hint from the error message "Initializer element is not a constant" and tried again a contrived example that made the pointers const:

sdcc -v
9504 MSVC

typedef struct { unsigned char file[13];} FILE;

extern int printf(char *format,...);
extern int fgetc(FILE *stream);

extern FILE * const SOCK0;
extern FILE * const SOCK1;
extern FILE * const SOCK2;

FILE* socket[3]={SOCK0,SOCK1,SOCK2};

void main()
{
   unsigned char i;

   for (i=0;i<=2;i++)
   {
      printf("SOCK%d says:%#02x\n",i,fgetc(socket[i]));
   }
}

sdcc -mz80 -S test.c

_socket:
    DEFW _SOCK0
    DEFW _SOCK1
    DEFW _SOCK2

and as before it is using the address of each FILE ptr rather than the value of the pointers to initialize the array.

[Maarten Brock]

Does gcc accept that code when it doesn't know at compile what the value of the constant is?

[Ben Shi]

I am afraid we can not match alvin's expectation, neither gcc nor clang accept the following code

typedef struct { unsigned char file[13];} FILE;

extern FILE * const SOCK0;
extern FILE * const SOCK1;
extern FILE * const SOCK2;

FILE* socket[3]={SOCK0,SOCK1,SOCK2};

gcc's error

benshi@debian:~$ gcc a.c -c
a.c:8:1: error: initializer element is not constant
 FILE* socket[3]={SOCK0,SOCK1,SOCK2};
 ^
a.c:8:1: error: (near initialization for ‘socket[0]’)
a.c:8:1: error: initializer element is not constant
a.c:8:1: error: (near initialization for ‘socket[1]’)
a.c:8:1: error: initializer element is not constant
a.c:8:1: error: (near initialization for ‘socket[2]’)

clang's error

benshi$ clang a.c -c
a.c:8:18: error: initializer element is not a compile-time constant
FILE* socket[3]={SOCK0,SOCK1,SOCK2};
                 ^~~~~
1 error generated.

[Peter Dons Tychsen]

I think Alvins point is that the code still does not emit the correct error code if the "const" keyword is used. So the fix does not check if its a global var, but simply that it is not const. So the bug is partly still there.

[Ben Shi]

I thought clang's error message is more clear, and will change sdcc's according to it.

However the above C code should not be accepted, and the same error message should be given for both const and nonconst global variables.

[alvin]

Yes as Peter says, I'm not saying the code should be accepted -- I'm saying that sdcc silently compiles incorrect code when it should be emitting an error as it now does for the original test case.

[Ben Shi]

OK. Alvin.

Please check if reversion [r9505] has been better to cop with your right/wrong code.

[Peter Dons Tychsen]

Hi,

I do not see how changing the error code fixes the problem.
Here it still fails with #9505:

It is still accepting the version with global const variables and gcc is not.

To fix this sdcc needs to check if the reference variable is "const" or global.

code:

typedef struct { unsigned char file[13];} FILE;

extern FILE * const SOCK0;
extern FILE * const SOCK1;
extern FILE * const SOCK2;

FILE* socket[3]={SOCK0,SOCK1,SOCK2};

gcc:

test.c:7:1: error: initializer element is not constant
 FILE* socket[3]={SOCK0,SOCK1,SOCK2};
 ^
test.c:7:1: error: (near initialization for 'socket[0]')
test.c:7:1: error: initializer element is not constant
test.c:7:1: error: (near initialization for 'socket[1]')
test.c:7:1: error: initializer element is not constant
test.c:7:1: error: (near initialization for 'socket[2]')

sdcc:

<no error>

Thanks,

/pedro

[Ben Shi]

Please try revision [r9506]. All cases should get error messages.

[alvin]

Thanks Ben - everything works as expected now.

[Maarten Brock]

Oops, sorry for reopening.