1

I was using a TCP library that has an incoming data handler with the following signature:

static void handleData(void *arg, AsyncClient *client, void *data, size_t len)

When I tried to cast the data like the following the access the field values of the structure, the board crashed.

MyStructure* m = (MyStructure*)data;

In an example of an unrelated communication library, I had seen it using memcpy like the following, so I changed the casting code above to memcpy then it worked. But why is the example using memcpy instead of casting?

// Callback when data is received
void OnDataRecv(uint8_t * mac, uint8_t *incomingData, uint8_t len) {
  memcpy(&incomingReadings, incomingData, sizeof(incomingReadings));
  incomingTemp = incomingReadings.temp;
  incomingHum = incomingReadings.hum;
}

The incomingReadings is declared as a global variable, but that variable is only used inside of that function, and only the fields which are copied to other global variables incomingTemp and incomingHum are used elsewhere. What if the example function were like the following, would it crash?

void OnDataRecv(uint8_t * mac, uint8_t *incomingData, uint8_t len) {
  struct_message* incoming = (struct_message*)incomingData;
  incomingTemp = incoming->temp;
  incomingHum = incoming->hum;
}

PS: About the crashing above, I have tested more things to reproduce it with simpler code. It seems that the board does not crash at casting, but at accessing the cast variable.

The structure is as simple as

typedef struct TEST_TYPE
{
    unsigned long a;
} TEST_TYPE;

and in the client, I sent a in

  TEST_TYPE *a = new TEST_TYPE();
  a->a = 1;

. In the server's handleData, I modified the code like below

static void handleData(void *arg, AsyncClient *client, void *data, size_t len)
{
    Serial.printf("Data length = %i\n", len);
    
    uint8_t* x = (uint8_t*)data;
    for(int i =0; i<len; i++)
    {
        Serial.printf("%X, ", x[i]);
    }

    Serial.println("Casting.");
    TEST_TYPE* a = (TEST_TYPE*)data;

    Serial.println("Printing.");
    Serial.printf("\nType = %i\n", a->a);

, and the output was

Data length = 4
1, 0, 0, 0, Casting.
Printing.

--------------- CUT HERE FOR EXCEPTION DECODER ---------------

Exception (9):
epc1=0x40201117 epc2=0x00000000 epc3=0x00000000 excvaddr=0x3fff3992 depc=0x00000000

>>>stack>>>

ctx: sys
sp: 3fffec30 end: 3fffffb0 offset: 0190

PS2: Seems like it indeed is an alignment issue. The exception code is 9 above, and according to this page, 9 means:

LoadStoreAlignmentCause Load or store to an unaligned address

I have found an old answer for a similar case. The author suggested some possible solutions

  1. adding __attribute__((aligned(4))) to the buffer: I think this is not applicable in my case, because I did not create the data parameter.

  2. adding __attribute__((packed)) to the structure: I have modified my structure like the following, and it did not crash this time.

     typedef struct TEST_TYPE
 {
     unsigned long a;
 } __attribute__((packed)) TEST_TYPE;

  3. Read it by each one byte and construct the fields manually: This seems too much work.

Damn Vegetables
  • 11,484
  • 13
  • 80
  • 135

2 Answers2

4

Without the full picture of the lifetimes of all the data, it's hard to say what's going wrong in your particular case. Some thoughts:

uint8_t *bytes;
...
MyStructure* m = (MyStructure*)bytes;

What the snippet above is doing is using m to interpret the region of memory pointed to by bytes as a MyStructure. It's important to note that m is only valid as long as bytes is valid. When bytes goes out of scope (or freed, etc.), m is no longer valid.

uint8_t *bytes;
MyStructure m;
...
memcpy(&m, bytes, sizeof(MyStructure));

This snippet is copying the data referred to by bytes into m. At this point, m's lifetime is separate from bytes. Note that you could do the same thing with this syntax:

uint8_t *bytes;
MyStructure m;
...
m = *((MyStructure*)bytes)

This snippet is saying "treat bytes as a pointer to a MyStructure, then dereference the pointer and make a copy of it".

As @danadam points out in a comment, memcpy() should be used in the case of alignment issues.

effect
  • 1,279
  • 6
  • 13
  • I think that from standard point of view, `memcpy` would be required even if alignment was right. But depending on which compiler you use, it might works. – Phil1970 Sep 13 '21 at 23:54
  • @effect It indeed was an alignment issue. Please see the added text to my original posting. – Damn Vegetables Sep 14 '21 at 04:32
4

Would it crash? Perhaps.

Essentially you're touching alignment and aliasing here. The rules are here: https://en.cppreference.com/w/cpp/language/object#Alignment

Your struct most probably has higher alignment requirements than 1 and therefore it depends on where in the memory the converted bytes are located if it will crash or not. As neither you nor the compiler can be sure of that, the cast is undefined behavior. The only way your cast from void* to MyStructure* wouldn't be UB is when the void* was casted from a MyStructure* in the first place. uint8 / char etc. have minimal alignment requirements (only 1 byte) and are therefore valid anywhere in a chunk of memory. That can be used to copy the memory into your correctly aligned object.

sego
  • 126
  • 2