Thread: stack smashing / segfault problem

  1. #1
    Registered User
    Join Date
    Oct 2009
    Posts
    48

    stack smashing / segfault problem

    Hi, I'm working on a simulator and I have found a spot where I repeatedly get a segfault and sometimes a stack smashing error. I believe with the code I will post here I do not get the stack smashing error, but I was earlier with similar code.

    The problem occurs when I call fscanf to read from a file. The format of the file is one char, one space, eight chars, then a newline. "r 0affbe01" for example. The first char is never read correctly as when I print it out, I get a question mark inside a diamond. I am using fscanf like so: "fscanf(fptr, "%c %s\n", &instr->op, instr->hex)".

    I'll try to give a little more background now.
    the function is like so:
    Code:
    void readNextInstr(FILE *fptr, instruction *instr)
    {
    	//char temp;
    	//char temp2[45];
    	
            /* With these uncommented, these values can be printed afterwards */
    	//instr->op = 'r';
    	//strcpy(instr->hex, "abcdef01");
    	
    #ifdef DEBUG_2
    	printf("*BDON: readNextInstr: Reading next line\n");
    #endif
    
    	// Read one line
    	fscanf(fptr, "%c %s\n", &instr->op, instr->hex);
     
            /* Alternatively, I've tried this with no luck */
    	//fscanf(fptr, "%c %s\n", &temp, temp2);
    	
    #ifdef DEBUG_2
    	printf("*BDON: readNextInstr: Read line. Op = %c\n", instr->op);
    	fflush(stdout);
    #endif
    	return;
    }

    That function is in smp.c, in cache.h I define instruction
    Code:
    typedef struct _instruction
    {
    	int        proc;
    	char     op;	 /* Operation (r/w) */
    	char     hex[9];	 /* String representation of address read from file */
    	address *addr; /* Pointer to address struct for the memory reference location */
    } instruction;

    Here is where I open the file in a function in smp.c. This is just a snippet.
    Code:
    // Open the trace file
    	trace_fptr = fopen(argv[6], "r");
    	if (trace_fptr == NULL)
    	{
    		fprintf(stderr, "ERROR: Could not open trace_file\n");
    		fflush(stderr);
    		return -1;
    	}

    And these are the relevant statements in main.c where readNextInstr is called.
    Code:
    int main(int argc, char **argv)
    {
    
            // Instruction being executed
    	instruction *instr;
    ...
    	// Malloc the instruction struct and address struct inside instruction
    	instr = (instruction *)malloc( sizeof( instruction ));
    	instr->addr = (address *)malloc( sizeof( *instr->addr));
    ...
    	// Read the first instruction
    	readNextInstr(trace_fptr, instr);
    ...
    }

    Here is some output from gdb
    Code:
    Program received signal SIGSEGV, Segmentation fault.
    0x00007f505fdd9f4d in _IO_vfscanf () from /lib/libc.so.6
    (gdb) info registers
    ...
    rip            0x7f505fdd9f4d   0x7f505fdd9f4d <_IO_vfscanf+1789>
    ...

    Here is output from command line
    Code:
    *BDON: readNextInstr: Reading next line
    *BDON: readNextInstr: Read line. Op = �
    
    Instruction: 1          Read �� Processor nBDON: executeInstr: hex: ��, tag: 0, index: 0, offset: 0 op:
    *BDON: DoRequest: tag: 0, index: 0, offset: 0 op:
    Segmentation fault
    After the "Instruction: 1" is output from other functions. Basically those are trying to print instr->op and instr->hex, too.


    Also, I use most of this code for another simulator as well. The other one imports multilevel.h instead of smp.h so I have a few functions like readNextInstr in both multilevel.c and smp.c that have the same declaration but do different things. When I compile for this other simulator, everything works perfectly. The only part of the code that I posted that is dependent on which simulator is the function readNextInstr and the function that opens the file. Here is readNextInstr from multilevel.c and again, everything works fine with this implementation.

    Code:
    void readNextInstr(FILE *fptr, instruction *instr)
    {
    	// Read one line
    	fscanf(fptr, "%c %s\n", &instr->op, instr->hex);
    
    #ifdef DEBUG_2
    	printf("*BDON: readNextInstr: Read line. Op = %c \n", instr->op);
    	fflush(stdout);
    #endif
    	
    	return;
    }

    I have a feeling it is not something obvious and I may have not posted enough information but I am just trying to keep this somewhat simple for now. Please let me know if I should post more code or explain more/better.

    Brandon

  2. #2
    Guest Sebastiani's Avatar
    Join Date
    Aug 2001
    Location
    Waterloo, Texas
    Posts
    5,708
    I can't really see anything wrong with it (from the code you posted, at least). I wrote a test program to see if I could duplicate the problem, but everything seemed to work fine:

    Code:
    
    
    #include <stdio.h>
    #include <stdlib.h>
    
    int main( int argc, char** argv )
    {
    	FILE
    		* in;
    	char
    		c, 
    		str[ 9 ];
    	int
    		result;
    	while( *( ++argv ) )
    	{
    		in = fopen( *argv , "r" );
    		if( !in )
    			fprintf( stderr, "Error: file not found\n" );
    		else
    		{
    			for( ;; )
    			{
    				result = fscanf( in, "%c %8s\n", &c, str );
    				if( result < 0 )
    					break;
    				if( result != 2 )
    				{
    					fprintf( stderr, "Error: malformed input\n" );
    					break;
    				}	
    				printf( "Char: '%c', Text: \"%s\"\n", c, str ); 	
    			}	
    			fclose( in );
    		}
    	}
    	return 0;
    }
    Also, it's always a good idea to limit the length of text read by scanf-type functions to prevent buffer overflows, eg:

    "%c %8s\n"
    Last edited by Sebastiani; 10-25-2009 at 03:17 PM. Reason: missing newline in fprintf

  3. #3
    Registered User
    Join Date
    Oct 2009
    Posts
    48
    Thanks for the reply, Sebastiani

    I changed it to "%c %8s" and collected the return value. The return value was 2 so that seems correct.

    I changed the fscanf call to store the results into two char *s created in the function instead of the instr struct.
    Code:
    void readNextInstr2(FILE *fptr, instruction *instr)
    {
    	int result;
    	char temp[6];
    	char temp2[45];
    	
    #ifdef DEBUG_2
    	printf("*BDON: readNextInstr: Reading next line\n");
    #endif
    	
    	// Read one line
    	//result = fscanf(fptr, "%c %8s\n", &instr->op, instr->hex);
    	result = fscanf(fptr, "%c %8s\n", &temp[0], temp2 );
    	
    #ifdef DEBUG_2
    	printf("*BDON: readNextInstr: Read line. Op = %c, hex = %s, read = %d\n", temp[0], temp2, result);
    	fflush(stdout);
    #endif
    	
    	return;
    }
    and I still get the error. Temp is size 6 because sometimes using gdb the value of temp would be 4 bytes (-96 and the first 3 bytes of temp2).

    So, I believe this means that the file is not being opened correctly. I don't know why and I'd appreciate any input while I look into it.

  4. #4
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Use %d instead of %c to print out your op, and see what actual value you've got in there. Does fscanf return 0 or 2?

  5. #5
    Guest Sebastiani's Avatar
    Join Date
    Aug 2001
    Location
    Waterloo, Texas
    Posts
    5,708
    Well, if there was an error opening the file then the processing function should never have been called in the first place. Anyway, to start with, print the current working directory and the filename as it is being passed to the fopen call, eg:

    Code:
    
    void check_filename( char const* name )
    {
    	char
    		path[ 1024 ];		
    	if( !getcwd( path, sizeof path ) )
    		fprintf( stderr, "Couldn't obtain current working directory\n" );
    	else 
    		printf( "Current working directory: '%s'\n", path );
    	printf( "File to be processed: '%s'\n", name );	
    }
    Verify that the path matches the directory your program 'thinks' it in, and that there is no excess space/newlines surrounding the filename.

  6. #6
    Registered User
    Join Date
    Oct 2009
    Posts
    48
    I ran into a little problem hopefully just from a typo so I need to do some clean up before I can test using %d. I was able to print the value of temp[0] using gdb before I made this typo.
    Code:
    (gdb) step                                                          
    *BDON: readNextInstr: Reading next line                             
    225             result = fscanf(fptr, "%1c %8s\n", &temp[0], temp2 );
    (gdb) print temp[0]                                                  
    $5 = -16 '�'
    And fscanf was returning 2.

  7. #7
    Guest Sebastiani's Avatar
    Join Date
    Aug 2001
    Location
    Waterloo, Texas
    Posts
    5,708
    Did you run the test program I listed to verify the output? Also, did you verify that the file was being opened properly?

  8. #8
    Registered User
    Join Date
    Oct 2009
    Posts
    48
    Ok, I changed the fscanf's format to "%d %s" and I added the function that you gave me, Sebastiani. Here is the output from both.
    Code:
    brandon003@desktop:/media/documents/XXXX/521/project1/src$ ./smp_cache 8192 8 64 1 0 ../traces/vortex_trace.txt
    Current working directory: '/media/documents/XXXX/521/project1/src'
    File to be processed: '../traces/vortex_trace.txt'
    ===== 506 SMP Simulator Configuration =====
    L1_SIZE:                        8192
    L1_ASSOC:                       8
    L1_BLOCKSIZE:                   64
    NUMBER OF PROCESSORS:           1
    COHERENCE PROTOCOL:             MSI
    TRACE FILE:                     ../traces/vortex_trace.txt
    *BDON: readNextInstr: Reading next line
    *BDON: readNextInstr: Read line. Op = 12, hex = , read = 0
    
    Instruction: 1          Read  Processor nBDON: executeInstr: hex: , tag: 0, index: 0, offset: 0 op:
    *BDON: DoRequest: tag: 0, index: 0, offset: 0 op:
    Segmentation fault
    brandon003@desktop:/media/documents/XXXX/521/project1/src$ ls ../traces/ -l
    total 5.3M
    -rw-r--r-- 1 brandon003 brandon003 1.1M 2009-10-02 20:56 compress_trace.txt
    -rw-r--r-- 1 brandon003 brandon003 1.1M 2009-09-26 18:11 gcc_trace.txt
    -rw-r--r-- 1 brandon003 brandon003 1.1M 2009-10-02 20:56 go_trace.txt
    -rw-r--r-- 1 brandon003 brandon003 1.1M 2009-10-02 20:56 perl_trace.txt
    -rw-r--r-- 1 brandon003 brandon003 1.1M 2009-10-02 20:56 vortex_trace.txt
    The first two lines show the current working directory and then the filename. After the segfault I did 'ls' to show that vortex_trace.txt does exist in ../traces/ and I do have read permission.

    The output from fscanf is on the line "*BDON: readNextInstr: Read line. Op = 12, hex = , read = 0".
    Op is from the %d, hex is from the %8s and read is the return value of fscanf. Vortex_trace.txt looks like this:
    Code:
    $ head ../traces/vortex_trace.txt
    w 40020760
    r 40020760
    r 7b0337fc
    r 7b0345f4
    r 7b0337fc
    r 40059770
    r 7b034514
    r 40059770
    w 40020464
    r 7b0337fc
    so fscanf is reading the first 'w' as the integer 12? and then hex is blank so I am not sure what is going on there. And now fscanf is returning 0.

    I will try changing readNextInstr to open the file, read a line, then close it to see if I can at least read one line.

  9. #9
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    You can't use %d to read in a letter; but you can use %d to print out a letter to see what it actually is.

  10. #10
    ATH0 quzah's Avatar
    Join Date
    Oct 2001
    Posts
    14,826
    Code:
    void foo( FILE *fp, ... )
    {
        if( fp )
        {
            char buf[BUFSIZ] = {0};
    
            if( fgets( buf, BUFSIZ, fp ) )
            {
                char c = 0, s[BUFSIZ] = {0};
    
                printf("buf: \'%s\'\n", buf );
                if( sscanf( buf, "%c %s", &c, s ) == 2 )
                {
                    printf( "c is \'%c\' %d; s is \'%s\'\n", c, c, s );
                }
                else
                {
                    printf( "sscanf failure\n" );
                }
            }
            else
            {
                printf( "fgets failure\n" );
            }
        }
    }
    When debugging, be generous with output statements and return value checks.


    Quzah.
    Last edited by quzah; 10-26-2009 at 12:23 PM. Reason: i to I
    Hope is the first step on the road to disappointment.

  11. #11
    Registered User
    Join Date
    Oct 2008
    Location
    TX
    Posts
    2,059
    Wonder if it is a problematic file? Try to read one char at a time from the file using fgetc() to see if that works. Isolate the problem is my suggestion; is it fscanf() or the file.
    What OS and platform are you on? Run cat -vet filename to see if there are any non-printing characters embedded in the file.

  12. #12
    Registered User
    Join Date
    Oct 2009
    Posts
    48
    Ok, I see what you mean. In that case, here is the output. I read in a char and a string and I print out the char as an int, then the string and then the return value.
    Code:
    *BDON: readNextInstr: fscanf: op = -96, hex = �, return = 2
    
    Instruction: 1          Read  Processor nBDON: executeInstr: hex: , tag: 0, index: 0, offset: 0 op:
    *BDON: DoRequest: tag: 0, index: 0, offset: 0 op:
    Segmentation fault
    When I opened the file inside readNextInstr I was able to read one line.

    So it seems I am either not passing the same pointer to readNextInstr that I use to originally open the file or somehow it is being altered. Let me see what I can find in that case.

  13. #13
    Registered User
    Join Date
    Oct 2009
    Posts
    48
    I've managed to fix the problem, though I am not sure exactly what it was.

    The FILE * I was using was called trace_fptr.
    I was declaring this variable in cache.h and externing it in smp.c and main.c.
    smp.c was opening the file in an initialization function.
    main.c was passing trace_fptr to readNextInstr.

    Somehow that was not working. I think the issue may have been related to smp.c importing cache.h. This is how I did get it to work.
    Declare trace_fptr in smp.c as a global variable.
    Extern trace_fptr in main.c
    smp.c opens the file in the initialization function.
    readNextInstr doesn't need trace_fptr passed to it since it is in smp.c.

    I still need main.c to have trace_fptr to tell when EOF is reached but I will change this soon to check a flag or other condition so main.c will not need trace_fptr.

    Thanks for all the help.

  14. #14
    Guest Sebastiani's Avatar
    Join Date
    Aug 2001
    Location
    Waterloo, Texas
    Posts
    5,708
    You could define the variable in 'smp.c', say, then just put an extern declaration in the header file, so that all other files can 'see' it. If it's more readable for you, you could of course put an extern declaration in every file.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Problem about stack and queue in C
    By neyul in forum C Programming
    Replies: 4
    Last Post: 04-07-2009, 10:34 PM
  2. A question related to strcmp
    By meili100 in forum C++ Programming
    Replies: 6
    Last Post: 07-07-2007, 02:51 PM
  3. Need Help with Stack
    By trongsi in forum C++ Programming
    Replies: 9
    Last Post: 05-23-2006, 04:14 PM
  4. Request for comments
    By Prelude in forum A Brief History of Cprogramming.com
    Replies: 15
    Last Post: 01-02-2004, 10:33 AM
  5. Array Stack Problem
    By Drew in forum C++ Programming
    Replies: 3
    Last Post: 09-04-2001, 06:58 PM