string copy into malloc'd char array fails with segmentation fault

Question

I am practicing a client-server application in C. In the application, the client first registers with the server and gets the client_id in return. Next time onwards, the client sends the messages to the server prepending the client_id to the message. Below code snippet is from the server, where after receiving the message, the server retrieves the client_id from the message, which is a 3 char long string with start address as 1 and end address as 4 from the message array. eg. In the message "1001CLIENT:001:MESSAGE:01", '1' at location 0 is for some purpose, "001" is the client_id and "CLIENT:001:MESSAGE:01" is the message from the client.

char *create_dynamic_string(int str_size)
{
        char *dyn_string = malloc(str_size*sizeof(char));
        if(!dyn_string)
        {
                printf("Dynamic string could not be created");
                return NULL;
        }
        dyn_string[0] = '\0';
        return dyn_string;
}

void free_dynamic_string(char *dyn_string)
{
        free(dyn_string);
}

//char *message is dynamically allocated char array.
char *retrieve_client_id(char *message)
{
        char *client_id;
        int i;
        client_id = create_dynamic_string(CLIENT_ID_SIZE + 1);

        if(!client_id)
        {
                printf("Client_id is NULL");
                return NULL;
        }
        //for(i = 1; i < (CLIENT_ID_SIZE + 1); i++)
        //      client_id[i-1] = message[i];

        //strncpy(client_id, message + 1, CLIENT_ID_SIZE);

        memcpy(client_id, message + 1, CLIENT_ID_SIZE);

        client_id[CLIENT_ID_SIZE] = '\0';
        printf("client_id retrieved=%s", client_id);
        return client_id;
}

The server accepts a connection from the clients and processes the messages in different threads. The working on multi-threading and processing of the messages is tested successfully. Below code compiled successfully and works most of the times. But some times it ends up in segmentation fault at memcpy() in retrieve_client_id(). I am not able to figure out why is it failing this way.

I used gdb to get more info. This is as below.

Program terminated with signal 11, Segmentation fault.
#0  0x00000000004017ef in retrieve_client_id (message=0x1b904f40 "1000CLIENT:000:MESSAGE:02") at src/server.c:46
46              memcpy(client_id, message + 1, CLIENT_ID_SIZE);
(gdb) print message
$1 = 0x1b904f40 "1000CLIENT:000:MESSAGE:02"
(gdb) print message+1
$2 = 0x1b904f41 "000CLIENT:000:MESSAGE:02"
(gdb) print CLIENT_ID_SIZE
$3 = 3
(gdb) print client_id
$4 = 0xffffffffcc0008c0 <Address 0xffffffffcc0008c0 out of bounds>
(gdb)

Need help in understanding what exactly might be happening when the application is failing. I have verified that the malloc was successful and client_id was not NULL. As you can see the commented code, I also tried with strcpy() and also copying the chars from source to dest array one by one. But there also I saw the failures.

Please [see why not to cast](http://stackoverflow.com/q/605845/2173917) the return value of `malloc()` and family in `C`. — Sourav Ghosh, Aug 10 '15 at 14:23
I am not casting it anyways. If you see in the gdb logs, the value of CLIENT_ID_SIZE is 3. — Peeyushpd, Aug 10 '15 at 14:26
oh is it? you seem very confident about it? what about `char *dyn_string = (char *)malloc(str_size*sizeof(char));` then? :-) — Sourav Ghosh, Aug 10 '15 at 14:27
Oops, missed it. :D I tried without casting as well. Issue is still seen. — Peeyushpd, Aug 10 '15 at 14:28
why don't you put a null check for incoming `message` pointer? — Sourav Ghosh, Aug 10 '15 at 14:32
Are you posting gdb info of same thread that got segfault? As you said there are multiple threads, so verify that. Also, print value of `client_id` before and after allocation function. — Rohan, Aug 10 '15 at 14:35
The message pointer is populated fine. The control will come to retrieve_client_id() only when the message was there. So I have not added any check here. — Peeyushpd, Aug 10 '15 at 14:36
@rohan I have used the core dump generated after the code seg faulted. I suppose it was the dump for the failed thread. — Peeyushpd, Aug 10 '15 at 14:39

Employed Russian · Accepted Answer · 2015-08-10T15:12:39.950

I am guessing that create_dynamic_string and retrieve_client_id are in different compilation units, and that you don't have correct prototype for the former visible when the latter is compiled.

This address 0xffffffffcc0008c0 looks like a reasonable heap address 0xcc0008c0 with extra bits tacked on.

Add this line:

extern char *create_dynamic_string(int);

just above definition of retrieve_client_id.

If that fixes the problem (it should), then add prototype for create_dynamic_string to appropriate header file, and #include it into the source that defines retrieve_client_id.

I did not understand why it did not give compilation error

In C, an undeclared function is assumed to have the following prototype:

int undeclared(...);

As such, any of the calls below will not be rejected:

int x = undeclared();
int y = undeclared(1);
char *z = undeclared(1, 2, 3);

You can ask GCC to warn you about missing prototypes specifically with -Wmissing-prototypes, but the best practice is to compile with -Wall and -Werror. The latter will not let you ignore warnings and force you to fix them.

Adding -Wextra is also recommended, although it does sometimes give false-positive warnings.

Thanks for pointing out about the prototype for create_dynamic_string(). I had missed the #include for the source file where create_dynamic_string() prototype was available. And not it looks like working fine. But I did not understand why it did not give compilation error. Can you please explain. — Peeyushpd, Aug 10 '15 at 15:00
Thanks for the explanation. Will use -Wall and -Werror going forward. I wanted to understand what exactly was happening at runtime? Why memcpy() was failing here if the address 0xffffffffcc0008c0 was a reasonable heap address. — Peeyushpd, Aug 11 '15 at 05:06

string copy into malloc'd char array fails with segmentation fault

1 Answers1