Opened 9 years ago

Closed 9 years ago

#8667 closed Bugs (fixed)

gmp_int wrongly assumes m_data is initialised on copy assignment

Reported by: andrea.bedini@… Owned by: John Maddock
Milestone: To Be Determined Component: multiprecision
Version: Boost 1.53.0 Severity: Problem
Keywords: Cc: john@…

Description

Also reported on github, I can't link it because otherwise trac thinks my report is spam.

... but m_data might be left uninitialised after a move and gmp will segfaults. Here is a test case.

#include <boost/multiprecision/gmp.hpp> 
#include <vector>

using boost::multiprecision::mpz_int;

int main(int argc, char const *argv[])
{
    std::vector<mpz_int> elements_(10, 123);
    elements_.resize(1); // this leaves garbage after the end
    mpz_int e = 321;
    elements_.insert(elements_.begin(), e);
    return 0;
}

Compiled with libc++

clang++ -g -O0 -std=c++11 -stdlib=libc++ -I$BOOST_ROOT -lgmp test.cpp -o test will segfaults on the insert.

* thread #1: tid = 0x1c03, 0x0000000100051ae4 libgmp.10.dylib`__gmpn_copyi + 548, stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x0000000100051ae4 libgmp.10.dylib`__gmpn_copyi + 548
    frame #1: 0x0000000100020e1b libgmp.10.dylib`__gmpz_set + 55
    frame #2: 0x00000001000033f7 test`boost::multiprecision::backends::gmp_int::operator=(this=0x00000001001039e0, o=0x00007fff5fbff550) + 55 at gmp.hpp:1019
    frame #3: 0x00000001000025ef test`boost::multiprecision::number<boost::multiprecision::backends::gmp_int, (this=0x00000001001039e0, e=0x00007fff5fbff550)1>::operator=(boost::multiprecision::number<boost::multiprecision::backends::gmp_int, (boost::multiprecision::expression_template_option)1> const&) + 47 at number.hpp:148
    frame #4: 0x0000000100001859 test`std::__1::vector<boost::multiprecision::number<boost::multiprecision::backends::gmp_int, (this=0x00007fff5fbff588, __position=(null), __x=0x00007fff5fbff550)1>, std::__1::allocator<boost::multiprecision::number<boost::multiprecision::backends::gmp_int, (boost::multiprecision::expression_template_option)1> > >::insert(std::__1::__wrap_iter<boost::multiprecision::number<boost::multiprecision::backends::gmp_int, (boost::multiprecision::expression_template_option)1> const*>, boost::multiprecision::number<boost::multiprecision::backends::gmp_int, (boost::multiprecision::expression_template_option)1> const&) + 841 at vector:1609
    frame #5: 0x0000000100001244 test`main(argc=1, argv=0x00007fff5fbff650) + 340 at test.cpp:11
    frame #6: 0x00007fff918ff7e1 libdyld.dylib`start + 1

The issue is in gmp_int& operator = (const gmp_int& o) which calls mpz_set(m_data, o.m_data) wrongly assuming m_data is a coherent state, which is false after a move since gmp_int(gmp_int&& o) sets o.m_data[0]._mp_d = 0.

The problem seems not to appear with libstdc++ (at least my version), perhaps because std::vector does a copy rather than a move to make space for the inserted elements.

I reckon the same problem might be presents in other places in gmp.hpp. Wouldn't also be better to replace o.m_data[0]._mp_d = 0 with a proper deinitialisation ?

Attachments (1)

gmp.hpp (90.9 KB ) - added by John Maddock 9 years ago.

Download all attachments as: .zip

Change History (5)

comment:1 by John Maddock, 9 years ago

I reckon the same problem might be presents in other places in gmp.hpp. Wouldn't also be better to replace o.m_data[0]._mp_d = 0 with a proper deinitialisation ?

Then it would do a copy, rather than a move.

Looks like a straight cut-and-paste error in the code to me, the other assignment operators have it right I believe, can you try the attached?

by John Maddock, 9 years ago

Attachment: gmp.hpp added

comment:2 by John Maddock, 9 years ago

Never mind, I have the test suite reproducing the problem now. Will post fixes shortly.

comment:3 by John Maddock, 9 years ago

(In [84687]) Fix assignment operations to be safe after a move. Added test cases to catch bug case. Refs #8667.

comment:4 by John Maddock, 9 years ago

Resolution: fixed
Status: newclosed

(In [84799]) Merge fixes for from Trunk. Fixes #8692. Fixes #8670. Fixes #8667.

Note: See TracTickets for help on using tickets.