After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 128603 - 4.3.28 crash on startup
4.3.28 crash on startup
Status: RESOLVED FIXED
Product: gnome-calculator
Classification: Core
Component: general
unspecified
Other other
: High critical
: ---
Assigned To: Rich Burridge
Rich Burridge
: 128710 129262 131058 133581 137593 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2003-12-05 17:14 UTC by Sebastien Bacher
Modified: 2004-12-22 21:47 UTC
See Also:
GNOME target: 2.6.next
GNOME version: ---


Attachments
A fix (I think), albeit a bad one. (541 bytes, text/plain)
2003-12-05 17:33 UTC, Rich Burridge
  Details
A better fix. (1.15 KB, text/plain)
2003-12-05 17:55 UTC, Rich Burridge
  Details
An even betterer fix (with locale support ;-) (1.32 KB, text/plain)
2003-12-05 20:41 UTC, Rich Burridge
  Details
Screenshot of debug session (65.84 KB, image/jpeg)
2003-12-31 21:59 UTC, Georg Wild
  Details
Proposed patch (1.96 KB, patch)
2004-01-01 18:54 UTC, Georg Wild
none Details | Review
Final version of the proposed patch (hopefully ;-) (2.12 KB, text/plain)
2004-01-05 16:50 UTC, Rich Burridge
  Details

Description Sebastien Bacher 2003-12-05 17:14:35 UTC
Distribution: Debian testing/unstable
Package: gcalctool
Severity: normal
Version: GNOME2.4.1 4.3.28
Gnome-Distributor: GNOME.Org
Synopsis: 4.3.28 crash on startup
Bugzilla-Product: gcalctool
Bugzilla-Component: general
Bugzilla-Version: 4.3.28
BugBuddy-GnomeVersion: 2.0 (2.4.0.1)
Description:
Description of the crash:
Crash on startup

Steps to reproduce the crash:
1. Launch it
2. 
3. 

Expected Results:
Start

How often does this happen?
Always


Additional Information:
Debian unstable, Gnome2.4 
4.3.16 works fine, I've just packaged 4.3.28, doesn't start




Debugging Information:

Backtrace was generated from '/usr/bin/gcalctool'

[New Thread 1086099584 (LWP 28209)]
0x4087830e in __waitpid_nocancel () from /lib/tls/libpthread.so.0

Thread 1 (Thread 1086099584 (LWP 28209))

  • #0 __waitpid_nocancel
    from /lib/tls/libpthread.so.0
  • #1 libgnomeui_module_info_get
    from /usr/lib/libgnomeui-2.so.0
  • #2 <signal handler called>
  • #3 remove_tsep
    at display.c line 87
  • #4 MPstr_to_num
    at display.c line 397
  • #5 init_vars
    at get.c line 256
  • #6 do_calctool
    at calctool.c line 1024
  • #7 main
    at gtk.c line 272
  • #0 __waitpid_nocancel
    from /lib/tls/libpthread.so.0




------- Bug moved to this database by unknown@bugzilla.gnome.org 2003-12-05 12:14 -------

Unknown version 4.3.28 in product gcalctool. Setting version to the default, "unspecified".
Reassigning to the default owner of the component, rich.burridge@sun.com.

Comment 1 Rich Burridge 2003-12-05 17:33:31 UTC
Created attachment 22129 [details]
A fix (I think), albeit a bad one.
Comment 2 Rich Burridge 2003-12-05 17:36:51 UTC
I think the problem is that the initial constant
strings are being put in read-only memory either 
by gcc (or because of the way that x86 arch is
designed). I tested this with the Sun compiler
on a Solaris SPARC machine which didn't exhibit 
a problem.

I've attach a small diff. Could you try it and 
see if it solves the problem please? It isn't 
a great fix as it will leak memory. If this works 
for you, I'll try to devise a better fix.

Thanks.
Comment 3 Rich Burridge 2003-12-05 17:55:39 UTC
Created attachment 22130 [details]
A better fix.
Comment 4 Rich Burridge 2003-12-05 17:58:12 UTC
Here's a better fix that doesn't leak memory.
Comment 5 Rich Burridge 2003-12-05 20:41:48 UTC
Created attachment 22137 [details]
An even betterer fix (with locale support ;-)
Comment 6 Rich Burridge 2003-12-05 20:54:41 UTC
Last patch checked into CVS HEAD. Fixed in v4.3.29.
Thanks again for the interactive debugging on IRC!
Comment 7 Stephane Loeuillet 2003-12-06 00:28:03 UTC
this bug also existed on a quite up to date gentoo x86 system

4.3.29 fixed the start-up crash too for us
Comment 8 Rich Burridge 2003-12-06 00:55:18 UTC
Ohh. Well it's hard to fix bugs that I don't know
about ;-), so if you have any others like this please
file 'em in Bugzilla. Thanks.
Comment 9 Rich Burridge 2003-12-07 01:43:13 UTC
*** Bug 128710 has been marked as a duplicate of this bug. ***
Comment 10 Rich Burridge 2003-12-13 16:35:13 UTC
*** Bug 129262 has been marked as a duplicate of this bug. ***
Comment 11 Georg Wild 2003-12-31 21:57:29 UTC
I have sth like this bug again in 4.3.31.
I get a SIGSEG when starting.
I'll attach a screenshot of ddd on this position which should explain
the problem.
I use gcc 3.2.2.
Comment 12 Georg Wild 2003-12-31 21:59:00 UTC
Created attachment 22806 [details]
Screenshot of debug session
Comment 13 Georg Wild 2004-01-01 10:02:45 UTC
What a mess - this function remove_tsep!

When this str is a constant string you write in read-only memory with
this *dstp++ = *srcp++. Well, you can simply work on a second string
and hope, that the constants are OK, but what a mess.

The following works for me:
diff display.c.orig display.c:
<     char *dstp = str;
---
>     char *dstp,*dstpptr; // = str;
90c90
< 
---
>     dstp=dstpptr=strdup(str);
101a102,103
>     if (strcmp(srcp,dstp)!=0) { memcpy(srcp,dstp,strlen(srcp)); }
>     free(dstpptr);
Comment 14 Rich Burridge 2004-01-01 16:37:55 UTC
Yes, remove_tsep() is a mess because it really
shouldn't have to care about whether the character
string is in read-only memory or not. I blame
the compiler. I certain don't have to jump through
hoops with the Sun compiler. 

Also the initial constant values shouldn't have any 
thousand separators in them, so you might be fixing the
wrong problem here and this is another variant of
bug #130282. Perhaps we should determine if you have
this problem if you unset the LANG variable.

Anyway...

A far better "fix", is to do the string duplication
in init_vars() in get.c. Something like:

    MPstr_to_num(strdup("0.621"), DEC, 
                 v->MPcon_vals[0]);  /* kms/hr <=> miles/hr. */
    MPstr_to_num(strdup("1.4142135623"), DEC, 
                 v->MPcon_vals[1]);  /* square root of 2 */        
    MPstr_to_num("2.7182818284", DEC, 
                 v->MPcon_vals[2]);  /* e */
    MPstr_to_num(strdup("3.1415926535"), DEC, 
                 v->MPcon_vals[3]);  /* pi */
    MPstr_to_num(strdup("0.3937007"),    DEC, 
                 v->MPcon_vals[4]);  /* cms <=> inch. */
    MPstr_to_num(strdup("57.295779513"), DEC, 
                 v->MPcon_vals[5]);  /* degrees/radian. */
    MPstr_to_num(strdup("1048576.0"),    DEC, 
                 v->MPcon_vals[6]);  /* 2 ^ 20. */
    MPstr_to_num(strdup("0.0353"), DEC, 
                 v->MPcon_vals[7]);  /* grams <=> ounce. */
    MPstr_to_num(strdup("0.948"),  DEC, 
                 v->MPcon_vals[8]);  /* Kjoules <=> BTU's. */
    MPstr_to_num(strdup("0.0610"), DEC, 
                 v->MPcon_vals[9]);  /* cms3 <=> inches3. */

We could even write a little macro for it:

#define INIT_CONSTANT(n, val) \
              MPstr_to_num(strdup(val), DEC, v->MPcon_vals[n]);

Then those ten lines become something like:

    INIT_CONSTANT(0, "0.621");   /* kms/hr <=> miles/hr. */
    ...

Much cleaner.

Georg, if unset LANG doesn't solve your problem, can you 
determine if this approach fixes the problem for you 
(with the current implementation of remove_tsep - i.e. 
without your suggested fix)?
Comment 15 Rich Burridge 2004-01-01 16:40:36 UTC
Adding Georg to the CC: line of this bug
so he can see my comments to the bug he
reopened. So why doesn't Bugzilla automatically
do that?
Comment 16 Georg Wild 2004-01-01 18:36:28 UTC
You're right, LC_ALL=C fixes the problem. Nevertheless I think it's
illegal programming.
Applying the patch of this other bug also fixes the problem!
Comment 17 Georg Wild 2004-01-01 18:54:57 UTC
Created attachment 22823 [details] [review]
Proposed patch
Comment 18 Georg Wild 2004-01-01 18:57:15 UTC
Setting LC_ALL only sometimes fixes this error.

Your proposed fix works. I've uploaded the patch in this bug report as 
an attachment (id=22823).
Comment 19 Rich Burridge 2004-01-01 20:24:47 UTC
Thanks Georg. Actually thinking about this some
more, there is still a problem here. The constant
values in init_vars() have "." hard-wired as the
radix separator. That's bogus. Those initial
values will need to be constructed (appended
together) using whatever is returned by get_radix() 
in get.c for the equivalent of ".".

What might be better here is to actually have them as
localizable strings.

Adding Christian to the CC: list for his take on this.

 
Comment 20 Georg Wild 2004-01-03 10:49:35 UTC
Let me just mention that the strdup isn't freed again in the proposed
patch!
Comment 21 Luis Villa 2004-01-04 00:30:47 UTC
Setting some bits, we want to track this and make sure it is fixed 
for 2.6.0. Thanks for all the work, guys.
Comment 22 Christian Rose 2004-01-04 01:27:14 UTC
Danilo, Christian, what are your opinions?
Comment 23 Rich Burridge 2004-01-04 17:43:53 UTC
Bug #130282 is related to this. Using the latest attachment
there, there is no need to localise the initial constant
strings. That just leaves the crash because those strings 
are in read-only memory on some systems. The latest attachment
from Georg will fix that (albeit the strings aren't freed which 
I can live with ;-)).

I'll check in this fix tomorrow. Thanks to all that have been
involved with this.
Comment 24 Georg Wild 2004-01-04 17:49:45 UTC
Well, I don't like unfree'd memory:
Why don't you use sth. like 
#define INIT_CONSTANT(n, val) \
              MPstr_to_num(a=strdup(val), DEC, v->MPcon_vals[n]); \
              free(a);
Comment 25 Rich Burridge 2004-01-04 18:03:54 UTC
Good point. We should probably use the glib calls
too. I'll adjust accordingly on Monday. Thanks.
Comment 26 Rich Burridge 2004-01-05 16:50:35 UTC
Created attachment 22943 [details]
Final version of the proposed patch (hopefully ;-)
Comment 27 Rich Burridge 2004-01-05 16:57:10 UTC
Changes checked into CVS HEAD. Fixed (hopefully
for the last time ;-) in v4.3.32.
Comment 28 Rich Burridge 2004-01-17 20:48:44 UTC
*** Bug 131058 has been marked as a duplicate of this bug. ***
Comment 29 Rich Burridge 2004-02-06 01:49:38 UTC
*** Bug 133581 has been marked as a duplicate of this bug. ***
Comment 30 Rich Burridge 2004-03-28 23:46:10 UTC
*** Bug 137593 has been marked as a duplicate of this bug. ***