Search code examples
c++stringmemory-managementexc-bad-access

C++ adding one line of code after while loop causes an error


I have tried my best to look for an answer that addresses my specific question but unfortunately, I wasn't able to find one that satisfied my needs.

I am writing an assembler in c++ for a language that I made up. Writing the assembler involves two classes that have been causing problems really tough to debug. My debugging techniques have devolved to adding and removing output stream commands like

std::cout << "TEST";

which end up either breaking or fixing the program. The IDE I am using is Xcode.

The two classes that I am working on are a LinkedList class and a SymbolTable class, which I am using as a hash table for symbol resolution.

LinkedList.h

#include <iostream>
using namespace std;

struct Node{
    string data;
    int address;
    Node* next = nullptr;
};

class LinkedList{
private:
    Node* main_ptr;
    int length;
public:
    LinkedList();

    void insertNode(string, int);
    void deleteNode(string);
    void displayList();

    bool contains(string, int* = nullptr);
    int getLength();

    ~LinkedList();
};

LinkedList.cpp

#include "LinkedList.h"

LinkedList::LinkedList(){
    main_ptr = nullptr;
    length = 0;
}

void LinkedList::insertNode(string data, int address){
    Node* newNode = new Node;
    newNode->data = data;
    newNode->address = address;

    if (!main_ptr){
        main_ptr = newNode;
    } else {
        newNode->next = main_ptr;
        main_ptr = newNode;
    }
    length++;
}

void LinkedList::deleteNode(string data){
    if (main_ptr){
        int current_length = length;
        if (main_ptr->data == data){
            main_ptr = main_ptr->next;
            return;
        }
        Node* q = main_ptr;
        Node* p = q;
        q = q->next;
        while (q){
            if (q->data == data){
                p->next = q->next;
                length--;
                return;
            }
            p = q;
            q = q->next;
        }
        if (current_length == length) cout << "Node was not found" << endl;
    } else {
        cout << "List is empty, cannot delete node!" << endl;
    }
}

void LinkedList::displayList(){
    if (!main_ptr){
        cout << "List is empty!\n";
    } else {
        Node* temp = main_ptr;
        while (temp){
            cout << temp->data;
            temp = temp->next;
        }
        cout << endl;
    }
}

bool LinkedList::contains(string data, int* address){
    if (main_ptr == nullptr) return false;
    else {
        Node* temp = main_ptr;
        while(temp != nullptr){
            if (temp->data == data) {
                address = &(temp->address);
                return true;
            }
            temp = temp->next;
        }
        return false;
    }
}

int LinkedList::getLength(){
    return length;
}

LinkedList::~LinkedList(){
    if (main_ptr){
        Node* q = main_ptr;
        Node* p = q;
        while (q){
            q = q->next;
            delete p;
            p = q;
        }
    }
}

SymbolTable.h

#include "LinkedList.h"
#include <iostream>

class SymbolTable{
private:
    LinkedList table[701];
public:
    SymbolTable();

    void addEntry(string, int);
    void printTable();
    bool contains(string, int*);

    int convertName(string);
};

SymbolTable.cpp

#include "SymbolTable.h"

SymbolTable::SymbolTable(){
}

void SymbolTable::addEntry(string name, int memory){
    int address = convertName(name);
    table[address].insertNode(name, memory);
}

void SymbolTable::printTable(){
    for (int i = 0; i < 701; i++)
        table[i].displayList();
}

bool SymbolTable::contains(string name, int* memory){
    return table[convertName(name)].contains(name, memory);
}

int SymbolTable::convertName(string name){
    int aggregate = 1;
    const char* c_name = name.c_str();
    for (int i = 0; i < name.length(); i++){
        aggregate *= (int)c_name[i];
    }
    return aggregate%701;
}

Now comes my actual question. The following is the main function:

#include "Parser.h"
#include "SymbolTable.h"

using namespace std;

int main(int argc, const char * argv[]) {

    ifstream inputFile;
    ofstream outputFile;

    inputFile.open("/Users/Azaldin/Desktop/nand2tetris/projects/06/max/Max.asm");
    outputFile.open("/Users/Azaldin/Desktop/nand2tetris/projects/06/max/Max.hack");

    Parser a(inputFile);
    SymbolTable s;

    int commandType = -1;
    string command;
    int a_position = 16;
    int currentLine = 0;

    while (a.hasMoreCommands()){
        a.advance();
        commandType = a.commandType();
        command = a.symbol();

        if (commandType == 0){
            if (!s.contains(command, nullptr)){
                s.addEntry(command, a_position);
                a_position++;
            }
        }

        if (commandType == 2){
            //cout << command << endl;
            if (!s.contains(command, nullptr)){
                s.addEntry(command, currentLine);
                //cout << command << endl;
            }
        }

        currentLine++;
    }

    string x;  //<<<<<<<<<< ADDING THIS LINE BREAKS THE PROGRAM
    cout << "Hi";

    inputFile.close();
    outputFile.close();

    return 0;
}

After adding the line pointed to above in the main function, the program breaks. Upon debugging, the following problem arises from the main thread:

Thread 1:EXC_BAD_ACCESS (code = 1, address=0x25fbfef80)

The chain of commands that leads to this problem:

  1. if (!s.contains(command, nullptr)){ from the main function

  2. return table[convertName(name)].contains(name, memory); from the SymbolTable.cpp "contains" function

  3. if (temp->data == data) { from LinkedList.cpp "contains" function

The rest of the chain command leads to the string class == operator, which then leads to the .size() function on the left-hand side.

Thank You!


Solution

  • The following comment by PaulMcKenzie helped solve the problem:

    Also, aggregate *= (int)c_name[i]; -- there is no guarantee that this will be a positive number, since a character may be signed, thus giving you values of -1 and below. Thus your return of return aggregate%701; isn't going to return what you expected (a number >= 0).

    It turns out that one of the arguments passed into the convertName function in the SymbolTable class have caused the program to corrupt the memory. I added an if statement to solve the problem.