0

Update 2: See the section after my code.

I'm using a thread for calculating PI using the GMP library, but somehow I'm now getting a segmentation fault when wxThread::OnExit() is internally in wxWidgets called.

Here is the line in wxWidgets source code: src/msw/thread.cpp#553

Here is the shortened code from my thread entry function:

while (i <= m_numIterations && !TestDestroy()) {
    mpf_div(result, perimeter, edgeCount);

    mpf_pow_ui(result, result, 2);
    mpf_ui_sub(result, 1, result);
    mpf_sqrt(result, result);
    mpf_div_ui(result, result, 2);

    mpf_sub(result, half, result);
    mpf_sqrt(result, result);
    mpf_mul_ui(result, result, 2);
    mpf_mul(result, result, edgeCount);

    mpf_set(perimeter, result);

    i++;
    mpf_mul_ui(edgeCount, edgeCount, 2);
}

// Free GMP variables we don't need anymore
mpf_clear(half);
mpf_clear(result);
mpf_clear(edgeCount);

// OUTPUT_DIGITS has a constant value, e.g. 12
char outputStr[OUTPUT_DIGITS];

mp_exp_t *expptr;

// If commented out, the error does not appear!
mpf_get_str(outputStr, expptr, 10, OUTPUT_DIGITS, perimeter);

Update 2: If I comment out the last line with mpf_get_str(), the error does not occur.
I also found a very old bug requests from 2003: http://gmplib.org/list-archives/gmp-discuss/2003-November/000888.html

Call stack from GCC Debugger:

#0 63AE80E9 wxThreadInternal::DoThreadOnExit(thread=0x2cfa978) (../../src/msw/thread.cpp:553)
#1 63B27ACF wxScopeGuardImpl1<void (*)(wxThread*) (../../include/wx/scopeguard.h:168)
#2 63B3F95B wxPrivate::OnScopeExit<wxScopeGuardImpl1<void (*)(wxThread*) (../../include/wx/scopeguard.h:67)
#3 63B27B36 wxScopeGuardImpl1<void (*)(wxThread*) (../../include/wx/scopeguard.h:166)
#4 63AE82FB wxThreadInternal::DoThreadStart(thread=0x2cfa978) (../../src/msw/thread.cpp:561)
#5 63AE83F2 wxThreadInternal::WinThreadStart(param=0x2cfa978) (../../src/msw/thread.cpp:602)
#6 75C4906A ui64tow() (C:\Windows\SysWOW64\msvcrt.dll:??)
#7 75C49147 msvcrt!iswalnum() (C:\Windows\SysWOW64\msvcrt.dll:??)
#8 76448543 UnregisterBadMemoryNotification() (C:\Windows\SysWOW64\kernel32.dll:??)
#9 00000000 0x02cfb178 in ??() (??:??)
#10 00000000    0x77e8ac69 in ??() (??:??)
#11 00000000    0x77e8ac3c in ??() (??:??)
#12 00000000    0x00000000 in ??() (??:??)
ComFreek
  • 29,044
  • 18
  • 104
  • 156
  • If it's a constant size, why are you dynamically allocating it? – chris Sep 19 '12 at 20:27
  • You're right. I've just tried it but it now throws a segmentation fault when `wxThread::OnExit()` is called! – ComFreek Sep 19 '12 at 20:33
  • It should throw an exception, not return `NULL`. Also, your assert that follows is wrong. – Nikolai Fetissov Sep 19 '12 at 20:33
  • **Note** that I've changed the question and the code. `outputStr` is now allocated on the stack. – ComFreek Sep 19 '12 at 20:37
  • Which version of GMP are you using? [V4.1])(http://cs.nyu.edu/exact/core/gmp/) was causing a similar error on Windows 7 x64 in this code: `char *data = mpz_get_str(NULL, base, gmpInt); std::string output(data); free(data);`. In my case, the segmentation fault was produced by the call to `free` and I assumed that `mpz_get_str` probably allocates memory in some funky way, without providing any (obvious) means of deallocating it. I managed to get around it by using MPIR on Windows and the latest version of GMP on Linux. – Mihai Todor Sep 24 '12 at 15:51
  • @MihaiTodor Thanks for your comment and thanks that you have remembered me to post the solution. The problem was that `expptr` was not initialized (i.e. you have to `new` it, GMP doesn't do it for you!) – ComFreek Sep 24 '12 at 15:53
  • @ComFreek I find it rather odd that you're using `new` on it, since you're calling the C version of the GMP API. I would expect to use `malloc` and `free` in this case, but anyway, I'm glad you managed to make it work. – Mihai Todor Sep 24 '12 at 15:58
  • @MihaiTodor I've already seen the C++ wrapper. But I don't see any reason to use it in my small PI program if it's only a wrapper. – ComFreek Sep 24 '12 at 16:02
  • Well, that's what I'm saying: If you're using C stuff, then don't mix it with C++ stuff like `new` and `delete`, because, by doing this, you're definitely asking for trouble ;) – Mihai Todor Sep 24 '12 at 16:03
  • @MihaiTodor Have you any links about this topic? I would be interested for my future projects. – ComFreek Sep 24 '12 at 16:07
  • It has been discussed plenty of times on SO: http://stackoverflow.com/questions/240212/what-is-the-difference-between-new-delete-and-malloc-free and here http://stackoverflow.com/questions/9972212/why-use-malloc-free-when-we-have-new-delete – Mihai Todor Sep 24 '12 at 16:10
  • @MihaiTodor Thanks. I'll consider them in my future projects. But AFAIK GMP does not `new` or `delete` any passed parameters. So it's safe (meaning that it won't break something directly) to use here. – ComFreek Sep 24 '12 at 16:24
  • @ComFreek Well, it's your code ;) – Mihai Todor Sep 24 '12 at 16:40

3 Answers3

0

new doesn't "fail", per se, you've got heap corruption somewhere.

mark
  • 5,269
  • 2
  • 21
  • 34
  • Thanks for your answer. I've changed my question since you answered. But I'm still getting a segmentation fault, see my text. – ComFreek Sep 19 '12 at 20:37
  • you still have corruption somewhere.. moving an item from the heap to the stack doesn't change that. run it in a debugger, it might fault near or on where the corruption actually happens. – mark Sep 19 '12 at 20:50
  • you might also try valgrind if you're on linux... it'll run a lot slower, effectively in an emulator, but it can pick up on invalid pointer access, stack overrun, etc. – mark Sep 19 '12 at 20:52
  • I've put the call stack into my question. I'm on Windows. – ComFreek Sep 19 '12 at 20:56
  • `new` can certainly fail, but that's probably not the problem here. `new` will throw an exception if it cannot allocate the requested amount of memory, and `new (nothrow)` will return a null pointer. – Ed S. Sep 19 '12 at 21:01
  • @EdS. Thanks for the info. I didn't know that before. – ComFreek Sep 19 '12 at 21:04
  • @Ed S: technically you are 100% correct, but for practical purposes, new doesn't fail if you're using it in a normal way... – mark Sep 19 '12 at 21:09
  • @mark: Sure it does, just not often. By your logic you should never check for `malloc` returning a null pointer either because you are using it in a "normal way". I'm also not sure how you would use it in an "abnormal way". – Ed S. Sep 19 '12 at 21:11
0

If it really crashes on this line (there is also a non negligible probability that gdb just gets lost and doesn't show the subsequent frames), then the thread pointer itself must be NULL or invalid which can't happen in normal execution, so I do agree with the other reply: something seems to corrupt your variables. But do check if your OnExit() gets entered at all just in case gdb is lying.

And as you're using cross-platform libraries, you should be able to rebuild under Linux and run it under valgrind which should pin point all the obvious problems.

VZ.
  • 21,740
  • 3
  • 39
  • 42
0

The exponent pointer (here expptr) has to be already initialized to an object.

This one-liner solves the problem:

expptr = new mp_exp_t();

// call mpf_get_str()

I also want to note that VZ. was right that GDB (GNU Debugger) can show the wrong line of source code in which the error should appear.

So do not only rely on that.

ComFreek
  • 29,044
  • 18
  • 104
  • 156