#1447 function parameters overlaid

closed-fixed
hc08 port (43)
5
2013-05-25
2008-04-02
No

I've noticed garbage when trying to Tx on RS232 some string which was copied from one buffer to another using my own function. It looks like this:

// main.c

void CopyString(char acSource[], char acDestination[])
{
unsigned char ucCurrentPosition;

for (ucCurrentPosition = 0; ; ucCurrentPosition++)
{
acDestination[ucCurrentPosition]=acSource[ucCurrentPosition];
if (0==acSource[ucCurrentPosition])
break;
}
}

void main(void){
char text1[] = "spam";
char text2[16];
CopyString(text1, text2);
}

// end of main.c

It compiles to code like:

106 ;------------------------------------------------------------
107 ;Allocation info for local variables in function 'CopyString'
108 ;------------------------------------------------------------
109 ;acDestination Allocated with name '_CopyString_acDestination_1_1'
110 ;acSource Allocated with name '_CopyString_acSource_1_1'
111 ;ucCurrentPosition Allocated to registers
112 ;sloc0 Allocated with name '_CopyString_sloc0_1_0'
113 ;sloc1 Allocated with name '_CopyString_sloc1_1_0'
114 ;sloc2 Allocated with name '_CopyString_sloc2_1_0'
115 ;------------------------------------------------------------
116 ;main.c:1: void CopyString(char acSource[], char acDestination[])
117 ; -----------------------------------------
118 ; function CopyString
119 ; -----------------------------------------
DC26 120 _CopyString:
DC26 C7 00 63 121 sta (_CopyString_acSource_1_1 + 1)
DC29 CF 00 62 122 stx _CopyString_acSource_1_1
DC2C C7 00 61 123 sta (_CopyString_acDestination_1_1 + 1)
DC2F CF 00 60 124 stx _CopyString_acDestination_1_1
125 ;main.c:5: for (ucCurrentPosition = 0; ; ucCurrentPosition++)
DC32 3F 60 126 clr *_CopyString_sloc0_1_0
DC34 127 00104$:
128 ;main.c:7: acDestination[ucCurrentPosition]=acSource[ucCurrentPosition];
DC34 C6 00 61 129 lda (_CopyString_acDestination_1_1 + 1)
DC37 BB 60 130 add *_CopyString_sloc0_1_0
DC39 B7 62 131 sta *(_CopyString_sloc1_1_0 + 1)
DC3B C6 00 60 132 lda _CopyString_acDestination_1_1
DC3E A9 00 133 adc #0x00
DC40 B7 61 134 sta *_CopyString_sloc1_1_0
DC42 C6 00 63 135 lda (_CopyString_acSource_1_1 + 1)
DC45 BB 60 136 add *_CopyString_sloc0_1_0
DC47 B7 64 137 sta *(_CopyString_sloc2_1_0 + 1)
DC49 C6 00 62 138 lda _CopyString_acSource_1_1
DC4C A9 00 139 adc #0x00
DC4E B7 63 140 sta *_CopyString_sloc2_1_0
DC50 55 63 141 ldhx *_CopyString_sloc2_1_0
DC52 F6 142 lda ,x
DC53 55 61 143 ldhx *_CopyString_sloc1_1_0
DC55 F7 144 sta ,x
145 ;main.c:8: if (0==acSource[ucCurrentPosition])
DC56 C6 00 63 146 lda (_CopyString_acSource_1_1 + 1)
DC59 BB 60 147 add *_CopyString_sloc0_1_0
DC5B B7 64 148 sta *(_CopyString_sloc2_1_0 + 1)
DC5D C6 00 62 149 lda _CopyString_acSource_1_1
DC60 A9 00 150 adc #0x00
DC62 B7 63 151 sta *_CopyString_sloc2_1_0
DC64 55 63 152 ldhx *_CopyString_sloc2_1_0
DC66 F6 153 lda ,x
DC67 A1 00 154 cmp #0x00
DC69 27 04 155 beq 00107$
DC6B 156 00112$:
157 ;main.c:5: for (ucCurrentPosition = 0; ; ucCurrentPosition++)
DC6B 3C 60 158 inc *_CopyString_sloc0_1_0
DC6D 20 C5 159 bra 00104$
DC6F 160 00107$:
DC6F 81 161 rts

Look at the lines 121-124. As far as I understand assembly, the code should write the parameters to allocated space in memory. However it translates to twice reading from A and X - thus resulting in acDestination being the same as asSource.

The function call is symmetric: first it writes the first operands to A and X, then overwrites them with the second.

Analogous things occur when declaring the function as reeantrant or compiling with --stack-auto.

The command I use to compile is:
sdcc.exe -mhc08 --stack-loc 0x015F --code-loc 0xDC00 --data-loc 0x0060 -o main.S19 main.c

The sdcc version is:
SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.8.0 #5117 (Mar 23 2008) (MINGW32)

Discussion

  • Jan Wicijowski

    Jan Wicijowski - 2008-04-07

    Logged In: YES
    user_id=2052961
    Originator: YES

    After a few experiments I guess, why such a bug could have exist unnoticed:

    void CopyString(char *acSource, char *acDestination);
    and
    void CopyString(char acSource[], char acDestination[]);

    are treated differently.

     
  • Maarten Brock

    Maarten Brock - 2011-05-14

    Fix ed in SDCC 3.0.2 #6517.

     
  • Maarten Brock

    Maarten Brock - 2011-05-14
    • milestone: --> fixed
    • assigned_to: nobody --> maartenbrock
    • status: open --> closed-fixed
     

Log in to post a comment.