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->clientHostNa me 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 deserializeLoca l 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_ARR AY, we use that expression. I would also put the API's website just in case.

https://github.com/redis/hiredis

REDIS_REPLY_ARR AY:

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_INT EGER */
int len; /* Length of string */
char *str; /* Used for both REDIS_REPLY_ERR OR and REDIS_REPLY_STR ING */
size_t elements; /* number of elements, for REDIS_REPLY_ARR AY */
struct redisReply **element; /* elements vector for REDIS_REPLY_ARR AY */
}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(r edisAsyncContex t *c, void *reply,void *privdata)
{
redisReply *r = reply;
if (reply == NULL) return;

if(r->type == REDIS_REPLY_ARR AY)
{
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 [U]*[/U]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(i sExists)) to malloc(sizeof(c har));
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_ARR AY){
memcpy(buffer,r ,sizeof(redisRe ply));
//printf("%d",r->elements);
for(int j =0; j<r->elements;j++ )
{
//buffer = r->element[j]->str;
printf("Printin g Redis Reply: %u) %s\n",j,r->element[j]->str);
//pch = (char*) memchr(r->element[j]->str,isExists,s trlen(r->element[j]->str));
//if (pch != NULL)
//executeParsing = true;
//else{
//executeParsing = false;
//printf("testtes t");
//}
}
}
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.