• Welcome to Andy's Workshop Forums. Please login or sign up.
 

Strange behaviour in the net_UDP_Receive_Async example

Started by Ozflip, November 08, 2014, 06:07:30 pm

Previous topic - Next topic

Ozflip

Sorry, but I'm being annoying again...

I have been messing with the UDP examples, and I get some weird behaviour with the UDP_Receive_Async example. \

If I set the size of the datagram data array to anything greater than 12 the program "blows up".

What happens is that  _datagramArrived is being set by something - I think it is a received DHCP packet. However, it is not being set by the onReceived event handler: if I break on  event.handled=true; then it never breaks there. Something is stomping on _datagramArrived. If I break in the loop where the datagram is sent to the UART then _datagramDataSize is 8192

The changes were:

1. Change the datagram size declaration:  volatile uint8_t _datagramData[25];
2. Change the memcopy size calc in onReceive: _datagramDataSize=_datagramDataSize<=25 ? _datagramDataSize : 25;

I can get the same behaviour by leaving the declaration of the _datagramData array at 10 and adding a following volatile array of anything bigger than 2 bytes: for example, changing all the volatiles to that below will also blow up the program:


    volatile uint8_t _datagramData[10];
    volatile uint8_t somerubbish[3];


Is there some limit to the amount of volatile storage that can be declared? The asm file stack is hex 400 bytes long, so the stack isn't overflowing.

Thanks,

Phil.


EDIT:

Some further debugging .. the program was being killed because when _datagramArrived is being stomped on, the uart output loop was trying to output 8192 bytes (the size of the _datagramDataSize volatile at that time).

If I do the following:

1. Change the datagram declaration to volatile uint8_t _datagramData[50];
2. Change the uart print loop as follows:

        if(_datagramArrived) {

          // a datagram has been received, print out the first 10 bytes

          // NEW: Check how much data we are attempting to output
          if (_datagramDataSize > 50 { _datagramDataSize = 50; }

          for(uint16_t i=0;i<_datagramDataSize;i++)
            (*_outputStream) << (uint16_t) _datagramData[i] << " ";

          (*_outputStream) << "\r\n";

          // ready for another

          _datagramArrived=false;
        }


3. Change the onReceive to _datagramDataSize=_datagramDataSize<=50 ? _datagramDataSize : 50;

The I get the following output:

143 78 0 8 120 9 0 32 152 9 0 32 120 9 0 32 136 9 0 32 0 36 25 0 255 36 3 10 22 36 23 0 0 0 0 0 1 0 0 0 136 1 0 0 0 0 0 0 248 255
255 36 3 10 22 36 23 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 7 9 11 7 222 12 17 3 254

The first line is rubbish and happened *without* onReceive passing the if(event.udpDatagram.udp_destinationPort!=NetUtil::htons(12345)) test. The second line is a real packet that I am receiving.

Something is still stomping on _datagramArrived whenever the array is greater than 12. That makes the total volatile storage 15 bytes, which is a suspiciously round binary number.


Andy Brown

Hi Phil,

Thanks for reporting this bug, it's fixed now on master. It was simply a case of the example code not pre-setting _datagramArrived to false in the run() method so it was undefined what it contained at runtime. When you changed the memory layout by increasing the buffer size you must have changed the storage location for _datagramArrived to somewhere that didn't contain zero.

There's no such concept as 'volatile storage'. All non-const data is in SRAM (const data is in flash). The volatile declaration is used to tell the compiler that the value may be changed by another execution context so it has to disable certain optimisations.
It's worse than that, it's physics Jim!

Ozflip

Thanks Andy - noob mistake on my part. 25 years since I coded in ordinary C! Too much reliance on managed languages the last few years and their nice (and inefficient) tendencies to do things like initialise class members for you.