CODEWITHSUNDEEP
c
Rocky Inde
Rocky Inde

Reputation: 1531

Return value of a function getting corrupted

I have the following code:

static char sessionid[SESSIONID_LEN+1] = { '\0' };

static void generate_sessionid() {
  char set[] = "0123456789"
    "abcdefghijklmnopqrstuvwxyz"
    "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
  int len;

  memset(sessionid, 0, sizeof(char) * SESSIONID_LEN);
  for (len = 0; len < SESSIONID_LEN; len++) {
    size_t index = (double) rand()/RAND_MAX*(sizeof(set) - 1);
    sessionid[len] = set[index];
  }
}

char *get_sessionid() {
  if (strlen(sessionid) == 0) generate_sessionid();
  char * dup = strdup(sessionid);
  return dup;
}

I call get_sessionid() from a different thread (whose function is defined in a different file). When I break in line 2 of get_sessionid() and examine the pointer address returned by strdup(), this is the value:

0x7f7788c004f50

Now, when I return from this function and examine the returned value caught in the thread function, the return value is different:

0xfffffffffc004f50

And subsequently when I call strlen() on this returned pointer value, I get a SIGSEGV and the reason being Address 0xfffffffffc004f50 out of bounds

The two files are part of a big application that have around 30 threads running and many more memory allocations happening elsewhere. I also think memory has been leaked by other threads. But I want to make sure that something is not wrong in the above code.

I verified the above code separately and everything works fine, the return value is not corrupt, i.e, it works perfectly fine when isolated from the rest of the program. But I don't understand why the return value is getting changed when run in the context of the whole program.

Edit (attachments)

In the thread function I call as:

thread.c

#include <header.h>

login_sessionid = (char*)get_sessionid();

In the header: header.h

char * get_sessionid(void);

Compiler warnings:

warning: implicit declaration of function ‘get_sessionid’ [-Wimplicit-function-declaration]
warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]

Both the above warnings are at the line shown (above) of the file thread.c

Upvotes: 1

Views: 1245

Answers (2)

Gene
Gene

Reputation: 46960

In addition to what @Jonathan Leffler said, if more than one thread can call this routine, there is a race condition allowing different threads to get different id's even though they are separate copies.

One example of what can occur:

  • Multiple threads inspect the static session id and see zero so begin initialization and stop.
  • One thread at a time completes initialization and makes its own copy. Thus each thread gets a different id.

In general, static storage (with no mutex) and multiple threads do not mix. This is a classic problem with the singleton pattern in multi-threaded environments.

Upvotes: 3

Jonathan Leffler
Jonathan Leffler

Reputation: 753695

The chances are rather high that the code in the other file doesn't realize that get_sessionid() returns a pointer so it assumes it returns an integer and then sign-extends that to create a pointer.

Your compiler is probably warning about undeclared functions; if it isn't, turn up the warning levels until it does. It should also be warning about converting an integer to a pointer of a different size.

Pay attention to the warnings your compiler gives. Remember, it knows more about C than you do.

Also make sure the compiler gives you as many warnings as possible. If you use gcc, you should be compiling cleanly with gcc -Wall -Wextra -Werror -Wmissing-prototypes -Wold-style-definition -Wold-style-declaration; that's my current benchmark (with -std=c11 or -std=c99). If your version doesn't support the old style warnings, you can simply leave them out.

Upvotes: 4

Related Questions