Marc Vandenhende

Codewarrior 7.2.1 compiler issue identified

Discussion created by Marc Vandenhende on Oct 23, 2010
Latest reply on Nov 5, 2010 by Marc Vandenhende

In the past I have reported about an issue with a modified Coldfire Lite when compiled with Codewarrior 7.2.x. It took me a while, but I have now found the cause of the issue (and the proof that there actually is one!).

 

There appears to be a change in order of assembler instructions that cause the function of an IF statement to not work correctly. This change in order happens at optimisation levels 2 or higher.

This is the piece of code in which it happens. This piece of code is part of the file tcpout.c in Coldfire Lite 3.2.

   /*    * If we're sending a SYN, check the IP address of the interface    * that we will (likely) use to send the IP datagram -- if it's    * changed from what is in the template (as it might if this is    * a retransmission, and the original SYN caused PPP to start    * bringing the interface up, and PPP has got a new IP address    * via IPCP), update the template and the inpcb with the new     * address.    */   if (flags & TH_SYN)   {      ip_addr src=ip_mymach(pip->ip_src);      if (src != pip->ip_src)      {         pip->ip_src=src;         tp->t_template->ti_i.ip_src=src;         so->lhost = src;      }   }

 

 

When compiled/disassembled at optimisation level 1, the result reads...

 

 

;  410:    ptcp->th_ack = (tp->rcv_nxt);
;  411:   
;  412:       /*
;  413:        * If we're sending a SYN, check the IP address of the interface
;  414:        * that we will (likely) use to send the IP datagram -- if it's
;  415:        * changed from what is in the template (as it might if this is
;  416:        * a retransmission, and the original SYN caused PPP to start
;  417:        * bringing the interface up, and PPP has got a new IP address
;  418:        * via IPCP), update the template and the inpcb with the new
;  419:        * address.
;  420:        */
;
0x00000262  0x256B00500008           move.l   80(a3),8(a2)
;
;  421:    if (flags & TH_SYN)
;  422:       {
;
0x00000268  0x08050001               btst     #1,d5
0x0000026C  0x6728                   beq.s    *+42                  ; 0x00000296
;
;  423:       ip_addr src=ip_mymach(pip->ip_src);
;  424:      
;
0x0000026E  0x207900000000           movea.l  _m_netp,a0
0x00000274  0x20280028               move.l   40(a0),d0
;
;  425:       if (src != pip->ip_src)
;  426:             {
;
0x00000278  0x206F0014               movea.l  20(a7),a0
0x0000027C  0xB0A8000C               cmp.l    12(a0),d0
0x00000280  0x6714                   beq.s    *+22                  ; 0x00000296
;
;  427:          pip->ip_src=src;
;
0x00000282  0x206F0014               movea.l  20(a7),a0
0x00000286  0x2140000C               move.l   d0,12(a0)
;
;  428:          tp->t_template->ti_i.ip_src=src;
;
0x0000028A  0x206B002C               movea.l  44(a3),a0
0x0000028E  0x2140000C               move.l   d0,12(a0)
;
;  429:          so->lhost = src;
;  430:                }
;  431:             }

 ...which is fine. The BEQ instruction follows directly after the BTST instruction.
But when the optimisation level is bumped one level higher, the disassembly reads...

 

 

0x0000028A  0x7002                   moveq    #2,d0
0x0000028C  0xC1AF001C               and.l    d0,28(a7)
;
;  410:    ptcp->th_ack = (tp->rcv_nxt);
;  411:   
;  412:       /*
;  413:        * If we're sending a SYN, check the IP address of the interface
;  414:        * that we will (likely) use to send the IP datagram -- if it's
;  415:        * changed from what is in the template (as it might if this is
;  416:        * a retransmission, and the original SYN caused PPP to start
;  417:        * bringing the interface up, and PPP has got a new IP address
;  418:        * via IPCP), update the template and the inpcb with the new
;  419:        * address.
;  420:        */
;
0x00000290  0x2D6B00500008           move.l   80(a3),8(a6)
;
;  421:    if (flags & TH_SYN)
;  422:       {
;
0x00000296  0x6728                   beq.s    *+42                  ; 0x000002c0
;
;  423:       ip_addr src=ip_mymach(pip->ip_src);
;  424:      
;
0x00000298  0x207900000000           movea.l  _m_netp,a0
0x0000029E  0x20280028               move.l   40(a0),d0
;
;  425:       if (src != pip->ip_src)
;  426:             {
;
0x000002A2  0x206F002C               movea.l  44(a7),a0
0x000002A6  0xB0A8000C               cmp.l    12(a0),d0
0x000002AA  0x6714                   beq.s    *+22                  ; 0x000002c0
;
;  427:          pip->ip_src=src;
;
0x000002AC  0x206F002C               movea.l  44(a7),a0
0x000002B0  0x2140000C               move.l   d0,12(a0)
;
;  428:          tp->t_template->ti_i.ip_src=src;
;
0x000002B4  0x206B002C               movea.l  44(a3),a0
0x000002B8  0x2140000C               move.l   d0,12(a0)
;
;  429:          so->lhost = src;
;  430:                }
;  431:             }

 

Here the comparison is done with a MOVEQ and AND.L instruction, but the MOVE.L instruction gets in the way, influencing the Z flag of the status register and leading to incorrect branching.

 

 

In the end this causes a wrong TCP checksum to be calculated as the instructions inside the IF statement are not executed, hence a TCP communication failure.

Outcomes