Why does my c++ program crash?

Discussion in 'OT Technology' started by D1G1T4L, May 15, 2005.

  1. D1G1T4L

    D1G1T4L Active Member

    Joined:
    May 4, 2001
    Messages:
    16,489
    Likes Received:
    0
    Location:
    Bay Area
    Code:
    
    // Purpose: A program that reads a few lines of text from keyboard
    // and prints the number of occurrences of each letter. Works with lower case letters.
    // Side Note: If a letter is upper case, it is converted to lower case. 
    // Numbers are ignored - only 26 characters of english alphabet are allowed
    
    
    #include<iostream>
    #include<cctype>
    #include<cstring>
    using namespace std;
    
    void readInput(char sentence[]);
    void calcNumOfAppearences(char *sPtr, int storage[], int counter, int counter2);
    void displayResults(int storage[]);
    
    //const GLOBAL variable
    //not modifiable
    //since they are const
    //ok to be global
    
    //user input max length
    const int INPUT = 1024;
    
    //alphabet array
    const char alphabet[26] = { 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 'j', 'k', 
    		'l', 'm', 'n', 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', 'w', 'x', 'y', 'z'};
    
    		
    //alphabet count
    const int ALPHABET = 26;
    	
    
    
    int main()
    {
    	//user input
    	char sentence[INPUT];
    
    	//will contain the letter count
    	int storage[26] = {0};
    
    	//counter for looping through user input
    	int counter = 0;
    	
    	//counter to loop through alphabet array
    	int counter2 = 0;
    
    
    	//gets user input
    	readInput(sentence);
    
    	//pointer that points to first value of
    	// the sentence, created just to see how pointers work
    	// might as well pass sentence array to all other functions
    	//instead of the sPtr pointer
    	char *sPtr;
    	sPtr = sentence;
    	
    
    	//calculates number of appearences
    	calcNumOfAppearences(sPtr, storage, counter, counter2);
    	
    	//displays the result
    	displayResults(storage);
    
    
    	return 0;
    }
    void readInput(char sentence[])
    {
    		
    	
    
    	int nLines; //number of lines of input
    	char *usrInput;
    	string combinedInput;
    
    	//asks the user for the number of lines he'll enter
    	//the input must contain not more than 1024 characters
    	cout << "\nHow many lines of input is there going to be?"
    		<< "\n(All combined input can only contain up to 1024 characters): ";
    	cin >> nLines;
    	usrInput = new char[INPUT+1];
    	cin.get(); // reads the empty character \n from the buffer
    	for(int i = 0; i < nLines; i++)
    	{
    		
    		cout << "\nEnter a sentence: "; 	//gets the input from the user
    		cin.getline(usrInput, INPUT, '\n');
    		usrInput[INPUT] = '\0';
    		strcat(sentence, usrInput);
    	}
    
    	//delete the memory and set pointer to null	
    	delete [] usrInput;
    	*usrInput = 0;
    }
    void calcNumOfAppearences(char *sPtr, int storage[], int counter, int counter2)
    {
    	
    	//goes through all the letters of the input 
    	//and counts up how many times the letter appears
    	for(counter = 0; counter < strlen(sPtr); counter++)
    	{
    		for(counter2 = 0; counter2 < ALPHABET; counter2++)
    		{
    			if(alphabet[counter2] == tolower(sPtr[counter]))
    			{
    				storage[counter2]++;
    			}
    		}
    
    	}
    
    
    }
    void displayResults(int storage[])
    {
    	cout << "Number of occurrences of each letter of the alphabet: \n";
    	for(int i = 0; i < ALPHABET; i++)
    	{
    		// prints out all the values in a column of 6
    		cout << (i % 6 == 0 ? '\n' : '\t') << alphabet[i] << ": " << storage[i];
    	}
    
    	cout << "\n";
    
    }
    
    ok this program used to work fine, the only problem is i had too many global variables (storage, counter, counter2) so i made them local and pass them to the function

    the thing is, the program works and shows correct stats but it CRASHES for some reason, i dont know why (crashes right after it shows the stats)

    it also gives me this warning

    Code:
    108) : warning C4018: '<' : signed/unsigned mismatch
    

    help me out, thx :hsd:
     
  2. skinjob

    skinjob Active Member

    Joined:
    Jan 6, 2001
    Messages:
    2,337
    Likes Received:
    0
    Location:
    Aztlán
    At the end of readInput(), you have:

    //delete the memory and set pointer to null
    delete [] usrInput;
    *usrInput = 0;

    You're not setting that pointer to NULL, you're trying to access memory you just deleted. And, what's the point of setting that pointer to NULL when the function returns at that point?
     
  3. D1G1T4L

    D1G1T4L Active Member

    Joined:
    May 4, 2001
    Messages:
    16,489
    Likes Received:
    0
    Location:
    Bay Area

    err didnt notice that but thats not the reason why the program crashes! i still dont get why it crashes!


    btw, what do you mean whats the point of setting that pointer to null when the function returns at that point? when a function exits, the pointers get set to null automatically? (sorry i am a pointer n00b)
     
  4. Krakerjak

    Krakerjak Active Member

    Joined:
    Jul 7, 2003
    Messages:
    8,288
    Likes Received:
    0
    Location:
    Edmonton eh
    Tested it.

    Works fine when you only input 1 line.
    2 or more lines crashes it.

    That kinda narrows down the problem for you.
     
  5. skinjob

    skinjob Active Member

    Joined:
    Jan 6, 2001
    Messages:
    2,337
    Likes Received:
    0
    Location:
    Aztlán
    I compiled your code using MS VC++, and commenting out the *usrInput = 0; line got rid of the crash.

    Your function ends after you delete the buffer. There is no purpose to setting that pointer to NULL since that pointer goes away when the function returns.
     
  6. D1G1T4L

    D1G1T4L Active Member

    Joined:
    May 4, 2001
    Messages:
    16,489
    Likes Received:
    0
    Location:
    Bay Area

    well first of all i set *usrInput = NULL; now (just in case?) it still crashes, i'll try removing it and see if it works, but that would be weird cause thats what i had before (with global variables) and it didnt crash
     
  7. skinjob

    skinjob Active Member

    Joined:
    Jan 6, 2001
    Messages:
    2,337
    Likes Received:
    0
    Location:
    Aztlán
    The state of the call stack has changed since you made some variables local instead of global. Something on the call stack is being corrupted, but whatever was being corrupted before didn't matter to the well-being of the program.
     
  8. skinjob

    skinjob Active Member

    Joined:
    Jan 6, 2001
    Messages:
    2,337
    Likes Received:
    0
    Location:
    Aztlán
    usrInput = 0; sets the pointer to point to NULL
    *usrInput = 0; sets the memory pointed to by usrInput to 0;

    since the memory pointed to by usrInput is no longer valid since you deleted it, it's bound to cause a problem.
     
  9. D1G1T4L

    D1G1T4L Active Member

    Joined:
    May 4, 2001
    Messages:
    16,489
    Likes Received:
    0
    Location:
    Bay Area
    ok well i just tried commenting out //usrInput = 0 and leaving usrInput = 0;

    it still crashes, weird... dont know why


    btw can i do usrInput = NULL; instead of 0?


    EDIT: i just found out it doesnt crash if i just include 1 letter, if i type more than 1 it crashes
     
  10. samm

    samm Next in Line

    Joined:
    Dec 22, 2000
    Messages:
    2,630
    Likes Received:
    0
    Location:
    San Jose, CA
    Your program worked fine for me:

    Code:
    Cochrane:~/Desktop samm$ g++ -o test test.cpp 
    Cochrane:~/Desktop samm$ gdb ./test
    GNU gdb 5.3-20030128 (Apple version gdb-330.1) (Fri Jul 16 21:42:28 GMT 2004)
    Copyright 2003 Free Software Foundation, Inc.
    GDB is free software, covered by the GNU General Public License, and you are
    welcome to change it and/or distribute copies of it under certain conditions.
    Type "show copying" to see the conditions.
    There is absolutely no warranty for GDB.  Type "show warranty" for details.
    This GDB was configured as "powerpc-apple-darwin".
    Reading symbols for shared libraries .. done
    (gdb) run
    Starting program: /Users/samm/Desktop/test 
    
    How many lines of input is there going to be?
    (All combined input can only contain up to 1024 characters): 3
    
    Enter a sentence: asdflkjaw;elkfja;lskdfj
    
    Enter a sentence: werfhapsioudfhwenfa;sfd
    
    Enter a sentence: asjkdfjh;whef;kjahd;kfjasd;fkasdfkjwe
    Number of occurrences of each letter of the alphabet: 
    
    a: 9    b: 0    c: 0    d: 8    e: 5    f: 12
    g: 0    h: 5    i: 1    j: 8    k: 8    l: 3
    m: 0    n: 1    o: 1    p: 1    q: 0    r: 1
    s: 7    t: 0    u: 1    v: 0    w: 5    x: 0
    y: 0    z: 0
    
    Program exited normally.
    (gdb) 
    
     
  11. samm

    samm Next in Line

    Joined:
    Dec 22, 2000
    Messages:
    2,630
    Likes Received:
    0
    Location:
    San Jose, CA
    NULL is the same as 0.
     
  12. turbo91

    turbo91 New Member

    Joined:
    Jun 14, 2002
    Messages:
    2,521
    Likes Received:
    0
    Location:
    San Diego, CA
    I can't get it to crash. Can you give me an example of input you are using that makes it crash?

    Btw, it's time to learn how to use a debugger.
     
  13. D1G1T4L

    D1G1T4L Active Member

    Joined:
    May 4, 2001
    Messages:
    16,489
    Likes Received:
    0
    Location:
    Bay Area

    i dont know maybe its something with my compiler? (using visual studio .net)
    it just randomly crushes, give me details on how to use the debugger? when i run it in debugging mode, it shows some break/continue stuff
     
  14. turbo91

    turbo91 New Member

    Joined:
    Jun 14, 2002
    Messages:
    2,521
    Likes Received:
    0
    Location:
    San Diego, CA
    Ok, I'm on the mac...lemme fire up the windows box.

    Basically, when you run it through the debugger, it should stop when the crash occurs and allow you to inspect things like the call stack, memory, etc.
     
  15. D1G1T4L

    D1G1T4L Active Member

    Joined:
    May 4, 2001
    Messages:
    16,489
    Likes Received:
    0
    Location:
    Bay Area

    do you by any chance have a link where i can read more about that?
     
  16. Joe_Cool

    Joe_Cool Never trust a woman or a government. Moderator

    Joined:
    Jun 30, 2003
    Messages:
    299,191
    Likes Received:
    509
    I couldn't crash it either. I used upper & lower case, punctuation, and did 1, 2, 3, and 4 lines of input.

    :dunno:
     
  17. D1G1T4L

    D1G1T4L Active Member

    Joined:
    May 4, 2001
    Messages:
    16,489
    Likes Received:
    0
    Location:
    Bay Area
    weird! heh, something with my laptop's os or compiler then
     
  18. turbo91

    turbo91 New Member

    Joined:
    Jun 14, 2002
    Messages:
    2,521
    Likes Received:
    0
    Location:
    San Diego, CA
    Ok, found the problem:

    You aren't initializing the memory allocated to "sentence." The initial memory is just going to be whatever junk happened to be there at the time. In debug mode, Visual Studio actually prefills the memory with the byte sequence "CD" to denote cleared memory--but this doesn't mean it's initialized...anyway.

    sentence is full of junk. In your readInput() function you do the following:

    Code:
    for (i=0; i<nLines; i+)
    {
    .
    .
    .
       usrInput[INPUT]='\0';
       strcat(sentence, usrInput);
    }
    
    The issue lies with your call to strcat() -- which APPENDS data. The strcat() function works on the principle that the destination has been null terminated at some point. It searches for the null terminator and appends another null terminated piece of data to it. Since sentence was never initialized, strcat just keeps searching through memory until it finds a null. In this case, it has overrun the amount of memory allocated to it and fucked up the stack. Your solution is to initialize sentence before using it. One method of accomplishing this is by using memset():

    memset(sentence, 0, sizeof(sentence));


    Let me know if you understand this...I can try to explain it better if you don't.
     
  19. turbo91

    turbo91 New Member

    Joined:
    Jun 14, 2002
    Messages:
    2,521
    Likes Received:
    0
    Location:
    San Diego, CA
    Well, it doesn't crash for me on Windows when I compiled an unchecked (non-debug) version.

    edit: The debug reference below is only in reference to Visual Studio. Other development environments / platforms do debugging differently.

    In debug mode, things are done differently. For one, all memory allocated in debug mode is marked with the byte sequence "CD" so that as you step through your program you can see which pieces of memory have been touched. Because of this, your variable "sentence" was full of the "CD" sequence--and had no NULLs...so strcat went right past the boundary.

    In a release/unchecked/non-debug build, the initial memory is what it is. Chances are it happens to be something with a NULL, which is preventing strcat from overrunning the allocation and romping at will with the stack.
     
  20. MrMan

    MrMan New Member

    Joined:
    Jul 13, 2004
    Messages:
    308
    Likes Received:
    0
    For signed/unsigned mismatch, you're comparing counter < strlen(sPtr)
    for(counter = 0; counter < strlen(sPtr); counter++)
    where counter is an integer (which is signed) and strlen is an unsigned long.

    One way is to cast one to make the two types equal.
    for(counter = 0; counter < (signed)strlen(sPtr); counter++)
    Here, we cast strlen to be signed.

    That removes the warning, but not the error.
    I believe that your error lies in the line:
    strcat(sentence, usrInput);

    let's look at some of your code:
    const int INPUT = 1024; // line 17
    char sentence[INPUT]; // line 26
    char *usrInput; // line 50
    usrInput = new char[INPUT+1]; // kine 57

    strcat(sentence, usrInput); // line 64

    What are you doing here... you're concatenating sentence to usrInput. But what is sentence?
    sentence is junk. Because you never initialized it.
    change
    char sentence[INPUT]; // line 26
    to
    char sentence[INPUT] = {0};

    By the way, I did not know this by looking at the code. I used Visual Studio debugger. I set breakpoints before
    strcat(sentence, usrInput);
    and after. What the debugger will tell you is what each variable contains before it reach a certain part of the code, and what the variable is after. If we did not initialize sentence, it looked something like this:
    sentence: "ÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍÍ"
    which basically means garbage to us, or in this case, uninitialized.

    I would advise you learn at least the basics of using the debugger. Can save lots of time and headache.


    Edit: ahh, someone beat me to it. lol. I write too much.
     
  21. D1G1T4L

    D1G1T4L Active Member

    Joined:
    May 4, 2001
    Messages:
    16,489
    Likes Received:
    0
    Location:
    Bay Area

    totally understand it except i never used memset before
    is it equivalent of me setting every element of sentence to 0?
     
  22. D1G1T4L

    D1G1T4L Active Member

    Joined:
    May 4, 2001
    Messages:
    16,489
    Likes Received:
    0
    Location:
    Bay Area
    err thx, but whenever i use debugger, it shows random stuff i dont understand :o
     
  23. D1G1T4L

    D1G1T4L Active Member

    Joined:
    May 4, 2001
    Messages:
    16,489
    Likes Received:
    0
    Location:
    Bay Area
    damn no wonder it worked before, i used to have char sentence[INPUT] = {0}; thats why it worked, must have removed = {0} somehow =(
     

Share This Page