#2189 Problems initializing global variables

[Mochilote]

The bug happens in the latest version 3.3.0, although it happened in the previous versións (at least in 3.2.0).

The sample source code to produce the bug is as follows:

////////////////////////////////////////////////////////////////
#include <stdio.h>

int n1 = 666;
int a1[] = { 666, 777};

main()
{
    int n2 = 666;
    int a2[] = { 666, 777};

    printf("n1 %d, n2 %d\n\n\r", n1, n2);
    printf("a1 [%d, %d], a2 [%d, %d]\n\n\r", a1[0], a1[1],a2[0], a2[1]);

    while(1) {};
}
////////////////////////////////////////////////////////////////

Compiled with: sdcc -mz80 --code-loc 0x0138 --data-loc 0 --no-std-crt0 crt0_cpc.rel putchar_cpc.rel sdcc01.c

If we execute the binary in an emulator (Amstrad CPC) we get the following screen output:

http://i.imgur.com/4UnWcvr.png

BUG: global variables (n1 and a1) have value -1

The asm code generated is as follows:

;--------------------------------------------------------
; File Created by SDCC : free open source ANSI-C Compiler
; Version 3.3.0 #8604 (May 11 2013) (MINGW32)
; This file was generated Sun Jun 16 10:31:44 2013
;--------------------------------------------------------
    .module sdcc01
    .optsdcc -mz80

;--------------------------------------------------------
; Public variables in this module
;--------------------------------------------------------
    .globl _main
    .globl _printf
    .globl _a1
    .globl _n1
;--------------------------------------------------------
; special function registers
;--------------------------------------------------------
;--------------------------------------------------------
; ram data
;--------------------------------------------------------
    .area _DATA
;--------------------------------------------------------
; ram data
;--------------------------------------------------------
    .area _INITIALIZED
_n1::
    .ds 2
_a1::
    .ds 4
;--------------------------------------------------------
; absolute external ram data
;--------------------------------------------------------
    .area _DABS (ABS)
;--------------------------------------------------------
; global & static initialisations
;--------------------------------------------------------
    .area _HOME
    .area _GSINIT
    .area _GSFINAL
    .area _GSINIT
;--------------------------------------------------------
; Home
;--------------------------------------------------------
    .area _HOME
    .area _HOME
;--------------------------------------------------------
; code
;--------------------------------------------------------
    .area _CODE
;sdcc01.c:6: main()
;   ---------------------------------
; Function main
; ---------------------------------
_main_start::
_main:
    push    ix
    ld  ix,#0
    add ix,sp
    ld  hl,#-8
    add hl,sp
    ld  sp,hl
;sdcc01.c:9: int a2[] = { 666, 777};
    ld  hl,#0x0000
    add hl,sp
    ld  e,l
    ld  d,h
    ld  (hl),#0x9A
    inc hl
    ld  (hl),#0x02
    ld  c, e
    ld  b, d
    inc bc
    inc bc
    ld  l, c
    ld  h, b
    ld  (hl),#0x09
    inc hl
    ld  (hl),#0x03
;sdcc01.c:11: printf("n1 %d, n2 %d\n\n\r", n1, n2);
    push    bc
    push    de
    ld  hl,#0x029A
    push    hl
    ld  hl,(_n1)
    push    hl
    ld  hl,#__str_0
    push    hl
    call    _printf
    ld  hl,#0x0006
    add hl,sp
    ld  sp,hl
    pop de
    pop bc
;sdcc01.c:12: printf("a1 [%d, %d], a2 [%d, %d]\n\n\r", a1[0], a1[1],a2[0], a2[1]);
    ld  a,(bc)
    ld  -2 (ix),a
    inc bc
    ld  a,(bc)
    ld  -1 (ix),a
    ld  a,(de)
    ld  -4 (ix),a
    inc de
    ld  a,(de)
    ld  -3 (ix),a
    ld  bc, (#_a1 + 2)
    ld  de, (#_a1 + 0)
    ld  l,-2 (ix)
    ld  h,-1 (ix)
    push    hl
    ld  l,-4 (ix)
    ld  h,-3 (ix)
    push    hl
    push    bc
    push    de
    ld  hl,#__str_1
    push    hl
    call    _printf
    ld  hl,#0x000A
    add hl,sp
    ld  sp,hl
;sdcc01.c:14: while(1) {};
00102$:
    jr  00102$
_main_end::
__str_0:
    .ascii "n1 %d, n2 %d"
    .db 0x0A
    .db 0x0A
    .db 0x0D
    .db 0x00
__str_1:
    .ascii "a1 [%d, %d], a2 [%d, %d]"
    .db 0x0A
    .db 0x0A
    .db 0x0D
    .db 0x00
    .area _CODE
    .area _INITIALIZER
__xinit__n1:
    .dw #0x029A
__xinit__a1:
    .dw #0x029A
    .dw #0x0309
    .area _CABS (ABS)

[Mochilote]

If we use "const" as follows:

const int n1 = 666;
const int a1[] = { 666, 777};

the bug disappears

[Philipp Klaus Krause]

The generated asm looks correct to me. The crt0.rel has to copy _INTIALIZER to _INITALIZED. This initialization method is much more efficient that the old one (which did not require support from crt0.rel). Your crt0.o does not do this copying. You might want to have a look at device/lib/z80/crt0.s in the sdcc source for an example of how to do the copying in crt0.rel.
For const variables sdcc already used an efficient initilization before, so those are not affected by lack of initialzation support in crt0.rel.

Philipp

P.S.: I'm wondering if it would be better to have the copying code placed by sdcc in the .rel that contains main(). That's how the stm8 port does it, and it does not place the burden of updating crt0.rel with changes in the initialization mehcanism on the user.

[Maarten Brock]

I would recommend to use a separate crtinit.asm (like crtxinit.asm for mcs51). This enables the option to link this from the library only when there are actual initializations to be done (not yet implemented for mcs51 either).

[Mochilote]

Thank you very much for the clarification, it seemed to me that this problem rarely escape the stress test.

I do not get crt0.s to compile with the new changes, even if I try to compile the original (3.3.0) file device/lib/z80/crt0.s, I get the same error:

sdasz80 -o crt0.s
crt0.s:91: Error: <u> undefined symbol encountered during assembly
crt0.s:95: Error: <u> undefined symbol encountered during assembly
crt0.s:96: Error: <u> undefined symbol encountered during assembly
removing crt0.rel

Specifically, these lines correspond to:

ld  bc, #l__INITIALIZER

and:

ld de, #s__INITIALIZED
ld hl, #s__INITIALIZER

greetings and thanks in advance.

[Philipp Klaus Krause]

l__INITIALIZER, etc are global symbols, so you'll need to add .globl directives for them or assemble using the -g option (e.g. sdasz80 -og crt0.s).

Philipp

[Mochilote]

Thank you very much, now it works perfectly

[Philipp Klaus Krause]

Maarten,

another reason why I thought about doing it in the main file was that this would allow to generate different code depending on --opt-code-size and --opt-code speed. Could we do that with the separate crtinit?

Philipp

[Maarten Brock]

I think so. Just write two different asm files for it.

The way I imagine it is to let any object that puts something in INITIALIZER/XINIT also reference a label that is defined in crtinit.s/crtxinit.asm. The linker should then link that object in. If there are no initializers then no code for it needs to be linked.

To prevent the crtxxxx code to end up anywhere in memory or requiring many jumps it is put in one of the GSINITx .areas which the sdcc and the linker try to keep together.

Maarten