A strange error which destroys array content

[simonsan]

Hi!

I use an 89S52 I/O port to read a keypad and store the inputs in a static key buffer array.

My code is something like this:

if(R1 & R2 & R3 & R4)
do_something();
else do_other_thing();

Where R1~R4 are I/O pins.

I found whenever a key press caused R4 to become low,  a particular key buffer array element's content would be destroyed. The stack started at 0x62, so it was not caused by stack overflow.

I complied it with version 2.6 and 2.7, same error occurred.
If I use logical AND(&&) instead of bitwise AND(&), then everything seems alright.
I noted bitwise AND(&) employed a local variable to handle the intermediate AND result, while logical AND(&&) used none.

I also compiled the program with KEIL compiler, and found no problem at all.

Is this a bug of the SDCC?

[Maarten Brock]

How are R1~R4 declared?
Can you show the generated asm code?

Maarten

[simonsan]

Hi! Maarten

My declaration is as follows:

sbit R1 = 0x90; // R1~R4 are rows, and C1~C3 are columns of a 4x3 keypad
sbit R2 = 0x92;
sbit R3 = 0x93;
sbit R4 = 0x94;
sbit C1 = 0x95;
sbit C2 = 0x96;
sbit C3 = 0x97;

The C code is as follows:

    P1 = 0xff;            /* set rows and columns high */
    C1 = C2 = C3 = 0;        /* all columns low */
    if (R1 & R2 & R3 & R4)
     {
        if (kp_flag == 1)    // no key press, clear flag
         {                      // and return
            kp_flag = 0;
             delay_50ms();
         }
        return 0xff;
     }
    else if (kp_flag == 1)
             return 0xff;
        ... key processing here

The ASM generated is as follows:

   02AB                    1100 _read_key:
                           1101 ;    wop.c:546: byte row_status = 0, key_code = 0;
   02AB 7A 00              1102     mov    r2,#0x00
                           1103 ;    wop.c:549: feed_dog();  /* feed watch dog */
   02AD C0 02              1104     push    ar2
   02AF 12 10 7B           1105     lcall    _feed_dog
   02B2 D0 02              1106     pop    ar2
                           1107 ;    wop.c:550: P1 = 0xff;    /* set rows and columns high */
   02B4 75 90 FF           1108     mov    _P1,#0xFF
                           1109 ;    wop.c:551: C1 = C2 = C3 = 0;    /* all columns low */
   02B7 C2 97              1110     clr    _C3
   02B9 C2 96              1111     clr    _C2
   02BB C2 95              1112     clr    _C1
                           1113 ;    wop.c:552: if (R1 & R2 & R3 & R4)
   02BD A2 90              1114     mov    c,_R1
   02BF 82 92              1115     anl    c,_R2
   02C1 92 62              1116     mov    _read_key_sloc0_1_0,c
   02C3 A2 93              1117     mov    c,_R3
   02C5 82 62              1118     anl    c,_read_key_sloc0_1_0
   02C7 92 62              1119     mov    _read_key_sloc0_1_0,c
   02C9 A2 62              1120     mov    c,_read_key_sloc0_1_0
   02CB 82 94              1121     anl    c,_R4
   02CD 50 0F              1122     jnc    00106$
                           1123 ;    wop.c:554: if (kp_flag == 1)    // no key press, clear flag
   02CF 74 01              1124     mov    a,#0x01
   02D1 B5 29 06           1125     cjne    a,_kp_flag,00102$
                           1126 ;    wop.c:556: kp_flag = 0;
   02D4 75 29 00           1127     mov    _kp_flag,#0x00
                           1128 ;    wop.c:557: delay_50ms();
   02D7 12 03 85           1129     lcall    _delay_50ms
   02DA                    1130 00102$:
                           1131 ;    wop.c:559: return 0xff;
   02DA 75 82 FF           1132     mov    dpl,#0xFF
   02DD 22                 1133     ret
   02DE                    1134 00106$:
                           1135 ;    wop.c:561: else if (kp_flag == 1)
   02DE 74 01              1136     mov    a,#0x01
   02E0 B5 29 04           1137     cjne    a,_kp_flag,00107$
                           1138 ;    wop.c:562: return 0xff;
   02E3 75 82 FF           1139     mov    dpl,#0xFF
   02E6 22                 1140     ret
   02E7                    1141 00107$:
                                        ... key processing here

[Raphael Neider]

I cannot read 8051 assembler, but probably you want

sbit __at(0x90) R1;

instead of (Keil'ish?)

sbit R1 = 0x90; // R1~R4 are rows, and C1~C3 are columns of a 4x3 keypad

The latter would create a bit variable somewhere and initialize it with 0x90... See compiler.h for a vendor-independant way do declare bits.

HTH,
Raphael

[simonsan]

Thank you,  tecodev.

The declaration on my last post is incorrect.

The actual declaration is as follows:
sbit at 0x90 R1;
sbit at 0x92 R2;
sbit at 0x93 R3;
sbit at 0x94 R4;
sbit at 0x95 C1;
sbit at 0x96 C2;
sbit at 0x97 C3;

[wek]

I think this fully qualifies as an error. The _read_key_sloc0_1_0
automatic variable is created apparently as a sfr bit and the assembler/linker allocates it as standard data.

There are two errors here: one is the compiler creating a temporary variable as sbit, two is the assembler/linker accepting it as data without throwing an error.

Of course I am too stupid to tell how to proceed, that's upon the gurus... :-)

JW

[Maarten Brock]

Hmm, this is weird indeed. The sloc should have been a __bit in BSEG. And RSEG should normally only contain EQU or = definitions (for either sfr or sbit) in which case it doesn't really matter what memory space is chosen.

How to proceed should now be obvious: file a bug report in the bug tracker.

[wek]

As I said already, I would see it as 2 problems:
1. the compiler should generate the sloc properly
2. the assembler should throw an error when non-EQU/= definition appears in RSEG.

Testing, I came to 3. too:
3. the compiler should not allow sbit without __at (there is no reason for the linker to allocate it).

A crappy test "program" below...

JW
-----
#include<8052.h>

sbit at 0x90 R1;
sbit at 0x92 R2;
sbit at 0x93 R3;
sbit at 0x94 R4;
sbit at 0x95 C1;
sbit at 0x96 C2;
sbit at 0x97 C3;

unsigned char kp_flag;

unsigned char key_scan(void){
sbit tmp;
volatile bit tmp2;

        tmp = C1;
        tmp2 = C2;
        C3 = tmp2;

    P1 = 0xff;            /* set rows and columns high */
    C1 = C2 = C3 = 0;        /* all columns low */
    if (R1 & R2 & R3 & R4)
     {
        if (kp_flag == 1)    // no key press, clear flag
         {                      // and return
            kp_flag = 0;
//             delay_50ms();
         }
        return 0xff;
     }
    else if (kp_flag == 1)
             return 0xff;
        return 0xff;
}

void main(void) {
  key_scan();
}

[Maarten Brock]

Re 3. I think it is already on the Requested Features list not to accept sfr and sbit definitions without an absolute address (unless maybe extern).

[simonsan]

Wow, the gurus have identified the bug in no time!
Thank you very much!

I will file a bug report accordingly.

[Maarten Brock]

Re 1. I've fixed this in SDCC 2.7.3 #4886.

[simonsan]

maartenbrock, you are amazing!
Many thanks for your great works!