Array error somewhere

[cyberCLoWn]

Compile the source and then type the following in:

+1007 ( read a )
+1008 ( read b )
+2007 ( load a )
+3008 ( add b )
+2109 ( store c )
+1109 ( write c )
+4300 ( halt )

Or even simpler:

+1099 ( read into 99 )
+1199 ( write from 99 )

It works correctly the first time it's run. However, run it a second time around and you'll see from the memory dump that there's a problem somewhere. However the answer is correct. I honestly don't have a clue where the problem could lie and believe me I've looked. Also, any code suggestions are welcome.

/*
#################################################################################
## Simpletron! Simulator
#################################################################################
##
## Written by    : CC
## Email         : [email protected]
## Last modified : 25/02/2004
## Description   : Allows the user to execute code written in SML language. A
##                 memory dump is given if an error occurs. New SML instructions
##                 can be coded. Many checks are run to ensure proper running of
##                 programs.
#################################################################################
## TODO List (in order of priority)
#################################################################################
##  
##   Finish Mem Dump              [ ]
##   Add new features             [ ] 
##   Code programs                [ ] 
##   Write documentation          [ ] 
#################################################################################
*/

#include <iostream>
using std::cout;
using std::cin;
using std::endl;
using std::right;
using std::left;

#include <iomanip>
using std::setw;
using std::setfill;
using std::showpos;
using std::noshowpos;

// Prototypes
void loadProgram( int[] );
void executeProgram( int *const, int *const, int *const, int *const, int *const, int [] );
void accumulatorCheck( const int *const , int *const );
void dump( const int *const, const int *const, const int *const, const int *const, const int *const, int [] );

#define MEMSIZE 100

int main()
{
   // Single subscripted array used for the memory          
   int memory[ MEMSIZE ] = { 0 };
   int accumulator = 0;
   // Keeps track of location in memory that contains current instruction
   int counter;
   // What operation to do (two left most digits)
   int operationCode;
   // Memory location of operation (two right most digits)
   int operand;
   // Copy of instruction from memory of what to do
   int instructionRegister;
   int again = 0;
   
   cout << "*** Welcome to Simpletron! ***\n\n"
        << "*** Please enter your program one instruction ***\n"
        << "*** (or data word) at a time. I will type the ***\n"
        << "*** location number and a quesion mark (?).   ***\n"
        << "*** You then type the word for that location. ***\n"
        << "*** Type the sentinel -99999 to stop entering ***\n"
        << "*** your program. Don't forget to HALT!       ***\n\n"; 

   // Load user's program into Simpletron's memory
   loadProgram( memory );
   
   while( again == 0 || again == 1 || again == 2 )
   {
      counter = 0;
      operationCode = 0;
      instructionRegister = 0;
      
      if( again == 1 )
         system( "cls" );
        
      if( again == 2 )
      {
         // Resets memory on new run
         for( int i = 0; i < MEMSIZE; i++)
            memory[ i ] = 0;
         system( "cls" );
         loadProgram( memory );
      }
      
      if( again == 1 )
         cout << "\n*** Program execution begins  ***\n";
      else
         cout << "*** Program execution begins  ***\n";
      
      // Execute all instructions until the halt instruction is found (or an error occurs)
      while( (operationCode != 43) && (operationCode != 50) )
      {
          executeProgram( &instructionRegister, &operationCode, &operand, &counter, &accumulator, memory );
          // Ensures accumulator stays between -9999 and 9999
          accumulatorCheck( &accumulator, &operationCode );
          ++counter;
      }

      if( operationCode == 50 )
      {            
         dump( &accumulator, &counter, &instructionRegister, &operationCode, &operand, memory ); // Computer dump
         // Reset accumulator
         accumulator = 0;  
      }
      
      dump( &accumulator, &counter, &instructionRegister, &operationCode, &operand, memory );
      cout << "\n[1 to rerun program; 2 to input new program; 3 to quit] ";
      cin >> again;
   }
   
   return 0; // indicates successful termination
}

void loadProgram( int mem[] )
{
   int location = 0;
     
   for( int i = 0; i < MEMSIZE - 1; i++)
   {
      cout << setw( 2 ) << right << noshowpos << setfill( '0' ) << i << " ? ";
      cin >> mem[ location ];
      
      while( (mem[ location ] < -9999) || (mem[ location ] > 9999) )
      {
         if( mem[ location ] == -99999 )
            break;
                        
         cout << "Invalid data input!\n"
              << setw( 2 ) << i << " ? ";
         cin >> mem[ location ];
      }
      
      if( mem[ location ] == -99999 )
         break;
         
      location++;
   }
   
   cout << "\n*** Program loading completed ***\n"; 
}

void executeProgram( int *const instructionRegister, int *const operationCode, int *const operand, int *const counter, int *const accumulator, int memory[] )
{
   *instructionRegister = memory[ *counter ];
   *operationCode = *instructionRegister / MEMSIZE;
   *operand = *instructionRegister % MEMSIZE;
   
   switch( *operationCode )
   {
      // Case 0 is placed here so no error is given when variables are specified mid-program
      case 0:
                  break;
      // Outputs a new line
      case 1:
                  cout << '\n';
                  break;
      // Read a word from the keyboard            
      case 10:
                  cin >> memory[ *operand ];
                  break;
      // Write a word from memory location
      case 11: 
                  cout << noshowpos << memory[ *operand ];
                  break;
   //--------------------------------------------------\\
      // Load word into accumulator
      case 20:
                  *accumulator = memory[ *operand ];
                  break;
      // Store a word from accumulator into memory
      case 21: 
                  memory[ *operand ] = *accumulator;
                  break;
   //--------------------------------------------------\\
      // addition
      case 30:
                  *accumulator += memory[ *operand ];
                  break;
      // subtraction            
      case 31:    
                  *accumulator -= memory[ *operand ];
                  break;
      // division with check for divide by zero
      case 32: 
                  if( memory[ *operand ] == 0 )
                  {  
                     cout << "\n*** Attempt to divide by zero ***\n"
                          << "*** Simpletron execution abnormally terminated ***";
                     *operationCode = 50;
                  }
                  else                            
                     *accumulator /= memory[ *operand ];
                  break;     
      // multiplication                      
      case 33:
                  *accumulator *= memory[ *operand ];
                  break;
   //--------------------------------------------------\\
      // branch to memory location
      case 40:
                  *counter = *operand - 1; // We -1 from the operand as the loop in main always increases int counter by one
                  break;
      // branch to mem location if negative
      case 41:
                  if( *accumulator < 0 )
                     *counter = *operand - 1;
                  break;
      // branch to mem location if accumulator is negative
      case 42:
                  if( *accumulator == 0 )
                     *counter = *operand - 1;
                  break;
      // halt            
      case 43:
                  cout << "\n*** Simpletron execution terminated ***\n";
                  break;
      default:
                  cout << "\n*** Error in code ***\n"
                       << "*** Simpletron execution abnormally terminated ***";
                  *operationCode = 50;  
   }
}

void accumulatorCheck( const int *const accumulator, int *const code )
{
   if( (*accumulator < -9999) || (*accumulator > 9999) )
   {
      cout << "*** Incorrect accumulator size ***\n"
           << "*** Simpletron execution abnormally terminated ***";
      *code = 50;
   }
}

void dump( const int *const accumulator, const int *const counter, const int *const instructionRegister, const int *const operationCode, const int *const operand, int mem[] )
{
   cout << "\n\nREGISTERS:\n";
   
   if( *accumulator > 0 )
      cout << setw( 4 ) << setfill( '0' ) << showpos << "accumulator\t\t" << *accumulator;
   else if( *accumulator == 0 )
      cout << "accumulator\t\t" << "+0000";
   else 
      cout << "accumulator\t\t" << *accumulator;
   
   cout << "\ncounter\t\t\t" << setfill( '0' ) << setw( 2 ) << noshowpos << (*counter - 1) // counter - 1 to take a step back to the problem
        << "\ninstructionRegister\t" << showpos << *instructionRegister
        << "\noperationCode\t\t" << setw( 2 ) << noshowpos << (*instructionRegister / MEMSIZE)
        << "\noperand\t\t\t" << setw( 2 ) << *operand << "\n\n";
        
   cout << "MEMORY:" << endl;
        
    // Display column header
    cout << "  ";
    for (int col = 0; col < 10; col++)
        cout<< setfill( ' ' ) << setw(7) << col;
    cout << '\n';
    
    // Display row: header-data-newline
    for (int row = 0; row < 10; row++) 
    {
       cout << setw(2) << right << noshowpos << setfill(' ') << row << "  ";
       for (int col = 0; col < 10; col++) 
       {
          static int number = 0;
          
          if( mem[ number ] == -99999 )
             mem[ number ] = 0;
             
          cout << setw(5) << left << showpos << setfill('0') << mem[ number ] << "  ";
          number++;
       }
       
       cout << '\n';
    }
}

I feel like I'm actually getting somewhere in this C++ world!

[ripper079]

Thats a lots of code to look through. My best advice would be to use a debugger and follow the code until the error occurs.

[Hammer]

My debugger says:

Program received signal SIGSEGV, Segmentation fault.
0x00402090 in dump(int const*, int const*, int const*, int const*, int const*, i
nt*) (accumulator=0x22ed84, counter=0x22ed80, instructionRegister=0x22ed74,
    operationCode=0x22ed7c, operand=0x22ed78, mem=0x22ed88) at junk1.cpp:275
275               if( mem[ number ] == -99999 )
(gdb) print number
$1 = 2206

The operation on the highlighted line caused the crash. number contains a value greater than the array size, and hence you go out of bounds.

Now all you have to do is find out why.

[cyberCLoWn]

I think I've found the problem. Amazing how things come to you in the shower

       for (int col = 0; col < 10; col++) 
       {
          static int number = 0;
          
          if( number >= MEMSIZE )
            number = 0;
          
          if( mem[ number ] == -99999 )
             mem[ number ] = 0;
             
          cout << setw(5) << left << showpos << setfill('0') << mem[ number ] << "  ";
          number++;
       }

I added that check in. What happens was on any second run of the program, a dump would be made, but int number is static and would remain at 99. So when the program ran again it would reference numbers outside of the array.

If the program does experience '-99999' in the array it will cause an error as it doesn't know what to do with it (yet). Therefore I stated that the halt command (+4300) should be used.