• Advertisement
Sign in to follow this  

Strange C++ Problem

This topic is 2260 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

Hiya,

I have some code doing something very strange, I was hoping someone might know what's going on!

The method in question :

const big_integer big_integer::operator-(const big_integer& rhs) const
{
return big_integer(*this) -= rhs;
}

// (defined in terms of)
big_integer& big_integer::operator-=(const big_integer& rhs)
{
// Snip...
}


As far as I can see, this should call:

  • the copy constructor, to make an unnamed copy of *this.
  • The compound subtraction operator.
  • ...and then return.


    For some reason instead of returning, the copy-constructor is called again. It is passed an invalid value (0x10) as a reference, which causes a segmentation fault. I can't seem to get any useful information out of gcc or gdb.


    If I make the following changes, it executes without problem.

    const big_integer big_integer::operator-(const big_integer& rhs) const
    {
    big_integer result(*this);
    result -= rhs;
    return result; // All OK.
    }



    Does anyone know why this is happening? I'm utterly confused.

    Any help is appreciated, thanks.

Share this post


Link to post
Share on other sites
Advertisement

I think the extra copy constructor call has something to do with the fact that your operator- returns a big_integer, while your operator-= returns a reference to a big_integer.


I'm not sure...

The only big_integer object that needs to be created is the copy of *this. As you say, the compound assignment returns a reference so it shouldn't be creating anything. The strange call to the copy-constructor to create a new big_integer occurs after operator-= has returned...

Thanks :)

Share this post


Link to post
Share on other sites
Hidden
That solution seems counter intuative, try defining "-=" in terms of "-"


const big_integer big_integer::operator-(const big_integer& rhs) const
{
big_integer result

return big_integer(*this) -= rhs;
}

// (defined in terms of)
big_integer& big_integer::operator-=(const big_integer& rhs)
{

}

Share this post


Link to post
This solution seems counter-intuitive. Perhaps you should try putting the subtraction logic in the subtraction operator and having the subtraction-assignment operator use the subtraction operator then assign the result. That would make this easier to debug and would make your logic easier to follow.


const big_integer big_integer::operator-(const big_integer& rhs) const
{
big_integer result;

// Subtraction logic

return result;
}

big_integer& big_integer::operator-=(const big_integer& rhs)
{
*this = *this-rhs;

return *this;
}

Share this post


Link to post
Share on other sites



I think you might be right.

I've made the changes you suggest. Thanks :)

Share this post


Link to post
Share on other sites
[font="Courier New"]return ...[/font] will call the copy constructor again because you are passing the result by value.

This is the expected order of execution, which VC9 demonstrates.

[font="Courier New"]operator -
copy constructor
operator -=
copy constructor

[/font]I couldn't reproduce the crash, so either you're doing something funky in operator -= that you're not showing us, or the problem lies in your driver program.

FYI, returning a const [font="Courier New"]big_integer[/font] by value is technically meaningless. If it were a reference or pointer then it would make sense.

Share this post


Link to post
Share on other sites

[font="Courier New"]return ...[/font] will call the copy constructor again because you are passing the result by value.

This is the expected order of execution, which VC9 demonstrates.

[font="Courier New"]operator -
copy constructor
operator -=
copy constructor

[/font]I couldn't reproduce the crash, so either you're doing something funky in operator -= that you're not showing us, or the problem lies in your driver program.


You're right, thanks.

I'm calling a single function in operator -=, which seems fine. The problem must be in the caller of operator -.


bool srp_ephemeral::is_valid() const
{
if(!_n.is_safe_prime())
return false;

/* Check that g is a generator of GF(n). */
if(_g.exp_mod((_n - big_integer::ONE()) / big_integer::TWO(), _n) + big_integer::ONE() != _n)
return false;

return true;
}


Anyways, I'll see if there's anything I can spot.


FYI, returning a const [font="Courier New"]big_integer[/font] by value is technically meaningless. If it were a reference or pointer then it would make sense.


Habit... It stops being doing things like:

big_integer a, b, c;
(a + b) = c;

By accident.

Thanks for your help, appreciated.

Share this post


Link to post
Share on other sites

[quote name='_Sauce_' timestamp='1323871956' post='4893842']
FYI, returning a const [font="Courier New"]big_integer[/font] by value is technically meaningless. If it were a reference or pointer then it would make sense.


Habit... It stops being doing things like:

big_integer a, b, c;
(a + b) = c;

By accident.
[/quote]

I've never seen const used this way but that's a fair point.

I still feel like I'm not seeing the actual code responsible for your crash so I'm afraid I have no further suggestions. I'll be interested to hear what the cause was if you figure it out.

Share this post


Link to post
Share on other sites
I'll be sure to say if I find it.

Oddly, I can plug the big_integer class into an empty driver and still cause a seg fault.


int main(int argc, char **argv)
{
big_integer a(7);
big_integer b(5);

big_integer c = a - b; // Crash.
}


The call stack shows:


> ctor ( 7 ) // initialise a
> ctor ( 5 ) // initialise b
> operator - ( b ) // a - b
> copy-ctor // initialise unnamed big_integer
> operator -= // actually do the subtraction
> copy-ctor // initialise the returned object - the reference parameter is 0x10, followed immediately by a segmentation fault.


Eh.

Share this post


Link to post
Share on other sites
You're more than welcome.

Source

Compiles with g++ 4.4.5.

Links with libgmp.a, libboost_thread.a and posix threads.

Thanks.

Share this post


Link to post
Share on other sites
Well - I made a quick port to VS2010 and found the problem.

I assume there's a reason the gnu compiler allows this, even with -Wall...


big_integer& big_integer::operator-=(const big_integer& rhs)
{
mpz_sub(_value, _value, rhs._value);
}


The method is not returning anything. I'm amazed it compiles, but I feel like a bit of an idiot to be honest.

Thanks very much for your help.

Share this post


Link to post
Share on other sites
Hmm...

$ g++ --version
g++ (Ubuntu/Linaro 4.6.1-9ubuntu3) 4.6.1
Copyright (C) 2011 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ g++ *.cpp -lboost_thread -lgmp -Wall -Werror && ./a.out
big_integer.cpp: In member function ‘big_integer& big_integer::operator+=(const big_integer&)’:
big_integer.cpp:46:1: error: no return statement in function returning non-void [-Werror=return-type]
big_integer.cpp: In member function ‘big_integer& big_integer::operator-=(const big_integer&)’:
big_integer.cpp:51:1: error: no return statement in function returning non-void [-Werror=return-type]
big_integer.cpp: In member function ‘big_integer& big_integer::operator*=(const big_integer&)’:
big_integer.cpp:56:1: error: no return statement in function returning non-void [-Werror=return-type]
big_integer.cpp: In member function ‘big_integer& big_integer::operator/=(const big_integer&)’:
big_integer.cpp:61:1: error: no return statement in function returning non-void [-Werror=return-type]
big_integer.cpp: In member function ‘big_integer& big_integer::operator%=(const big_integer&)’:
big_integer.cpp:66:1: error: no return statement in function returning non-void [-Werror=return-type]
cc1plus: all warnings being treated as errors
[/quote]

Share this post


Link to post
Share on other sites
Indeed... I guess it's the older version of g++ that is provided with oracle linux.

Share this post


Link to post
Share on other sites

This solution seems counter-intuitive. Perhaps you should try putting the subtraction logic in the subtraction operator and having the subtraction-assignment operator use the subtraction operator then assign the result. That would make this easier to debug and would make your logic easier to follow.


const big_integer big_integer::operator-(const big_integer& rhs) const
{
big_integer result;

// Subtraction logic

return result;
}

big_integer& big_integer::operator-=(const big_integer& rhs)
{
*this = *this-rhs;

return *this;
}

No! That is exactly the wrong thing to do, as anyone who knows much about operator overloading will tell you. Just look at previous forum responses by some of the most talented people on this forum and you'll see that 99.9% of the advice is to do it the way that it was.

Seriously, change it back again.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement