-1

I encountered an interesting problem in C, when calling an external method from main that tries to strtok a local (main) string that is passed to it by reference. If I strtok the string in main, it works as expected, but if I call the external method, it fails with segfault. Valgrind's output on accessing the first element of the strtok result:

Address 0xfffffffffeffebc0 is not stack'd, malloc'd or (recently) free'd

--- test.c ---

extern void tt(char* x);

main(argc, argv) 
    int argc;
    char *argv[];
{
    char line[5000];
    // if I uncomment the following two lines and comment above it works as expected
    // char * line;
    // line = malloc(5000);
    strcpy(line, "hello world");
    printf("%d\n", sizeof(line));
    tt(line);
}

--- test2.c ---

void tt(char* l) {
    char* x = strtok(l, " \t");
    printf("%p\n", x);
    printf("%c\n", x[0]);
}

compile by

gcc -c test2.c -o test2.o
gcc test.c test2.o

IMHO it should print out something like:

./a.out
0x.....
h

But instead I get segfault on printing the "h".

Can somebody please explain this behavior?

Thanks.

ticcky
  • 688
  • 6
  • 17
  • 1
    Not a reason for a downvote, but you should absolutely stop using K&R style function definitions. – Daniel Fischer May 14 '13 at 16:05
  • 1
    runs on http://codepad.org/7KUrMWdl – dchhetri May 14 '13 at 16:06
  • @ticcky, why don't you step through the debugger and figure out which line it crashes and why – dchhetri May 14 '13 at 16:08
  • @user814628 looks that you put the tt function and main in the same file. but the question was the both functions are in separated files – MOHAMED May 14 '13 at 16:08
  • 1
    @MOHAMED, Can you explain why that should matter? Isn't that just a matter of linkage – dchhetri May 14 '13 at 16:10
  • It run also on separate files. Which compiler do you use, on which OS and with which compile flags ? – lucasg May 14 '13 at 16:12
  • 1
    There's also a second bug in the code. Your line `printf("%d\n", sizeof(line));` should read `printf("%zu\n", sizeof(line));` or `printf("%d\n", (int)sizeof(line));`. The `sizeof` operator returns a `size_t` which in 64 bit mode is defined as `unsigned long` not `unsigned int` as in 32 bit mode. So if you want to be portable you have to use the right format specifier or cast the return value. – Patrick Schlüter May 14 '13 at 16:24
  • @georgesl when pointers and integers have the same size then it may work, when they aren't it might still work if the pointer is exactly representable in the range of int, but in this case the lower 32 bits of the pointer are sign extended to the full 64 bits of a pointer creating an invalid address – Geoff Reedy May 14 '13 at 16:25
  • Just so that you know. Your program printed the right value of the sizeof because you compiled it on a little endian machine (x86, ARM), on a big endian machine it would have probably printed 0 (SPARC, POWER, MIPS). – Patrick Schlüter May 14 '13 at 16:29
  • guys, thanks for the insightful comments. this (as you correctly pointed out) filthy piece of code is a stripped-off version of someone else's code that I had to make work. I am sorry that it distracted the attention from the real problem. it is solved by adding #include as @DanielFischer suggested. – ticcky May 14 '13 at 20:27

1 Answers1

7

In the file

--- test2.c ---

void tt(char* l) {
    char* x = strtok(l, " \t");
    printf("%p\n", x);
    printf("%c\n", x[0]);
}

you haven't included string.h, so the compiler assumes a return type of int for strtok. Thus the line

char *x = strtok(l, " \t");

doesn't assign the proper pointer to x (As the valgrind output shows, your pointers are 64 bits at least, but int is very probably only 32 bits). Then dereferencing the corrupted pointer causes a segmentation fault.

Daniel Fischer
  • 181,706
  • 17
  • 308
  • 431
  • 1
    wow that's incredible of you to find the solution, given the few amount of informations written by the OP ! – lucasg May 14 '13 at 16:15
  • 3
    This is one of the reasons to compile with warnings on and pay attention to them. When I don't include `string.h` gcc gives a warning about initialization of pointer from integer without a cast and when I add `-Wall` I get a warning for implicit declaration of `strtok`. If your compiler isn't giving these diagnostics get a better one :) – Geoff Reedy May 14 '13 at 16:18