0

My program runs on all numbers, but not when calculating BigInt MAX(INT_MAX). When I try to run this, it gives me a segmentation fault. Here is the BigInt class:

#include <iostream>
#include <limits.h>
#include <vector>
#include <string>
#include <algorithm>
using namespace std;
class BigInt
{
    private:
        vector<char> v; 
    public:
        BigInt() 
        {
            v.push_back('0');
        }
        
        BigInt(int num) 
        {
            while (num != 0)
            {
                v.push_back(num % 10);
                num /= 10;
            }
        }
        
        BigInt(string str) //3
        {
            for (int i = str.length() - 1; i >= 0; i--)
                v.push_back(str[i] - '0');
        }
        
        BigInt operator+(BigInt b) 
        {
            BigInt result;
            int carry = 0;
            for (int i = 0; i < max(v.size(), b.v.size()); i++)
            {
                int sum = carry;
                if (i < v.size())
                    sum += v[i];
                if (i < b.v.size())
                    sum += b.v[i];
                result.v.push_back(sum % 10);
                carry = sum / 10;
            }
            if (carry != 0)
                result.v.push_back(carry);
            return result;
        }
        
        BigInt operator++( ) 
        {
            *this = *this + BigInt(1);
            return *this;
        }
        
        BigInt operator++(int) 
        {
            BigInt temp = *this;
            *this = *this + BigInt(1);
            return temp;
        }
        
        BigInt operator*(BigInt b) 
        {
            BigInt result;
            result.v.resize(v.size() + b.v.size());
            
            for (int i = 0; i < v.size(); i++)
                for (int j = 0; j < b.v.size(); j++)
                    result.v[i + j] += v[i] * b.v[j];
            
            for (int i = 0; i < result.v.size() - 1; i++)
            {
                result.v[i + 1] += result.v[i] / 10;
                result.v[i] %= 10;
            }
            
            while (result.v.back() == 0 && result.v.size() > 1)
                result.v.pop_back();
            
            return result;
        }
        
        BigInt operator+=(BigInt b) 
        {
            *this = *this + b;
            return *this;
        }
        
        BigInt half() 
        {
            BigInt result;
            
            int remainder = 0;
            
            for (int i = v.size() - 1; i >= 0; i--)
            {
                int current = remainder * 10 + v[i];
                int digit = current / 2;
                
                if (digit != 0 || !result.v.empty())
                    result.v.push_back(digit);
                
                remainder = current % 2;
            }
            
            if (result.v.empty())
                result.v.push_back(0);
            
            return result;
        }
        
        bool isOdd()
        {
            return v[0] % 2 == 1;
        }
        
        bool isEven()
        {
            return !isOdd();
        }
        
        bool operator==(BigInt b)  
        {
            if (v.size() != b.v.size())
                return false;
            
            for (int i = 0; i < v.size(); i++)
                if (v[i] != b.v[i])
                    return false;
            
            return true;
        }
        
        bool operator<(BigInt b) 
        {
            if (v.size() != b.v.size())
                return v.size() < b.v.size();
            
            for (int i = v.size() - 1; i >= 0; i--)
                if (v[i] != b.v[i])
                    return v[i] < b.v[i];
            
            return false;
        }
        
friend ostream& operator<<(ostream& os, const BigInt& b){
    if (b.v.size() < 9)
    {
        for (auto it = b.v.rbegin(); it != b.v.rend(); ++it)
        {
            os << static_cast<int>(*it);
        }
    }
    else
    {
        os << static_cast<int>(b.v.back()) << ".";
        for (size_t i = 1; i <= 7; ++i)
        {
            os << static_cast<int>(b.v[b.v.size() - i - 1]);
        }
        os << "e" << b.v.size() - 1;
    }
    return os;
    }
};

and here is the driver code:

struct ThreeNp1
{
    BigInt start;
    BigInt steps;
    BigInt max;
    BigInt odd;
    BigInt even;
};
void print(ThreeNp1 temp)
{
    cout << "start:" << temp.start << endl;
    cout << "steps:" << temp.steps << endl;
    cout << "max:" << temp.max << endl;
    cout << "odds:" << temp.odd << endl;
    cout << "evens:" << temp.even << endl;
}
void findThreeNp1(BigInt n, ThreeNp1 &Np1, bool printSteps = false)
{
    if (printSteps)
    {
        cout << "->" << '(' << n << ')';
    }
    if (Np1.max < n) 
        Np1.max = n; 

    if (n == BigInt(1)) 
    {
        return; 
    }
    else if (n.isEven()) 
    {
        Np1.even++; 
        Np1.steps++;
        findThreeNp1(n.half(), Np1, printSteps); 
    }
    else if (n.isOdd()) 
    {
        Np1.odd++;
        Np1.steps++;
        BigInt tempN(n);                                       
        findThreeNp1(tempN * BigInt(3) + BigInt(1), Np1, printSteps); 
    }
    else
        {
            cout << "How the hell did I get here?\n";
            return;
        }
}
int main()
{
    BigInt MAX(INT_MAX);
    cout << "The largest integer is " << MAX << endl;
    cout << "Twice the largest integer is " << MAX + MAX << endl;
    BigInt start(INT_MAX); 
    bool printSteps = false;
    ThreeNp1 N = {start, 0, 0, 0, 0};   
    findThreeNp1(start, N, printSteps);
    cout << endl;
    print(N);
    return 0;
}

I have rewritten this code so many times, I've checked for errors in my logic. I've tried everything I can think of. If anyone sees anything wrong or I could improve to get this thing running, please let me know.

Allan Wind
  • 23,068
  • 5
  • 28
  • 38
  • Removed C tag as it's a C++ question. – Allan Wind May 04 '23 at 05:34
  • 2
    Instead of rewriting your program on faults; run it through a debugger to figure out where it crashes. – Allan Wind May 04 '23 at 05:35
  • @AllanWind I'm new to programing so I'm not sure how to do that – Stephen Martinez May 04 '23 at 05:40
  • For me it crashes in `__gnu_cxx::new_allocator::_M_max_size` as a result of `v.push_back('0');` and it has 18724 stack frames. In C++ you should prefer an iterative solution instead of a recursive one (as I don't believe C++ requires tail call optimization). – Allan Wind May 04 '23 at 05:41
  • 4
    [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) – Jason May 04 '23 at 05:43
  • @AllanWind teach me your ways plz so to fix this problem I would need to change the recursive calls and the v.push_back('0')? – Stephen Martinez May 04 '23 at 05:45
  • @StephenMartinez You're welcome. Using the debugger(in the above mentioned link), you'll be able to find out the problem. There are also youtube tutorials on using a debugger. – Jason May 04 '23 at 05:47
  • @Jason thank you so much, you have no idea how long I've been trying to get this code to work – Stephen Martinez May 04 '23 at 05:49
  • findThreeNp1() is an recursive function as it calls itself. You want to express that as a loop instead. – Allan Wind May 04 '23 at 05:49
  • 1
    Investing the time to use a debugger is well worth your time. It's a deep subject but you get a lot of value from just knowing a few things. – Allan Wind May 04 '23 at 05:49
  • @AllanWind that and unit tests. With unit test you try to think of all possible calls to your code (including wrong input), in fact you should try to break your code. The combination of unit tests + debugger is really powerful. – Pepijn Kramer May 04 '23 at 05:54
  • Side note : unlearn `using namespace std;` (using namespace can have unintended consequences in bigger projects, or even when including windows header files). Just learn to type std::string etc. – Pepijn Kramer May 04 '23 at 05:55
  • @PepijnKramer thank you! I'm taking taking all this into consideration and I'll probably spend the weekend learning it. Thank you again so much – Stephen Martinez May 04 '23 at 05:57
  • It's not specific to INT_MAX, btw, INT_MAX-1 also segfaults. – Allan Wind May 04 '23 at 06:06
  • @StephenMartinez `BigInt half()` -- Maybe you should have implemented `operator /`, and then all `half()` would have been is `{return b / 2};`. You implemented all the other operators, so leaving out `/` doesn't make a lot of sense. Also, `operator %` could have been implemented alongside `/`, since they are related. Then instead of `isOdd` and `isEven`, it is just `b % 2 == 1` and `b % 2 == 0;`, respectively. – PaulMcKenzie May 04 '23 at 06:43
  • BTW, [here is the output when using a BigInt class that works](https://godbolt.org/z/WM4Esdxac) – PaulMcKenzie May 04 '23 at 06:46
  • The default constructor (which accepts no arguments) pushes a digit `'0'` to the vector, but the other constructors push numeric values (`0` to `9` - which are different from digits) to the vector. Given that some of your operator functions (and other members) default construct a `BigInt`, and then push numeric values, there is an inconsistency (your operators and member functions ASSUME that the vector contains numeric values, but the default constructor does not satisfy that assumption). Such inconsistencies tend to result in all sorts of bugs. – Peter May 04 '23 at 07:09
  • BigInt(1) + BigInt(1) probably shouldn't be 20. Write some units tests. – Allan Wind May 04 '23 at 07:55
  • BigInt(99) * BigInt(99) probably shouldn't be 73-61. 9*9+9*9 overflows. – Allan Wind May 04 '23 at 08:25

2 Answers2

0

Your error is here

BigInt() 
{
    v.push_back('0');
}

that should be

BigInt() 
{
    v.push_back(0);
}

because your class stores integers not digits. So zero is 0 not "0".

Specifically your multiplication code starts with result initialised to zero with BigInt result; but because your default constructor does not correctly initialise to zero, the multiplication routine is also bugged and this leads to an infinite recursion and a stack overflow when you try to calculate your Collatz Conjecture series.

A debugger found this error fairly quickly, you should learn how to use one.

UPDATE (corrected)

There's a problem with your half routine. You process the digits of the number you are halving from most significant to least significant. But you push the resulting digits onto the result variable in the same order, most significant first. This contradicts how your class works, which stores the least significant digit first. The result is that after the half operation the digits are correct but stored in the reverse order. There also an extra zero digit that gets included because of the way that the default constructor works.

Probably the default constructor should leave the vector empty. Printing a zero BigInt as "0" is a special case that should be handled in the print routine, not in the calculation routines. This will simplify the code. For example BigInt x; and BigInt y(0); result in different internal vectors. This is also a bug.

john
  • 85,011
  • 4
  • 57
  • 81
0

The segfault is due to the recursive function findThreeNp1() exhausting its stack space, and I/@john attribute the root cause to the following defects:

  1. The default constructor should leave vector<...> a empty. Otherwise BigInt(1) + BigInt(1) becomes 20. This requires a special case for when vector.size() == 0 in operator<< and isOdd().

  2. vector<char> v means it's implemented defined if that is signed or unsigned. On my system it's signed which means 99 * 99 triggers an overflow. Change the type to unsigned char.

  3. half() doesn't work. BigInt(20).half() shouldn't be 01. I rewrote it and tested it minimally.

Here is the resulting program:

#include <iostream>
#include <limits.h>
#include <vector>
#include <string>
#include <algorithm>

using namespace std;
class BigInt
{
    private:
        // stores numbers in reverse order, i.e. 123 as [3, 2, 1]
        vector<unsigned char> v;
    public:
        BigInt() {
        }

        BigInt(int num) {
            for(; num; num /= 10)
                v.push_back(num % 10);
        }

        BigInt operator+(BigInt b) {
            BigInt result;
            int carry = 0;
            int n = max(v.size(), b.v.size());
            for (int i = 0; i < n; i++) {
                int sum = carry;
                if (i < v.size())
                    sum += v[i];
                if (i < b.v.size())
                    sum += b.v[i];
                result.v.push_back(sum % 10);
                carry = sum / 10;
            }
            if (carry)
                result.v.push_back(carry);
            return result;
        }

        BigInt operator++() {
            *this = *this + BigInt(1);
            return *this;
        }

        BigInt operator++(int) {
            BigInt temp = *this;
            *this = *this + BigInt(1);
            return temp;
        }

        BigInt operator*(BigInt b) {
            BigInt result;
            result.v.resize(v.size() + b.v.size());
            for (int i = 0; i < v.size(); i++)
                for (int j = 0; j < b.v.size(); j++)
                    result.v[i + j] += v[i] * b.v[j];

            for (int i = 0; i < result.v.size() - 1; i++) {
                result.v[i + 1] += result.v[i] / 10;
                result.v[i] %= 10;
            }

            while (result.v.back() == 0 && result.v.size() > 1)
                result.v.pop_back();

            return result;
        }

        BigInt operator+=(const BigInt &b) {
            *this = *this + b;
            return *this;
        }

        BigInt half() {
            BigInt result;
            for(int i = 0; i < v.size(); i++) {
                // losing the most significant digit
                if(i + 1 != v.size() || v[i] / 2)
                    result.v.push_back((v[i]) / 2);
                // truncate least significant digit, i.e. 3/2 = 1
                if(i && (v[i] % 2))
                    result.v[i-1] += 5;
            }
            return result;
        }

        bool isOdd() {
            return v.size() && (v[0] % 2 == 1);
        }

        bool isEven() {
            return !isOdd();
        }

        bool operator==(BigInt b) {
            if (v.size() != b.v.size())
                return false;

            for (int i = 0; i < v.size(); i++)
                if (v[i] != b.v[i])
                    return false;

            return true;
        }

        bool operator<(BigInt b) {
            if (v.size() != b.v.size())
                return v.size() < b.v.size();

            for (int i = v.size() - 1; i >= 0; i--)
                if (v[i] != b.v[i])
                    return v[i] < b.v[i];

            return false;
        }

        friend ostream& operator<<(ostream& os, const BigInt& b){
            if(!b.v.size()) {
                os << 0;
            } else if (b.v.size() < 9) {
                for (auto it = b.v.rbegin(); it != b.v.rend(); ++it)
                    os << static_cast<int>(*it);
            } else {
                os << static_cast<int>(b.v.back()) << ".";
                for (size_t i = 1; i <= 7; ++i) {
                    os << static_cast<int>(b.v[b.v.size() - i - 1]);
                }
                os << "e" << b.v.size() - 1;
            }
            return os;
        }
};

struct ThreeNp1 {
    BigInt start;
    BigInt steps;
    BigInt max;
    BigInt odd;
    BigInt even;
};

void print(ThreeNp1 temp) {
    cout << "start:" << temp.start << endl;
    cout << "steps:" << temp.steps << endl;
    cout << "max:" << temp.max << endl;
    cout << "odds:" << temp.odd << endl;
    cout << "evens:" << temp.even << endl;
}

void findThreeNp1(BigInt n, ThreeNp1 &Np1, bool printSteps = false) {
    if (printSteps)
    {
        cout << "->" << '(' << n << ')';
    }
    if (Np1.max < n)
        Np1.max = n;

    if (n == BigInt(1)) {
        return;
    } else if (n.isEven()) {
        ++Np1.even;
        ++Np1.steps;
        findThreeNp1(n.half(), Np1, printSteps);
    }
    else if (n.isOdd()) {
        ++Np1.odd;
        ++Np1.steps;
        BigInt tempN(n);
        findThreeNp1(tempN * BigInt(3) + BigInt(1), Np1, printSteps);
    } else {
        cout << "How the hell did I get here?\n";
        return;
    }
}

int main(int argc, char **argv) {
    BigInt MAX(INT_MAX);
    cout << "The largest integer is " << MAX << endl;
    cout << "Twice the largest integer is " << MAX + MAX << endl;
    BigInt start(INT_MAX);
    bool printSteps = false;
    ThreeNp1 N = {start, 0, 0, 0, 0};   
    findThreeNp1(start, N, printSteps);
    cout << endl;
    print(N);
    return 0;
}

and the output is:

$ time ./a.out
The largest integer is 2.1474836e9
Twice the largest integer is 4.2949672e9

start:2.1474836e9
steps:450
max:1.2353467e15
odds:162
evens:288

real    0m0.015s
user    0m0.010s
sys     0m0.005s

Here is the iterative version of findThreeNp1(). It doesn't make much difference once the other issues are fixed (it's about ~2.5% faster on 10k external runs):

void findThreeNp1(BigInt n, ThreeNp1 &Np1, bool printSteps = false) {
    while(!(n == BigInt(1))) {
        if (printSteps)
            cout << "->" << '(' << n << ')';
        if (Np1.max < n) 
            Np1.max = n;
        ++Np1.steps;
        if (n.isEven()) {
            ++Np1.even; 
            n = n.half();
        } else {
            ++Np1.odd;
            n = BigInt(3) * n + BigInt(1);
        }
    }
}

Np1.steps == Np1.odd + Np1.even so we only need two of those variables.

Allan Wind
  • 23,068
  • 5
  • 28
  • 38
  • That part was not written by me but by my professor, the only thing I have written is the BigInt stuff but I agree that it's bad haha – Stephen Martinez May 04 '23 at 06:16
  • I will leave this up till I get the first down-vote. It's been running for a long time but not crashed. @john will probably square you away with the bug in your code. – Allan Wind May 04 '23 at 06:18
  • 1
    @AllanWind The code is testing the [Collatz Conjecture](https://en.wikipedia.org/wiki/Collatz_conjecture) – john May 04 '23 at 06:19
  • @AllanWind if you want you can keep it running but some ppl had it running for hours before they got an answer – Stephen Martinez May 04 '23 at 06:24
  • Some people (possible me) are doing it wrong if it takes hours to run with INT_MAX as input. I did validate that I was getting the correct result for the two exampled listed in the article that @john pointed me to. – Allan Wind May 04 '23 at 09:36
  • @AllanWind thank you so much brother, I wish there was a way you could mentor me so I can become as good as you one day. Thank you so much – Stephen Martinez May 04 '23 at 19:02