Tcl Source Code

View Ticket
Login
Ticket UUID: 474335
Title: logic of matherr is backwards
Type: Bug Created on: 2001-10-24 03:45:18
Submitter: dgp Assigned to: dgp
Subsystem: 52. Portability Support Severity:
Priority: 5 Medium Last modified: 2002-06-01 05:21:28
Status: Closed Closed by: dgp
Resolution: Fixed Closed on: 2002-05-31 22:21:28
Version: obsolete: 8.4a4
Description:
A simple look at the cvs history of 
unix/tclMtherr.c shows the introduction of
a bug:

cvs diff -r core-8-0-5 -r core-8-1-0 tclMtherr.c
...
matherr(xPtr)
     struct exception *xPtr;    /* Describes error that
occurred. */
 {
-    if (!tcl_MathInProgress) {
+    if (TclMathInProgress()) {
        return 0;
...

The "!" did not survive the transition from
access of a global variable to call of an internal
function.  The logic of matherr() became reversed.
The obvious fix is to restore the proper logic.

It's worth pausing though and asking why this
bug hasn't been noticed in the 3 years since it
was introduced.  I am unable to come up with
any script on any platform that exposes this error.

It seems that Tcl behaves the same whether the
logic is forwards or backwards.
It would seem then that Tcl would also behave the
same if the logic were completely removed.  I welcome
second opinions on this, but my inclination would
be to remove the matherr() routine altogether
unless someone can demonstrate it's serving a
useful purpose somewhere.  (And how useful can
it be if it's been wrong since Tcl 8.1.0 ?)
User Comments:
hobbs added on 2001-10-25 02:42:07:
Logged In: YES 
user_id=72656

And note that win/tclWinMtherr.c actually has the correct ! 
in that compat function.  Note that it is a compat 
function.  You'd like have to ask the older core guys 
(JohnO or Brian Lewis who did the original compiler) 
directly if/what meaning this had.

hobbs added on 2001-10-25 02:43:24:
Logged In: YES 
user_id=72656

Of course, the comments for the Windows version say only 
Borland C++ uses it.  However, that should be verified, as 
that used to be the official compiler of the sources, and 
the comment may be incorrect.

dgp added on 2001-10-25 03:13:31:
Logged In: YES 
user_id=80530

Agreed that second opinions from the old-timers
will be very helpful.

I also note, that matherr() and _matherr() are the
only callers of TclMathInProgress() so if they go,
so can that.

mdejong added on 2002-01-12 01:56:38:
Logged In: YES 
user_id=90858

I don't have anything useful to add, assigning this
bug back to hobbs.

jenglish added on 2002-01-16 09:21:46:
Logged In: YES 
user_id=68433

On the Unices that I have available, 'matherr()' is only
used  if you statically  link against a special library
instead of the default -lm (IRIX: -lmx; AIX: -lmsaa; Linux
(RH 6.2, 7.0,7.1,7.2, SuSE 6.2): apparently not supported). 

A google search shows that matherr is supported on HP-UX and
a few others, but the default behaviour (set errno and
continue) is exactly the same as  what's in
unix/tclMtherr.c, so the latter is redundant.

It would appear that this is either unused or redundant on
all Unix platforms, and can safely be removed (except for
one problem, see below).

In unix/tclAppInit.c the following should also be taken out:

/* 
 * The following variable is a special hack that is needed
in order for
 * Sun shared libraries to be used for Tcl.
 */
extern int matherr();
int *tclDummyMathPtr = (int *) matherr;

This bit of code has been around since Tcl 7.4, but has long
since been rendered obsolete by Stubs and newer versions of
SunOS.
 
The only problem with removing unix/tclMtherr.c is that all
the custom tclsh's and wishes out there that have retained
the tclDummyMathPtr definition for cargo-cult reasons will
fail to link on platforms that don't have matherr().  It's
an easy fix -- just get rid of the definition -- so this
probably isn't much of an issue.

dgp added on 2002-02-20 08:25:16:
Logged In: YES 
user_id=80530

davgrvy, can you comment on whether the
_matherr() function in win/tclWinMtherr.c
is still needed by Borland compilers?

dgp added on 2002-02-22 00:31:28:
Logged In: YES 
user_id=80530

Since Tcl 8.4 should be released soon, now is not
the time to risk breaking anything, but as soon
as we get a HEAD aimed at 8.5a development I plan
to yank out all this matherr business.

jenglish added on 2002-05-14 04:50:27:
Logged In: YES 
user_id=68433

Another data point:  On HP-UX 10.20, the function _matherr()
is used when linking with the default math library (note
leading underscore, NOT 'matherr()', which is what
unix/tclMtherr.c defines).  Also, the man page says the
following:

"The matherr() and _matherr() functions are obsolete and
will not be supported at the next release of HP-UX  [that
would be HP-UX 11 -JE].  The matherr() function is not
required by any version of XPG nor by ANSI C, and has not
been part of any standard since SVID2.  In the HP-UX math
library, the SVID2 matherr() function has been renamed to
_matherr(), and no error messages are printed to the
standard error output.  _matherr() is provided in libm.a in
order to assist in supporting old programs.  Executables
that use the old matherr() or _matherr() will continue to
run indefinitely, even on future releases of HP-UX."

"However, if you intend to recompile or relink any of your
current programs that use matherr() or _matherr() on a
future release of HP-UX, you should be aware that any
function called matherr() or _matherr() will no longer be
called in case of library errors."

davygrvy added on 2002-05-28 08:19:49:
Logged In: YES 
user_id=7549

Should this be assigned to me?  I don't know how to follow 
this up.

dgp added on 2002-05-28 10:50:05:
Logged In: YES 
user_id=80530

Hi, davygrvy,

This was assigned to you looking for comments on
whether the Borland compiler still had any need 
for a matherr() implementation.  See the 2002-02-19
comment below.

davygrvy added on 2002-05-28 11:04:49:
Logged In: YES 
user_id=7549

There are 2 mentions of it in include/math.h:

struct  _exception
{
    int type;
    char   _FAR *name;
    double  arg1, arg2, retval;
};

struct  _exceptionl
{
    int type;
    char   _FAR *name;
    long double  arg1, arg2, retval;
};

typedef enum
{
    DOMAIN = 1,    /* argument domain error -- log (-1)        */
    SING,          /* argument singularity  -- pow (0,-2))     */
    OVERFLOW,      /* overflow range error  -- exp (1000)      */
    UNDERFLOW,     /* underflow range error -- exp (-1000)     
*/
    TLOSS,         /* total loss of significance -- sin(10e70) */
    PLOSS,         /* partial loss of signif. -- not used      */
    STACKFAULT     /* floating point unit stack overflow       */
}   _mexcep;

int         _RTLENTRY          _matherr(struct _exception  *__e);
int         _RTLENTRY          _matherrl(struct _exceptionl 
*__e);

I don't know what to say beyond the existence of it.  I don't 
have any documentation on Borland's RT to contribute any 
useful concerns.  fastmath.h says the following, though:

/*
   fastmath.h - header file for the FastMath routines.  These 
are high
                performance math routines that don't bother 
checking for
                most error conditions and never call matherr.  
They are
                coded for maximum speed and most are written in 
assembly.

                These functions are never exported from the 
RTLDLL, which
                means that they always get linked directly into the 
PE file
                which is being created.

*/

dgp added on 2002-05-29 23:47:54:
Logged In: YES 
user_id=80530

OK, thanks.  I'll stick with the plan to remove all
the matherr stuff from Tcl once we hit 8.5a development.

jenglish added on 2002-05-30 00:34:45:
Logged In: YES 
user_id=68433

> I'll stick with the plan to remove all the matherr stuff
from Tcl once we hit 8.5a development.

Why not remove it now, while we're still in the 8.4 alpha
series?

See also patch #555635 "Dead code: matherr,
TclMathInProgress".  This is extremely unlikely to break
anything.  I realize that code cleanup is not a high
priority -- "if it ain't broke, don't fix it" -- but dead
code *does* have a cost.

dgp added on 2002-06-01 05:21:28:
Logged In: YES 
user_id=80530

Patch 555635 accepted in Tcl 8.4a5.  Fixed.  Closed.