Re: Memory corruption (?) I don't understand

[Senthil Kumar]

This part

>     a340:   68 01           movw    r12, r16
>     a342:   ff e4           ldi r31, 0x4F   ; 79
>     a344:   cf 1a           sub r12, r31
>     a346:   fe ef           ldi r31, 0xFE   ; 254
>     a348:   df 0a           sbc r13, r31
>     a34a:   e3 50           subi    r30, 0x03   ; 3
>     a34c:   ff 4f           sbci    r31, 0xFF   ; 255
>     a34e:   01 90           ld  r0, Z+
>     a350:   f0 81           ld  r31, Z
>     a352:   e0 2d           mov r30, r0

looks suspicious - the Z register is loaded with 0xFE4F and then just
subtracted with 0xFF03 to obtain self->handler, whereas self->app is
obtained by offsetting R13:R12.

Explicitly checking if self->handler is NULL also has the sub, but I
presume R31:R30 are loaded with the address of self beforehand?

>     a33c:   e3 50           subi    r30, 0x03   ; 3
>     a33e:   ff 4f           sbci    r31, 0xFF   ; 255
>     a340:   01 90           ld  r0, Z+
>     a342:   f0 81           ld  r31, Z
>     a344:   e0 2d           mov r30, r0

Regards
Senthil

On Wed, Jun 23, 2021 at 2:07 PM BERTRAND Joël <joel.bertrand@systella.fr> wrote:
>
> Comparaison between good.S and bad.S.
>
> Good.elf is compiled with :
>
>         app_handler(self->app, LDL_MAC_DEV_NONCE_UPDATED, &arg);
>
> instead of bad.elf that is compiled with :
>
>         self->handler(self->app, LDL_MAC_DEV_NONCE_UPDATED, &arg);
>
> in enum ldl_mac_status LDL_MAC_otaa(struct ldl_mac *self)
>
> In both .S files:
> - app_handler is written @000022f2.
> - argmac.handler = app_handler gives following code :
>
>     2886:   89 e7           ldi r24, 0x79   ; 121
>     2888:   91 e1           ldi r25, 0x11   ; 17
>     288a:   9c 87           std Y+12, r25   ; 0x0c
>     288c:   8b 87           std Y+11, r24   ; 0x0b
>
> In good.S, app_handler(self->app, LDL_MAC_DEV_NONCE_UPDATED, &arg) is
> built as :
>     a340:   68 01           movw    r12, r16
>     a342:   ff e4           ldi r31, 0x4F   ; 79
>     a344:   cf 1a           sub r12, r31
>     a346:   fe ef           ldi r31, 0xFE   ; 254
>     a348:   df 0a           sbc r13, r31
>     a34a:   ae 01           movw    r20, r28
>     a34c:   4f 5a           subi    r20, 0xAF   ; 175
>     a34e:   5f 4f           sbci    r21, 0xFF   ; 255
> // 0x05 0x00 = LDL_MAC_DEV_NONCE_UPDATED
>     a350:   65 e0           ldi r22, 0x05   ; 5
>     a352:   70 e0           ldi r23, 0x00   ; 0
>     a354:   d6 01           movw    r26, r12
>     a356:   8d 91           ld  r24, X+
>     a358:   9c 91           ld  r25, X
>     a35a:   0e 94 79 11     call    0x22f2  ; 0x22f2 <app_handler>
>
> In bad.S, self->handler(self->app, LDL_MAC_DEV_NONCE_UPDATED, &arg) gives :
>     a340:   68 01           movw    r12, r16
>     a342:   ff e4           ldi r31, 0x4F   ; 79
>     a344:   cf 1a           sub r12, r31
>     a346:   fe ef           ldi r31, 0xFE   ; 254
>     a348:   df 0a           sbc r13, r31
>     a34a:   e3 50           subi    r30, 0x03   ; 3
>     a34c:   ff 4f           sbci    r31, 0xFF   ; 255
>     a34e:   01 90           ld  r0, Z+
>     a350:   f0 81           ld  r31, Z
>     a352:   e0 2d           mov r30, r0
>     a354:   ae 01           movw    r20, r28
>     a356:   4f 5a           subi    r20, 0xAF   ; 175
>     a358:   5f 4f           sbci    r21, 0xFF   ; 255
>     a35a:   65 e0           ldi r22, 0x05   ; 5
>     a35c:   70 e0           ldi r23, 0x00   ; 0
>     a35e:   d6 01           movw    r26, r12
>     a360:   8d 91           ld  r24, X+
>     a362:   9c 91           ld  r25, X
>     a364:   09 95           icall
>
>         I have rebuilt a second firmware that works as expected with:
>
> if (self->handler == NULL) for(;;);
>             self->handler(self->app, LDL_MAC_DEV_NONCE_UPDATED, &arg);
>
>         Asmb output contains :
>
> if (self->handler == NULL) for(;;);
>     a33c:   e3 50           subi    r30, 0x03   ; 3
>     a33e:   ff 4f           sbci    r31, 0xFF   ; 255
>     a340:   01 90           ld  r0, Z+
>     a342:   f0 81           ld  r31, Z
>     a344:   e0 2d           mov r30, r0
>     a346:   30 97           sbiw    r30, 0x00   ; 0
>     a348:   09 f4           brne    .+2         ; 0xa34c
>     a34a:   4a c0           rjmp    .+148       ; 0xa3e0
>             self->handler(self->app, LDL_MAC_DEV_NONCE_UPDATED, &arg);
>     a34c:   68 01           movw    r12, r16
>     a34e:   2f e4           ldi r18, 0x4F   ; 79
>     a350:   c2 1a           sub r12, r18
>     a352:   2e ef           ldi r18, 0xFE   ; 254
>     a354:   d2 0a           sbc r13, r18
>     a356:   ae 01           movw    r20, r28
>     a358:   4f 5a           subi    r20, 0xAF   ; 175
>     a35a:   5f 4f           sbci    r21, 0xFF   ; 255
>     a35c:   65 e0           ldi r22, 0x05   ; 5
>     a35e:   70 e0           ldi r23, 0x00   ; 0
>     a360:   d6 01           movw    r26, r12
>     a362:   8d 91           ld  r24, X+
>     a364:   9c 91           ld  r25, X
>     a366:   09 95           icall
>
>         I'm not a AVR ASMB specialist, but I don't understand why
> self->handler(self->app, LDL_MAC_DEV_NONCE_UPDATED, &arg) produces a
> different code (with of course the same compilation options) when it was
> preceded by if (self->handler == NULL) for(;;);
>
>         Best regards,
>
>         JKB
>