Finding out Segmentation Fault Problem

[akadayifci]

Dear Bytes community,

I would like to get your opinion on how to identify a segmentation fault problem in my code which is written in C language. I would share my code for your valuable comments,

Kindly regards.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stddef.h>
#include <signal.h>
#include <assert.h>
#include <hiredis/hiredis.h>
#include <hiredis/async.h>
#include <hiredis/adapters/libevent.h>
#include <event.h>
#include <unistd.h>
#include <time.h>
#include <stdbool.h>
#define PACKETSIZE sizeof(cloudRANMessage)
#define COMMANDSIZE 256
 
typedef struct cloudRANMessage
{
    unsigned int      station_id;
    unsigned int      location_area;
    unsigned int      counterRedis;
    char              clientHostName[1024];
    char              command[COMMANDSIZE];
 
}cloudRANMessage;
 
char *accessKey;
char *accessHash;
 
void callbackDeserialize();
void serialize();
 
void printMyMessage(cloudRANMessage *message)
{
    printf("%d\n", message->location_area);
    printf("%d\n", message->station_id);
    printf("%s\n", message->command);
    printf("%s\n", message->counterRedis);
    printf("%s\n", message->clientHostName);
}
 
void serialize(cloudRANMessage *message, char *data)
{
    assert(data != NULL);
    memcpy(data, message, sizeof *message);
}
 
 
void deserialize(char *data, cloudRANMessage *tempMessage)
{
    memset(tempMessage, 0, sizeof(cloudRANMessage));
    memcpy(tempMessage, data, sizeof(cloudRANMessage));
    printMyMessage(tempMessage);
}
 
void deserializeLocal(char *data, cloudRANMessage *tempMessageLocal)
{
    memset(tempMessageLocal, 0, sizeof(cloudRANMessage));
    memcpy(tempMessageLocal, data, sizeof(cloudRANMessage));
    printMyMessage(tempMessageLocal);
}
 
void getCallback(redisAsyncContext *c, void *r, void *privdata)
{
    redisReply *reply = r;
    printf("%s\n", reply->str);                   // Call deserializaton function for the data retrieval.;
    /* Disconnect after receiving the reply to GET */
    redisAsyncDisconnect(c);
}
 
void replyParsing(void *reply)  // Parsing will be used to handle subscripton message reply to get the hash-key pair of the data that is written.
{
    redisReply *parsing = reply;
    printf("parsing array %s", parsing->element[2]->str);
    char *parsingArray = parsing->element[2]->str;
    char *p;
    p = strtok(parsingArray,"[ ""].");
    int i= 0;
    while(p !=NULL)
    {
        p = strtok(NULL,"[ ""].");
        if(i == 7)
        {
            accessHash = p;
            printf("%s\n",p);
        }
        else if(i == 8)
        {
            accessKey = p;
            printf("%s\n",p);
        }
    i++;
    }
     // send pointer here !
    // GET command here with the appropiate keys
}
// for the key hash value in the char. [8&9]
 
void listenChannel(redisAsyncContext *c, void *reply, void *privdata)
{
    struct event_base *base = (struct event_base*)privdata;
    char isExists = malloc(sizeof(isExists));
    isExists = "eNB";
    bool executeParsing = false;
    redisReply *r = reply;
    if (reply == NULL)
    return;
 
    printf("Client successfully subscribed to channel !\n");
    if(r->type == REDIS_REPLY_ARRAY){
 
        for(int j =0; j<r->elements;j++)
        {
            printf("Printing Redis Reply: %u) %s\n",j,r->element[j]->str);
            if (strstr(r->element[j]->str,isExists) != NULL)
                executeParsing = true;
            else
                executeParsing = false;
        }
 
    }
        if (executeParsing){
        replyParsing(r);
        printf("test received key !\n");
        event_base_loopexit(base,NULL);
    }
        else
        event_base_loopcontinue(base);
}
 
void callbackDeserialize(redisAsyncContext *c, void *r, cloudRANMessage *tempMessage) {
    redisReply *reply = r;
    if (reply == NULL) return;
    printf("%s\n", reply->str);                   // Call deserializaton function for the data retrieval.
    char *stringReply = reply->str;
    deserialize(stringReply, tempMessage);
    /* Disconnect after receiving the reply to GET */
    //
}
 
void callbackDeserializeLocal(redisAsyncContext *c, void *r, cloudRANMessage *tempMessageLocal) {
    redisReply *reply = r;
    if (reply == NULL) return;
    printf("%s\n", reply->str);                   // Call deserializaton function for the data retrieval.
    char *stringReply = reply->str;
    deserializeLocal(stringReply, tempMessageLocal);
    /* Disconnect after receiving the reply to GET */
    //
}
 
 
void connectCallback(const redisAsyncContext *c, int status) {
    if (status != REDIS_OK) {
        printf("Error: %s\n", c->errstr);
        return;
    }
    printf("Connected...\n");
}
 
void disconnectCallback(const redisAsyncContext *c, int status) {
    if (status != REDIS_OK) {
        printf("Error: %s\n", c->errstr);
        return;
    }
    printf("Disconnected...\n");
 
}
 
void connectCallback2(const redisAsyncContext *c2, int status) {
    if (status != REDIS_OK) {
        printf("Error: %s\n", c2->errstr);
        return;
    }
    printf("Connected...\n");
}
 
void disconnectCallback2(const redisAsyncContext *c2, int status) {
    if (status != REDIS_OK) {
        printf("Error: %s\n", c2->errstr);
        return;
    }
    printf("Disconnected...\n");
 
}
void connectCallback3(const redisAsyncContext *c3, int status) {
    if (status != REDIS_OK) {
        printf("Error: %s\n", c3->errstr);
        return;
    }
    printf("Connected...\n");
}
 
void disconnectCallback3(const redisAsyncContext *c3, int status) {
    if (status != REDIS_OK) {
        printf("Error: %s\n", c3->errstr);
        return;
    }
    printf("Disconnected...\n");
 
}
 
void exitCallback(redisAsyncContext *c, void *reply, void *privdata){
 
    struct event_base *base = (struct event_base*)privdata;
    event_base_loopexit(base,NULL);
}
 
int main (int argc, char **argv) {
 
    cloudRANMessage *cloudRANptr = malloc(sizeof(cloudRANMessage));
    cloudRANMessage *receivedMsg = malloc(sizeof(cloudRANMessage));
    cloudRANMessage *initialMessage = malloc(sizeof(cloudRANMessage));
    void *data = calloc(1,sizeof(cloudRANMessage));
 
    accessKey = malloc(sizeof(char));
    accessHash = malloc(sizeof(char));
 
    struct timeval start;
 
    initialMessage->location_area = 7214;
    initialMessage->station_id = 45632;
    initialMessage->counterRedis = 0;
    gethostname(initialMessage->clientHostName,1023);
    strcpy(initialMessage->command, "HANDOVER\0");
    gethostname(initialMessage->clientHostName,1023); // Gets the hostname of the compiled computer and puts in struct.
    printf("%s\n",initialMessage->clientHostName);
 
        signal(SIGPIPE, SIG_IGN);
        struct event_base *base = event_base_new();
        struct event_base *base3 = event_base_new();
        struct event_base *base2 = event_base_new();
        struct event_base *base4 = event_base_new();
 
        redisAsyncContext *localCon = redisAsyncConnect("localhost", 6379); // Connection will be used for get operation
        if (localCon->err) {
        printf("Error on localhost connection: %s\n", localCon->errstr);
        return 1;
        }
 
        redisAsyncContext *clientCon = redisAsyncConnect("192.168.1.103", 6379); // Connection will be used to write data on master
        if (clientCon->err) {
        printf("Error on master connection: %s\n", clientCon->errstr);
        return 1;
        }
 
        redisAsyncContext *subCon = redisAsyncConnect("localhost", 6379); // Connection will be used for SUBSCRIBE & UNSUBSCRIBE command
        if (subCon->err) {
        printf("Error on subscribe connection: %s\n", subCon->errstr);
        return 1;
        }
        unsigned int counter = malloc(sizeof(unsigned int));
        counter = 0;
 
        redisLibeventAttach(subCon, base2);
 
        redisAsyncSetConnectCallback(subCon,connectCallback3);
        redisAsyncSetDisconnectCallback(subCon,disconnectCallback3);
        counter++;
        redisAsyncCommand(subCon,listenChannel,base2,"SUBSCRIBE cloudRAN");
        WAITSUBSCRIBE:
        event_base_dispatch(base2);
        //initialize data on master !
        serialize(initialMessage,data);
        redisLibeventAttach(clientCon,base4);
        // Should run once to set the data !
        redisAsyncCommand(clientCon,exitCallback,base4,"HSET TA_1 eNB_1 %b",data,sizeof(cloudRANMessage));
 
        event_base_dispatch(base4);
 
        if(counter != 1){
        redisLibeventAttach(localCon, base);
        redisAsyncSetConnectCallback(localCon,connectCallback);
        redisAsyncSetDisconnectCallback(localCon,disconnectCallback);
        // Make an initial SET operation to the MASTER !
        // Access the data locally
 
        redisAsyncCommand(localCon,callbackDeserialize,receivedMsg,"HGET %s %s",accessHash,accessKey);
        printf("Execution in UNIX time for HGET:%ld\n", (start.tv_sec * 1000000 + start.tv_usec));
        event_base_loop(base,EVLOOP_ONCE);
        printf("Hash and Key value %s %s",accessHash,accessKey);
 
         if(initialMessage->clientHostName != receivedMsg->clientHostName){
 
            redisLibeventAttach(clientCon, base3);
 
            redisAsyncSetConnectCallback(clientCon,connectCallback2);
            redisAsyncSetDisconnectCallback(clientCon,disconnectCallback2);
 
            cloudRANptr = receivedMsg;   // Now access and change the data after verification
            cloudRANptr->counterRedis++;
            cloudRANptr->location_area= 56789; // Assign arbitrary location area value on this client
            serialize(cloudRANptr,data); // try with a different data pointer as well !!
            gettimeofday(&start, NULL);
            printf("Execution in UNIX time for HSET:%ld\n", (start.tv_sec * 1000000 + start.tv_usec));
            redisAsyncCommand(clientCon,exitCallback,base3,"HSET %s %s %b",accessHash,accessKey,data,sizeof(cloudRANMessage));
 
            event_base_dispatch(base3);
            goto WAITSUBSCRIBE;
        }
    }
        else
        goto WAITSUBSCRIBE;
    //redisAsyncCommand(c,NULL, NULL, "PUBLISH cloudRAN %b", data, sizeof(cloudRANMessage));
    return 0;
}
 
 
 

[weaknessforcats]

The only way to find these kinds of problems is to use your debugger and step through the code verifying every step of the way that your variable and memory contents are what you expect.

I do notice use of memcpy which I never advise. The mem functions are the quickest way to corrupt memory that I can think of.

I also notice that your memcpy copies from a char* 1292 bytes to a address pointing to a cloudRANMessage. 1) does the char* point to 1292 bytes and not one byte less, 2) does the target cloudRANMessage pointer point to 1292 bytes, 3) is the memory pointed at by the char* actually pointing at a cloudRANMessage? I would expect compiler warnings here.

[donbock]

Line 38 uses "%s" to print message->counterRedis. However, counterRedis is an unsigned int not a pointer.

What else could go wrong?

Function printMyMessage does not check if message is NULL before dereferencing it.
Function printMyMessage prints the message->command string without knowing if the string is properly terminated.
Function printMyMessage prints the message->clientHostName string without knowing if the string is properly terminated.

Function serialize does not check if message is NULL before dereferencing it.
Function serialize does know if *data is smaller than *message. Why not declare data as pointing to cloudRANMessage too? If both are the same type then you can simply use assignment statement instead of memcpy.

Function deserializeLocal does not check if data and tempMessage are NULL before dereferencing them.
Function deserialize does not know if data is big enough to pull that many bytes out of.

Function getCallback does not check if r is NULL before dereferencing it.
Function getCallback prints reply->str without knowing if the string is properly terminated.

And so on. Some of the more common causes of segmentation faults are dereferencing a NULL pointer, reading or writing past the end of a variable or buffer, and accessing memory after it has been freed.

[akadayifci]

I really appreciate your answers that helped to understand a lot. Finally i realized to find out where the program gives memory dump it is here:

 if (reply == NULL)
    return;
 
    printf("Client successfully subscribed to channel !\n");
    if(r->type == REDIS_REPLY_ARRAY){
 
        for(int j =0; j<r->elements;j++)
        {
            printf("Printing Redis Reply: %u) %s\n",j,r->element[j]->str);
            if (strstr(r->element[j]->str,isExists) != NULL)
                executeParsing = true;
            else
                executeParsing = false;
        }
 
    }

strstr causes a memory dump which i didn't understand the reason. Because when i comment out that part i can successfully receive the message response from the server which is being handled by printf line in the loop.

Thanks.

[weaknessforcats]

Maybe you could post additional code explaining the variables used by the strstr.

[akadayifci]

isExists is simply "eNB" string whereas the pointer with elements array is described as like this in this API. When we get a response with the type of REDIS_REPLY_ARRAY, we use that expression. I would also put the API's website just in case.

https://github.com/redis/hiredis

REDIS_REPLY_ARRAY:

A multi bulk reply. The number of elements in the multi bulk reply is stored in reply->elements. Every element in the multi bulk reply is a redisReply object as well and can be accessed via reply->element[..index..]. Redis may reply with nested arrays but this is fully supported.

Many thanks.

[weaknessforcats]

So you are saying that each argument of strstr is a const char* to a null terminated string?

If that's the case strstr won't crash.

But it is crashing so I think there is something wrong with those arguments OR the strstr is not the one in the C library.

[akadayifci]

Actually when i'm going to dig into the API library i found out that this structure holds a struct for the reply that is coming for the answer.
So i guess arguments of the each element is not a const char according to this structure below. You think being not a const as a problem when it is being used by strstr ?


typedef struct redisReply {
int type; /* REDIS_REPLY_* */
long long integer; /* The integer when type is REDIS_REPLY_INTEGER */
int len; /* Length of string */
char *str; /* Used for both REDIS_REPLY_ERROR and REDIS_REPLY_STRING */
size_t elements; /* number of elements, for REDIS_REPLY_ARRAY */
struct redisReply **element; /* elements vector for REDIS_REPLY_ARRAY */
}redisReply;

[donbock]

Starting from line 102...

char isExists = malloc(sizeof(isExists));
isExists = "eNB";
...
if (strstr(r->element[j]->str,isExists) != NULL)

isExists is a char, not the char* that strstr expects. You should have gotten a compiler error.
What compiler are you using?
Are you setting a command line option that suppresses function prototype checking?

[weaknessforcats]

This code crashes:

int main()
{
    redisReply var;
    redisReply* r = &var;
 
    strstr(r->element[0]->str, "hello");
 
}

From what I see both arguments to strstr are char* values. The crash is because the elements pointer is invalid and the str pointer is invalid.

Compiles and links fine but since these variables are screwed up the strstr never gets a chance to work because the crash occurs trying to access the string pointed at by
r->element[0]->str.

Now I think your redisReply variables are not correct. For example is elements correct for the number of element in the array? Where is this checked? It would have to be checked every time an element was added to or removed from the array.

Finally, the const char* arguments to strstr to not require your char* to be const. The const char* argument means that strstr will treat the argument as const (won't try to change it). Therefore, if your char* used to call strstr is invalid it's not strstr that did it.

[akadayifci]

I'm using GCC compiler on ubuntu with an option of std=C99.

In fact it is weird that when the API gets value (where r is a pointer of message reply) it prints out the response without any problem with this loop whenever i want to use another function like strstr it crashes.

void listenChannel(redisAsyncContext *c, void *reply,void *privdata)
{
redisReply *r = reply;
if (reply == NULL) return;

if(r->type == REDIS_REPLY_ARRAY)
{
for(int j =0; j<r->elements;j++)
{
printf("%u) %s\n",j,r->element[j]->str);
}
}
}

[donbock]

Please confirm lines 102, 103, and 115 appear in your source code exactly as they are shown in your original post, namely

    char isExists = malloc(sizeof(isExists));
    isExists = "eNB";
...
            if (strstr(r->element[j]->str,isExists) != NULL)

Especially, that line 102 is not

    char *isExists = malloc(sizeof(isExists));

If the original post is correct, then please examine your compiler output to see if there are any errors or warnings for lines 103 or 115 -- because your compiler ought to be complaining.

[weaknessforcats]

@akadayifci post #11:

The printf is being told to display an unsigned integer (%u) rather than a string (%s).

You might try changing the printf to %s and see what happens. The %s will display characters until the first null. The %u will just display the bytes occupied by an unsigned integer.

This function has void* arguments so it's just the function who says the address is a redisReply address.

Let me know what you find out.

[akadayifci]

I've changed malloc(sizeof(isExists)) to malloc(sizeof(char));
There is no error for these lines actually.

When i run the code without comments (İ'm trying this string search function now it runs one time and gives a segfault)

if(r->type == REDIS_REPLY_ARRAY){
memcpy(buffer,r,sizeof(redisReply));
//printf("%d",r->elements);
for(int j =0; j<r->elements;j++)
{
//buffer = r->element[j]->str;
printf("Printing Redis Reply: %u) %s\n",j,r->element[j]->str);
//pch = (char*) memchr(r->element[j]->str,isExists,strlen(r->element[j]->str));
//if (pch != NULL)
//executeParsing = true;
//else{
//executeParsing = false;
//printf("testtest");
//}
}
}
Normally, if i remove the string search condition it will wait for the future responses in the event loop but this somehow causes a segfault after the for loop completition.

[weaknessforcats]

I'm not sure what you are trying to do. redisReply has got pointers for members. After a memcpy the buffer and the redisReply variable now each point to the same arrays. Whichever of these variables changes one of those pointers screws up the other one.

There's a warning in your loop because you compare signed and unsigned integers. size_t is unsigned.

Where is pch defined?

This code:

pch = (char*) memchr(r->element[j]->str,isExists,strlen(r->element[j]->str));

says to look for an integer in memory starting at isExists that is equal to the integer returned by strlen?

Isn't isExists a char*? How is it pointing to an integer?

This code:

buffer = r->element[j]->str;

trashes buffer by putting the address of r->element[j]->str at the beginning.

As you can see I'm having considerable difficulty in understanding this logic.

[akadayifci]

Maybe i wrote it in the search function in a wrong manner, i simply want to search for the strings that are in the r->element[j]->str by looking to isExists string. I've defined the pch as char *pch in the same function. Since r->element[j] contains the responses that i want to look for i want to make a comparison with every element.

Could you have any suggestion to to that because it is my first time to use these string manipulation functions in C, I used to work on C++. I guess I've tried several things at once that's what makes you confused to understand the code, I'm sorry because of that.

Thanks for your help again.

[weaknessforcats]

Stop using those mem... functions.

This code does not crash:

int main()
{
    redisReply var[10];
    redisReply* r = var;
 
    char* result = 0;
 
    r->element = (redisReply**)malloc(sizeof(redisReply*));
 
    r->element[0] = (redisReply*)malloc(sizeof(redisReply));
 
    r->element[0]->str = malloc(100);
 
    strcpy(r->element[0]->str, "123hello456");
 
    result = strstr(r->element[0]->str, "hello");
 
    printf("result %p\n", result);
 
    free(r->element[0]->str);
    free(r->element[0]);
    free(r->element);
 
}

1) First I create an array of redisReply variables
2) Next, I assign the address of the array to an redisReply pointer. This is to simulate your code. r points to var[0].
3) Next I will I need to allocate memory for an redisReply* that the redisReply** in r points to.
4) Next I will need to allocate memory for an redisReply* that r->element points to. This allocation is for r->element[0], which is all I will use in this demo code.
5) Next I will allocate memory for the string that is a member of r->element[0]. That string is r->element[0]->str.
6) Next I copy some contents to r->element[0]->str.
7) Next I search those contents for hello and capture the result. strstr returns a char* to the location where the h of hello is located when hello is found. If hello is not found, strstr returns a 0.
8) Next I display that address.
9) Next I release memory in the EXACT REVERSE ORDER of how it was allocated.

If any of these memory allocations are not done, you crash.
If any of these memory allocations are overrun or you have a pointer pointing to an address outside these allocatons, you crash.
If you copy contents to the str string that is larger than strlen(contents +1), you crash. That 1 is for the null termination which strlen does not count.
If you fail to free the allocations in the exact reverse order of the allocations, you leak.

I am convinced that there is a memory corruption problem surrounding your redisReply variables which is causing strstr to crash. I also believe this is why the printf crashes in the listenChannel function.

Let me know what you think. I can offer a solution but only if you agree the problem is not strstr but is instead a memory management issue in the code itself.